Skip to content

Commit f914ff7

Browse files
authored
[Types] Fix missing inverse check for described field in isValidSupertype (#8378)
## Summary - Add the missing `else` branch in `isValidSupertype` for the `described` field check, making it symmetric with the existing `descriptor` field check. - Without this fix, a type without a `describes` clause can illegally subtype a type that has a `describes` clause (i.e., a descriptor type), as long as all features are enabled. - The `descriptor` check already handles both directions (if sub has no descriptor, super must also have no descriptor), but the `described` check was missing the inverse direction. ## Details In `isValidSupertype` (src/wasm/wasm-type.cpp), the validation for `descriptor` is fully symmetric: ```cpp if (sub.descriptor) { if (super.descriptor && sub.descriptor->supertype != super.descriptor) { return false; } } else { if (super.descriptor) { return false; } } ``` But the `described` check was missing the else branch: ```cpp if (sub.described) { if (!super.described || sub.described->supertype != super.described) { return false; } } // Missing: else { if (super.described) { return false; } } ``` The spec test at `test/spec/descriptors.wast` (lines 152-161) already covers this case with an `assert_invalid` for "supertype of non-descriptor type cannot be a descriptor". However, the test currently passes for the wrong reason: `wasm-shell` re-parses `assert_invalid` modules as quoted text modules without features enabled, so `TypeBuilder::build()` rejects the module with `RequiresCustomDescriptors` before ever reaching `isValidSupertype`. When the module is loaded through the normal inline parse path (with all features enabled), the invalid subtyping relationship is incorrectly accepted. ## Test plan - Verified that `wasm-shell test/spec/descriptors.wast` passes all 14 checks (including the one at line 152) - Verified that all 309 GTest unit tests pass
1 parent 67e7170 commit f914ff7

2 files changed

Lines changed: 7 additions & 0 deletions

File tree

src/tools/wasm-shell.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ struct Shell {
102102
std::shared_ptr<Module> wasm;
103103
if (auto* quoted = std::get_if<QuotedModule>(&mod.module)) {
104104
wasm = std::make_shared<Module>();
105+
wasm->features = FeatureSet::All;
105106
switch (quoted->type) {
106107
case QuotedModuleType::Text: {
107108
CHECK_ERR(parseModule(*wasm, quoted->module));

src/wasm/wasm-type.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2418,6 +2418,12 @@ bool isValidSupertype(const HeapTypeInfo& sub, const HeapTypeInfo& super) {
24182418
if (!super.described || sub.described->supertype != super.described) {
24192419
return false;
24202420
}
2421+
} else {
2422+
// A supertype of a type without a describes clause must also not have a
2423+
// describes clause.
2424+
if (super.described) {
2425+
return false;
2426+
}
24212427
}
24222428
SubTyper typer;
24232429
switch (sub.kind) {

0 commit comments

Comments
 (0)