Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix encoding of heap type definitions
The leading bytes that indicate what kind of heap type is being defined
are bytes, but we were previously treating them as SLEB128-encoded
values. Since we emit the smallest LEB encodings possible, we were
writing the correct bytes in output files, but we were also improperly
accepting binaries that used more than one byte to encode these values.
This was caught by an upstream spec test.
  • Loading branch information
tlively committed Aug 21, 2024
commit bf10d42ff36887284b138328d69f2a326ab39cc3
5 changes: 2 additions & 3 deletions scripts/test/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,8 @@ def get_tests(test_dir, extensions=[], recursive=False):
'elem.wast',
]
SPEC_TESTSUITE_TESTS_TO_SKIP = [
'address.wast',
'align.wast',
'binary-leb128.wast',
'address.wast', # 64-bit offset allowed by memory64
'align.wast', # Alignment bit 6 used by multi-memory
'binary.wast',
'block.wast',
'br_table.wast',
Expand Down
18 changes: 9 additions & 9 deletions src/wasm-binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,15 @@ enum EncodedType {
// string reference types
stringref = -0x19, // 0x67
// type forms
Func = -0x20, // 0x60
Cont = -0x23, // 0x5d
Struct = -0x21, // 0x5f
Array = -0x22, // 0x5e
Sub = -0x30, // 0x50
SubFinal = -0x31, // 0x4f
Shared = -0x1b, // 0x65
// isorecursive recursion groups
Rec = -0x32, // 0x4e
Func = 0x60,
Cont = 0x5d,
Struct = 0x5f,
Array = 0x5e,
Sub = 0x50,
SubFinal = 0x4f,
SharedDef = 0x65,
Shared = -0x1b, // Also 0x65 as an SLEB128
Rec = 0x4e,
// block_type
Empty = -0x40, // 0x40
};
Expand Down
28 changes: 14 additions & 14 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,17 +269,17 @@ void WasmBinaryWriter::writeTypes() {
// size 1 are implicit, so only emit a group header for larger groups.
auto currGroup = type.getRecGroup();
if (lastGroup != currGroup && currGroup.size() > 1) {
o << S32LEB(BinaryConsts::EncodedType::Rec) << U32LEB(currGroup.size());
o << uint8_t(BinaryConsts::EncodedType::Rec) << U32LEB(currGroup.size());
}
lastGroup = currGroup;
// Emit the type definition.
BYN_TRACE("write " << type << std::endl);
auto super = type.getDeclaredSuperType();
if (super || type.isOpen()) {
if (type.isOpen()) {
o << S32LEB(BinaryConsts::EncodedType::Sub);
o << uint8_t(BinaryConsts::EncodedType::Sub);
} else {
o << S32LEB(BinaryConsts::EncodedType::SubFinal);
o << uint8_t(BinaryConsts::EncodedType::SubFinal);
}
if (super) {
o << U32LEB(1);
Expand All @@ -289,11 +289,11 @@ void WasmBinaryWriter::writeTypes() {
}
}
if (type.isShared()) {
o << S32LEB(BinaryConsts::EncodedType::Shared);
o << uint8_t(BinaryConsts::EncodedType::SharedDef);
}
switch (type.getKind()) {
case HeapTypeKind::Func: {
o << S32LEB(BinaryConsts::EncodedType::Func);
o << uint8_t(BinaryConsts::EncodedType::Func);
auto sig = type.getSignature();
for (auto& sigType : {sig.params, sig.results}) {
o << U32LEB(sigType.size());
Expand All @@ -304,7 +304,7 @@ void WasmBinaryWriter::writeTypes() {
break;
}
case HeapTypeKind::Struct: {
o << S32LEB(BinaryConsts::EncodedType::Struct);
o << uint8_t(BinaryConsts::EncodedType::Struct);
auto fields = type.getStruct().fields;
o << U32LEB(fields.size());
for (const auto& field : fields) {
Expand All @@ -313,11 +313,11 @@ void WasmBinaryWriter::writeTypes() {
break;
}
case HeapTypeKind::Array:
o << S32LEB(BinaryConsts::EncodedType::Array);
o << uint8_t(BinaryConsts::EncodedType::Array);
writeField(type.getArray().element);
break;
case HeapTypeKind::Cont:
o << S32LEB(BinaryConsts::EncodedType::Cont);
o << uint8_t(BinaryConsts::EncodedType::Cont);
writeHeapType(type.getContinuation().type);
break;
case HeapTypeKind::Basic:
Expand Down Expand Up @@ -2405,7 +2405,7 @@ void WasmBinaryReader::readTypes() {

for (size_t i = 0; i < builder.size(); i++) {
BYN_TRACE("read one\n");
auto form = getS32LEB();
auto form = getInt8();
if (form == BinaryConsts::EncodedType::Rec) {
uint32_t groupSize = getU32LEB();
if (groupSize == 0u) {
Expand All @@ -2416,7 +2416,7 @@ void WasmBinaryReader::readTypes() {
// allocate space for the extra types.
builder.grow(groupSize - 1);
builder.createRecGroup(i, groupSize);
form = getS32LEB();
form = getInt8();
}
std::optional<uint32_t> superIndex;
if (form == BinaryConsts::EncodedType::Sub ||
Expand All @@ -2432,11 +2432,11 @@ void WasmBinaryReader::readTypes() {
}
superIndex = getU32LEB();
}
form = getS32LEB();
form = getInt8();
}
if (form == BinaryConsts::Shared) {
if (form == BinaryConsts::SharedDef) {
builder[i].setShared();
form = getS32LEB();
form = getInt8();
}
if (form == BinaryConsts::EncodedType::Func) {
builder[i] = readSignatureDef();
Expand Down Expand Up @@ -4741,7 +4741,7 @@ Index WasmBinaryReader::readMemoryAccess(Address& alignment, Address& offset) {
rawAlignment = rawAlignment & ~(1 << 6);
}

if (rawAlignment > 8) {
if (rawAlignment >= 8) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate fix?

throwError("Alignment must be of a reasonable size");
}

Expand Down
Loading