Skip to content

Commit 32fe916

Browse files
sbc100sbc100
committed
Reland "Fix renaming in FixInvokeFunctionNamesWalker (#2513)"
This reverts commit 132daae.
1 parent 1a0530d commit 32fe916

File tree

8 files changed

+462
-72
lines changed

8 files changed

+462
-72
lines changed

‎scripts/test/generate_lld_tests.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ def files_with_extensions(path, extensions):
3030
yield file, ext
3131

3232

33-
def generate_wast_files(llvm_bin, emscripten_root):
34-
print('\n[ building wast files from C sources... ]\n')
33+
def generate_wat_files(llvm_bin, emscripten_root):
34+
print('\n[ building wat files from C sources... ]\n')
3535

3636
lld_path = os.path.join(shared.options.binaryen_test, 'lld')
3737
for src_file, ext in files_with_extensions(lld_path, ['.c', '.cpp']):
@@ -42,11 +42,11 @@ def generate_wast_files(llvm_bin, emscripten_root):
4242
obj_path = os.path.join(lld_path, obj_file)
4343

4444
wasm_file = src_file.replace(ext, '.wasm')
45-
wast_file = src_file.replace(ext, '.wast')
45+
wat_file = src_file.replace(ext, '.wat')
4646

4747
obj_path = os.path.join(lld_path, obj_file)
4848
wasm_path = os.path.join(lld_path, wasm_file)
49-
wast_path = os.path.join(lld_path, wast_file)
49+
wat_path = os.path.join(lld_path, wat_file)
5050
is_shared = 'shared' in src_file
5151

5252
compile_cmd = [
@@ -70,6 +70,10 @@ def generate_wast_files(llvm_bin, emscripten_root):
7070
'--export', '__data_end',
7171
'--global-base=568',
7272
]
73+
# We had a regression where this test only worked if debug names
74+
# were included.
75+
if 'longjmp' in src_file:
76+
link_cmd.append('--strip-debug')
7377
if is_shared:
7478
compile_cmd.append('-fPIC')
7579
compile_cmd.append('-fvisibility=default')
@@ -80,7 +84,7 @@ def generate_wast_files(llvm_bin, emscripten_root):
8084
try:
8185
support.run_command(compile_cmd)
8286
support.run_command(link_cmd)
83-
support.run_command(shared.WASM_DIS + [wasm_path, '-o', wast_path])
87+
support.run_command(shared.WASM_DIS + [wasm_path, '-o', wat_path])
8488
finally:
8589
# Don't need the .o or .wasm files, don't leave them around
8690
shared.delete_from_orbit(obj_path)
@@ -91,4 +95,4 @@ def generate_wast_files(llvm_bin, emscripten_root):
9195
if len(shared.options.positional_args) != 2:
9296
print('Usage: generate_lld_tests.py [llvm/bin/dir] [path/to/emscripten]')
9397
sys.exit(1)
94-
generate_wast_files(*shared.options.positional_args)
98+
generate_wat_files(*shared.options.positional_args)

‎scripts/test/shared.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ def parse_args(args):
8787

8888
options = parse_args(sys.argv[1:])
8989
requested = options.positional_args
90+
script_dir = os.path.dirname(os.path.abspath(__file__))
9091

9192
num_failures = 0
9293
warnings = []
@@ -123,8 +124,7 @@ def warn(text):
123124

124125
# Locate Binaryen source directory if not specified.
125126
if not options.binaryen_root:
126-
path_parts = os.path.abspath(__file__).split(os.path.sep)
127-
options.binaryen_root = os.path.sep.join(path_parts[:-3])
127+
options.binaryen_root = os.path.dirname(os.path.dirname(script_dir))
128128

129129
options.binaryen_test = os.path.join(options.binaryen_root, 'test')
130130

@@ -206,8 +206,7 @@ def wrap_with_valgrind(cmd):
206206

207207

