Commit b5e9eab6 authored by Thibaud Michaud's avatar Thibaud Michaud Committed by V8 LUCI CQ

[wasm] Fix module prefix hash for streaming decoder

The module's "prefix hash" is based on a prefix of the module bytes that
starts at the beginning of the module and stops at the code section.

In the case of the streaming decoder, if the code section is empty,
`AsyncStreamingProcessor::ProcessCodeSectionHeader()` is never called,
and we keep accumulating bytes in the hash after the code section. Fix
this by always calling into the streaming processor even if the code
section is empty.

R=ahaas@chromium.org
CC=clemensb@chromium.org

Bug: chromium:1334651
Change-Id: Id2a03468b355867868e589523c994c268c7b4eaf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695564
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81060}
parent ae41f7df
......@@ -2816,18 +2816,19 @@ bool AsyncStreamingProcessor::ProcessCodeSectionHeader(
before_code_section_ = false;
TRACE_STREAMING("Start the code section with %d functions...\n",
num_functions);
decoder_.StartCodeSection();
prefix_hash_ = base::hash_combine(prefix_hash_,
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());
return false;
}
if (num_functions == 0) return true;
decoder_.StartCodeSection();
decoder_.set_code_section(code_section_start,
static_cast<uint32_t>(code_section_length));
prefix_hash_ = base::hash_combine(prefix_hash_,
static_cast<uint32_t>(code_section_length));
if (!GetWasmEngine()->GetStreamingCompilationOwnership(prefix_hash_)) {
// Known prefix, wait until the end of the stream and check the cache.
prefix_cache_hit_ = true;
......
......@@ -645,14 +645,6 @@ AsyncStreamingDecoder::DecodeNumberOfFunctions::NextWithValue(
}
memcpy(payload_buf.begin(), buffer().begin(), bytes_consumed_);
// {value} is the number of functions.
if (value_ == 0) {
if (payload_buf.size() != bytes_consumed_) {
return streaming->Error("not all code section bytes were used");
}
return std::make_unique<DecodeSectionID>(streaming->module_offset());
}
DCHECK_GE(kMaxInt, section_buffer_->module_offset() +
section_buffer_->payload_offset());
int code_section_start = static_cast<int>(section_buffer_->module_offset() +
......@@ -664,6 +656,15 @@ AsyncStreamingDecoder::DecodeNumberOfFunctions::NextWithValue(
streaming->section_buffers_.back(),
code_section_start, code_section_len);
if (!streaming->ok()) return nullptr;
// {value} is the number of functions.
if (value_ == 0) {
if (payload_buf.size() != bytes_consumed_) {
return streaming->Error("not all code section bytes were used");
}
return std::make_unique<DecodeSectionID>(streaming->module_offset());
}
return std::make_unique<DecodeFunctionLength>(
section_buffer_, section_buffer_->payload_offset() + bytes_consumed_,
value_);
......
......@@ -301,12 +301,7 @@ size_t NativeModuleCache::PrefixHash(base::Vector<const uint8_t> wire_bytes) {
section_id = static_cast<SectionCode>(decoder.consume_u8());
uint32_t section_size = decoder.consume_u32v("section size");
if (section_id == SectionCode::kCodeSectionCode) {
uint32_t num_functions = decoder.consume_u32v("num functions");
// If {num_functions} is 0, the streaming decoder skips the section. Do
// the same here to ensure hashes are consistent.
if (num_functions != 0) {
hash = base::hash_combine(hash, section_size);
}
hash = base::hash_combine(hash, section_size);
break;
}
const uint8_t* payload_start = decoder.pc();
......
......@@ -1548,6 +1548,17 @@ STREAM_TEST(TierDownWithError) {
tester.RunCompilerTasks();
}
STREAM_TEST(Regress1334651) {
StreamTester tester(isolate);
const uint8_t bytes[] = {WASM_MODULE_HEADER, SECTION(Code, ENTRY_COUNT(0)),
SECTION(Unknown, 0)};
tester.OnBytesReceived(bytes, arraysize(bytes));
tester.FinishStream();
tester.RunCompilerTasks();
}
#undef STREAM_TEST
} // namespace v8::internal::wasm
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