Skip to content

Commit 7d229cf

Browse files
authored
wasm2c: Fix handling of locals in setjmp targets (#2479)
It is UB to read local variables after a call to `setjmp` returns, if those variables have been modified between `setjmp` and `longjmp`, unless they're marked as `volatile`. This marks them as `volatile`. Closes #2469
1 parent 84ea5d3 commit 7d229cf

File tree

2 files changed

+60
-14
lines changed

2 files changed

+60
-14
lines changed

src/c-writer.cc

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -426,11 +426,12 @@ class CWriter {
426426
void WriteImportProperties(CWriterPhase);
427427
void WriteFuncs();
428428
void BeginFunction(const Func&);
429-
void FinishFunction();
429+
void FinishFunction(size_t);
430430
void Write(const Func&);
431431
void WriteTailCallee(const Func&);
432432
void WriteParamsAndLocals();
433-
void WriteParams(const std::vector<std::string>& index_to_name);
433+
void WriteParams(const std::vector<std::string>& index_to_name,
434+
bool setjmp_safe = false);
434435
void WriteParamSymbols(const std::vector<std::string>& index_to_name);
435436
void WriteParamTypes(const FuncDeclaration& decl);
436437
void WriteLocals(const std::vector<std::string>& index_to_name);
@@ -444,7 +445,7 @@ class CWriter {
444445
void Unspill(const TypeVector&);
445446

446447
template <typename Vars, typename TypeOf, typename ToDo>
447-
void WriteVarsByType(const Vars&, const TypeOf&, const ToDo&);
448+
void WriteVarsByType(const Vars&, const TypeOf&, const ToDo&, bool);
448449

449450
template <typename sources>
450451
void Spill(const TypeVector&, const sources& src);
@@ -3043,15 +3044,15 @@ void CWriter::BeginFunction(const Func& func) {
30433044
Write(Newline());
30443045
}
30453046

3046-
void CWriter::FinishFunction() {
3047+
void CWriter::FinishFunction(size_t stack_var_section) {
30473048
for (size_t i = 0; i < func_sections_.size(); ++i) {
30483049
auto& [condition, stream] = func_sections_.at(i);
30493050
std::unique_ptr<OutputBuffer> buf = stream.ReleaseOutputBuffer();
30503051
if (condition.empty() || func_includes_.count(condition)) {
30513052
stream_->WriteData(buf->data.data(), buf->data.size());
30523053
}
30533054

3054-
if (i == 0) {
3055+
if (i == stack_var_section) {
30553056
WriteStackVarDeclarations(); // these come immediately after section #0
30563057
// (return type/name/params/locals)
30573058
}
@@ -3071,6 +3072,7 @@ void CWriter::Write(const Func& func) {
30713072
WriteParamsAndLocals();
30723073
Write("FUNC_PROLOGUE;", Newline());
30733074

3075+
size_t stack_vars_section = func_sections_.size() - 1;
30743076
PushFuncSection();
30753077

30763078
std::string label = DefineLabelName(kImplicitFuncLabel);
@@ -3094,13 +3096,14 @@ void CWriter::Write(const Func& func) {
30943096
}
30953097

30963098
stream_ = prev_stream;
3097-
FinishFunction();
3099+
FinishFunction(stack_vars_section);
30983100
}
30993101

31003102
template <typename Vars, typename TypeOf, typename ToDo>
31013103
void CWriter::WriteVarsByType(const Vars& vars,
31023104
const TypeOf& typeoffunc,
3103-
const ToDo& todo) {
3105+
const ToDo& todo,
3106+
bool setjmp_safe) {
31043107
for (Type type : {Type::I32, Type::I64, Type::F32, Type::F64, Type::V128,
31053108
Type::FuncRef, Type::ExternRef}) {
31063109
Index var_index = 0;
@@ -3109,6 +3112,11 @@ void CWriter::WriteVarsByType(const Vars& vars,
31093112
if (typeoffunc(var) == type) {
31103113
if (count == 0) {
31113114
Write(type, " ");
3115+
if (setjmp_safe) {
3116+
PushFuncSection("exceptions");
3117+
Write("volatile ");
3118+
PushFuncSection();
3119+
}
31123120
Indent(4);
31133121
} else {
31143122
Write(", ");
@@ -3144,14 +3152,15 @@ void CWriter::WriteTailCallee(const Func& func) {
31443152
if (func.GetNumParams()) {
31453153
WriteVarsByType(
31463154
func.decl.sig.param_types, [](auto x) { return x; },
3147-
[&](Index i, Type) { Write(DefineParamName(index_to_name[i])); });
3155+
[&](Index i, Type) { Write(DefineParamName(index_to_name[i])); }, true);
31483156
Write(OpenBrace(), func.decl.sig.param_types, " tmp;", Newline(),
31493157
"wasm_rt_memcpy(&tmp, tail_call_stack, sizeof(tmp));", Newline());
31503158
Unspill(func.decl.sig.param_types,
31513159
[&](auto i) { return ParamName(index_to_name[i]); });
31523160
Write(CloseBrace(), Newline());
31533161
}
31543162

3163+
size_t stack_vars_section = func_sections_.size() - 1;
31553164
WriteLocals(index_to_name);
31563165

31573166
PushFuncSection();
@@ -3175,19 +3184,20 @@ void CWriter::WriteTailCallee(const Func& func) {
31753184
Write("next->fn = NULL;", Newline());
31763185

31773186
stream_ = prev_stream;
3178-
FinishFunction();
3187+
FinishFunction(stack_vars_section);
31793188
}
31803189

31813190
void CWriter::WriteParamsAndLocals() {
31823191
std::vector<std::string> index_to_name;
31833192
MakeTypeBindingReverseMapping(func_->GetNumParamsAndLocals(), func_->bindings,
31843193
&index_to_name);
3185-
WriteParams(index_to_name);
3194+
WriteParams(index_to_name, true);
31863195
Write(" ", OpenBrace());
31873196
WriteLocals(index_to_name);
31883197
}
31893198

3190-
void CWriter::WriteParams(const std::vector<std::string>& index_to_name) {
3199+
void CWriter::WriteParams(const std::vector<std::string>& index_to_name,
3200+
bool setjmp_safe) {
31913201
Write(ModuleInstanceTypeName(), "* instance");
31923202
if (func_->GetNumParams() != 0) {
31933203
Indent(4);
@@ -3196,7 +3206,13 @@ void CWriter::WriteParams(const std::vector<std::string>& index_to_name) {
31963206
if (i != 0 && (i % 8) == 0) {
31973207
Write(Newline());
31983208
}
3199-
Write(func_->GetParamType(i), " ", DefineParamName(index_to_name[i]));
3209+
Write(func_->GetParamType(i), " ");
3210+
if (setjmp_safe) {
3211+
PushFuncSection("exceptions");
3212+
Write("volatile ");
3213+
PushFuncSection();
3214+
}
3215+
Write(DefineParamName(index_to_name[i]));
32003216
}
32013217
Dedent(4);
32023218
}
@@ -3240,13 +3256,17 @@ void CWriter::WriteLocals(const std::vector<std::string>& index_to_name) {
32403256
} else {
32413257
Write("0");
32423258
}
3243-
});
3259+
},
3260+
true);
32443261
}
32453262

32463263
void CWriter::WriteStackVarDeclarations() {
3264+
// NOTE: setjmp_safe must be false, can't push func sections in here because
3265+
// this is called from FinishFunction.
3266+
// (luckily, we don't need setjmp_safe for these.)
32473267
WriteVarsByType(
32483268
stack_var_sym_map_, [](auto stp_name) { return stp_name.first.second; },
3249-
[&](Index, auto& stp_name) { Write(stp_name.second); });
3269+
[&](Index, auto& stp_name) { Write(stp_name.second); }, false);
32503270
}
32513271

32523272
void CWriter::Write(const Block& block) {
@@ -3262,6 +3282,7 @@ void CWriter::Write(const Block& block) {
32623282
}
32633283

32643284
size_t CWriter::BeginTry(const TryExpr& tryexpr) {
3285+
func_includes_.insert("exceptions");
32653286
Write(OpenBrace()); /* beginning of try-catch */
32663287
const std::string tlabel = DefineLabelName(tryexpr.block.label);
32673288
Write("WASM_RT_UNWIND_TARGET *", tlabel,
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
;;; TOOL: run-spec-wasm2c
2+
;;; ARGS*: --enable-exceptions
3+
;;; NOTE: ref: issue-2469
4+
(module
5+
(tag $e0)
6+
(func $longjmp-bait (throw $e0))
7+
(func (export "setjmp-bait") (param $return-early i32) (result i32)
8+
(local $value i32)
9+
(try $try
10+
(do
11+
(br_if $try (local.get $return-early))
12+
(local.set $value (i32.const 1))
13+
(call $longjmp-bait)
14+
)
15+
(catch $e0)
16+
)
17+
(local.get $value)
18+
)
19+
)
20+
21+
(assert_return (invoke "setjmp-bait" (i32.const 1)) (i32.const 0))
22+
(assert_return (invoke "setjmp-bait" (i32.const 0)) (i32.const 1))
23+
(;; STDOUT ;;;
24+
2/2 tests passed.
25+
;;; STDOUT ;;)

0 commit comments

Comments
 (0)