Skip to content

Commit

Permalink
wasm2c: Fix handling of locals in setjmp targets (#2479)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@SoniEx2
SoniEx2 authored Oct 2, 2024
1 parent 84ea5d3 commit 7d229cf
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 14 deletions.
49 changes: 35 additions & 14 deletions src/c-writer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,11 +426,12 @@ class CWriter {
void WriteImportProperties(CWriterPhase);
void WriteFuncs();
void BeginFunction(const Func&);
void FinishFunction();
void FinishFunction(size_t);
void Write(const Func&);
void WriteTailCallee(const Func&);
void WriteParamsAndLocals();
void WriteParams(const std::vector<std::string>& index_to_name);
void WriteParams(const std::vector<std::string>& index_to_name,
bool setjmp_safe = false);
void WriteParamSymbols(const std::vector<std::string>& index_to_name);
void WriteParamTypes(const FuncDeclaration& decl);
void WriteLocals(const std::vector<std::string>& index_to_name);
Expand All @@ -444,7 +445,7 @@ class CWriter {
void Unspill(const TypeVector&);

template <typename Vars, typename TypeOf, typename ToDo>
void WriteVarsByType(const Vars&, const TypeOf&, const ToDo&);
void WriteVarsByType(const Vars&, const TypeOf&, const ToDo&, bool);

template <typename sources>
void Spill(const TypeVector&, const sources& src);
Expand Down Expand Up @@ -3043,15 +3044,15 @@ void CWriter::BeginFunction(const Func& func) {
Write(Newline());
}

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

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

size_t stack_vars_section = func_sections_.size() - 1;
PushFuncSection();

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

stream_ = prev_stream;
FinishFunction();
FinishFunction(stack_vars_section);
}

template <typename Vars, typename TypeOf, typename ToDo>
void CWriter::WriteVarsByType(const Vars& vars,
const TypeOf& typeoffunc,
const ToDo& todo) {
const ToDo& todo,
bool setjmp_safe) {
for (Type type : {Type::I32, Type::I64, Type::F32, Type::F64, Type::V128,
Type::FuncRef, Type::ExternRef}) {
Index var_index = 0;
Expand All @@ -3109,6 +3112,11 @@ void CWriter::WriteVarsByType(const Vars& vars,
if (typeoffunc(var) == type) {
if (count == 0) {
Write(type, " ");
if (setjmp_safe) {
PushFuncSection("exceptions");
Write("volatile ");
PushFuncSection();
}
Indent(4);
} else {
Write(", ");
Expand Down Expand Up @@ -3144,14 +3152,15 @@ void CWriter::WriteTailCallee(const Func& func) {
if (func.GetNumParams()) {
WriteVarsByType(
func.decl.sig.param_types, [](auto x) { return x; },
[&](Index i, Type) { Write(DefineParamName(index_to_name[i])); });
[&](Index i, Type) { Write(DefineParamName(index_to_name[i])); }, true);
Write(OpenBrace(), func.decl.sig.param_types, " tmp;", Newline(),
"wasm_rt_memcpy(&tmp, tail_call_stack, sizeof(tmp));", Newline());
Unspill(func.decl.sig.param_types,
[&](auto i) { return ParamName(index_to_name[i]); });
Write(CloseBrace(), Newline());
}

size_t stack_vars_section = func_sections_.size() - 1;
WriteLocals(index_to_name);

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

stream_ = prev_stream;
FinishFunction();
FinishFunction(stack_vars_section);
}

void CWriter::WriteParamsAndLocals() {
std::vector<std::string> index_to_name;
MakeTypeBindingReverseMapping(func_->GetNumParamsAndLocals(), func_->bindings,
&index_to_name);
WriteParams(index_to_name);
WriteParams(index_to_name, true);
Write(" ", OpenBrace());
WriteLocals(index_to_name);
}

void CWriter::WriteParams(const std::vector<std::string>& index_to_name) {
void CWriter::WriteParams(const std::vector<std::string>& index_to_name,
bool setjmp_safe) {
Write(ModuleInstanceTypeName(), "* instance");
if (func_->GetNumParams() != 0) {
Indent(4);
Expand All @@ -3196,7 +3206,13 @@ void CWriter::WriteParams(const std::vector<std::string>& index_to_name) {
if (i != 0 && (i % 8) == 0) {
Write(Newline());
}
Write(func_->GetParamType(i), " ", DefineParamName(index_to_name[i]));
Write(func_->GetParamType(i), " ");
if (setjmp_safe) {
PushFuncSection("exceptions");
Write("volatile ");
PushFuncSection();
}
Write(DefineParamName(index_to_name[i]));
}
Dedent(4);
}
Expand Down Expand Up @@ -3240,13 +3256,17 @@ void CWriter::WriteLocals(const std::vector<std::string>& index_to_name) {
} else {
Write("0");
}
});
},
true);
}

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

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

size_t CWriter::BeginTry(const TryExpr& tryexpr) {
func_includes_.insert("exceptions");
Write(OpenBrace()); /* beginning of try-catch */
const std::string tlabel = DefineLabelName(tryexpr.block.label);
Write("WASM_RT_UNWIND_TARGET *", tlabel,
Expand Down
25 changes: 25 additions & 0 deletions test/regress/wasm2c-ehv3-setjmp-volatile.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
;;; TOOL: run-spec-wasm2c
;;; ARGS*: --enable-exceptions
;;; NOTE: ref: issue-2469
(module
(tag $e0)
(func $longjmp-bait (throw $e0))
(func (export "setjmp-bait") (param $return-early i32) (result i32)
(local $value i32)
(try $try
(do
(br_if $try (local.get $return-early))
(local.set $value (i32.const 1))
(call $longjmp-bait)
)
(catch $e0)
)
(local.get $value)
)
)

(assert_return (invoke "setjmp-bait" (i32.const 1)) (i32.const 0))
(assert_return (invoke "setjmp-bait" (i32.const 0)) (i32.const 1))
(;; STDOUT ;;;
2/2 tests passed.
;;; STDOUT ;;)

0 comments on commit 7d229cf

Please sign in to comment.