[wasm] Use a consistent error message for validation failures

The validation paths during decoding and compilation should generate the
same error message. To achieve this, we move the {GetWasmErrorWithName}
function from the compiler to the decoder. As a drive-by, we replace the
{WasmFunction&} parameter by just an integer, because that is all we
need.

R=ahaas@chromium.org

Bug: v8:13447
Change-Id: I469dd871c7471c0f5af12c56e19b71be136557cd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4037268
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84399}
This commit is contained in:
Clemens Backes 2022-11-18 17:54:24 +01:00 committed by V8 LUCI CQ
parent c0743da924
commit e9639b550b
5 changed files with 34 additions and 37 deletions

View File

@ -1072,22 +1072,6 @@ class CompilationUnitBuilder {
js_to_wasm_wrapper_units_;
};
WasmError GetWasmErrorWithName(ModuleWireBytes wire_bytes,
const WasmFunction* func,
const WasmModule* module, WasmError error) {
WasmName name = wire_bytes.GetNameOrNull(func, module);
if (name.begin() == nullptr) {
return WasmError(error.offset(), "Compiling function #%d failed: %s",
func->func_index, error.message().c_str());
} else {
TruncatedUserString<> truncated_name(name);
return WasmError(error.offset(),
"Compiling function #%d:\"%.*s\" failed: %s",
func->func_index, truncated_name.length(),
truncated_name.start(), error.message().c_str());
}
}
DecodeResult ValidateSingleFunction(const WasmModule* module, int func_index,
base::Vector<const uint8_t> code,
AccountingAllocator* allocator,
@ -1231,9 +1215,9 @@ void ThrowLazyCompilationError(Isolate* isolate,
CHECK(decode_result.failed());
wasm::ErrorThrower thrower(isolate, nullptr);
thrower.CompileFailed(
GetWasmErrorWithName(ModuleWireBytes{native_module->wire_bytes()}, func,
module, std::move(decode_result).error()));
thrower.CompileFailed(GetWasmErrorWithName(native_module->wire_bytes(),
func_index, module,
std::move(decode_result).error()));
}
class TransitiveTypeFeedbackProcessor {
@ -1879,7 +1863,8 @@ WasmError ValidateFunctions(const WasmModule* module,
DecodeResult function_result = ValidateSingleFunction(
module, function.func_index, code, allocator, enabled_features);
if (function_result.failed()) {
return GetWasmErrorWithName(wire_bytes, &function, module,
return GetWasmErrorWithName(wire_bytes.module_bytes(),
function.func_index, module,
std::move(function_result).error());
}
module->set_function_validated(function.func_index);

View File

@ -475,14 +475,7 @@ class ValidateFunctionsTask : public JobTask {
if (!error_out_->empty() && error_out_->offset() <= error.offset()) {
return;
}
// Wrap the error message from the function decoder.
const WasmFunction& function = module_->functions[func_index];
WasmFunctionName func_name{
&function,
ModuleWireBytes{wire_bytes_}.GetNameOrNull(&function, module_)};
std::ostringstream error_msg;
error_msg << "in function " << func_name << ": " << error.message();
*error_out_ = WasmError{error.offset(), error_msg.str()};
*error_out_ = GetWasmErrorWithName(wire_bytes_, func_index, module_, error);
}
const base::Vector<const uint8_t> wire_bytes_;
@ -533,6 +526,22 @@ WasmError ValidateFunctions(const WasmModule* module,
return validation_error;
}
WasmError GetWasmErrorWithName(base::Vector<const uint8_t> wire_bytes,
int func_index, const WasmModule* module,
WasmError error) {
WasmName name = ModuleWireBytes{wire_bytes}.GetNameOrNull(func_index, module);
if (name.begin() == nullptr) {
return WasmError(error.offset(), "Compiling function #%d failed: %s",
func_index, error.message().c_str());
} else {
TruncatedUserString<> truncated_name(name);
return WasmError(error.offset(),
"Compiling function #%d:\"%.*s\" failed: %s", func_index,
truncated_name.length(), truncated_name.start(),
error.message().c_str());
}
}
DecodedNameSection::DecodedNameSection(base::Vector<const uint8_t> wire_bytes,
WireBytesRef name_section) {
if (name_section.is_empty()) return; // No name section.

View File

@ -140,6 +140,10 @@ V8_EXPORT_PRIVATE WasmError ValidateFunctions(
const WasmModule*, WasmFeatures enabled_features,
base::Vector<const uint8_t> wire_bytes, std::function<bool(int)> filter);
WasmError GetWasmErrorWithName(base::Vector<const uint8_t> wire_bytes,
int func_index, const WasmModule* module,
WasmError error);
class ModuleDecoderImpl;
class ModuleDecoder {

View File

@ -194,14 +194,14 @@ WasmName ModuleWireBytes::GetNameOrNull(WireBytesRef ref) const {
}
// Get a string stored in the module bytes representing a function name.
WasmName ModuleWireBytes::GetNameOrNull(const WasmFunction* function,
WasmName ModuleWireBytes::GetNameOrNull(int func_index,
const WasmModule* module) const {
return GetNameOrNull(module->lazily_generated_names.LookupFunctionName(
*this, function->func_index));
return GetNameOrNull(
module->lazily_generated_names.LookupFunctionName(*this, func_index));
}
std::ostream& operator<<(std::ostream& os, const WasmFunctionName& name) {
os << "#" << name.function_->func_index;
os << "#" << name.func_index_;
if (!name.name_.empty()) {
if (name.name_.begin()) {
os << ":";

View File

@ -703,8 +703,7 @@ struct V8_EXPORT_PRIVATE ModuleWireBytes {
WasmName GetNameOrNull(WireBytesRef ref) const;
// Get a string stored in the module bytes representing a function name.
WasmName GetNameOrNull(const WasmFunction* function,
const WasmModule* module) const;
WasmName GetNameOrNull(int func_index, const WasmModule* module) const;
// Checks the given reference is contained within the module bytes.
bool BoundsCheck(WireBytesRef ref) const {
@ -730,10 +729,10 @@ ASSERT_TRIVIALLY_COPYABLE(ModuleWireBytes);
// A helper for printing out the names of functions.
struct WasmFunctionName {
WasmFunctionName(const WasmFunction* function, WasmName name)
: function_(function), name_(name) {}
WasmFunctionName(int func_index, WasmName name)
: func_index_(func_index), name_(name) {}
const WasmFunction* function_;
const int func_index_;
const WasmName name_;
};