Skip to content

Commit

Permalink
[wasm][streaming] Report deterministic error location
Browse files Browse the repository at this point in the history
This fixes a bug in the offset computation when instantiating the
decoder to decode a VarInt32.
It also extends the streaming decoder test to check the error location.

[email protected]

Bug: v8:8814
Change-Id: Id8ce31ce7e494cce14231febbb5b0c7d91a26e01
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1505453
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Clemens Hammacher <[email protected]>
Cr-Commit-Position: refs/heads/master@{#60067}
  • Loading branch information
backes authored and Commit Bot committed Mar 6, 2019
1 parent 9584b6b commit 3a16ee8
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 55 deletions.
3 changes: 2 additions & 1 deletion src/wasm/streaming-decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ size_t StreamingDecoder::DecodeVarInt32::ReadBytes(
TRACE_STREAMING("ReadBytes of a VarInt\n");
memcpy(remaining_buf.start(), &bytes.first(), new_bytes);
buf.Truncate(offset() + new_bytes);
Decoder decoder(buf, streaming->module_offset());
Decoder decoder(buf,
streaming->module_offset() - static_cast<uint32_t>(offset()));
value_ = decoder.consume_u32v(field_name_);
// The number of bytes we actually needed to read.
DCHECK_GT(decoder.pc(), buffer().start());
Expand Down
117 changes: 63 additions & 54 deletions test/unittests/wasm/streaming-decoder-unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ class WasmStreamingDecoderTest : public ::testing::Test {
}
}

void ExpectFailure(Vector<const uint8_t> data, const char* message) {
void ExpectFailure(Vector<const uint8_t> data, uint32_t error_offset,
const char* message) {
for (int split = 0; split <= data.length(); ++split) {
MockStreamingResult result;
StreamingDecoder stream(
Expand All @@ -119,6 +120,7 @@ class WasmStreamingDecoderTest : public ::testing::Test {
stream.OnBytesReceived(data.SubVector(split, data.length()));
stream.Finish();
EXPECT_FALSE(result.ok());
EXPECT_EQ(error_offset, result.error.offset());
EXPECT_EQ(message, result.error.message());
}
}
Expand All @@ -136,12 +138,12 @@ TEST_F(WasmStreamingDecoderTest, IncompleteModuleHeader) {
{
MockStreamingResult result;
StreamingDecoder stream(base::make_unique<MockStreamingProcessor>(&result));
stream.OnBytesReceived(Vector<const uint8_t>(data, 1));
stream.OnBytesReceived(VectorOf(data, 1));
stream.Finish();
EXPECT_FALSE(result.ok());
}
for (int length = 1; length < static_cast<int>(arraysize(data)); ++length) {
ExpectFailure(Vector<const uint8_t>(data, length),
for (uint32_t length = 1; length < sizeof(data); ++length) {
ExpectFailure(VectorOf(data, length), length - 1,
"unexpected end of stream");
}
}
Expand All @@ -154,14 +156,14 @@ TEST_F(WasmStreamingDecoderTest, MagicAndVersion) {
TEST_F(WasmStreamingDecoderTest, BadMagic) {
for (uint32_t x = 1; x; x <<= 1) {
const uint8_t data[] = {U32_LE(kWasmMagic ^ x), U32_LE(kWasmVersion)};
ExpectFailure(ArrayVector(data), "expected wasm magic");
ExpectFailure(ArrayVector(data), 0, "expected wasm magic");
}
}

TEST_F(WasmStreamingDecoderTest, BadVersion) {
for (uint32_t x = 1; x; x <<= 1) {
const uint8_t data[] = {U32_LE(kWasmMagic), U32_LE(kWasmVersion ^ x)};
ExpectFailure(ArrayVector(data), "expected wasm version");
ExpectFailure(ArrayVector(data), 4, "expected wasm version");
}
}

Expand Down Expand Up @@ -248,7 +250,8 @@ TEST_F(WasmStreamingDecoderTest, OneSectionNotEnoughPayload1) {
0x0, // 4
0x0 // 5
};
ExpectFailure(ArrayVector(data), "unexpected end of stream");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"unexpected end of stream");
}

TEST_F(WasmStreamingDecoderTest, OneSectionNotEnoughPayload2) {
Expand All @@ -259,7 +262,8 @@ TEST_F(WasmStreamingDecoderTest, OneSectionNotEnoughPayload2) {
0x6, // Section Length
0x0 // Payload
};
ExpectFailure(ArrayVector(data), "unexpected end of stream");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"unexpected end of stream");
}

TEST_F(WasmStreamingDecoderTest, OneSectionInvalidLength) {
Expand All @@ -273,7 +277,7 @@ TEST_F(WasmStreamingDecoderTest, OneSectionInvalidLength) {
0x80, // --
0x80, // --
};
ExpectFailure(ArrayVector(data), "expected section length");
ExpectFailure(ArrayVector(data), sizeof(data) - 1, "expected section length");
}

TEST_F(WasmStreamingDecoderTest, TwoLongSections) {
Expand Down Expand Up @@ -386,9 +390,10 @@ TEST_F(WasmStreamingDecoderTest, EmptyFunction) {
kCodeSectionCode, // Section ID
0x2, // Section Length
0x1, // Number of Functions
0x0, // Function Length
0x0, // Function Length -- ERROR
};
ExpectFailure(ArrayVector(data), "invalid function length (0)");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"invalid function length (0)");
}

TEST_F(WasmStreamingDecoderTest, TwoFunctions) {
Expand Down Expand Up @@ -445,7 +450,8 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthZero) {
kCodeSectionCode, // Section ID
0x0, // Section Length
};
ExpectFailure(ArrayVector(data), "code section cannot have size 0");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"code section cannot have size 0");
}

TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooHigh) {
Expand All @@ -466,7 +472,8 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooHigh) {
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "not all code section bytes were used");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"not all code section bytes were used");
}

TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooHighZeroFunctions) {
Expand All @@ -477,7 +484,8 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooHighZeroFunctions) {
0xD, // Section Length
0x0, // Number of Functions
};
ExpectFailure(ArrayVector(data), "not all code section bytes were used");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"not all code section bytes were used");
}

TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLow) {
Expand All @@ -486,19 +494,20 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLow) {
U32_LE(kWasmVersion), // --
kCodeSectionCode, // Section ID
0x9, // Section Length
0x2, // Number of Functions
0x7, // Function Length
0x0, // Function
0x0, // 2
0x0, // 3
0x0, // 4
0x0, // 5
0x0, // 6
0x0, // 7
0x1, // Function Length
0x2, // Number of Functions <0>
0x7, // Function Length <1>
0x0, // Function <2>
0x0, // 2 <3>
0x0, // 3 <3>
0x0, // 4 <4>
0x0, // 5 <5>
0x0, // 6 <6>
0x0, // 7 <7>
0x1, // Function Length <8> -- ERROR
0x0, // Function
};
ExpectFailure(ArrayVector(data), "read past code section end");
ExpectFailure(ArrayVector(data), sizeof(data) - 2,
"read past code section end");
}

TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInNumFunctions) {
Expand All @@ -507,8 +516,8 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInNumFunctions) {
U32_LE(kWasmVersion), // --
kCodeSectionCode, // Section ID
0x1, // Section Length
0x82, // Number of Functions
0x80, // --
0x82, // Number of Functions <0>
0x80, // -- <1> -- ERROR
0x00, // --
0x7, // Function Length
0x0, // Function
Expand All @@ -521,7 +530,7 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInNumFunctions) {
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "invalid code section length");
ExpectFailure(ArrayVector(data), 12, "invalid code section length");
}

TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInFunctionLength) {
Expand All @@ -530,12 +539,12 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInFunctionLength) {
U32_LE(kWasmVersion), // --
kCodeSectionCode, // Section ID
0x5, // Section Length
0x82, // Number of Functions
0x80, // --
0x00, // --
0x87, // Function Length
0x80, // --
0x00, // --
0x82, // Number of Functions <0>
0x80, // -- <1>
0x00, // -- <2>
0x87, // Function Length <3>
0x80, // -- <4>
0x00, // -- <5> -- ERROR
0x0, // Function
0x0, // 2
0x0, // 3
Expand All @@ -546,7 +555,7 @@ TEST_F(WasmStreamingDecoderTest, CodeSectionLengthTooLowEndsInFunctionLength) {
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "read past code section end");
ExpectFailure(ArrayVector(data), 15, "read past code section end");
}

TEST_F(WasmStreamingDecoderTest, NumberOfFunctionsTooHigh) {
Expand All @@ -567,31 +576,27 @@ TEST_F(WasmStreamingDecoderTest, NumberOfFunctionsTooHigh) {
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "unexpected end of stream");
ExpectFailure(ArrayVector(data), sizeof(data) - 1,
"unexpected end of stream");
}

TEST_F(WasmStreamingDecoderTest, NumberOfFunctionsTooLow) {
const uint8_t data[] = {
U32_LE(kWasmMagic), // --
U32_LE(kWasmVersion), // --
kCodeSectionCode, // Section ID
0xE, // Section Length
0x8, // Section Length
0x2, // Number of Functions
0x1, // Function Length
0x0, // Function
0x2, // Function Length
0x0, // Function
0x0, // 2
0x7, // Function Length
0x0, // Function
0x0, // 2
0x0, // 3
0x0, // 4
0x0, // 5
0x0, // 6
0x0, // 7
0x0, // Function byte#0
0x0, // Function byte#1 -- ERROR
0x1, // Function Length
0x0 // Function
};
ExpectFailure(ArrayVector(data), "not all code section bytes were used");
ExpectFailure(ArrayVector(data), sizeof(data) - 3,
"not all code section bytes were used");
}

TEST_F(WasmStreamingDecoderTest, TwoCodeSections) {
Expand All @@ -603,13 +608,15 @@ TEST_F(WasmStreamingDecoderTest, TwoCodeSections) {
0x1, // Number of Functions
0x1, // Function Length
0x0, // Function
kCodeSectionCode, // Section ID
0x3, // Section Length
kCodeSectionCode, // Section ID -- ERROR (where it should be)
0x3, // Section Length -- ERROR (where it is reported)
0x1, // Number of Functions
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "code section can only appear once");
// TODO(wasm): This should report at the second kCodeSectionCode.
ExpectFailure(ArrayVector(data), sizeof(data) - 4,
"code section can only appear once");
}

TEST_F(WasmStreamingDecoderTest, UnknownSection) {
Expand Down Expand Up @@ -644,13 +651,15 @@ TEST_F(WasmStreamingDecoderTest, UnknownSectionSandwich) {
0x1, // Name Length
0x1, // Name
0x0, // Content
kCodeSectionCode, // Section ID
0x3, // Section Length
kCodeSectionCode, // Section ID -- ERROR (where it should be)
0x3, // Section Length -- ERROR (where it is reported)
0x1, // Number of Functions
0x1, // Function Length
0x0, // Function
};
ExpectFailure(ArrayVector(data), "code section can only appear once");
// TODO(wasm): This should report at the second kCodeSectionCode.
ExpectFailure(ArrayVector(data), sizeof(data) - 4,
"code section can only appear once");
}

} // namespace wasm
Expand Down

0 comments on commit 3a16ee8

Please sign in to comment.