[wasm][refactor] Use MessageTemplate to describe errors

- Functions related to table initialization now return an optional
  {MessageTemplate} if they fail. This is used to emit the correct error
  message in one test.
- InitExprInterface now uses {MessageTemplate} to describe errors.

Change-Id: I2428f7823859b95d14b6e81c8200f78da4510ceb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695579
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81045}
This commit is contained in:
Manos Koukoutos 2022-06-09 15:04:35 +00:00 committed by V8 LUCI CQ
parent 3efa5f6d07
commit ccc8389f14
11 changed files with 160 additions and 92 deletions

View File

@ -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<MessageTemplate> 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();
}

View File

@ -223,12 +223,12 @@ void InitExprInterface::ArrayInitFromSegment(
// Error handling.
if (length >
static_cast<uint32_t>(WasmArray::MaxLength(array_imm.array_type))) {
error_ = "length for array.init_from_data too large";
error_ = MessageTemplate::kWasmTrapArrayTooLarge;
return;
}
if (!base::IsInBounds<uint32_t>(offset, length_in_bytes,
data_segment.source.length())) {
error_ = "data segment is out of bounds";
error_ = MessageTemplate::kWasmTrapDataSegmentOutOfBounds;
return;
}

View File

@ -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_;

View File

@ -897,10 +897,19 @@ bool HasDefaultToNumberBehaviour(Isolate* isolate,
return true;
}
V8_INLINE WasmValue EvaluateInitExpression(Zone* zone, ConstantExpression expr,
ValueType expected, Isolate* isolate,
Handle<WasmInstanceObject> 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<WasmInstanceObject> 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<WasmInstanceObject> 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<size_t>::max()}, which is always an
// invalid offset.
DCHECK_GT(std::numeric_limits<size_t>::max(), instance->memory_size());
dest_offset = static_cast<size_t>(std::min(
dest_offset_64, uint64_t{std::numeric_limits<size_t>::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<size_t>(dest_offset, size, instance->memory_size())) {
@ -1719,15 +1723,14 @@ void InstanceBuilder::InitGlobals(Handle<WasmInstanceObject> 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<byte>(global));
to_value(result).CopyTo(GetRawUntaggedGlobalPtr<byte>(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<WasmInstanceObject> instance,
Handle<WasmTableObject> 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<MessageTemplate> LoadElemSegmentImpl(
Zone* zone, Isolate* isolate, Handle<WasmInstanceObject> instance,
Handle<WasmTableObject> 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<uint64_t>(dst, count, table_object->current_length()) ||
!base::IsInBounds<uint64_t>(
if (!base::IsInBounds<uint64_t>(dst, count, table_object->current_length())) {
return {MessageTemplate::kWasmTrapTableOutOfBounds};
}
if (!base::IsInBounds<uint64_t>(
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<WasmInstanceObject> 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<WasmValue>(value).to_u32();
if (thrower_->error()) return;
uint32_t src = 0;
size_t count = elem_segment.entries.size();
bool success = LoadElemSegmentImpl(
base::Optional<MessageTemplate> 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<WasmInstanceObject> 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<WasmInstanceObject> instance) {
}
}
bool LoadElemSegment(Isolate* isolate, Handle<WasmInstanceObject> instance,
uint32_t table_index, uint32_t segment_index, uint32_t dst,
uint32_t src, uint32_t count) {
base::Optional<MessageTemplate> LoadElemSegment(
Isolate* isolate, Handle<WasmInstanceObject> 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

View File

@ -11,7 +11,12 @@
#include <stdint.h>
#include <variant>
#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 <typename T>
class MaybeHandle;
namespace wasm {
class ConstantExpression;
class ErrorThrower;
MaybeHandle<WasmInstanceObject> InstantiateToInstanceObject(
@ -37,9 +42,29 @@ MaybeHandle<WasmInstanceObject> InstantiateToInstanceObject(
Handle<WasmModuleObject> module_object, MaybeHandle<JSReceiver> imports,
MaybeHandle<JSArrayBuffer> memory);
bool LoadElemSegment(Isolate* isolate, Handle<WasmInstanceObject> 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<MessageTemplate> LoadElemSegment(
Isolate* isolate, Handle<WasmInstanceObject> 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<WasmValue, MessageTemplate>;
V8_INLINE bool is_error(ValueOrError result) {
return std::holds_alternative<MessageTemplate>(result);
}
V8_INLINE MessageTemplate to_error(ValueOrError result) {
return std::get<MessageTemplate>(result);
}
V8_INLINE WasmValue to_value(ValueOrError result) {
return std::get<WasmValue>(result);
}
ValueOrError EvaluateInitExpression(Zone* zone, ConstantExpression expr,
ValueType expected, Isolate* isolate,
Handle<WasmInstanceObject> instance);
} // namespace wasm
} // namespace internal

View File

@ -1355,11 +1355,9 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
}
// static
bool WasmInstanceObject::InitTableEntries(Isolate* isolate,
Handle<WasmInstanceObject> instance,
uint32_t table_index,
uint32_t segment_index, uint32_t dst,
uint32_t src, uint32_t count) {
base::Optional<MessageTemplate> WasmInstanceObject::InitTableEntries(
Isolate* isolate, Handle<WasmInstanceObject> 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.

View File

@ -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<WasmInstanceObject> 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<MessageTemplate> InitTableEntries(
Isolate* isolate, Handle<WasmInstanceObject> 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;

View File

@ -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();
}
}

View File

@ -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);

View File

@ -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<uint32_t>();
auto dst = Pop().to<uint32_t>();
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<MessageTemplate> 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<Decoder::kNoValidation> imm(decoder, code->at(pc + *len),

View File

@ -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());
})();