Skip to content

Commit 6a9acea

Browse files
kripkenkripken
authored
Add asserts in Asyncify (#2338)
With the optional asserts, we throw if we see an unwind begin in code that we thought could never unwind (say, because the user incorrectly blacklisted it). Helps with emscripten-core/emscripten#9389
1 parent 19bc69a commit 6a9acea

3 files changed

Lines changed: 316 additions & 7 deletions

‎src/passes/Asyncify.cpp‎

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,12 @@
241241
//
242242
// As with --asyncify-imports, you can use a response file here.
243243
//
244+
// --pass-arg=asyncify-asserts
245+
//
246+
// This enables extra asserts in the output, like checking if we in
247+
// an unwind/rewind in an invalid place (this can be helpful for manual
248+
// tweaking of the whitelist/blacklist).
249+
//
244250
// TODO When wasm has GC, extending the live ranges of locals can keep things
245251
// alive unnecessarily. We may want to set locals to null at the end
246252
// of their original range.
@@ -363,9 +369,10 @@ class ModuleAnalyzer {
363369
std::function<bool(Name, Name)> canImportChangeState,
364370
bool canIndirectChangeState,
365371
const String::Split& blacklistInput,
366-
const String::Split& whitelistInput)
372+
const String::Split& whitelistInput,
373+
bool asserts)
367374
: module(module), canIndirectChangeState(canIndirectChangeState),
368-
globals(module) {
375+
globals(module), asserts(asserts) {
369376

370377
blacklist.insert(blacklistInput.begin(), blacklistInput.end());
371378
whitelist.insert(whitelistInput.begin(), whitelistInput.end());
@@ -561,6 +568,7 @@ class ModuleAnalyzer {
561568
GlobalHelper globals;
562569
std::set<Name> blacklist;
563570
std::set<Name> whitelist;
571+
bool asserts;
564572
};
565573

566574
// Checks if something performs a call: either a direct or indirect call,
@@ -606,6 +614,10 @@ class AsyncifyBuilder : public Builder {
606614
makeGlobalGet(ASYNCIFY_STATE, i32),
607615
makeConst(Literal(int32_t(value))));
608616
}
617+
618+
Expression* makeNegatedStateCheck(State value) {
619+
return makeUnary(EqZInt32, makeStateCheck(value));
620+
}
609621
};
610622

