Commit 7bace1d4 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm] Pass correct code section start in streaming compilation

The streaming decoder computed the code section start from the passed
"offset". That offset is computed from the module offset *after* the
number of functions has been read. Hence 1 is subtracted, with the
comment:
// The offset passed to {ProcessCodeSectionHeader} is an error offset and
// not the start offset of a buffer. Therefore we need the -1 here.

That subtraction of 1 worked when the number of functions was encoded in
a 1-byte LEB, otherwise it was off.

This CL fixes the immediate issue of passing the right code offset. The
usage of the previously existing offset also seems wrong, and I will try
to clean that up in a follow-up CL.

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

Bug: chromium:1150303
Change-Id: I64bb2ececeb4749b7ba2096cd148ccb4079eca4f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2562383
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71503}
parent 073ff8b4
......@@ -1752,6 +1752,7 @@ class AsyncStreamingProcessor final : public StreamingProcessor {
bool ProcessCodeSectionHeader(int num_functions, uint32_t offset,
std::shared_ptr<WireBytesStorage>,
int code_section_start,
int code_section_length) override;
bool ProcessFunctionBody(Vector<const uint8_t> bytes,
......@@ -2487,7 +2488,7 @@ bool AsyncStreamingProcessor::ProcessSection(SectionCode section_code,
bool AsyncStreamingProcessor::ProcessCodeSectionHeader(
int num_functions, uint32_t offset,
std::shared_ptr<WireBytesStorage> wire_bytes_storage,
int code_section_length) {
int code_section_start, int code_section_length) {
DCHECK_LE(0, code_section_length);
before_code_section_ = false;
TRACE_STREAMING("Start the code section with %d functions...\n",
......@@ -2499,7 +2500,8 @@ bool AsyncStreamingProcessor::ProcessCodeSectionHeader(
return false;
}
decoder_.set_code_section(offset, static_cast<uint32_t>(code_section_length));
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));
......
......@@ -184,13 +184,13 @@ class V8_EXPORT_PRIVATE AsyncStreamingDecoder : public StreamingDecoder {
void StartCodeSection(int num_functions,
std::shared_ptr<WireBytesStorage> wire_bytes_storage,
int code_section_length) {
int code_section_start, int code_section_length) {
if (!ok()) return;
// The offset passed to {ProcessCodeSectionHeader} is an error offset and
// not the start offset of a buffer. Therefore we need the -1 here.
if (!processor_->ProcessCodeSectionHeader(
num_functions, module_offset() - 1, std::move(wire_bytes_storage),
code_section_length)) {
code_section_start, code_section_length)) {
Fail();
}
}
......@@ -638,12 +638,16 @@ AsyncStreamingDecoder::DecodeNumberOfFunctions::NextWithValue(
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() +
section_buffer_->payload_offset());
DCHECK_GE(kMaxInt, payload_buf.length());
int code_section_len = static_cast<int>(payload_buf.length());
DCHECK_GE(kMaxInt, value_);
streaming->StartCodeSection(static_cast<int>(value_),
streaming->section_buffers_.back(),
code_section_len);
code_section_start, code_section_len);
if (!streaming->ok()) return nullptr;
return std::make_unique<DecodeFunctionLength>(
section_buffer_, section_buffer_->payload_offset() + bytes_consumed_,
......
......@@ -39,6 +39,7 @@ class V8_EXPORT_PRIVATE StreamingProcessor {
// finished successfully and the decoding should continue.
virtual bool ProcessCodeSectionHeader(int num_functions, uint32_t offset,
std::shared_ptr<WireBytesStorage>,
int code_section_start,
int code_section_length) = 0;
// Process a function body. Returns true if the processing finished
......
......@@ -283,6 +283,8 @@ struct V8_EXPORT_PRIVATE WasmModule {
uint32_t num_declared_functions = 0; // excluding imported
uint32_t num_exported_functions = 0;
uint32_t num_declared_data_segments = 0; // From the DataCount section.
// Position and size of the code section (payload only, i.e. without section
// ID and length).
WireBytesRef code = {0, 0};
WireBytesRef name = {0, 0};
std::vector<TypeDefinition> types; // by type index
......
......@@ -6,4 +6,4 @@ Wasm script parsed: ID 3, startColumn: 0, endColumn: 32, codeOffset: 0
Wasm script parsed: ID 4, startColumn: 0, endColumn: 40, codeOffset: 36
Wasm script parsed: ID 5, startColumn: 0, endColumn: 40, codeOffset: 36
Wasm script parsed: ID 6, startColumn: 0, endColumn: 44, codeOffset: 36
Wasm script parsed: ID 7, startColumn: 0, endColumn: 44, codeOffset: 40
Wasm script parsed: ID 7, startColumn: 0, endColumn: 44, codeOffset: 36
......@@ -58,6 +58,7 @@ class MockStreamingProcessor : public StreamingProcessor {
bool ProcessCodeSectionHeader(int num_functions, uint32_t offset,
std::shared_ptr<WireBytesStorage>,
int code_section_start,
int code_section_length) override {
return true;
}
......
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