208208
def in_binaryen(*args):
209-
__rootpath__ = os.path.abspath(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
210-
return os.path.join(__rootpath__, *args)
209+
return os.path.join(options.binaryen_root, *args)
211210

212211

213212
os.environ['BINARYEN'] = in_binaryen()

‎src/wasm/wasm-emscripten.cpp

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -994,11 +994,11 @@ struct FixInvokeFunctionNamesWalker
994994
: public PostWalker<FixInvokeFunctionNamesWalker> {
995995
Module& wasm;
996996
std::map<Name, Name> importRenames;
997-
std::vector<Name> toRemove;
998-
std::set<Name> newImports;
997+
std::map<Name, Name> functionReplace;
999998
std::set<Signature> invokeSigs;
999+
ImportInfo imports;
10001000

1001-
FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm) {}
1001+
FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm), imports(wasm) {}
10021002

10031003
// Converts invoke wrapper names generated by LLVM backend to real invoke
10041004
// wrapper names that are expected by JavaScript glue code.
@@ -1053,27 +1053,38 @@ struct FixInvokeFunctionNamesWalker
10531053
return;
10541054
}
10551055

1056-
assert(importRenames.count(curr->name) == 0);
1057-
BYN_TRACE("renaming: " << curr->name << " -> " << newname << "\n");
1058-
importRenames[curr->name] = newname;
1059-
// Either rename or remove the existing import
1060-
if (wasm.getFunctionOrNull(newname) || !newImports.insert(newname).second) {
1061-
toRemove.push_back(curr->name);
1056+
BYN_TRACE("renaming import: " << curr->module << "." << curr->base << " ("
1057+
<< curr->name << ") -> " << newname << "\n");
1058+
assert(importRenames.count(curr->base) == 0);
1059+
importRenames[curr->base] = newname;
1060+
// Either rename the import, or replace it with an existing one
1061+
Function* existingFunc = imports.getImportedFunction(curr->module, newname);
1062+
if (existingFunc) {
1063+
BYN_TRACE("replacing with an existing import: " << existingFunc->name
1064+
<< "\n");
1065+
functionReplace[curr->name] = existingFunc->name;
10621066
} else {
1067+
BYN_TRACE("renaming the import in place\n");
10631068
curr->base = newname;
1064-
curr->name = newname;
10651069
}
10661070
}
10671071

