Skip to content

Commit

Permalink
[wasm] Simplify LEB decoding
Browse files Browse the repository at this point in the history 
Remove one "mode" of LEB decoding by eliminating the {AdvancePCFlag},
and doing the PC advance in the caller instead.
The returned length is now always zero in case of an error, thus remove
the respective checks from the unit tests. The returned length does not
really matter if we ran into an error.

R=thibaudm@chromium.org

Change-Id: Ibfd94dd981cefa2fc24c7af560c85afd1c826f2c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2449972
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70404}
  • Loading branch information
@backes
backes authored and Commit Bot committed Oct 8, 2020
1 parent 637d1fc commit 5bf1619
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 40 deletions.
53 changes: 26 additions & 27 deletions src/wasm/decoder.h
View file
Expand Up @@ -46,8 +46,6 @@ class Decoder {
kFullValidation // Run full validation with error message and location.
};

enum AdvancePCFlag : bool { kAdvancePc = true, kNoAdvancePc = false };

enum TraceFlag : bool { kTrace = true, kNoTrace = false };

Decoder(const byte* start, const byte* end, uint32_t buffer_offset = 0)
Expand Down Expand Up @@ -101,40 +99,35 @@ class Decoder {
template <ValidateFlag validate>
uint32_t read_u32v(const byte* pc, uint32_t* length,
const char* name = "LEB32") {
return read_leb<uint32_t, validate, kNoAdvancePc, kNoTrace>(pc, length,
name);
return read_leb<uint32_t, validate, kNoTrace>(pc, length, name);
}

// Reads a variable-length signed integer (little endian).
template <ValidateFlag validate>
int32_t read_i32v(const byte* pc, uint32_t* length,
const char* name = "signed LEB32") {
return read_leb<int32_t, validate, kNoAdvancePc, kNoTrace>(pc, length,
name);
return read_leb<int32_t, validate, kNoTrace>(pc, length, name);
}

// Reads a variable-length unsigned integer (little endian).
template <ValidateFlag validate>
uint64_t read_u64v(const byte* pc, uint32_t* length,
const char* name = "LEB64") {
return read_leb<uint64_t, validate, kNoAdvancePc, kNoTrace>(pc, length,
name);
return read_leb<uint64_t, validate, kNoTrace>(pc, length, name);
}

// Reads a variable-length signed integer (little endian).
template <ValidateFlag validate>
int64_t read_i64v(const byte* pc, uint32_t* length,
const char* name = "signed LEB64") {
return read_leb<int64_t, validate, kNoAdvancePc, kNoTrace>(pc, length,
name);
return read_leb<int64_t, validate, kNoTrace>(pc, length, name);
}

// Reads a variable-length 33-bit signed integer (little endian).
template <ValidateFlag validate>
int64_t read_i33v(const byte* pc, uint32_t* length,
const char* name = "signed LEB33") {
return read_leb<int64_t, validate, kNoAdvancePc, kNoTrace, 33>(pc, length,
name);
return read_leb<int64_t, validate, kNoTrace, 33>(pc, length, name);
}

// Reads a prefixed-opcode, possibly with variable-length index.
Expand Down Expand Up @@ -188,22 +181,28 @@ class Decoder {
// Reads a LEB128 variable-length unsigned 32-bit integer and advances {pc_}.
uint32_t consume_u32v(const char* name = nullptr) {
uint32_t length = 0;
return read_leb<uint32_t, kFullValidation, kAdvancePc, kTrace>(pc_, &length,
name);
uint32_t result =
read_leb<uint32_t, kFullValidation, kTrace>(pc_, &length, name);
pc_ += length;
return result;
}

// Reads a LEB128 variable-length signed 32-bit integer and advances {pc_}.
int32_t consume_i32v(const char* name = nullptr) {
uint32_t length = 0;
return read_leb<int32_t, kFullValidation, kAdvancePc, kTrace>(pc_, &length,
name);
int32_t result =
read_leb<int32_t, kFullValidation, kTrace>(pc_, &length, name);
pc_ += length;
return result;
}

// Reads a LEB128 variable-length unsigned 64-bit integer and advances {pc_}.
uint64_t consume_u64v(const char* name = nullptr) {
uint32_t length = 0;
return read_leb<uint64_t, kFullValidation, kAdvancePc, kTrace>(pc_, &length,
name);
uint64_t result =
read_leb<uint64_t, kFullValidation, kTrace>(pc_, &length, name);
pc_ += length;
return result;
}

// Consume {size} bytes and send them to the bit bucket, advancing {pc_}.
Expand Down Expand Up @@ -386,20 +385,19 @@ class Decoder {
return val;
}

template <typename IntType, ValidateFlag validate, AdvancePCFlag advance_pc,
TraceFlag trace, size_t size_in_bits = 8 * sizeof(IntType)>
template <typename IntType, ValidateFlag validate, TraceFlag trace,
size_t size_in_bits = 8 * sizeof(IntType)>
IntType read_leb(const byte* pc, uint32_t* length,
const char* name = "varint") {
DCHECK_IMPLIES(advance_pc, pc == pc_);
static_assert(size_in_bits <= 8 * sizeof(IntType),
"leb does not fit in type");
TRACE_IF(trace, " +%u %-20s: ", pc_offset(), name);
return read_leb_tail<IntType, validate, advance_pc, trace, size_in_bits, 0>(
pc, length, name, 0);
return read_leb_tail<IntType, validate, trace, size_in_bits, 0>(pc, length,
name, 0);
}

template <typename IntType, ValidateFlag validate, AdvancePCFlag advance_pc,
TraceFlag trace, size_t size_in_bits, int byte_index>
template <typename IntType, ValidateFlag validate, TraceFlag trace,
size_t size_in_bits, int byte_index>
IntType read_leb_tail(const byte* pc, uint32_t* length, const char* name,
IntType result) {
constexpr bool is_signed = std::is_signed<IntType>::value;
Expand All @@ -422,10 +420,9 @@ class Decoder {
// Compilers are not smart enough to figure out statically that the
// following call is unreachable if is_last_byte is false.
constexpr int next_byte_index = byte_index + (is_last_byte ? 0 : 1);
return read_leb_tail<IntType, validate, advance_pc, trace, size_in_bits,
return read_leb_tail<IntType, validate, trace, size_in_bits,
next_byte_index>(pc + 1, length, name, result);
}
if (advance_pc) pc_ = pc + (at_end ? 0 : 1);
*length = byte_index + (at_end ? 0 : 1);
if (validate && V8_UNLIKELY(at_end || (b & 0x80))) {
TRACE_IF(trace, at_end ? "<end> " : "<length overflow> ");
Expand All @@ -435,6 +432,7 @@ class Decoder {
MarkError();
}
result = 0;
*length = 0;
}
if (is_last_byte) {
// A signed-LEB128 must sign-extend the final byte, excluding its
Expand All @@ -459,6 +457,7 @@ class Decoder {
MarkError();
}
result = 0;
*length = 0;
}
}
constexpr int sign_ext_shift =
Expand Down
9 changes: 5 additions & 4 deletions src/wasm/streaming-decoder.cc
View file
Expand Up @@ -517,10 +517,6 @@ size_t AsyncStreamingDecoder::DecodeVarInt32::ReadBytes(
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().begin());
bytes_consumed_ = static_cast<size_t>(decoder.pc() - buf.begin());
TRACE_STREAMING(" ==> %zu bytes consumed\n", bytes_consumed_);

if (decoder.failed()) {
if (new_bytes == remaining_buf.size()) {
Expand All @@ -531,6 +527,11 @@ size_t AsyncStreamingDecoder::DecodeVarInt32::ReadBytes(
return new_bytes;
}

// The number of bytes we actually needed to read.
DCHECK_GT(decoder.pc(), buffer().begin());
bytes_consumed_ = static_cast<size_t>(decoder.pc() - buf.begin());
TRACE_STREAMING(" ==> %zu bytes consumed\n", bytes_consumed_);

// We read all the bytes we needed.
DCHECK_GT(bytes_consumed_, offset());
new_bytes = bytes_consumed_ - offset();
Expand Down
9 changes: 0 additions & 9 deletions test/unittests/wasm/decoder-unittest.cc
View file
Expand Up @@ -380,7 +380,6 @@ TEST_F(DecoderTest, ReadU32v_off_end1) {
unsigned length = 0;
decoder.Reset(data, data);
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(0u, length);
EXPECT_FALSE(decoder.ok());
}

Expand All @@ -390,7 +389,6 @@ TEST_F(DecoderTest, ReadU32v_off_end2) {
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(i, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -401,7 +399,6 @@ TEST_F(DecoderTest, ReadU32v_off_end3) {
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(i, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -412,7 +409,6 @@ TEST_F(DecoderTest, ReadU32v_off_end4) {
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(i, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -423,7 +419,6 @@ TEST_F(DecoderTest, ReadU32v_off_end5) {
unsigned length = 0;
decoder.Reset(data, data + i);
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(i, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -435,7 +430,6 @@ TEST_F(DecoderTest, ReadU32v_extra_bits) {
unsigned length = 0;
decoder.Reset(data, data + sizeof(data));
decoder.read_u32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(5u, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -456,7 +450,6 @@ TEST_F(DecoderTest, ReadI32v_extra_bits_positive) {
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x77};
decoder.Reset(data, data + sizeof(data));
decoder.read_i32v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(5u, length);
EXPECT_FALSE(decoder.ok());
}

Expand Down Expand Up @@ -654,7 +647,6 @@ TEST_F(DecoderTest, ReadU64v_extra_bits) {
unsigned length = 0;
decoder.Reset(data, data + sizeof(data));
decoder.read_u64v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(10u, length);
EXPECT_FALSE(decoder.ok());
}
}
Expand All @@ -675,7 +667,6 @@ TEST_F(DecoderTest, ReadI64v_extra_bits_positive) {
byte data[] = {0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x77};
decoder.Reset(data, data + sizeof(data));
decoder.read_i64v<Decoder::kFullValidation>(decoder.start(), &length);
EXPECT_EQ(10u, length);
EXPECT_FALSE(decoder.ok());
}

Expand Down

0 comments on commit 5bf1619

Please sign in to comment.