diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc index 62349a2397..c6cda42105 100644 --- a/src/runtime/runtime-wasm.cc +++ b/src/runtime/runtime-wasm.cc @@ -489,9 +489,12 @@ RUNTIME_FUNCTION(Runtime_WasmTableInit) { DCHECK(!isolate->context().is_null()); - bool oob = !WasmInstanceObject::InitTableEntries( - isolate, instance, table_index, elem_segment_index, dst, src, count); - if (oob) return ThrowTableOutOfBounds(isolate, instance); + base::Optional opt_error = + WasmInstanceObject::InitTableEntries(isolate, instance, table_index, + elem_segment_index, dst, src, count); + if (opt_error.has_value()) { + return ThrowWasmError(isolate, opt_error.value()); + } return ReadOnlyRoots(isolate).undefined_value(); } diff --git a/src/wasm/init-expr-interface.cc b/src/wasm/init-expr-interface.cc index 4441a61a32..f4a5ec8808 100644 --- a/src/wasm/init-expr-interface.cc +++ b/src/wasm/init-expr-interface.cc @@ -223,12 +223,12 @@ void InitExprInterface::ArrayInitFromSegment( // Error handling. if (length > static_cast(WasmArray::MaxLength(array_imm.array_type))) { - error_ = "length for array.init_from_data too large"; + error_ = MessageTemplate::kWasmTrapArrayTooLarge; return; } if (!base::IsInBounds(offset, length_in_bytes, data_segment.source.length())) { - error_ = "data segment is out of bounds"; + error_ = MessageTemplate::kWasmTrapDataSegmentOutOfBounds; return; } diff --git a/src/wasm/init-expr-interface.h b/src/wasm/init-expr-interface.h index 5a0da7cfa1..a50ce897b2 100644 --- a/src/wasm/init-expr-interface.h +++ b/src/wasm/init-expr-interface.h @@ -70,19 +70,26 @@ class InitExprInterface { INTERFACE_CONSTANT_FUNCTIONS(DECLARE_INTERFACE_FUNCTION) #undef DECLARE_INTERFACE_FUNCTION - WasmValue result() { - DCHECK_NOT_NULL(isolate_); + WasmValue computed_value() const { + DCHECK(generate_value()); + // The value has to be initialized. + DCHECK_NE(computed_value_.type(), kWasmVoid); return computed_value_; } - bool end_found() { return end_found_; } - bool runtime_error() { return error_ != nullptr; } - const char* runtime_error_msg() { return error_; } + bool end_found() const { return end_found_; } + bool has_error() const { return error_ != MessageTemplate::kNone; } + MessageTemplate error() const { + DCHECK(has_error()); + DCHECK_EQ(computed_value_.type(), kWasmVoid); + return error_; + } private: - bool generate_value() { return isolate_ != nullptr && !runtime_error(); } + bool generate_value() const { return isolate_ != nullptr && !has_error(); } + bool end_found_ = false; - const char* error_ = nullptr; WasmValue computed_value_; + MessageTemplate error_ = MessageTemplate::kNone; const WasmModule* module_; WasmModule* outer_module_; Isolate* isolate_; diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc index e7da2847b4..b9cacd7539 100644 --- a/src/wasm/module-instantiate.cc +++ b/src/wasm/module-instantiate.cc @@ -897,10 +897,19 @@ bool HasDefaultToNumberBehaviour(Isolate* isolate, return true; } -V8_INLINE WasmValue EvaluateInitExpression(Zone* zone, ConstantExpression expr, - ValueType expected, Isolate* isolate, - Handle instance, - ErrorThrower* thrower) { +bool MaybeMarkError(ValueOrError value, ErrorThrower* thrower) { + if (is_error(value)) { + thrower->RuntimeError("%s", + MessageFormatter::TemplateString(to_error(value))); + return true; + } + return false; +} +} // namespace + +ValueOrError EvaluateInitExpression(Zone* zone, ConstantExpression expr, + ValueType expected, Isolate* isolate, + Handle instance) { switch (expr.kind()) { case ConstantExpression::kEmpty: UNREACHABLE(); @@ -938,16 +947,12 @@ V8_INLINE WasmValue EvaluateInitExpression(Zone* zone, ConstantExpression expr, decoder.DecodeFunctionBody(); - if (decoder.interface().runtime_error()) { - thrower->RuntimeError("%s", decoder.interface().runtime_error_msg()); - return {}; - } - - return decoder.interface().result(); + return decoder.interface().has_error() + ? ValueOrError(decoder.interface().error()) + : ValueOrError(decoder.interface().computed_value()); } } } -} // namespace // Look up an import value in the {ffi_} object specifically for linking an // asm.js module. This only performs non-observable lookups, which allows @@ -1007,22 +1012,21 @@ void InstanceBuilder::LoadDataSegments(Handle instance) { size_t dest_offset; if (module_->is_memory64) { - uint64_t dest_offset_64 = - EvaluateInitExpression(&init_expr_zone_, segment.dest_addr, kWasmI64, - isolate_, instance, thrower_) - .to_u64(); - if (thrower_->error()) return; + ValueOrError result = EvaluateInitExpression( + &init_expr_zone_, segment.dest_addr, kWasmI64, isolate_, instance); + if (MaybeMarkError(result, thrower_)) return; + uint64_t dest_offset_64 = to_value(result).to_u64(); + // Clamp to {std::numeric_limits::max()}, which is always an // invalid offset. DCHECK_GT(std::numeric_limits::max(), instance->memory_size()); dest_offset = static_cast(std::min( dest_offset_64, uint64_t{std::numeric_limits::max()})); } else { - dest_offset = - EvaluateInitExpression(&init_expr_zone_, segment.dest_addr, kWasmI32, - isolate_, instance, thrower_) - .to_u32(); - if (thrower_->error()) return; + ValueOrError result = EvaluateInitExpression( + &init_expr_zone_, segment.dest_addr, kWasmI32, isolate_, instance); + if (MaybeMarkError(result, thrower_)) return; + dest_offset = to_value(result).to_u32(); } if (!base::IsInBounds(dest_offset, size, instance->memory_size())) { @@ -1719,15 +1723,14 @@ void InstanceBuilder::InitGlobals(Handle instance) { // Happens with imported globals. if (!global.init.is_set()) continue; - WasmValue value = - EvaluateInitExpression(&init_expr_zone_, global.init, global.type, - isolate_, instance, thrower_); - if (thrower_->error()) return; + ValueOrError result = EvaluateInitExpression( + &init_expr_zone_, global.init, global.type, isolate_, instance); + if (MaybeMarkError(result, thrower_)) return; if (global.type.is_reference()) { - tagged_globals_->set(global.offset, *value.to_ref()); + tagged_globals_->set(global.offset, *to_value(result).to_ref()); } else { - value.CopyTo(GetRawUntaggedGlobalPtr(global)); + to_value(result).CopyTo(GetRawUntaggedGlobalPtr(global)); } } } @@ -1986,14 +1989,14 @@ void InstanceBuilder::InitializeNonDefaultableTables( SetFunctionTableNullEntry(isolate_, table_object, entry_index); } } else { - WasmValue value = + ValueOrError result = EvaluateInitExpression(&init_expr_zone_, table.initial_value, - table.type, isolate_, instance, thrower_); - if (thrower_->error()) return; + table.type, isolate_, instance); + if (MaybeMarkError(result, thrower_)) return; for (uint32_t entry_index = 0; entry_index < table.initial_size; entry_index++) { WasmTableObject::Set(isolate_, table_object, entry_index, - value.to_ref()); + to_value(result).to_ref()); } } } @@ -2001,23 +2004,26 @@ void InstanceBuilder::InitializeNonDefaultableTables( } namespace { -bool LoadElemSegmentImpl(Zone* zone, Isolate* isolate, - Handle instance, - Handle table_object, - uint32_t table_index, uint32_t segment_index, - uint32_t dst, uint32_t src, size_t count) { +// If the operation succeeds, returns an empty {Optional}. Otherwise, returns an +// {Optional} containing the {MessageTemplate} code of the error. +base::Optional LoadElemSegmentImpl( + Zone* zone, Isolate* isolate, Handle instance, + Handle table_object, uint32_t table_index, + uint32_t segment_index, uint32_t dst, uint32_t src, size_t count) { DCHECK_LT(segment_index, instance->module()->elem_segments.size()); auto& elem_segment = instance->module()->elem_segments[segment_index]; // TODO(wasm): Move this functionality into wasm-objects, since it is used // for both instantiation and in the implementation of the table.init // instruction. - if (!base::IsInBounds(dst, count, table_object->current_length()) || - !base::IsInBounds( + if (!base::IsInBounds(dst, count, table_object->current_length())) { + return {MessageTemplate::kWasmTrapTableOutOfBounds}; + } + if (!base::IsInBounds( src, count, instance->dropped_elem_segments()[segment_index] == 0 ? elem_segment.entries.size() : 0)) { - return false; + return {MessageTemplate::kWasmTrapElementSegmentOutOfBounds}; } bool is_function_table = @@ -2035,13 +2041,14 @@ bool LoadElemSegmentImpl(Zone* zone, Isolate* isolate, entry.kind() == ConstantExpression::kRefNull) { SetFunctionTableNullEntry(isolate, table_object, entry_index); } else { - WasmValue value = EvaluateInitExpression(zone, entry, elem_segment.type, - isolate, instance, &thrower); - if (thrower.error()) return false; - WasmTableObject::Set(isolate, table_object, entry_index, value.to_ref()); + ValueOrError result = EvaluateInitExpression( + zone, entry, elem_segment.type, isolate, instance); + if (is_error(result)) return to_error(result); + WasmTableObject::Set(isolate, table_object, entry_index, + to_value(result).to_ref()); } } - return true; + return {}; } } // namespace @@ -2053,15 +2060,15 @@ void InstanceBuilder::LoadTableSegments(Handle instance) { if (elem_segment.status != WasmElemSegment::kStatusActive) continue; uint32_t table_index = elem_segment.table_index; - uint32_t dst = - EvaluateInitExpression(&init_expr_zone_, elem_segment.offset, kWasmI32, - isolate_, instance, thrower_) - .to_u32(); + ValueOrError value = EvaluateInitExpression( + &init_expr_zone_, elem_segment.offset, kWasmI32, isolate_, instance); + if (MaybeMarkError(value, thrower_)) return; + uint32_t dst = std::get(value).to_u32(); if (thrower_->error()) return; uint32_t src = 0; size_t count = elem_segment.entries.size(); - bool success = LoadElemSegmentImpl( + base::Optional opt_error = LoadElemSegmentImpl( &init_expr_zone_, isolate_, instance, handle(WasmTableObject::cast( instance->tables().get(elem_segment.table_index)), @@ -2070,8 +2077,9 @@ void InstanceBuilder::LoadTableSegments(Handle instance) { // Set the active segments to being already dropped, since table.init on // a dropped passive segment and an active segment have the same behavior. instance->dropped_elem_segments()[segment_index] = 1; - if (!success) { - thrower_->RuntimeError("table initializer is out of bounds"); + if (opt_error.has_value()) { + thrower_->RuntimeError( + "%s", MessageFormatter::TemplateString(opt_error.value())); return; } } @@ -2086,9 +2094,9 @@ void InstanceBuilder::InitializeTags(Handle instance) { } } -bool LoadElemSegment(Isolate* isolate, Handle instance, - uint32_t table_index, uint32_t segment_index, uint32_t dst, - uint32_t src, uint32_t count) { +base::Optional LoadElemSegment( + Isolate* isolate, Handle instance, uint32_t table_index, + uint32_t segment_index, uint32_t dst, uint32_t src, uint32_t count) { AccountingAllocator allocator; // This {Zone} will be used only by the temporary WasmFullDecoder allocated // down the line from this call. Therefore it is safe to stack-allocate it diff --git a/src/wasm/module-instantiate.h b/src/wasm/module-instantiate.h index 35f63c4a6f..87788ee9d5 100644 --- a/src/wasm/module-instantiate.h +++ b/src/wasm/module-instantiate.h @@ -11,7 +11,12 @@ #include +#include + #include "include/v8config.h" +#include "src/base/optional.h" +#include "src/common/message-template.h" +#include "src/wasm/wasm-value.h" namespace v8 { namespace internal { @@ -29,7 +34,7 @@ template class MaybeHandle; namespace wasm { - +class ConstantExpression; class ErrorThrower; MaybeHandle InstantiateToInstanceObject( @@ -37,9 +42,29 @@ MaybeHandle InstantiateToInstanceObject( Handle module_object, MaybeHandle imports, MaybeHandle memory); -bool LoadElemSegment(Isolate* isolate, Handle instance, - uint32_t table_index, uint32_t segment_index, uint32_t dst, - uint32_t src, uint32_t count) V8_WARN_UNUSED_RESULT; +// Loads a range of elements from element segment into a table. +// Returns the empty {Optional} if the operation succeeds, or an {Optional} with +// the error {MessageTemplate} if it fails. +base::Optional LoadElemSegment( + Isolate* isolate, Handle instance, uint32_t table_index, + uint32_t segment_index, uint32_t dst, uint32_t src, + uint32_t count) V8_WARN_UNUSED_RESULT; + +using ValueOrError = std::variant; + +V8_INLINE bool is_error(ValueOrError result) { + return std::holds_alternative(result); +} +V8_INLINE MessageTemplate to_error(ValueOrError result) { + return std::get(result); +} +V8_INLINE WasmValue to_value(ValueOrError result) { + return std::get(result); +} + +ValueOrError EvaluateInitExpression(Zone* zone, ConstantExpression expr, + ValueType expected, Isolate* isolate, + Handle instance); } // namespace wasm } // namespace internal diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index ee1da9ec46..5ea2a2d141 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -1355,11 +1355,9 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate, } // static -bool WasmInstanceObject::InitTableEntries(Isolate* isolate, - Handle instance, - uint32_t table_index, - uint32_t segment_index, uint32_t dst, - uint32_t src, uint32_t count) { +base::Optional WasmInstanceObject::InitTableEntries( + Isolate* isolate, Handle instance, uint32_t table_index, + uint32_t segment_index, uint32_t dst, uint32_t src, uint32_t count) { // Note that this implementation just calls through to module instantiation. // This is intentional, so that the runtime only depends on the object // methods, and not the module instantiation logic. diff --git a/src/wasm/wasm-objects.h b/src/wasm/wasm-objects.h index b5d12831fe..7b0ac44e3f 100644 --- a/src/wasm/wasm-objects.h +++ b/src/wasm/wasm-objects.h @@ -19,6 +19,7 @@ #include "src/objects/js-function.h" #include "src/objects/js-objects.h" #include "src/objects/objects.h" +#include "src/wasm/module-instantiate.h" #include "src/wasm/stacks.h" #include "src/wasm/struct-types.h" #include "src/wasm/value-type.h" @@ -475,13 +476,13 @@ class V8_EXPORT_PRIVATE WasmInstanceObject : public JSObject { uint32_t src, uint32_t count) V8_WARN_UNUSED_RESULT; - // Copy table entries from an element segment. Returns {false} if the ranges - // are out-of-bounds. - static bool InitTableEntries(Isolate* isolate, - Handle instance, - uint32_t table_index, uint32_t segment_index, - uint32_t dst, uint32_t src, - uint32_t count) V8_WARN_UNUSED_RESULT; + // Loads a range of elements from element segment into a table. + // Returns the empty {Optional} if the operation succeeds, or an {Optional} + // with the error {MessageTemplate} if it fails. + static base::Optional InitTableEntries( + Isolate* isolate, Handle instance, + uint32_t table_index, uint32_t segment_index, uint32_t dst, uint32_t src, + uint32_t count) V8_WARN_UNUSED_RESULT; // Iterates all fields in the object except the untagged fields. class BodyDescriptor; diff --git a/src/wasm/wasm-opcodes-inl.h b/src/wasm/wasm-opcodes-inl.h index adf1d464e7..6707af522a 100644 --- a/src/wasm/wasm-opcodes-inl.h +++ b/src/wasm/wasm-opcodes-inl.h @@ -733,8 +733,21 @@ constexpr MessageTemplate WasmOpcodes::TrapReasonToMessageId( return MessageTemplate::kWasm##name; FOREACH_WASM_TRAPREASON(TRAPREASON_TO_MESSAGE) #undef TRAPREASON_TO_MESSAGE + case kTrapCount: + UNREACHABLE(); + } +} + +constexpr TrapReason WasmOpcodes::MessageIdToTrapReason( + MessageTemplate message) { + switch (message) { +#define MESSAGE_TO_TRAPREASON(name) \ + case MessageTemplate::kWasm##name: \ + return k##name; + FOREACH_WASM_TRAPREASON(MESSAGE_TO_TRAPREASON) +#undef MESSAGE_TO_TRAPREASON default: - return MessageTemplate::kNone; + UNREACHABLE(); } } diff --git a/src/wasm/wasm-opcodes.h b/src/wasm/wasm-opcodes.h index 3a0acd6513..5a50063418 100644 --- a/src/wasm/wasm-opcodes.h +++ b/src/wasm/wasm-opcodes.h @@ -884,6 +884,7 @@ class V8_EXPORT_PRIVATE WasmOpcodes { static constexpr bool IsBreakable(WasmOpcode); static constexpr MessageTemplate TrapReasonToMessageId(TrapReason); + static constexpr TrapReason MessageIdToTrapReason(MessageTemplate message); // Extract the prefix byte (or 0x00) from a {WasmOpcode}. static constexpr byte ExtractPrefix(WasmOpcode); diff --git a/test/common/wasm/wasm-interpreter.cc b/test/common/wasm/wasm-interpreter.cc index 718b82284b..c5587fddc1 100644 --- a/test/common/wasm/wasm-interpreter.cc +++ b/test/common/wasm/wasm-interpreter.cc @@ -1401,6 +1401,10 @@ class WasmInterpreterInternals { CommitPc(pc); } + void DoTrap(MessageTemplate message, pc_t pc) { + DoTrap(WasmOpcodes::MessageIdToTrapReason(message), pc); + } + // Check if there is room for a function's activation. void EnsureStackSpaceForCall(InterpreterCode* code) { EnsureStackSpace(code->side_table->max_stack_height_ + @@ -1897,11 +1901,16 @@ class WasmInterpreterInternals { auto src = Pop().to(); auto dst = Pop().to(); HandleScope scope(isolate_); // Avoid leaking handles. - bool ok = WasmInstanceObject::InitTableEntries( - instance_object_->GetIsolate(), instance_object_, imm.table.index, - imm.element_segment.index, dst, src, size); - if (!ok) DoTrap(kTrapTableOutOfBounds, pc); - return ok; + base::Optional opt_error = + WasmInstanceObject::InitTableEntries( + instance_object_->GetIsolate(), instance_object_, + imm.table.index, imm.element_segment.index, dst, src, size); + if (opt_error.has_value()) { + DoTrap(opt_error.value(), pc); + return false; + } + + return true; } case kExprElemDrop: { IndexImmediate imm(decoder, code->at(pc + *len), diff --git a/test/mjsunit/wasm/externref-table.js b/test/mjsunit/wasm/externref-table.js index e481c329e8..10c81e0edd 100644 --- a/test/mjsunit/wasm/externref-table.js +++ b/test/mjsunit/wasm/externref-table.js @@ -34,14 +34,17 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js"); print(arguments.callee.name); const builder = new WasmModuleBuilder(); - const table_index = builder.addImportedTable("imp", "table", 3, 10, kWasmExternRef); + const table_index = builder.addImportedTable( + "imp", "table", 3, 10, kWasmExternRef); builder.addFunction('get', kSig_r_v) .addBody([kExprI32Const, 0, kExprTableGet, table_index]); - let table_ref = new WebAssembly.Table({element: "externref", initial: 3, maximum: 10}); + let table_ref = new WebAssembly.Table( + { element: "externref", initial: 3, maximum: 10 }); builder.instantiate({imp:{table: table_ref}}); - let table_func = new WebAssembly.Table({ element: "anyfunc", initial: 3, maximum: 10 }); + let table_func = new WebAssembly.Table( + { element: "anyfunc", initial: 3, maximum: 10 }); assertThrows(() => builder.instantiate({ imp: { table: table_func } }), WebAssembly.LinkError, /imported table does not match the expected type/); })(); @@ -77,7 +80,7 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js"); .exportFunc(); const instance = builder.instantiate(); - assertTraps(kTrapTableOutOfBounds, () => instance.exports.init()); + assertTraps(kTrapElementSegmentOutOfBounds, () => instance.exports.init()); })();