10681072
void visitModule(Module* curr) {
1069-
for (auto importName : toRemove) {
1070-
wasm.removeFunction(importName);
1073+
// For each replaced function first remove the function itself then
1074+
// rename all uses to the point to the new function.
1075+
for (auto& pair : functionReplace) {
1076+
BYN_TRACE("removeFunction " << pair.first << "\n");
1077+
wasm.removeFunction(pair.first);
10711078
}
1072-
ModuleUtils::renameFunctions(wasm, importRenames);
1073-
ImportInfo imports(wasm);
1079+
// Rename all uses of the removed functions
1080+
ModuleUtils::renameFunctions(wasm, functionReplace);
1081+
1082+
// For imports that for renamed, update any associated GOT.func imports.
10741083
for (auto& pair : importRenames) {
1075-
// Update any associated GOT.func import.
1084+
BYN_TRACE("looking for: GOT.func." << pair.first << "\n");
10761085
if (auto g = imports.getImportedGlobal("GOT.func", pair.first)) {
1086+
BYN_TRACE("renaming corresponding GOT entry: " << g->base << " -> "
1087+
<< pair.second << "\n");
10771088
g->base = pair.second;
10781089
}
10791090
}

‎test/lld/longjmp.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
typedef struct jmp_buf_buf {
2+
int thing;
3+
} jmp_buf;
4+
5+
void longjmp(jmp_buf env, int val);
6+
int setjmp(jmp_buf env);
7+
8+
int __THREW__;
9+
int __threwValue;
10+
11+
int main() {
12+
jmp_buf jmp;
13+
if (setjmp(jmp) == 0) {
14+
longjmp(jmp, 1);
15+
}
16+
return 0;
17+
}

‎test/lld/longjmp.wat

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
(module
2+
(type $i32_=>_none (func (param i32)))
3+
(type $i32_i32_=>_none (func (param i32 i32)))
4+
(type $none_=>_i32 (func (result i32)))
5+
(type $none_=>_none (func))
6+
(type $i32_i32_i32_=>_none (func (param i32 i32 i32)))
7+
(type $i32_=>_i32 (func (param i32) (result i32)))
8+
(type $i32_i32_=>_i32 (func (param i32 i32) (result i32)))
9+
(type $i32_i32_i32_=>_i32 (func (param i32 i32 i32) (result i32)))
10+
(type $i32_i32_i32_i32_=>_i32 (func (param i32 i32 i32 i32) (result i32)))
11+
(import "env" "malloc" (func $fimport$0 (param i32) (result i32)))
12+
(import "env" "saveSetjmp" (func $fimport$1 (param i32 i32 i32 i32) (result i32)))
13+
(import "env" "getTempRet0" (func $fimport$2 (result i32)))
14+
(import "env" "emscripten_longjmp_jmpbuf" (func $fimport$3 (param i32 i32)))
15+
(import "env" "__invoke_void_i32_i32" (func $fimport$4 (param i32 i32 i32)))
16+
(import "env" "testSetjmp" (func $fimport$5 (param i32 i32 i32) (result i32)))
17+
(import "env" "setTempRet0" (func $fimport$6 (param i32)))
18+
(import "env" "free" (func $fimport$7 (param i32)))
19+
(import "env" "emscripten_longjmp" (func $fimport$8 (param i32 i32)))
20+
(memory $0 2)
21+
(table $0 2 2 funcref)
22+
(elem (i32.const 1) $fimport$3)
23+
(global $global$0 (mut i32) (i32.const 66112))
24+
(global $global$1 i32 (i32.const 576))
25+
(export "memory" (memory $0))
26+
(export "__wasm_call_ctors" (func $0))
27+
(export "main" (func $2))
28+
(export "__data_end" (global $global$1))
29+
(func $0 (; 9 ;)
30+
)
31+
(func $1 (; 10 ;) (result i32)
32+
(local $0 i32)
33+
(local $1 i32)
34+
(local $2 i32)
35+
(local $3 i32)
36+
(i32.store
37+
(local.tee $0
38+
(call $fimport$0
39+
(i32.const 40)
40+
)
41+
)
42+
(i32.const 0)
43+
)
44+
(local.set $1
45+
(call $fimport$1
46+
(local.get $0)
47+
(i32.const 1)
48+
(local.get $0)
49+
(i32.const 4)
50+
)
51+
)
52+
(local.set $2
53+
(call $fimport$2)
54+
)
55+
(local.set $0
56+
(i32.const 0)
57+
)
58+
(block $label$1
59+
(block $label$2
60+
(loop $label$3
61+
(br_if $label$2
62+
(local.get $0)
63+
)
64+
(i32.store offset=568
65+
(i32.const 0)
66+
(i32.const 0)
67+
)
68+
(call $fimport$4
69+
(i32.const 1)
70+
(local.get $0)
71+
(i32.const 1)
72+
)
73+
(local.set $0
74+
(i32.load offset=568
75+
(i32.const 0)
76+
)
77+
)
78+
(i32.store offset=568
79+
(i32.const 0)
80+
(i32.const 0)
81+
)
82+
(block $label$4
83+
(br_if $label$4
84+
(i32.eqz
85+
(local.get $0)
86+
)
87+
)
88+
(br_if $label$4
89+
(i32.eqz
90+
(local.tee $3
91+
(i32.load offset=572
92+
(i32.const 0)
93+
)
94+
)
95+
)
96+
)
97+
(br_if $label$1
98+
(i32.eqz
99+
(call $fimport$5
100+
(i32.load
101+
(local.get $0)
102+
)
103+
(local.get $1)
104+
(local.get $2)
105+
)
106+
)
107+
)
108+
(call $fimport$6
109+
(local.get $3)
110+
)
111+
)
112+
(local.set $0
113+
(call $fimport$2)
114+
)
115+
(br $label$3)
116+
)
117+
)
118+
(call $fimport$7
119+
(local.get $1)
120+
)
121+
(return
122+
(i32.const 0)
123+
)
124+
)
125+
(call $fimport$8
126+
(local.get $0)
127+
(local.get $3)
128+
)
129+
(unreachable)
130+
)
131+
(func $2 (; 11 ;) (param $0 i32) (param $1 i32) (result i32)
132+
(call $1)
133+
)
134+
;; custom section "producers", size 112
135+
)
136+

0 commit comments

Comments
 (0)