Skip to content

Commit 2b989ae

Browse files
tlivelykripken
andauthored
[custom-descriptors] Update struct.new parsing and validation (#7643)
When allocating structs that have custom descriptors, struct.new and struct.new_default must take an additional operand for the descriptor. --------- Co-authored-by: Alon Zakai <[email protected]>
1 parent a1fa205 commit 2b989ae

File tree

11 files changed

+294
-63
lines changed

11 files changed

+294
-63
lines changed

scripts/test/fuzzing.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@
114114
# TODO: fuzzer support for custom descriptors
115115
'custom-descriptors.wast',
116116
'br_on_cast_desc.wast',
117-
'ref.get_cast.wast',
117+
'ref.get_desc.wast',
118118
'ref.cast_desc.wast',
119+
'struct.new-desc.wast',
119120
# TODO: fix split_wast() on tricky escaping situations like a string ending
120121
# in \\" (the " is not escaped - there is an escaped \ before it)
121122
'string-lifting-section.wast',

src/ir/child-typer.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -934,16 +934,19 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
934934
}
935935

936936
void visitStructNew(StructNew* curr) {
937-
if (curr->isWithDefault()) {
938-
return;
939-
}
940937
if (self().skipUnreachable() && !curr->type.isRef()) {
941938
return;
942939
}
943-
const auto& fields = curr->type.getHeapType().getStruct().fields;
944-
assert(fields.size() == curr->operands.size());
945-
for (size_t i = 0; i < fields.size(); ++i) {
946-
note(&curr->operands[i], fields[i].type);
940+
if (!curr->isWithDefault()) {
941+
const auto& fields = curr->type.getHeapType().getStruct().fields;
942+
assert(fields.size() == curr->operands.size());
943+
for (size_t i = 0; i < fields.size(); ++i) {
944+
note(&curr->operands[i], fields[i].type);
945+
}
946+
}
947+
auto desc = curr->type.getHeapType().getDescriptorType();
948+
if (desc) {
949+
note(&curr->descriptor, Type(*desc, NonNullable, Exact));
947950
}
948951
}
949952

src/ir/cost.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,7 @@ struct CostAnalyzer : public OverriddenVisitor<CostAnalyzer, CostType> {
693693
for (auto* child : curr->operands) {
694694
ret += visit(child);
695695
}
696+
ret += maybeVisit(curr->descriptor);
696697
return ret;
697698
}
698699
CostType visitStructGet(StructGet* curr) {

src/wasm-builder.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -921,23 +921,32 @@ class Builder {
921921
return ret;
922922
}
923923
StructNew* makeStructNew(HeapType type,
924-
std::initializer_list<Expression*> args) {
924+
std::initializer_list<Expression*> args,
925+
Expression* descriptor = nullptr) {
925926
auto* ret = wasm.allocator.alloc<StructNew>();
926927
ret->operands.set(args);
928+
ret->descriptor = descriptor;
927929
ret->type = Type(type, NonNullable, Exact);
928930
ret->finalize();
929931
return ret;
930932
}
931-
StructNew* makeStructNew(HeapType type, ExpressionList&& args) {
933+
StructNew* makeStructNew(HeapType type,
934+
ExpressionList&& args,
935+
Expression* descriptor = nullptr) {
932936
auto* ret = wasm.allocator.alloc<StructNew>();
933937
ret->operands = std::move(args);
938+
ret->descriptor = descriptor;
934939
ret->type = Type(type, NonNullable, Exact);
935940
ret->finalize();
936941
return ret;
937942
}
938-
template<typename T> StructNew* makeStructNew(HeapType type, const T& args) {
943+
template<typename T>
944+
StructNew* makeStructNew(HeapType type,
945+
const T& args,
946+
Expression* descriptor = nullptr) {
939947
auto* ret = wasm.allocator.alloc<StructNew>();
940948
ret->operands.set(args);
949+
ret->descriptor = descriptor;
941950
ret->type = Type(type, NonNullable, Exact);
942951
ret->finalize();
943952
return ret;

src/wasm-delegations-fields.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,7 @@ DELEGATE_FIELD_CHILD(BrOn, ref)
667667
DELEGATE_FIELD_CASE_END(BrOn)
668668

669669
DELEGATE_FIELD_CASE_START(StructNew)
670+
DELEGATE_FIELD_OPTIONAL_CHILD(StructNew, descriptor)
670671
DELEGATE_FIELD_CHILD_VECTOR(StructNew, operands)
671672
DELEGATE_FIELD_CASE_END(StructNew)
672673

src/wasm.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,6 +1672,8 @@ class StructNew : public SpecificExpression<Expression::StructNewId> {
16721672
// case, and binaryen doesn't guarantee roundtripping binaries anyhow.
16731673
ExpressionList operands;
16741674

1675+
Expression* descriptor = nullptr;
1676+
16751677
bool isWithDefault() { return operands.empty(); }
16761678

16771679
void finalize();

src/wasm/wasm-ir-builder.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,15 +2214,17 @@ Result<> IRBuilder::makeBrOn(
22142214
Result<> IRBuilder::makeStructNew(HeapType type) {
22152215
StructNew curr(wasm.allocator);
22162216
curr.type = Type(type, NonNullable, Exact);
2217-
// Differentiate from struct.new_default with a non-empty expression list.
22182217
curr.operands.resize(type.getStruct().fields.size());
22192218
CHECK_ERR(visitStructNew(&curr));
2220-
push(builder.makeStructNew(type, std::move(curr.operands)));
2219+
push(builder.makeStructNew(type, std::move(curr.operands), curr.descriptor));
22212220
return Ok{};
22222221
}
22232222

22242223
Result<> IRBuilder::makeStructNewDefault(HeapType type) {
2225-
push(builder.makeStructNew(type, {}));
2224+
StructNew curr(wasm.allocator);
2225+
curr.type = Type(type, NonNullable, Exact);
2226+
CHECK_ERR(visitStructNew(&curr));
2227+
push(builder.makeStructNew(type, {}, curr.descriptor));
22262228
return Ok{};
22272229
}
22282230

src/wasm/wasm-validator.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3147,6 +3147,18 @@ void FunctionValidator::visitStructNew(StructNew* curr) {
31473147
}
31483148
}
31493149
}
3150+
3151+
auto descType = curr->type.getHeapType().getDescriptorType();
3152+
if (!descType) {
3153+
shouldBeFalse(curr->descriptor,
3154+
curr,
3155+
"struct.new of type without descriptor should lack one");
3156+
} else {
3157+
shouldBeSubType(curr->descriptor->type,
3158+
Type(*descType, Nullable, Exact),
3159+
curr,
3160+
"struct.new descriptor operand should have proper type");
3161+
}
31503162
}
31513163

31523164
void FunctionValidator::visitStructGet(StructGet* curr) {

0 commit comments

Comments
 (0)