Reland "[wasm] Remove field to store "intermediate" error"

This is a reland of commit c5e56d450b

No changes since revert. Revert was incorrect in suspecting CL for
GC stress failures.

Original change's description:
> [wasm] Remove field to store "intermediate" error
>
> We used to store the first error during function validation there, and
> then later return it instead of the outer (module-wide) decoding error.
> This is redundant, we can just directly store a function validation
> error in the outer error field.
> This also removes the boolean parameter to {Decoder::FinishDecoding}.
> This was always redundant, since the "intermediate error" was only set
> if function validation was enabled before.
>
> R=jkummerow@chromium.org
>
> Bug: v8:13371
> Change-Id: Ib812d4ba0ea10e9a5dda46cac4e33d427dd9c16c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3942084
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#83612}

Bug: v8:13371
Change-Id: If53599a843201f83fb77671d6398e7150632acd8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3943130
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83618}
This commit is contained in:
Clemens Backes 2022-10-10 17:25:55 +02:00 committed by V8 LUCI CQ
parent 477c7c5815
commit aa9dda4540
4 changed files with 16 additions and 29 deletions

View File

@ -2823,7 +2823,7 @@ bool AsyncStreamingProcessor::ProcessModuleHeader(
job_->context_id(), GetWasmEngine()->allocator());
decoder_.DecodeModuleHeader(bytes, offset);
if (!decoder_.ok()) {
FinishAsyncCompileJobWithError(decoder_.FinishDecoding(false).error());
FinishAsyncCompileJobWithError(decoder_.FinishDecoding().error());
return false;
}
prefix_hash_ = GetWireBytesHash(bytes);
@ -2849,7 +2849,7 @@ bool AsyncStreamingProcessor::ProcessSection(SectionCode section_code,
size_t bytes_consumed = ModuleDecoder::IdentifyUnknownSection(
&decoder_, bytes, offset, &section_code);
if (!decoder_.ok()) {
FinishAsyncCompileJobWithError(decoder_.FinishDecoding(false).error());
FinishAsyncCompileJobWithError(decoder_.FinishDecoding().error());
return false;
}
if (section_code == SectionCode::kUnknownSectionCode) {
@ -2863,7 +2863,7 @@ bool AsyncStreamingProcessor::ProcessSection(SectionCode section_code,
constexpr bool verify_functions = false;
decoder_.DecodeSection(section_code, bytes, offset, verify_functions);
if (!decoder_.ok()) {
FinishAsyncCompileJobWithError(decoder_.FinishDecoding(false).error());
FinishAsyncCompileJobWithError(decoder_.FinishDecoding().error());
return false;
}
return true;
@ -2882,7 +2882,7 @@ bool AsyncStreamingProcessor::ProcessCodeSectionHeader(
static_cast<uint32_t>(code_section_length));
if (!decoder_.CheckFunctionsCount(static_cast<uint32_t>(num_functions),
functions_mismatch_error_offset)) {
FinishAsyncCompileJobWithError(decoder_.FinishDecoding(false).error());
FinishAsyncCompileJobWithError(decoder_.FinishDecoding().error());
return false;
}
@ -2987,7 +2987,7 @@ void AsyncStreamingProcessor::OnFinishedStream(
base::OwnedVector<uint8_t> bytes) {
TRACE_STREAMING("Finish stream...\n");
DCHECK_EQ(NativeModuleCache::PrefixHash(bytes.as_vector()), prefix_hash_);
ModuleResult result = decoder_.FinishDecoding(false);
ModuleResult result = decoder_.FinishDecoding();
if (result.failed()) {
FinishAsyncCompileJobWithError(result.error());
return;

View File

@ -1627,7 +1627,7 @@ class ModuleDecoderTemplate : public Decoder {
return true;
}
ModuleResult FinishDecoding(bool validate_functions = true) {
ModuleResult FinishDecoding() {
if (ok() && CheckMismatchedCounts()) {
// We calculate the global offsets here, because there may not be a
// global section and code section that would have triggered the
@ -1636,12 +1636,7 @@ class ModuleDecoderTemplate : public Decoder {
CalculateGlobalOffsets(module_.get());
}
ModuleResult result = toResult(std::move(module_));
if (validate_functions && result.ok() && intermediate_error_.has_error()) {
// Copy error message and location.
return ModuleResult{std::move(intermediate_error_)};
}
return result;
return toResult(std::move(module_));
}
// Decodes an entire module.
@ -1652,7 +1647,7 @@ class ModuleDecoderTemplate : public Decoder {
base::Vector<const byte> orig_bytes(start(), end() - start());
DecodeModuleHeader(base::VectorOf(start(), end() - start()), offset);
if (failed()) {
return FinishDecoding(validate_functions);
return FinishDecoding();
}
// Size of the module header.
offset += 8;
@ -1679,7 +1674,7 @@ class ModuleDecoderTemplate : public Decoder {
return decoder.toResult<std::shared_ptr<WasmModule>>(nullptr);
}
return FinishDecoding(validate_functions);
return FinishDecoding();
}
// Decodes a single anonymous function starting at {start_}.
@ -1687,18 +1682,13 @@ class ModuleDecoderTemplate : public Decoder {
Zone* zone, const ModuleWireBytes& wire_bytes, const WasmModule* module) {
pc_ = start_;
expect_u8("type form", kWasmFunctionTypeCode);
if (!ok()) return FunctionResult{std::move(intermediate_error_)};
WasmFunction function;
function.sig = consume_sig(zone);
function.code = {off(pc_), static_cast<uint32_t>(end_ - pc_)};
if (!ok()) return FunctionResult{std::move(error_)};
if (ok()) {
ValidateFunctionBody(zone->allocator(), 0, wire_bytes, module, &function);
}
if (intermediate_error_.has_error()) {
return FunctionResult{std::move(intermediate_error_)};
}
if (!ok()) return FunctionResult{std::move(error_)};
return FunctionResult{std::make_unique<WasmFunction>(function)};
}
@ -1747,7 +1737,6 @@ class ModuleDecoderTemplate : public Decoder {
kBitsPerByte * sizeof(ModuleDecoderTemplate::seen_unordered_sections_) >
kLastKnownModuleSection,
"not enough bits");
WasmError intermediate_error_;
ModuleOrigin origin_;
AccountingAllocator allocator_;
Zone init_expr_zone_{&allocator_, "constant expr. zone"};
@ -1834,14 +1823,14 @@ class ModuleDecoderTemplate : public Decoder {
// If the decode failed and this is the first error, set error code and
// location.
if (result.failed() && intermediate_error_.empty()) {
if (result.failed() && error_.empty()) {
// Wrap the error message from the function decoder.
WasmFunctionName func_name(function,
wire_bytes.GetNameOrNull(function, module));
std::ostringstream error_msg;
error_msg << "in function " << func_name << ": "
<< result.error().message();
intermediate_error_ = WasmError{result.error().offset(), error_msg.str()};
error_ = WasmError{result.error().offset(), error_msg.str()};
}
}

View File

@ -191,9 +191,7 @@ bool ModuleDecoder::CheckFunctionsCount(uint32_t functions_count,
return impl_->CheckFunctionsCount(functions_count, error_offset);
}
ModuleResult ModuleDecoder::FinishDecoding(bool validate_functions) {
return impl_->FinishDecoding(validate_functions);
}
ModuleResult ModuleDecoder::FinishDecoding() { return impl_->FinishDecoding(); }
size_t ModuleDecoder::IdentifyUnknownSection(ModuleDecoder* decoder,
base::Vector<const uint8_t> bytes,

View File

@ -159,7 +159,7 @@ class ModuleDecoder {
void DecodeFunctionBody(uint32_t index, uint32_t size, uint32_t offset,
bool verify_functions = true);
ModuleResult FinishDecoding(bool verify_functions = true);
ModuleResult FinishDecoding();
const std::shared_ptr<WasmModule>& shared_module() const;