From 68f6c03a67a4a333e94e81451846004ba332bd59 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Mon, 1 Jun 2026 16:41:36 +0200 Subject: [PATCH] LibWasm: Correctly handle failed validation --- .../AbstractMachine/BytecodeInterpreter.cpp | 74 +++++++------------ .../LibWasm/AbstractMachine/Validator.cpp | 5 +- Libraries/LibWasm/CraneliftBridge.cpp | 18 ++++- Libraries/LibWasm/CraneliftStubs.cpp | 1 + Libraries/LibWasm/Types.h | 1 + 5 files changed, 49 insertions(+), 50 deletions(-) diff --git a/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp b/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp index d9f4787e50..3b0dcfd9d8 100644 --- a/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp +++ b/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp @@ -190,43 +190,6 @@ static void install_compiled_fault_handlers() { } #endif -class ScopedCompiledFaultRecovery { -public: - ScopedCompiledFaultRecovery(Wasm::BytecodeInterpreter& interpreter, Wasm::Configuration& configuration) - { - m_context.interpreter = &interpreter; - m_context.configuration = &configuration; - m_context.previous = s_compiled_fault_recovery; - } - - ~ScopedCompiledFaultRecovery() - { - if (m_armed) - s_compiled_fault_recovery = m_context.previous; - } - - bool arm() - { - install_compiled_fault_handlers(); - s_compiled_fault_recovery = &m_context; - m_armed = true; - if (setjmp(m_context.jump_buffer) != 0) { - // Disarm immediately after longjmp return; the compiled code may - // have corrupted our stack frame, so the destructor must be a no-op. - s_compiled_fault_recovery = m_context.previous; - m_armed = false; - return false; - } - return true; - } - - bool faulted() const { return m_context.faulted; } - -private: - CompiledFaultRecoveryContext m_context; - bool m_armed { false }; -}; - } #ifdef AK_COMPILER_CLANG @@ -354,10 +317,17 @@ void BytecodeInterpreter::interpret(Configuration& configuration) { m_trap = Empty {}; auto& expression = configuration.frame().expression(); - Optional compiled_fault_recovery; + CompiledFaultRecoveryContext compiled_fault_recovery; + bool did_install_compiled_fault_recovery = false; if (expression.compiled_instructions.cranelift_compiled && !s_compiled_fault_recovery) { - compiled_fault_recovery.emplace(*this, configuration); - if (!compiled_fault_recovery->arm()) { + install_compiled_fault_handlers(); + compiled_fault_recovery.interpreter = this; + compiled_fault_recovery.configuration = &configuration; + compiled_fault_recovery.previous = s_compiled_fault_recovery; + s_compiled_fault_recovery = &compiled_fault_recovery; + did_install_compiled_fault_recovery = true; + if (setjmp(compiled_fault_recovery.jump_buffer) != 0) { + s_compiled_fault_recovery = compiled_fault_recovery.previous; m_trap = Trap::from_string("Memory access out of bounds"); return; } @@ -365,15 +335,25 @@ void BytecodeInterpreter::interpret(Configuration& configuration) auto const should_limit_instruction_count = configuration.should_limit_instruction_count(); if (!expression.compiled_instructions.dispatches.is_empty()) { if (expression.compiled_instructions.direct) { - if (should_limit_instruction_count) - return interpret_impl(configuration, expression); - return interpret_impl(configuration, expression); + if (should_limit_instruction_count) { + interpret_impl(configuration, expression); + goto done; + } + interpret_impl(configuration, expression); + goto done; } - return interpret_impl(configuration, expression); + interpret_impl(configuration, expression); + goto done; } - if (should_limit_instruction_count) - return interpret_impl(configuration, expression); - return interpret_impl(configuration, expression); + if (should_limit_instruction_count) { + interpret_impl(configuration, expression); + goto done; + } + interpret_impl(configuration, expression); + +done: + if (did_install_compiled_fault_recovery) + s_compiled_fault_recovery = compiled_fault_recovery.previous; } constexpr static u32 default_sources_and_destination = (to_underlying(Dispatch::RegisterOrStack::Stack) | (to_underlying(Dispatch::RegisterOrStack::Stack) << 2) | (to_underlying(Dispatch::RegisterOrStack::Stack) << 4)); diff --git a/Libraries/LibWasm/AbstractMachine/Validator.cpp b/Libraries/LibWasm/AbstractMachine/Validator.cpp index 6924266750..517161d3f1 100644 --- a/Libraries/LibWasm/AbstractMachine/Validator.cpp +++ b/Libraries/LibWasm/AbstractMachine/Validator.cpp @@ -287,7 +287,10 @@ ErrorOr Validator::validate(CodeSection const& section) ScopeGuard cleanup = [&] { auto cranelift_start = MonotonicTime::now(); - flush_cranelift_batch(); + if (validation_succeeded) + flush_cranelift_batch(); + else + discard_cranelift_batch(); auto cranelift_duration = MonotonicTime::now() - cranelift_start; if (installing) diff --git a/Libraries/LibWasm/CraneliftBridge.cpp b/Libraries/LibWasm/CraneliftBridge.cpp index 2da2804039..20ed59cb6a 100644 --- a/Libraries/LibWasm/CraneliftBridge.cpp +++ b/Libraries/LibWasm/CraneliftBridge.cpp @@ -137,7 +137,7 @@ struct CacheState { Vector pending_batch; }; -static CacheState s_cranelift_cache_state; +static thread_local CacheState s_cranelift_cache_state; static thread_local u32 s_active_function_index = NumericLimits::max(); static u64 compute_layout_hash(RuntimeHelpers const& h) @@ -775,7 +775,7 @@ static ALWAYS_INLINE i32 wasm_cl_direct_call_impl(BytecodeInterpreter& interpret auto const& entry = (*table)[index]; if (config.depth() > 500) [[unlikely]] { - interpreter.set_trap("call stack exhausted"sv); + interpreter.set_trap(Constants::stack_exhaustion_message); return 1; } @@ -1195,6 +1195,8 @@ static void try_cranelift_compile_batch(Vector& batch) auto code_offset = static_cast(output->code_offset); auto code_size = static_cast(output->code_size); + if (code_offset > code_region_size || code_size > code_region_size - code_offset) + continue; auto code_start = code_base_offset + code_offset; if (code_start + code_size > total_size) continue; @@ -1202,6 +1204,10 @@ static void try_cranelift_compile_batch(Vector& batch) auto const reloc_offset = static_cast(output->reloc_offset); auto const reloc_count = static_cast(output->reloc_count); auto const reloc_bytes = reloc_count * sizeof(HelperReloc); + if (reloc_count != 0 && reloc_bytes / sizeof(HelperReloc) != reloc_count) + continue; + if (reloc_offset > reloc_region_size || reloc_bytes > reloc_region_size - reloc_offset) + continue; if (reloc_region_start + reloc_offset + reloc_bytes > total_size) continue; @@ -1245,6 +1251,9 @@ bool try_cranelift_compile(CompiledInstructions& compiled, u32 result_arity) if (compiled.cranelift_compiled) return true; + if (s_active_function_index == NumericLimits::max()) + return false; + // Cache hit: install from the parsed blob instead of going through cranelift. // dispatches[] has just been populated by try_compile_instructions, so handler_ptr is ready to be set. if (s_cranelift_cache_state.pending_install.active && s_active_function_index != NumericLimits::max()) { @@ -1388,6 +1397,11 @@ void flush_cranelift_batch() s_cranelift_cache_state.pending_batch.clear(); } +void discard_cranelift_batch() +{ + s_cranelift_cache_state.pending_batch.clear(); +} + void free_cranelift_code(void* handle) { if (handle) { diff --git a/Libraries/LibWasm/CraneliftStubs.cpp b/Libraries/LibWasm/CraneliftStubs.cpp index a319c7ec10..e973a6c879 100644 --- a/Libraries/LibWasm/CraneliftStubs.cpp +++ b/Libraries/LibWasm/CraneliftStubs.cpp @@ -10,6 +10,7 @@ namespace Wasm { bool try_cranelift_compile(CompiledInstructions&, u32) { return false; } void flush_cranelift_batch() { } +void discard_cranelift_batch() { } void free_cranelift_code(void*) { } void set_cranelift_active_function_index(u32) { } void begin_cranelift_cache_capture() { } diff --git a/Libraries/LibWasm/Types.h b/Libraries/LibWasm/Types.h index 80405b1b92..c879c277f7 100644 --- a/Libraries/LibWasm/Types.h +++ b/Libraries/LibWasm/Types.h @@ -1548,6 +1548,7 @@ private: CompiledInstructions try_compile_instructions(Expression const&, Span functions); bool try_cranelift_compile(CompiledInstructions& compiled, u32 result_arity = 0); void flush_cranelift_batch(); +void discard_cranelift_batch(); // Caller-supplied hooks for the Cranelift on-disk cache. // - `wasm_hash` is a 32-byte digest of the wasm bytes; embedded in produced blobs