diff options
author | Alon Zakai <azakai@google.com> | 2020-04-07 17:55:40 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-07 17:55:40 -0700 |
commit | e77d85fecd2bfe707f3e9b7ab84cfd6744bdc7b7 (patch) | |
tree | cd674b0322cec50feeb132e634bc17eeedf40ab5 | |
parent | eaa7240105811ffa565dc97768f1399e7855744b (diff) | |
download | binaryen-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.cpp | 18 | ||||
-rw-r--r-- | test/lld/main_module.wat | 58 | ||||
-rw-r--r-- | test/lld/main_module.wat.out | 165 | ||||
-rw-r--r-- | test/lld/shared.wat.out | 36 | ||||
-rw-r--r-- | test/lld/shared_add_to_table.wasm.out | 45 |
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", |