611623
// Instrument control flow, around calls and adding skips for rewinding.
@@ -622,13 +634,16 @@ struct AsyncifyFlow : public Pass {
622634
runOnFunction(PassRunner* runner, Module* module_, Function* func_) override {
623635
module = module_;
624636
func = func_;
637+
builder = make_unique<AsyncifyBuilder>(*module);
625638
// If the function cannot change our state, we have nothing to do -
626639
// we will never unwind or rewind the stack here.
627640
if (!analyzer->needsInstrumentation(func)) {
641+
if (analyzer->asserts) {
642+
addAssertsInNonInstrumented(func);
643+
}
628644
return;
629645
}
630646
// Rewrite the function body.
631-
builder = make_unique<AsyncifyBuilder>(*module);
632647
// Each function we enter will pop one from the stack, which is the index
633648
// of the next call to make.
634649
auto* block = builder->makeBlock(
@@ -841,6 +856,65 @@ struct AsyncifyFlow : public Pass {
841856
// don't want it to be seen by asyncify itself.
842857
return builder->makeCall(ASYNCIFY_GET_CALL_INDEX, {}, none);
843858
}
859+
860+
// Given a function that is not instrumented - because we proved it doesn't
861+
// need it, or depending on the whitelist/blacklist - add assertions that
862+
// verify that property at runtime.
863+
// Note that it is ok to run code while sleeping (if you are careful not
864+
// to break assumptions in the program!) - so what is actually
865+
// checked here is if the state *changes* in an uninstrumented function.
866+
// That is, if in an uninstrumented function, a sleep should not begin
867+
// from any call.
868+
void addAssertsInNonInstrumented(Function* func) {
869+
auto oldState = builder->addVar(func, i32);
870+
// Add a check at the function entry.
871+
func->body = builder->makeSequence(
872+
builder->makeLocalSet(oldState,
873+
builder->makeGlobalGet(ASYNCIFY_STATE, i32)),
874+
func->body);
875+
// Add a check around every call.
876+
struct Walker : PostWalker<Walker> {
877+
void visitCall(Call* curr) {
878+
// Tail calls will need another type of check, as they wouldn't reach
879+
// this assertion.
880+
assert(!curr->isReturn);
881+
handleCall(curr);
882+
}
883+
void visitCallIndirect(CallIndirect* curr) {
884+
// Tail calls will need another type of check, as they wouldn't reach
885+
// this assertion.
886+
assert(!curr->isReturn);
887+
handleCall(curr);
888+
}
889+
void handleCall(Expression* call) {
890+
auto* check = builder->makeIf(
891+
builder->makeBinary(NeInt32,
892+
builder->makeGlobalGet(ASYNCIFY_STATE, i32),
893+
builder->makeLocalGet(oldState, i32)),
894+
builder->makeUnreachable());
895+
Expression* rep;
896+
if (isConcreteType(call->type)) {
897+
auto temp = builder->addVar(func, call->type);
898+
rep = builder->makeBlock({
899+
builder->makeLocalSet(temp, call),
900+
check,
901+
builder->makeLocalGet(temp, call->type),
902+
});
903+
} else {
904+
rep = builder->makeSequence(call, check);
905+
}
906+
replaceCurrent(rep);
907+
}
908+
Function* func;
909+
AsyncifyBuilder* builder;
910+
Index oldState;
911+
};
912+
Walker walker;
913+
walker.func = func;
914+
walker.builder = builder.get();
915+
walker.oldState = oldState;
916+
walker.walk(func->body);
917+
}
844918
};
845919

846920
// Instrument local saving/restoring.
@@ -1048,8 +1122,8 @@ struct Asyncify : public Pass {
10481122
bool allImportsCanChangeState =
10491123
stateChangingImports == "" && ignoreImports == "";
10501124
String::Split listedImports(stateChangingImports, ",");
1051-
auto ignoreIndirect =
1052-
runner->options.getArgumentOrDefault("asyncify-ignore-indirect", "");
1125+
auto ignoreIndirect = runner->options.getArgumentOrDefault(
1126+
"asyncify-ignore-indirect", "") == "";
10531127
String::Split blacklist(
10541128
String::trim(read_possible_response_file(
10551129
runner->options.getArgumentOrDefault("asyncify-blacklist", ""))),
@@ -1058,6 +1132,8 @@ struct Asyncify : public Pass {
10581132
String::trim(read_possible_response_file(
10591133
runner->options.getArgumentOrDefault("asyncify-whitelist", ""))),
10601134
",");
1135+
auto asserts =
1136+
runner->options.getArgumentOrDefault("asyncify-asserts", "") != "";
10611137

10621138
blacklist = handleBracketingOperators(blacklist);
10631139
whitelist = handleBracketingOperators(whitelist);
@@ -1105,9 +1181,10 @@ struct Asyncify : public Pass {
11051181
// Scan the module.
11061182
ModuleAnalyzer analyzer(*module,
11071183
canImportChangeState,
1108-
ignoreIndirect == "",
1184+
ignoreIndirect,
11091185
blacklist,
1110-
whitelist);
1186+
whitelist,
1187+
asserts);
11111188

11121189
// Add necessary globals before we emit code to use them.
11131190
addGlobals(module);

‎test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-whitelist@waka.txt‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
(module
2+
(type $f (func))
3+
(type $FUNCSIG$i (func (result i32)))
4+
(type $FUNCSIG$vi (func (param i32)))
5+
(import "env" "import" (func $import))
6+
(import "env" "import2" (func $import2 (result i32)))
7+
(import "env" "import3" (func $import3 (param i32)))
8+
(memory $0 1 2)
9+
(table $0 2 2 funcref)
10+
(elem (i32.const 0) $calls-import2-drop $calls-import2-drop)
11+
(global $__asyncify_state (mut i32) (i32.const 0))
12+
(global $__asyncify_data (mut i32) (i32.const 0))
13+
(export "asyncify_start_unwind" (func $asyncify_start_unwind))
14+
(export "asyncify_stop_unwind" (func $asyncify_stop_unwind))
15+
(export "asyncify_start_rewind" (func $asyncify_start_rewind))
16+
(export "asyncify_stop_rewind" (func $asyncify_stop_rewind))
17+
(func $calls-import (; 3 ;) (type $f)
18+
(local $0 i32)
19+
(local.set $0
20+
(global.get $__asyncify_state)
21+
)
22+
(block
23+
(block
24+
(call $import)
25+
(if
26+
(i32.ne
27+
(global.get $__asyncify_state)
28+
(local.get $0)
29+
)
30+
(unreachable)
31+
)
32+
)
33+
(nop)
34+
)
35+
)
36+
(func $calls-import2-drop (; 4 ;) (type $f)
37+
(local $0 i32)
38+
(local $1 i32)
39+
(local $2 i32)
40+
(local.set $1
41+
(global.get $__asyncify_state)
42+
)
43+
(block
44+
(local.set $0
45+
(block (result i32)
46+
(local.set $2
47+
(call $import2)
48+
)
49+
(if
50+
(i32.ne
51+
(global.get $__asyncify_state)
52+
(local.get $1)
53+
)
54+
(unreachable)
55+
)
56+
(local.get $2)
57+
)
58+
)
59+
(drop
60+
(local.get $0)
61+
)
62+
(nop)
63+
)
64+
)
65+
(func $returns (; 5 ;) (type $FUNCSIG$i) (result i32)
66+
(local $x i32)
67+
(local $1 i32)
68+
(local $2 i32)
69+
(local $3 i32)
70+
(local $4 i32)
71+
(local $5 i32)
72+
(local $6 i32)
73+
(local.set $5
74+
(global.get $__asyncify_state)
75+
)
76+
(block
77+
(block
78+
(local.set $1
79+
(block (result i32)
80+
(local.set $6
81+
(call $import2)
82+
)
83+
(if
84+
(i32.ne
85+
(global.get $__asyncify_state)
86+
(local.get $5)
87+
)
88+
(unreachable)
89+
)
90+
(local.get $6)
91+
)
92+
)
93+
(local.set $x
94+
(local.get $1)
95+
)
96+
(nop)
97+
(local.set $2
98+
(local.get $x)
99+
)
100+
(local.set $3
101+
(local.get $2)
102+
)
103+
)
104+
(local.set $4
105+
(local.get $3)
106+
)
107+
(return
108+
(local.get $4)
109+
)
110+
)
111+
)
112+
(func $calls-indirect (; 6 ;) (type $FUNCSIG$vi) (param $x i32)
113+
(local $1 i32)
114+
(local $2 i32)
115+
(local.set $2
116+
(global.get $__asyncify_state)
117+
)
118+
(block
119+
(local.set $1
120+
(local.get $x)
121+
)
122+
(block
123+
(call_indirect (type $f)
124+
(local.get $1)
125+
)
126+
(if
127+
(i32.ne
128+
(global.get $__asyncify_state)
129+
(local.get $2)
130+
)
131+
(unreachable)
132+
)
133+
)
134+
(nop)
135+
)
136+
)
137+
(func $asyncify_start_unwind (; 7 ;) (param $0 i32)
138+
(global.set $__asyncify_state
139+
(i32.const 1)
140+
)
141+
(global.set $__asyncify_data
142+
(local.get $0)
143+
)
144+
(if
145+
(i32.gt_u
146+
(i32.load
147+
(global.get $__asyncify_data)
148+
)
149+
(i32.load offset=4
150+
(global.get $__asyncify_data)
151+
)
152+
)
153+
(unreachable)
154+
)
155+
)
156+
(func $asyncify_stop_unwind (; 8 ;)
157+
(global.set $__asyncify_state
158+
(i32.const 0)
159+
)
160+
(if
161+
(i32.gt_u
162+
(i32.load
163+
(global.get $__asyncify_data)
164+
)
165+
(i32.load offset=4
166+
(global.get $__asyncify_data)
167+
)
168+
)
169+
(unreachable)
170+
)
171+
)
172+
(func $asyncify_start_rewind (; 9 ;) (param $0 i32)
173+
(global.set $__asyncify_state
174+
(i32.const 2)
175+
)
176+
(global.set $__asyncify_data
177+
(local.get $0)
178+
)
179+
(if
180+
(i32.gt_u
181+
(i32.load
182+
(global.get $__asyncify_data)
183+
)
184+
(i32.load offset=4
185+
(global.get $__asyncify_data)
186+
)
187+
)
188+
(unreachable)
189+
)
190+
)
191+
(func $asyncify_stop_rewind (; 10 ;)
192+
(global.set $__asyncify_state
193+
(i32.const 0)
194+
)
195+
(if
196+
(i32.gt_u
197+
(i32.load
198+
(global.get $__asyncify_data)
199+
)
200+
(i32.load offset=4
201+
(global.get $__asyncify_data)
202+
)
203+
)
204+
(unreachable)
205+
)
206+
)
207+
)

‎test/passes/asyncify_pass-arg=asyncify-asserts_pass-arg=asyncify-whitelist@waka.wast‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
(module
2+
(memory 1 2)
3+
(type $f (func))
4+
(import "env" "import" (func $import))
5+
(import "env" "import2" (func $import2 (result i32)))
6+
(import "env" "import3" (func $import3 (param i32)))
7+
(table 1 1)
8+
(func $calls-import
9+
(call $import)
10+
)
11+
(func $calls-import2-drop
12+
(drop (call $import2))
13+
)
14+
(func $returns (result i32)
15+
(local $x i32)
16+
(local.set $x (call $import2))
17+
(local.get $x)
18+
)
19+
(func $calls-indirect (param $x i32)
20+
(call_indirect (type $f)
21+
(local.get $x)
22+
)
23+
)
24+
)
25+

0 commit comments

Comments
 (0)