Skip to content

Commit f154364

Browse files
kripkenkripken
authored
Bysyncify: Assertion improvements (#2193)
Add assertions on stack overflow in all 4 Bysyncify API calls (previously only 2 did it). Also add a check that those assertions are hit.
1 parent c3e45d4 commit f154364

9 files changed

+375
-132
lines changed

‎src/passes/Bysyncify.cpp‎

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,17 @@
174174
// we are recreating the call stack. At that point you should call
175175
// bysyncify_stop_rewind and then execution can resume normally.
176176
//
177+
// Note that the bysyncify_* API calls will verify that the data structure
178+
// is valid, checking if the current location is past the end. If so, they
179+
// will throw a wasm unreachable. No check is done during unwinding or
180+
// rewinding, to keep things fast - in principle, from when unwinding begins
181+
// and until it ends there should be no memory accesses aside from the
182+
// bysyncify code itself (and likewise when rewinding), so there should be
183+
// no chance of memory corruption causing odd errors. However, the low
184+
// overhead of this approach does cause an error only to be shown when
185+
// unwinding/rewinding finishes, and not at the specific spot where the
186+
// stack end was exceeded.
187+
//
177188
// By default this pass is very carefuly: it assumes that any import and
178189
// any indirect call may start an unwind/rewind operation. If you know more
179190
// specific information you can inform bysyncify about that, which can reduce
@@ -1056,57 +1067,47 @@ struct Bysyncify : public Pass {
10561067

10571068
void addFunctions(Module* module) {
10581069
Builder builder(*module);
1059-
auto makeFunction = [&](Name name,
1060-
bool setData,
1061-
State state,
1062-
Expression* extra) {
1070+
auto makeFunction = [&](Name name, bool setData, State state) {
10631071
std::vector<Type> params;
10641072
if (setData) {
10651073
params.push_back(i32);
10661074
}
10671075
auto* body = builder.makeBlock();
1068-
if (setData) {
1069-
// Verify the data is valid.
1070-
auto* stackPos = builder.makeLoad(4,
1071-
false,
1072-
int32_t(DataOffset::BStackPos),
1073-
4,
1074-
builder.makeLocalGet(0, i32),
1075-
i32);
1076-
auto* stackEnd = builder.makeLoad(4,
1077-
false,
1078-
int32_t(DataOffset::BStackEnd),
1079-
4,
1080-
builder.makeLocalGet(0, i32),
1081-
i32);
1082-
body->list.push_back(
1083-
builder.makeIf(builder.makeBinary(GtUInt32, stackPos, stackEnd),
1084-
builder.makeUnreachable()));
1085-
}
10861076
body->list.push_back(builder.makeGlobalSet(
10871077
BYSYNCIFY_STATE, builder.makeConst(Literal(int32_t(state)))));
1088-
if (extra) {
1089-
body->list.push_back(extra);
1078+
if (setData) {
1079+
body->list.push_back(
1080+
builder.makeGlobalSet(BYSYNCIFY_DATA, builder.makeLocalGet(0, i32)));
10901081
}
1082+
// Verify the data is valid.
1083+
auto* stackPos =
1084+
builder.makeLoad(4,
1085+
false,
1086+
int32_t(DataOffset::BStackPos),
1087+
4,
1088+
builder.makeGlobalGet(BYSYNCIFY_DATA, i32),
1089+
i32);
1090+
auto* stackEnd =
1091+
builder.makeLoad(4,
1092+
false,
1093+
int32_t(DataOffset::BStackEnd),
1094+
4,
1095+
builder.makeGlobalGet(BYSYNCIFY_DATA, i32),
1096+
i32);
1097+
body->list.push_back(
1098+
builder.makeIf(builder.makeBinary(GtUInt32, stackPos, stackEnd),
1099+
builder.makeUnreachable()));
10911100
body->finalize();
10921101
auto* func = builder.makeFunction(
10931102
name, std::move(params), none, std::vector<Type>{}, body);
10941103
module->addFunction(func);
10951104
module->addExport(builder.makeExport(name, name, ExternalKind::Function));
10961105
};
10971106

1098-
makeFunction(
1099-
BYSYNCIFY_START_UNWIND,
1100-
true,
1101-
State::Unwinding,
1102-
builder.makeGlobalSet(BYSYNCIFY_DATA, builder.makeLocalGet(0, i32)));
1103-
makeFunction(BYSYNCIFY_STOP_UNWIND, false, State::Normal, nullptr);
1104-
makeFunction(
1105-
BYSYNCIFY_START_REWIND,
1106-
true,
1107-
State::Rewinding,
1108-
builder.makeGlobalSet(BYSYNCIFY_DATA, builder.makeLocalGet(0, i32)));
1109-
makeFunction(BYSYNCIFY_STOP_REWIND, false, State::Normal, nullptr);
1107+
makeFunction(BYSYNCIFY_START_UNWIND, true, State::Unwinding);
1108+
makeFunction(BYSYNCIFY_STOP_UNWIND, false, State::Normal);
1109+
makeFunction(BYSYNCIFY_START_REWIND, true, State::Rewinding);
1110+
makeFunction(BYSYNCIFY_STOP_REWIND, false, State::Normal);
11101111
}
11111112
};
11121113

‎test/passes/bysyncify.txt‎

Lines changed: 102 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -297,52 +297,74 @@
297297
(nop)
298298
)
299299
(func $bysyncify_start_unwind (; 6 ;) (param $0 i32)
300+
(global.set $__bysyncify_state
301+
(i32.const 1)
302+
)
303+
(global.set $__bysyncify_data
304+
(local.get $0)
305+
)
300306
(if
301307
(i32.gt_u
302308
(i32.load
303-
(local.get $0)
309+
(global.get $__bysyncify_data)
304310
)
305311
(i32.load offset=4
306-
(local.get $0)
312+
(global.get $__bysyncify_data)
307313
)
308314
)
309315
(unreachable)
310316
)
311-
(global.set $__bysyncify_state
312-
(i32.const 1)
313-
)
314-
(global.set $__bysyncify_data
315-
(local.get $0)
316-
)
317317
)
318318
(func $bysyncify_stop_unwind (; 7 ;)
319319
(global.set $__bysyncify_state
320320
(i32.const 0)
321321
)
322-
)
323-
(func $bysyncify_start_rewind (; 8 ;) (param $0 i32)
324322
(if
325323
(i32.gt_u
326324
(i32.load
327-
(local.get $0)
325+
(global.get $__bysyncify_data)
328326
)
329327
(i32.load offset=4
330-
(local.get $0)
328+
(global.get $__bysyncify_data)
331329
)
332330
)
333331
(unreachable)
334332
)
333+
)
334+
(func $bysyncify_start_rewind (; 8 ;) (param $0 i32)
335335
(global.set $__bysyncify_state
336336
(i32.const 2)
337337
)
338338
(global.set $__bysyncify_data
339339
(local.get $0)
340340
)
341+
(if
342+
(i32.gt_u
343+
(i32.load
344+
(global.get $__bysyncify_data)
345+
)
346+
(i32.load offset=4
347+
(global.get $__bysyncify_data)
348+
)
349+
)
350+
(unreachable)
351+
)
341352
)
342353
(func $bysyncify_stop_rewind (; 9 ;)
343354
(global.set $__bysyncify_state
344355
(i32.const 0)
345356
)
357+
(if
358+
(i32.gt_u
359+
(i32.load
360+
(global.get $__bysyncify_data)
361+
)
362+
(i32.load offset=4
363+
(global.get $__bysyncify_data)
364+
)
365+
)
366+
(unreachable)
367+
)
346368
)
347369
)
348370
(module
@@ -2808,52 +2830,74 @@
28082830
(nop)
28092831
)
28102832
(func $bysyncify_start_unwind (; 19 ;) (param $0 i32)
2833+
(global.set $__bysyncify_state
2834+
(i32.const 1)
2835+
)
2836+
(global.set $__bysyncify_data
2837+
(local.get $0)
2838+
)
28112839
(if
28122840
(i32.gt_u
28132841
(i32.load
2814-
(local.get $0)
2842+
(global.get $__bysyncify_data)
28152843
)
28162844
(i32.load offset=4
2817-
(local.get $0)
2845+
(global.get $__bysyncify_data)
28182846
)
28192847
)
28202848
(unreachable)
28212849
)
2822-
(global.set $__bysyncify_state
2823-
(i32.const 1)
2824-
)
2825-
(global.set $__bysyncify_data
2826-
(local.get $0)
2827-
)
28282850
)
28292851
(func $bysyncify_stop_unwind (; 20 ;)
28302852
(global.set $__bysyncify_state
28312853
(i32.const 0)
28322854
)
2833-
)
2834-
(func $bysyncify_start_rewind (; 21 ;) (param $0 i32)
28352855
(if
28362856
(i32.gt_u
28372857
(i32.load
2838-
(local.get $0)
2858+
(global.get $__bysyncify_data)
28392859
)
28402860
(i32.load offset=4
2841-
(local.get $0)
2861+
(global.get $__bysyncify_data)
28422862
)
28432863
)
28442864
(unreachable)
28452865
)
2866+
)
2867+
(func $bysyncify_start_rewind (; 21 ;) (param $0 i32)
28462868
(global.set $__bysyncify_state
28472869
(i32.const 2)
28482870
)
28492871
(global.set $__bysyncify_data
28502872
(local.get $0)
28512873
)
2874+
(if
2875+
(i32.gt_u
2876+
(i32.load
2877+
(global.get $__bysyncify_data)
2878+
)
2879+
(i32.load offset=4
2880+
(global.get $__bysyncify_data)
2881+
)
2882+
)
2883+
(unreachable)
2884+
)
28522885
)
28532886
(func $bysyncify_stop_rewind (; 22 ;)
28542887
(global.set $__bysyncify_state
28552888
(i32.const 0)
28562889
)
2890+
(if
2891+
(i32.gt_u
2892+
(i32.load
2893+
(global.get $__bysyncify_data)
2894+
)
2895+
(i32.load offset=4
2896+
(global.get $__bysyncify_data)
2897+
)
2898+
)
2899+
(unreachable)
2900+
)
28572901
)
28582902
)
28592903
(module
@@ -2865,51 +2909,73 @@
28652909
(export "bysyncify_start_rewind" (func $bysyncify_start_rewind))
28662910
(export "bysyncify_stop_rewind" (func $bysyncify_stop_rewind))
28672911
(func $bysyncify_start_unwind (; 0 ;) (param $0 i32)
2912+
(global.set $__bysyncify_state
2913+
(i32.const 1)
2914+
)
2915+
(global.set $__bysyncify_data
2916+
(local.get $0)
2917+
)
28682918
(if
28692919
(i32.gt_u
28702920
(i32.load
2871-
(local.get $0)
2921+
(global.get $__bysyncify_data)
28722922
)
28732923
(i32.load offset=4
2874-
(local.get $0)
2924+
(global.get $__bysyncify_data)
28752925
)
28762926
)
28772927
(unreachable)
28782928
)
2879-
(global.set $__bysyncify_state
2880-
(i32.const 1)
2881-
)
2882-
(global.set $__bysyncify_data
2883-
(local.get $0)
2884-
)
28852929
)
28862930
(func $bysyncify_stop_unwind (; 1 ;)
28872931
(global.set $__bysyncify_state
28882932
(i32.const 0)
28892933
)
2890-
)
2891-
(func $bysyncify_start_rewind (; 2 ;) (param $0 i32)
28922934
(if
28932935
(i32.gt_u
28942936
(i32.load
2895-
(local.get $0)
2937+
(global.get $__bysyncify_data)
28962938
)
28972939
(i32.load offset=4
2898-
(local.get $0)
2940+
(global.get $__bysyncify_data)
28992941
)
29002942
)
29012943
(unreachable)
29022944
)
2945+
)
2946+
(func $bysyncify_start_rewind (; 2 ;) (param $0 i32)
29032947
(global.set $__bysyncify_state
29042948
(i32.const 2)
29052949
)
29062950
(global.set $__bysyncify_data
29072951
(local.get $0)
29082952
)
2953+
(if
2954+
(i32.gt_u
2955+
(i32.load
2956+
(global.get $__bysyncify_data)
2957+
)
2958+
(i32.load offset=4
2959+
(global.get $__bysyncify_data)
2960+
)
2961+
)
2962+
(unreachable)
2963+
)
29092964
)
29102965
(func $bysyncify_stop_rewind (; 3 ;)
29112966
(global.set $__bysyncify_state
29122967
(i32.const 0)
29132968
)
2969+
(if
2970+
(i32.gt_u
2971+
(i32.load
2972+
(global.get $__bysyncify_data)
2973+
)
2974+
(i32.load offset=4
2975+
(global.get $__bysyncify_data)
2976+
)
2977+
)
2978+
(unreachable)
2979+
)
29142980
)
29152981
)

0 commit comments

Comments
 (0)