Commit 43f0f49d authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm] Add a "boolean validation" mode

All instantiations of the function body decoder (validation, Liftoff,
TurboFan) currently generate precise error messages. For Liftoff though,
the error message and location is never used. Thus we can save some
binary size and performance by only keeping a flag whether an error
occured or not. In the error case, the TurboFan compiler will execute
right afterwards anyway, generating a proper error message.

As as follow-up, we can avoid storing the pc in {ValueBase} and
{ControlBase}, because that's only used for error reporting.

R=thibaudm@chromium.org

Bug: v8:10969
Change-Id: I65c46cb9d8b654f9476f2c34ca9a8dd45d6bbbc5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2436347
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70235}
parent 29bcdaad
...@@ -258,7 +258,7 @@ class DebugSideTableBuilder { ...@@ -258,7 +258,7 @@ class DebugSideTableBuilder {
class LiftoffCompiler { class LiftoffCompiler {
public: public:
// TODO(clemensb): Make this a template parameter. // TODO(clemensb): Make this a template parameter.
static constexpr Decoder::ValidateFlag validate = Decoder::kFullValidation; static constexpr Decoder::ValidateFlag validate = Decoder::kBooleanValidation;
using Value = ValueBase; using Value = ValueBase;
...@@ -4116,7 +4116,7 @@ WasmCompilationResult ExecuteLiftoffCompilation( ...@@ -4116,7 +4116,7 @@ WasmCompilationResult ExecuteLiftoffCompilation(
if (debug_sidetable) { if (debug_sidetable) {
debug_sidetable_builder = std::make_unique<DebugSideTableBuilder>(); debug_sidetable_builder = std::make_unique<DebugSideTableBuilder>();
} }
WasmFullDecoder<Decoder::kFullValidation, LiftoffCompiler> decoder( WasmFullDecoder<Decoder::kBooleanValidation, LiftoffCompiler> decoder(
&zone, env->module, env->enabled_features, detected, func_body, &zone, env->module, env->enabled_features, detected, func_body,
call_descriptor, env, &zone, instruction_buffer->CreateView(), call_descriptor, env, &zone, instruction_buffer->CreateView(),
debug_sidetable_builder.get(), for_debugging, func_index, breakpoints, debug_sidetable_builder.get(), for_debugging, func_index, breakpoints,
...@@ -4173,7 +4173,7 @@ std::unique_ptr<DebugSideTable> GenerateLiftoffDebugSideTable( ...@@ -4173,7 +4173,7 @@ std::unique_ptr<DebugSideTable> GenerateLiftoffDebugSideTable(
auto call_descriptor = compiler::GetWasmCallDescriptor(&zone, func_body.sig); auto call_descriptor = compiler::GetWasmCallDescriptor(&zone, func_body.sig);
DebugSideTableBuilder debug_sidetable_builder; DebugSideTableBuilder debug_sidetable_builder;
WasmFeatures detected; WasmFeatures detected;
WasmFullDecoder<Decoder::kFullValidation, LiftoffCompiler> decoder( WasmFullDecoder<Decoder::kBooleanValidation, LiftoffCompiler> decoder(
&zone, env->module, env->enabled_features, &detected, func_body, &zone, env->module, env->enabled_features, &detected, func_body,
call_descriptor, env, &zone, call_descriptor, env, &zone,
NewAssemblerBuffer(AssemblerBase::kDefaultBufferSize), NewAssemblerBuffer(AssemblerBase::kDefaultBufferSize),
......
...@@ -40,10 +40,9 @@ using DecodeResult = VoidResult; ...@@ -40,10 +40,9 @@ using DecodeResult = VoidResult;
class Decoder { class Decoder {
public: public:
// {ValidateFlag} can be used in a boolean manner ({if (!validate) ...}). // {ValidateFlag} can be used in a boolean manner ({if (!validate) ...}).
// TODO(clemensb): Introduce a "boolean validation" mode where error messages
// are locations are not tracked.
enum ValidateFlag : int8_t { enum ValidateFlag : int8_t {
kNoValidation = 0, // Don't run validation, assume valid input. kNoValidation = 0, // Don't run validation, assume valid input.
kBooleanValidation, // Run validation but only store a generic error.
kFullValidation // Run full validation with error message and location. kFullValidation // Run full validation with error message and location.
}; };
...@@ -231,6 +230,14 @@ class Decoder { ...@@ -231,6 +230,14 @@ class Decoder {
return true; return true;
} }
// Use this for "boolean validation", i.e. if the error message is not used
// anyway.
void V8_NOINLINE MarkError() {
if (!ok()) return;
error_ = {0, "validation failed"};
onFirstError();
}
// Do not inline error methods. This has measurable impact on validation time, // Do not inline error methods. This has measurable impact on validation time,
// see https://crbug.com/910432. // see https://crbug.com/910432.
void V8_NOINLINE error(const char* msg) { errorf(pc_offset(), "%s", msg); } void V8_NOINLINE error(const char* msg) { errorf(pc_offset(), "%s", msg); }
...@@ -241,6 +248,13 @@ class Decoder { ...@@ -241,6 +248,13 @@ class Decoder {
errorf(offset, "%s", msg); errorf(offset, "%s", msg);
} }
void V8_NOINLINE PRINTF_FORMAT(2, 3) errorf(const char* format, ...) {
va_list args;
va_start(args, format);
verrorf(pc_offset(), format, args);
va_end(args);
}
void V8_NOINLINE PRINTF_FORMAT(3, 4) void V8_NOINLINE PRINTF_FORMAT(3, 4)
errorf(uint32_t offset, const char* format, ...) { errorf(uint32_t offset, const char* format, ...) {
va_list args; va_list args;
...@@ -350,7 +364,7 @@ class Decoder { ...@@ -350,7 +364,7 @@ class Decoder {
onFirstError(); onFirstError();
} }
template <typename IntType, bool validate> template <typename IntType, ValidateFlag validate>
inline IntType read_little_endian(const byte* pc, const char* msg) { inline IntType read_little_endian(const byte* pc, const char* msg) {
if (!validate) { if (!validate) {
DCHECK(validate_size(pc, sizeof(IntType), msg)); DCHECK(validate_size(pc, sizeof(IntType), msg));
...@@ -368,7 +382,7 @@ class Decoder { ...@@ -368,7 +382,7 @@ class Decoder {
pc_ = end_; pc_ = end_;
return IntType{0}; return IntType{0};
} }
IntType val = read_little_endian<IntType, false>(pc_, name); IntType val = read_little_endian<IntType, kNoValidation>(pc_, name);
traceByteRange(pc_, pc_ + sizeof(IntType)); traceByteRange(pc_, pc_ + sizeof(IntType));
TRACE("= %d\n", val); TRACE("= %d\n", val);
pc_ += sizeof(IntType); pc_ += sizeof(IntType);
...@@ -418,7 +432,11 @@ class Decoder { ...@@ -418,7 +432,11 @@ class Decoder {
*length = byte_index + (at_end ? 0 : 1); *length = byte_index + (at_end ? 0 : 1);
if (validate && V8_UNLIKELY(at_end || (b & 0x80))) { if (validate && V8_UNLIKELY(at_end || (b & 0x80))) {
TRACE_IF(trace, at_end ? "<end> " : "<length overflow> "); TRACE_IF(trace, at_end ? "<end> " : "<length overflow> ");
if (validate == kFullValidation) {
errorf(pc, "expected %s", name); errorf(pc, "expected %s", name);
} else {
MarkError();
}
result = 0; result = 0;
} }
if (is_last_byte) { if (is_last_byte) {
...@@ -438,7 +456,11 @@ class Decoder { ...@@ -438,7 +456,11 @@ class Decoder {
if (!validate) { if (!validate) {
DCHECK(valid_extra_bits); DCHECK(valid_extra_bits);
} else if (V8_UNLIKELY(!valid_extra_bits)) { } else if (V8_UNLIKELY(!valid_extra_bits)) {
if (validate == kFullValidation) {
error(pc, "extra bits in varint"); error(pc, "extra bits in varint");
} else {
MarkError();
}
result = 0; result = 0;
} }
} }
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment