Skip to content

Commit

Permalink
Switch to treating segment flags as a bitfield. NFC (WebAssembly#1232)
Browse files Browse the repository at this point in the history 
This is in preparation for updating to latest version reference-types
proposal where there is an additional flag and they can be combined.

See: WebAssembly/bulk-memory-operations#98

Also, add ERROR_IF to binary-reader.cc as logical corollary to the
existing ERROR_UNLESS.
  • Loading branch information
@sbc100
sbc100 committed Nov 18, 2019
1 parent cc1edc0 commit 43fef7d
Show file tree
Hide file tree
Showing 23 changed files with 152 additions and 133 deletions.
14 changes: 8 additions & 6 deletions src/binary-reader-ir.cc
View file
Expand Up @@ -204,7 +204,7 @@ class BinaryReaderIR : public BinaryReaderNop {
Result OnElemSegmentCount(Index count) override;
Result BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) override;
Result BeginElemSegmentInitExpr(Index index) override;
Result EndElemSegmentInitExpr(Index index) override;
Expand All @@ -214,7 +214,9 @@ class BinaryReaderIR : public BinaryReaderNop {
Index func_index) override;

Result OnDataSegmentCount(Index count) override;
Result BeginDataSegment(Index index, Index memory_index, bool passive) override;
Result BeginDataSegment(Index index,
Index memory_index,
uint8_t flags) override;
Result BeginDataSegmentInitExpr(Index index) override;
Result EndDataSegmentInitExpr(Index index) override;
Result OnDataSegmentData(Index index,
Expand Down Expand Up @@ -1003,12 +1005,12 @@ Result BinaryReaderIR::OnElemSegmentCount(Index count) {

Result BinaryReaderIR::BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) {
auto field = MakeUnique<ElemSegmentModuleField>(GetLocation());
ElemSegment& elem_segment = field->elem_segment;
elem_segment.table_var = Var(table_index, GetLocation());
elem_segment.passive = passive;
elem_segment.flags = flags;
elem_segment.elem_type = elem_type;
module_->AppendField(std::move(field));
return Result::Ok;
Expand Down Expand Up @@ -1059,11 +1061,11 @@ Result BinaryReaderIR::OnDataSegmentCount(Index count) {

Result BinaryReaderIR::BeginDataSegment(Index index,
Index memory_index,
bool passive) {
uint8_t flags) {
auto field = MakeUnique<DataSegmentModuleField>(GetLocation());
DataSegment& data_segment = field->data_segment;
data_segment.memory_var = Var(memory_index, GetLocation());
data_segment.passive = passive;
data_segment.flags = flags;
module_->AppendField(std::move(field));
return Result::Ok;
}
Expand Down
23 changes: 12 additions & 11 deletions src/binary-reader-logging.cc
View file
Expand Up @@ -363,12 +363,12 @@ Result BinaryReaderLogging::OnSimdShuffleOpExpr(Opcode opcode, v128 value) {

Result BinaryReaderLogging::BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) {
LOGF("BeginElemSegment(index: %" PRIindex ", table_index: %" PRIindex
", passive: %s, elem_type: %s)\n",
index, table_index, passive ? "true" : "false", GetTypeName(elem_type));
return reader_->BeginElemSegment(index, table_index, passive, elem_type);
", flags: %d, elem_type: %s)\n",
index, table_index, flags, GetTypeName(elem_type));
return reader_->BeginElemSegment(index, table_index, flags, elem_type);
}

Result BinaryReaderLogging::OnDataSegmentData(Index index,
Expand Down Expand Up @@ -623,12 +623,13 @@ Result BinaryReaderLogging::OnComdatEntry(ComdatType kind, Index index) {
return reader_->name(value0, value1); \
}

#define DEFINE_INDEX_INDEX_BOOL(name, desc0, desc1, desc2) \
Result BinaryReaderLogging::name(Index value0, Index value1, bool value2) { \
LOGF(#name "(" desc0 ": %" PRIindex ", " desc1 ": %" PRIindex \
", " desc2 ": %s)\n", \
value0, value1, value2 ? "true" : "false"); \
return reader_->name(value0, value1, value2); \
#define DEFINE_INDEX_INDEX_U8(name, desc0, desc1, desc2) \
Result BinaryReaderLogging::name(Index value0, Index value1, \
uint8_t value2) { \
LOGF(#name "(" desc0 ": %" PRIindex ", " desc1 ": %" PRIindex ", " desc2 \
": %d)\n", \
value0, value1, value2); \
return reader_->name(value0, value1, value2); \
}

#define DEFINE_OPCODE(name) \
Expand Down Expand Up @@ -761,7 +762,7 @@ DEFINE_END(EndElemSection)

DEFINE_BEGIN(BeginDataSection)
DEFINE_INDEX(OnDataSegmentCount)
DEFINE_INDEX_INDEX_BOOL(BeginDataSegment, "index", "memory_index", "passive")
DEFINE_INDEX_INDEX_U8(BeginDataSegment, "index", "memory_index", "flags")
DEFINE_INDEX(BeginDataSegmentInitExpr)
DEFINE_INDEX(EndDataSegmentInitExpr)
DEFINE_INDEX(EndDataSegment)
Expand Down
6 changes: 4 additions & 2 deletions src/binary-reader-logging.h
View file
Expand Up @@ -230,7 +230,7 @@ class BinaryReaderLogging : public BinaryReaderDelegate {
Result OnElemSegmentCount(Index count) override;
Result BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) override;
Result BeginElemSegmentInitExpr(Index index) override;
Result EndElemSegmentInitExpr(Index index) override;
Expand All @@ -243,7 +243,9 @@ class BinaryReaderLogging : public BinaryReaderDelegate {

Result BeginDataSection(Offset size) override;
Result OnDataSegmentCount(Index count) override;
Result BeginDataSegment(Index index, Index memory_index, bool passive) override;
Result BeginDataSegment(Index index,
Index memory_index,
uint8_t flags) override;
Result BeginDataSegmentInitExpr(Index index) override;
Result EndDataSegmentInitExpr(Index index) override;
Result OnDataSegmentData(Index index,
Expand Down
6 changes: 4 additions & 2 deletions src/binary-reader-nop.h
View file
Expand Up @@ -301,7 +301,7 @@ class BinaryReaderNop : public BinaryReaderDelegate {
Result OnElemSegmentCount(Index count) override { return Result::Ok; }
Result BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) override {
return Result::Ok;
}
Expand All @@ -323,7 +323,9 @@ class BinaryReaderNop : public BinaryReaderDelegate {
/* Data section */
Result BeginDataSection(Offset size) override { return Result::Ok; }
Result OnDataSegmentCount(Index count) override { return Result::Ok; }
Result BeginDataSegment(Index index, Index memory_index, bool passive) override {
Result BeginDataSegment(Index index,
Index memory_index,
uint8_t flags) override {
return Result::Ok;
}
Result BeginDataSegmentInitExpr(Index index) override { return Result::Ok; }
Expand Down
29 changes: 15 additions & 14 deletions src/binary-reader-objdump.cc
View file
Expand Up @@ -791,7 +791,7 @@ class BinaryReaderObjdump : public BinaryReaderObjdumpBase {
Result OnElemSegmentCount(Index count) override;
Result BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) override;
Result OnElemSegmentElemExprCount(Index index, Index count) override;
Result OnElemSegmentElemExpr_RefNull(Index segment_index) override;
Expand All @@ -810,7 +810,7 @@ class BinaryReaderObjdump : public BinaryReaderObjdumpBase {
Result OnDataSegmentCount(Index count) override;
Result BeginDataSegment(Index index,
Index memory_index,
bool passive) override;
uint8_t flags) override;
Result OnDataSegmentData(Index index,
const void* data,
Address size) override;
Expand Down Expand Up @@ -896,8 +896,8 @@ class BinaryReaderObjdump : public BinaryReaderObjdumpBase {
bool in_elem_section_ = false;
InitExpr data_init_expr_;
InitExpr elem_init_expr_;
bool data_is_passive_ = false;
bool elem_is_passive_ = false;
uint8_t data_flags_ = 0;
uint8_t elem_flags_ = 0;
Index data_mem_index_ = 0;
uint32_t data_offset_ = 0;
uint32_t elem_offset_ = 0;
Expand Down Expand Up @@ -1264,20 +1264,21 @@ Result BinaryReaderObjdump::OnElemSegmentCount(Index count) {

Result BinaryReaderObjdump::BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) {
table_index_ = table_index;
elem_index_ = 0;
elem_is_passive_ = passive;
elem_flags_ = flags;
return Result::Ok;
}

Result BinaryReaderObjdump::OnElemSegmentElemExprCount(Index index,
Index count) {
PrintDetails(" - segment[%" PRIindex "] table=%" PRIindex " count=%" PRIindex,
index, table_index_, count);
if (elem_is_passive_) {
PrintDetails(" passive\n");
PrintDetails(" - segment[%" PRIindex "] flags=%d table=%" PRIindex
" count=%" PRIindex,
index, elem_flags_, table_index_, count);
if (elem_flags_ & SegPassive) {
PrintDetails("\n");
} else {
PrintInitExpr(elem_init_expr_);
}
Expand Down Expand Up @@ -1463,9 +1464,9 @@ Result BinaryReaderObjdump::OnDataSegmentCount(Index count) {

Result BinaryReaderObjdump::BeginDataSegment(Index index,
Index memory_index,
bool passive) {
uint8_t flags) {
data_mem_index_ = memory_index;
data_is_passive_ = passive;
data_flags_ = flags;
return Result::Ok;
}

Expand All @@ -1481,13 +1482,13 @@ Result BinaryReaderObjdump::OnDataSegmentData(Index index,
if (!name.empty()) {
PrintDetails(" <" PRIstringview ">", WABT_PRINTF_STRING_VIEW_ARG(name));
}
if (data_is_passive_) {
if (data_flags_ & SegPassive) {
PrintDetails(" passive");
} else {
PrintDetails(" memory=%" PRIindex, data_mem_index_);
}
PrintDetails(" size=%" PRIaddress, size);
if (data_is_passive_) {
if (data_flags_ & SegPassive) {
PrintDetails("\n");
} else {
PrintInitExpr(data_init_expr_);
Expand Down
54 changes: 27 additions & 27 deletions src/binary-reader.cc
View file
Expand Up @@ -36,14 +36,16 @@
#include <alloca.h>
#endif

#define ERROR_UNLESS(expr, ...) \
do { \
if (!(expr)) { \
PrintError(__VA_ARGS__); \
return Result::Error; \
} \
#define ERROR_IF(expr, ...) \
do { \
if (expr) { \
PrintError(__VA_ARGS__); \
return Result::Error; \
} \
} while (0)

#define ERROR_UNLESS(expr, ...) ERROR_IF(!(expr), __VA_ARGS__)

#define ERROR_UNLESS_OPCODE_ENABLED(opcode) \
do { \
if (!opcode.IsEnabled(options_.features)) { \
Expand Down Expand Up @@ -519,7 +521,7 @@ Result BinaryReader::ReadTable(Type* out_elem_type, Limits* out_elem_limits) {
CHECK_RESULT(ReadU32Leb128(&initial, "table initial elem count"));
bool has_max = flags & WABT_BINARY_LIMITS_HAS_MAX_FLAG;
bool is_shared = flags & WABT_BINARY_LIMITS_IS_SHARED_FLAG;
ERROR_UNLESS(!is_shared, "tables may not be shared");
ERROR_IF(is_shared, "tables may not be shared");
if (has_max) {
CHECK_RESULT(ReadU32Leb128(&max, "table max elem count"));
ERROR_UNLESS(initial <= max,
Expand All @@ -541,7 +543,7 @@ Result BinaryReader::ReadMemory(Limits* out_page_limits) {
ERROR_UNLESS(initial <= WABT_MAX_PAGES, "invalid memory initial size");
bool has_max = flags & WABT_BINARY_LIMITS_HAS_MAX_FLAG;
bool is_shared = flags & WABT_BINARY_LIMITS_IS_SHARED_FLAG;
ERROR_UNLESS(!is_shared || has_max, "shared memory must have a max size");
ERROR_IF(is_shared && !has_max, "shared memory must have a max size");
if (has_max) {
CHECK_RESULT(ReadU32Leb128(&max, "memory max page count"));
ERROR_UNLESS(max <= WABT_MAX_PAGES, "invalid memory max size");
Expand Down Expand Up @@ -2130,38 +2132,37 @@ Result BinaryReader::ReadElemSection(Offset section_size) {
ERROR_UNLESS(num_elem_segments == 0 || NumTotalTables() > 0,
"elem section without table section");
for (Index i = 0; i < num_elem_segments; ++i) {
uint32_t flags_u32;
CHECK_RESULT(ReadU32Leb128(&flags_u32, "elem segment flags"));
ERROR_UNLESS(flags_u32 <= static_cast<uint32_t>(SegmentFlags::IndexOther),
"invalid elem segment flags");
SegmentFlags flags = static_cast<SegmentFlags>(flags_u32);
uint32_t flags;
CHECK_RESULT(ReadU32Leb128(&flags, "elem segment flags"));
ERROR_IF(flags > ~(~0u << SegFlagMax), "invalid elem segment flags: %#x",
flags);
Index table_index(0);
if (flags == SegmentFlags::IndexOther) {
if (flags & SegIndexOther) {
CHECK_RESULT(ReadIndex(&table_index, "elem segment table index"));
}
Type elem_type;
if (flags == SegmentFlags::Passive) {
if (flags & SegPassive) {
CHECK_RESULT(ReadType(&elem_type, "table elem type"));
ERROR_UNLESS(elem_type == Type::Funcref || elem_type == Type::Anyref,
"segment elem type must by funcref or anyref");
} else {
elem_type = Type::Funcref;
}

CALLBACK(BeginElemSegment, i, table_index, flags == SegmentFlags::Passive,
elem_type);
CALLBACK(BeginElemSegment, i, table_index, flags, elem_type);

if (flags != SegmentFlags::Passive) {
if (!(flags & SegPassive)) {
CALLBACK(BeginElemSegmentInitExpr, i);
CHECK_RESULT(ReadI32InitExpr(i));
CALLBACK(EndElemSegmentInitExpr, i);
}

Index num_elem_exprs;
CHECK_RESULT(ReadCount(&num_elem_exprs, "elem expr count"));

CALLBACK(OnElemSegmentElemExprCount, i, num_elem_exprs);
for (Index j = 0; j < num_elem_exprs; ++j) {
if (flags == SegmentFlags::Passive) {
if (flags & SegPassive) {
Opcode opcode;
CHECK_RESULT(ReadOpcode(&opcode, "elem expr opcode"));
if (opcode == Opcode::RefNull) {
Expand Down Expand Up @@ -2240,17 +2241,16 @@ Result BinaryReader::ReadDataSection(Offset section_size) {
ERROR_UNLESS(data_count_ == kInvalidIndex || data_count_ == num_data_segments,
"data segment count does not equal count in DataCount section");
for (Index i = 0; i < num_data_segments; ++i) {
uint32_t flags_u32;
CHECK_RESULT(ReadU32Leb128(&flags_u32, "data segment flags"));
ERROR_UNLESS(flags_u32 <= static_cast<uint32_t>(SegmentFlags::IndexOther),
"invalid data segment flags");
SegmentFlags flags = static_cast<SegmentFlags>(flags_u32);
uint32_t flags;
CHECK_RESULT(ReadU32Leb128(&flags, "data segment flags"));
ERROR_IF(flags > ~(~0u << SegFlagMax), "invalid data segment flags: %#x",
flags);
Index memory_index(0);
if (flags == SegmentFlags::IndexOther) {
if (flags & SegIndexOther) {
CHECK_RESULT(ReadIndex(&memory_index, "data segment memory index"));
}
CALLBACK(BeginDataSegment, i, memory_index, flags == SegmentFlags::Passive);
if (flags != SegmentFlags::Passive) {
CALLBACK(BeginDataSegment, i, memory_index, flags);
if (!(flags & SegPassive)) {
CALLBACK(BeginDataSegmentInitExpr, i);
CHECK_RESULT(ReadI32InitExpr(i));
CALLBACK(EndDataSegmentInitExpr, i);
Expand Down
4 changes: 2 additions & 2 deletions src/binary-reader.h
View file
Expand Up @@ -286,7 +286,7 @@ class BinaryReaderDelegate {
virtual Result OnElemSegmentCount(Index count) = 0;
virtual Result BeginElemSegment(Index index,
Index table_index,
bool passive,
uint8_t flags,
Type elem_type) = 0;
virtual Result BeginElemSegmentInitExpr(Index index) = 0;
virtual Result EndElemSegmentInitExpr(Index index) = 0;
Expand All @@ -302,7 +302,7 @@ class BinaryReaderDelegate {
virtual Result OnDataSegmentCount(Index count) = 0;
virtual Result BeginDataSegment(Index index,
Index memory_index,
bool passive) = 0;
uint8_t flags) = 0;
virtual Result BeginDataSegmentInitExpr(Index index) = 0;
virtual Result EndDataSegmentInitExpr(Index index) = 0;
virtual Result OnDataSegmentData(Index index,
Expand Down
19 changes: 9 additions & 10 deletions src/binary-writer.cc
View file
Expand Up @@ -1032,19 +1032,18 @@ Result BinaryWriter::WriteModule() {
for (size_t i = 0; i < module_->elem_segments.size(); ++i) {
ElemSegment* segment = module_->elem_segments[i];
WriteHeader("elem segment header", i);
if (segment->passive) {
stream_->WriteU8(static_cast<uint8_t>(SegmentFlags::Passive));
stream_->WriteU8(segment->flags, "segment flags");
if (segment->is_passive()) {
WriteType(stream_, segment->elem_type);
} else if (module_->GetTableIndex(segment->table_var)) {
stream_->WriteU8(static_cast<uint8_t>(SegmentFlags::IndexOther));
} else if (segment->flags & SegIndexOther) {
WriteU32Leb128(stream_, module_->GetTableIndex(segment->table_var), "table index");
WriteInitExpr(segment->offset);
} else {
stream_->WriteU8(static_cast<uint8_t>(SegmentFlags::IndexZero));
assert(module_->GetTableIndex(segment->table_var) == 0);
WriteInitExpr(segment->offset);
}
WriteU32Leb128(stream_, segment->elem_exprs.size(), "num elem exprs");
if (segment->passive) {
WriteU32Leb128(stream_, segment->elem_exprs.size(), "num elems");
if (segment->is_passive()) {
for (const ElemExpr& elem_expr : segment->elem_exprs) {
switch (elem_expr.kind) {
case ElemExprKind::RefNull:
Expand Down Expand Up @@ -1104,11 +1103,11 @@ Result BinaryWriter::WriteModule() {
for (size_t i = 0; i < module_->data_segments.size(); ++i) {
const DataSegment* segment = module_->data_segments[i];
WriteHeader("data segment header", i);
if (segment->passive) {
stream_->WriteU8(static_cast<uint8_t>(SegmentFlags::Passive));
if (segment->is_passive()) {
stream_->WriteU8(SegPassive);
} else {
assert(module_->GetMemoryIndex(segment->memory_var) == 0);
stream_->WriteU8(static_cast<uint8_t>(SegmentFlags::IndexZero));
stream_->WriteU8(SegIndexZero);
WriteInitExpr(segment->offset);
}
WriteU32Leb128(stream_, segment->data.size(), "data segment size");
Expand Down

0 comments on commit 43fef7d

Please sign in to comment.