summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2020-04-07 17:55:40 -0700
committerGitHub <noreply@github.com>2020-04-07 17:55:40 -0700
commite77d85fecd2bfe707f3e9b7ab84cfd6744bdc7b7 (patch)
treecd674b0322cec50feeb132e634bc17eeedf40ab5
parenteaa7240105811ffa565dc97768f1399e7855744b (diff)
downloadbinaryen-e77d85fecd2bfe707f3e9b7ab84cfd6744bdc7b7.tar.gz
binaryen-e77d85fecd2bfe707f3e9b7ab84cfd6744bdc7b7.tar.bz2
binaryen-e77d85fecd2bfe707f3e9b7ab84cfd6744bdc7b7.zip
Only do fp$ optimization in the main module (#2720)
Weak symbols and interposition etc. mean that we should not replace an fp$ call with a symbol from the module itself if there is a chance there is another symbol that would have overridden it. In side modules this risk exists and so this PR makes us stop doing that. In main modules it is ok because they are loaded first and so any symbol they provide will "win" over others anyhow.
-rw-r--r--src/wasm/wasm-emscripten.cpp18
-rw-r--r--test/lld/main_module.wat58
-rw-r--r--test/lld/main_module.wat.out165
-rw-r--r--test/lld/shared.wat.out36
-rw-r--r--test/lld/shared_add_to_table.wasm.out45
5 files changed, 264 insertions, 58 deletions
diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index 0d71e8f00..75598875c 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -306,13 +306,13 @@ Function* EmscriptenGlueGenerator::generateAssignGOTEntriesFunction() {
// Note that we don't search for the function by name since its internal
// name may be different.
auto* ex = wasm.getExportOrNull(g->base);
- if (ex) {
+ // If this is exported then it must be one of the functions implemented
+ // here, and if this is a main module, then we can simply place the function
+ // in the table: the loader will see it there and resolve all other uses
+ // to this one.
+ if (ex && !sideModule) {
assert(ex->kind == ExternalKind::Function);
auto* f = wasm.getFunction(ex->value);
- // This is exported, so must be one of the functions implemented here.
- // Simply add it to the table, and use that index. The loader will
- // know to reuse that index for other modules so they all share the
- // same index and function pointer equality works.
if (f->imported()) {
Fatal() << "GOT.func entry is both imported and exported: " << g->base;
}
@@ -337,10 +337,14 @@ Function* EmscriptenGlueGenerator::generateAssignGOTEntriesFunction() {
block->list.push_back(globalSet);
continue;
}
- // This is imported. Create an fp$ import to get the function table index.
+ // This is imported or in a side module. Create an fp$ import to get the
+ // function table index from the dynamic loader.
auto* f = importInfo.getImportedFunction(ENV, g->base);
if (!f) {
- Fatal() << "GOT.func entry with no import/export: " << g->base;
+ if (!ex) {
+ Fatal() << "GOT.func entry with no import/export: " << g->base;
+ }
+ f = wasm.getFunction(ex->value);
}
Name getter(
(std::string("fp$") + g->base.c_str() + std::string("$") + getSig(f))
diff --git a/test/lld/main_module.wat b/test/lld/main_module.wat
new file mode 100644
index 000000000..fb3263424
--- /dev/null
+++ b/test/lld/main_module.wat
@@ -0,0 +1,58 @@
+(module
+ (type $0 (func (param i32) (result i32)))
+ (type $1 (func))
+ (type $2 (func (result i32)))
+ (import "env" "memory" (memory $0 0))
+ (data (global.get $gimport$2) "Hello, world\00\00\00\00\00\00\00\00\00\00\00\00")
+ (import "env" "__stack_pointer" (global $sp (mut i32)))
+ (import "env" "__indirect_function_table" (table $timport$1 0 funcref))
+ (import "env" "__memory_base" (global $gimport$2 i32))
+ (import "env" "__table_base" (global $gimport$3 i32))
+ (import "GOT.mem" "external_var" (global $gimport$5 (mut i32)))
+ (import "GOT.func" "puts" (global $gimport$6 (mut i32)))
+ (import "GOT.func" "_Z13print_messagev" (global $gimport$7 (mut i32)))
+ (import "env" "puts" (func $puts (param i32) (result i32)))
+ (global $global$0 i32 (i32.const 16))
+ (global $global$1 i32 (i32.const 20))
+ (global $global i32 (i32.const 42))
+ (export "__wasm_call_ctors" (func $__wasm_call_ctors))
+ (export "_Z13print_messagev" (func $print_message\28\29))
+ (export "ptr_puts" (global $global$0))
+ (export "ptr_local_func" (global $global$1))
+ (export "__data_end" (global $global))
+ (func $__wasm_call_ctors (; 1 ;) (type $1)
+ (call $__wasm_apply_relocs)
+ )
+ (func $__wasm_apply_relocs (; 2 ;) (type $1)
+ (i32.store
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 16)
+ )
+ (global.get $gimport$6)
+ )
+ (i32.store
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 20)
+ )
+ (global.get $gimport$7)
+ )
+ )
+ (func $print_message\28\29 (; 3 ;) (type $2) (result i32)
+ (drop
+ (call $puts
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 0)
+ )
+ )
+ )
+ (i32.load
+ (global.get $gimport$5)
+ )
+ )
+ ;; custom section "dylink", size 5
+ ;; custom section "producers", size 112
+)
+
diff --git a/test/lld/main_module.wat.out b/test/lld/main_module.wat.out
new file mode 100644
index 000000000..478a7a08f
--- /dev/null
+++ b/test/lld/main_module.wat.out
@@ -0,0 +1,165 @@
+(module
+ (type $none_=>_i32 (func (result i32)))
+ (type $i32_=>_i32 (func (param i32) (result i32)))
+ (type $none_=>_none (func))
+ (type $i32_=>_none (func (param i32)))
+ (import "env" "memory" (memory $0 0))
+ (data (global.get $gimport$2) "Hello, world\00\00\00\00\00\00\00\00\00\00\00\00")
+ (import "env" "table" (table $0 1 funcref))
+ (elem (global.get $gimport$3) $print_message\28\29)
+ (import "env" "__stack_pointer" (global $sp_import i32))
+ (import "env" "__memory_base" (global $gimport$2 i32))
+ (import "env" "__table_base" (global $gimport$3 i32))
+ (import "env" "puts" (func $puts (param i32) (result i32)))
+ (import "env" "g$external_var" (func $g$external_var (result i32)))
+ (import "env" "fp$puts$ii" (func $fp$puts$ii (result i32)))
+ (global $gimport$5 (mut i32) (i32.const 0))
+ (global $gimport$6 (mut i32) (i32.const 0))
+ (global $gimport$7 (mut i32) (i32.const 0))
+ (global $global$0 i32 (i32.const 16))
+ (global $global$1 i32 (i32.const 20))
+ (global $global i32 (i32.const 42))
+ (global $sp (mut i32) (global.get $sp_import))
+ (export "__wasm_call_ctors" (func $__wasm_call_ctors))
+ (export "_Z13print_messagev" (func $print_message\28\29))
+ (export "ptr_puts" (global $global$0))
+ (export "ptr_local_func" (global $global$1))
+ (export "__data_end" (global $global))
+ (export "stackSave" (func $stackSave))
+ (export "stackAlloc" (func $stackAlloc))
+ (export "stackRestore" (func $stackRestore))
+ (export "__growWasmMemory" (func $__growWasmMemory))
+ (export "__assign_got_enties" (func $__assign_got_enties))
+ (export "dynCall_i" (func $dynCall_i))
+ (func $__wasm_call_ctors (; 3 ;)
+ (call $__wasm_apply_relocs)
+ )
+ (func $__wasm_apply_relocs (; 4 ;)
+ (i32.store
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 16)
+ )
+ (global.get $gimport$6)
+ )
+ (i32.store
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 20)
+ )
+ (global.get $gimport$7)
+ )
+ )
+ (func $print_message\28\29 (; 5 ;) (result i32)
+ (drop
+ (call $puts
+ (i32.add
+ (global.get $gimport$2)
+ (i32.const 0)
+ )
+ )
+ )
+ (i32.load
+ (global.get $gimport$5)
+ )
+ )
+ (func $stackSave (; 6 ;) (result i32)
+ (global.get $sp)
+ )
+ (func $stackAlloc (; 7 ;) (param $0 i32) (result i32)
+ (local $1 i32)
+ (global.set $sp
+ (local.tee $1
+ (i32.and
+ (i32.sub
+ (global.get $sp)
+ (local.get $0)
+ )
+ (i32.const -16)
+ )
+ )
+ )
+ (local.get $1)
+ )
+ (func $stackRestore (; 8 ;) (param $0 i32)
+ (global.set $sp
+ (local.get $0)
+ )
+ )
+ (func $__growWasmMemory (; 9 ;) (param $newSize i32) (result i32)
+ (memory.grow
+ (local.get $newSize)
+ )
+ )
+ (func $__assign_got_enties (; 10 ;)
+ (global.set $gimport$5
+ (call $g$external_var)
+ )
+ (global.set $gimport$6
+ (call $fp$puts$ii)
+ )
+ (global.set $gimport$7
+ (i32.add
+ (global.get $gimport$3)
+ (i32.const 0)
+ )
+ )
+ )
+ (func $dynCall_i (; 11 ;) (param $fptr i32) (result i32)
+ (call_indirect (type $none_=>_i32)
+ (local.get $fptr)
+ )
+ )
+)
+(;
+--BEGIN METADATA --
+{
+ "staticBump": 4294966770,
+ "tableSize": 1,
+ "initializers": [
+ "__assign_got_enties",
+ "__wasm_call_ctors"
+ ],
+ "declares": [
+ "puts",
+ "g$external_var",
+ "fp$puts$ii"
+ ],
+ "externs": [
+ "___stack_pointer",
+ "___memory_base",
+ "___table_base"
+ ],
+ "implementedFunctions": [
+ "___wasm_call_ctors",
+ "__Z13print_messagev",
+ "_stackSave",
+ "_stackAlloc",
+ "_stackRestore",
+ "___growWasmMemory",
+ "___assign_got_enties",
+ "_dynCall_i"
+ ],
+ "exports": [
+ "__wasm_call_ctors",
+ "_Z13print_messagev",
+ "stackSave",
+ "stackAlloc",
+ "stackRestore",
+ "__growWasmMemory",
+ "__assign_got_enties",
+ "dynCall_i"
+ ],
+ "namedGlobals": {
+ "ptr_puts" : "16",
+ "ptr_local_func" : "20",
+ "__data_end" : "42"
+ },
+ "invokeFuncs": [
+ ],
+ "features": [
+ ],
+ "mainReadsParams": 0
+}
+-- END METADATA --
+;)
diff --git a/test/lld/shared.wat.out b/test/lld/shared.wat.out
index 64f97b984..5cc1cf88f 100644
--- a/test/lld/shared.wat.out
+++ b/test/lld/shared.wat.out
@@ -4,13 +4,13 @@
(type $i32_=>_i32 (func (param i32) (result i32)))
(import "env" "memory" (memory $0 0))
(data (global.get $gimport$2) "Hello, world\00\00\00\00\00\00\00\00\00\00\00\00")
- (import "env" "table" (table $0 1 funcref))
- (elem (global.get $gimport$3) $print_message\28\29)
+ (import "env" "table" (table $0 0 funcref))
(import "env" "__memory_base" (global $gimport$2 i32))
(import "env" "__table_base" (global $gimport$3 i32))
(import "env" "puts" (func $puts (param i32) (result i32)))
(import "env" "g$external_var" (func $g$external_var (result i32)))
(import "env" "fp$puts$ii" (func $fp$puts$ii (result i32)))
+ (import "env" "fp$_Z13print_messagev$i" (func $fp$_Z13print_messagev$i (result i32)))
(global $gimport$5 (mut i32) (i32.const 0))
(global $gimport$6 (mut i32) (i32.const 0))
(global $gimport$7 (mut i32) (i32.const 0))
@@ -20,11 +20,10 @@
(export "ptr_puts" (global $global$0))
(export "ptr_local_func" (global $global$1))
(export "__post_instantiate" (func $__post_instantiate))
- (export "dynCall_i" (func $dynCall_i))
- (func $__wasm_call_ctors (; 3 ;)
+ (func $__wasm_call_ctors (; 4 ;)
(call $__wasm_apply_relocs)
)
- (func $__wasm_apply_relocs (; 4 ;)
+ (func $__wasm_apply_relocs (; 5 ;)
(i32.store
(i32.add
(global.get $gimport$2)
@@ -40,7 +39,7 @@
(global.get $gimport$7)
)
)
- (func $print_message\28\29 (; 5 ;) (result i32)
+ (func $print_message\28\29 (; 6 ;) (result i32)
(drop
(call $puts
(i32.add
@@ -53,11 +52,11 @@
(global.get $gimport$5)
)
)
- (func $__post_instantiate (; 6 ;)
+ (func $__post_instantiate (; 7 ;)
(call $__assign_got_enties)
(call $__wasm_call_ctors)
)
- (func $__assign_got_enties (; 7 ;)
+ (func $__assign_got_enties (; 8 ;)
(global.set $gimport$5
(call $g$external_var)
)
@@ -65,15 +64,7 @@
(call $fp$puts$ii)
)
(global.set $gimport$7
- (i32.add
- (global.get $gimport$3)
- (i32.const 0)
- )
- )
- )
- (func $dynCall_i (; 8 ;) (param $fptr i32) (result i32)
- (call_indirect (type $none_=>_i32)
- (local.get $fptr)
+ (call $fp$_Z13print_messagev$i)
)
)
)
@@ -81,11 +72,12 @@
--BEGIN METADATA --
{
"staticBump": 0,
- "tableSize": 1,
+ "tableSize": 0,
"declares": [
"puts",
"g$external_var",
- "fp$puts$ii"
+ "fp$puts$ii",
+ "fp$_Z13print_messagev$i"
],
"externs": [
"___memory_base",
@@ -93,13 +85,11 @@
],
"implementedFunctions": [
"__Z13print_messagev",
- "___post_instantiate",
- "_dynCall_i"
+ "___post_instantiate"
],
"exports": [
"_Z13print_messagev",
- "__post_instantiate",
- "dynCall_i"
+ "__post_instantiate"
],
"namedGlobals": {
"ptr_puts" : "16",
diff --git a/test/lld/shared_add_to_table.wasm.out b/test/lld/shared_add_to_table.wasm.out
index c561a89d3..5343b7089 100644
--- a/test/lld/shared_add_to_table.wasm.out
+++ b/test/lld/shared_add_to_table.wasm.out
@@ -1,18 +1,18 @@
(module
- (type $none_=>_none (func))
(type $none_=>_i32 (func (result i32)))
+ (type $none_=>_none (func))
(type $i32_=>_i32 (func (param i32) (result i32)))
(type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
(import "env" "memory" (memory $0 0))
(data (global.get $gimport$3) "*\00\00\00")
- (import "env" "table" (table $timport$1 1 funcref))
- (elem (global.get $gimport$4) $waka_func_mine\28int\29)
+ (import "env" "table" (table $timport$1 0 funcref))
(import "env" "__memory_base" (global $gimport$3 i32))
(import "env" "__table_base" (global $gimport$4 i32))
(import "env" "_Z16waka_func_theirsi" (func $waka_func_theirs\28int\29 (param i32) (result i32)))
(import "env" "g$waka_mine" (func $g$waka_mine (result i32)))
(import "env" "g$waka_others" (func $g$waka_others (result i32)))
(import "env" "fp$_Z16waka_func_theirsi$ii" (func $fp$_Z16waka_func_theirsi$ii (result i32)))
+ (import "env" "fp$_Z14waka_func_minei$ii" (func $fp$_Z14waka_func_minei$ii (result i32)))
(global $gimport$6 (mut i32) (i32.const 0))
(global $gimport$7 (mut i32) (i32.const 0))
(global $gimport$8 (mut i32) (i32.const 0))
@@ -26,19 +26,18 @@
(export "main" (func $main))
(export "__dso_handle" (global $global$1))
(export "__post_instantiate" (func $__post_instantiate))
- (export "dynCall_ii" (func $dynCall_ii))
- (func $__wasm_call_ctors (; 4 ;)
+ (func $__wasm_call_ctors (; 5 ;)
(call $__wasm_apply_relocs)
)
- (func $__wasm_apply_relocs (; 5 ;)
+ (func $__wasm_apply_relocs (; 6 ;)
)
- (func $waka_func_mine\28int\29 (; 6 ;) (param $0 i32) (result i32)
+ (func $waka_func_mine\28int\29 (; 7 ;) (param $0 i32) (result i32)
(i32.add
(local.get $0)
(i32.const 1)
)
)
- (func $__original_main (; 7 ;) (result i32)
+ (func $__original_main (; 8 ;) (result i32)
(local $0 i32)
(local $1 i32)
(local.set $0
@@ -62,14 +61,14 @@
)
)
)
- (func $main (; 8 ;) (param $0 i32) (param $1 i32) (result i32)
+ (func $main (; 9 ;) (param $0 i32) (param $1 i32) (result i32)
(call $__original_main)
)
- (func $__post_instantiate (; 9 ;)
+ (func $__post_instantiate (; 10 ;)
(call $__assign_got_enties)
(call $__wasm_call_ctors)
)
- (func $__assign_got_enties (; 10 ;)
+ (func $__assign_got_enties (; 11 ;)
(global.set $gimport$8
(call $g$waka_mine)
)
@@ -80,22 +79,13 @@
(call $fp$_Z16waka_func_theirsi$ii)
)
(global.set $gimport$7
- (i32.add
- (global.get $gimport$4)
- (i32.const 0)
- )
- )
- )
- (func $dynCall_ii (; 11 ;) (param $fptr i32) (param $0 i32) (result i32)
- (call_indirect (type $i32_=>_i32)
- (local.get $0)
- (local.get $fptr)
+ (call $fp$_Z14waka_func_minei$ii)
)
)
;; dylink section
;; memorysize: 4
;; memoryalignment: 2
- ;; tablesize: 1
+ ;; tablesize: 0
;; tablealignment: 0
;; custom section "producers", size 157
)
@@ -103,12 +93,13 @@
--BEGIN METADATA --
{
"staticBump": 0,
- "tableSize": 1,
+ "tableSize": 0,
"declares": [
"_Z16waka_func_theirsi",
"g$waka_mine",
"g$waka_others",
- "fp$_Z16waka_func_theirsi$ii"
+ "fp$_Z16waka_func_theirsi$ii",
+ "fp$_Z14waka_func_minei$ii"
],
"externs": [
"___memory_base",
@@ -119,16 +110,14 @@
"__Z14waka_func_minei",
"___original_main",
"_main",
- "___post_instantiate",
- "_dynCall_ii"
+ "___post_instantiate"
],
"exports": [
"__wasm_apply_relocs",
"_Z14waka_func_minei",
"__original_main",
"main",
- "__post_instantiate",
- "dynCall_ii"
+ "__post_instantiate"
],
"namedGlobals": {
"waka_mine" : "0",