Skip to content

Commit 6a67b87

Browse files
[NFC] Followup code changes to wasm-validator.cpp (#8344)
Followup changes to #8342 * Extract out function for `validateExactReferences` (the existing body of `validateTypes`) and add a note that new validations should probably go in wasm-type.cpp. * Use the anonymous namespace instead of `static` for free functions: https://stackoverflow.com/questions/4977252/why-an-unnamed-namespace-is-a-superior-alternative-to-static
1 parent a8c0a50 commit 6a67b87

1 file changed

Lines changed: 46 additions & 39 deletions

File tree

src/wasm/wasm-validator.cpp

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737

3838
namespace wasm {
3939

40+
namespace {
41+
4042
// Print anything that can be streamed to an ostream
4143
template<typename T,
4244
typename std::enable_if<!std::is_base_of<
@@ -239,6 +241,34 @@ struct ValidationInfo {
239241
}
240242
};
241243

244+
// Check that public types do not contain any exact references if custom
245+
// descriptors is not enabled. If they did, we would erase the exactness
246+
// during binary writing and change the public type identities.
247+
void validateExactReferences(Module& module, ValidationInfo& info) {
248+
if (module.features.hasCustomDescriptors()) {
249+
return;
250+
}
251+
252+
for (auto type : ModuleUtils::getPublicHeapTypes(module)) {
253+
for (auto child : type.getTypeChildren()) {
254+
if (child.isExact()) {
255+
std::string typeName;
256+
if (auto it = module.typeNames.find(type);
257+
it != module.typeNames.end()) {
258+
typeName = '$' + it->second.name.toString();
259+
} else {
260+
typeName = type.toString();
261+
}
262+
info.fail("Exact reference in public type not allowed without custom "
263+
"descriptors [--enable-custom-descriptors]",
264+
typeName,
265+
nullptr);
266+
break;
267+
}
268+
}
269+
}
270+
}
271+
242272
std::string getMissingFeaturesList(Module& wasm, FeatureSet feats) {
243273
std::stringstream ss;
244274
bool first = true;
@@ -4358,7 +4388,7 @@ void FunctionValidator::validateAlignment(
43584388
}
43594389
}
43604390

4361-
static void validateBinaryenIR(Module& wasm, ValidationInfo& info) {
4391+
void validateBinaryenIR(Module& wasm, ValidationInfo& info) {
43624392
struct BinaryenIRValidator
43634393
: public PostWalker<BinaryenIRValidator,
43644394
UnifiedExpressionVisitor<BinaryenIRValidator>> {
@@ -4401,35 +4431,12 @@ static void validateBinaryenIR(Module& wasm, ValidationInfo& info) {
44014431

44024432
// Main validator class
44034433

4404-
static void validateTypes(Module& module, ValidationInfo& info) {
4405-
// Check that public types do not contain any exact references if custom
4406-
// descriptors is not enabled. If they did, we would erase the exactness
4407-
// during binary writing and change the public type identities.
4408-
if (module.features.hasCustomDescriptors()) {
4409-
return;
4410-
}
4411-
4412-
for (auto type : ModuleUtils::getPublicHeapTypes(module)) {
4413-
for (auto child : type.getTypeChildren()) {
4414-
if (child.isExact()) {
4415-
std::string typeName;
4416-
if (auto it = module.typeNames.find(type);
4417-
it != module.typeNames.end()) {
4418-
typeName = '$' + it->second.name.toString();
4419-
} else {
4420-
typeName = type.toString();
4421-
}
4422-
info.fail("Exact reference in public type not allowed without custom "
4423-
"descriptors [--enable-custom-descriptors]",
4424-
typeName,
4425-
nullptr);
4426-
break;
4427-
}
4428-
}
4429-
}
4434+
void validateTypes(Module& module, ValidationInfo& info) {
4435+
// Most validations belong in `validateTypeInfo` in wasm-type.cpp.
4436+
validateExactReferences(module, info);
44304437
}
44314438

4432-
static void validateImports(Module& module, ValidationInfo& info) {
4439+
void validateImports(Module& module, ValidationInfo& info) {
44334440
ModuleUtils::iterImportedFunctions(module, [&](Function* curr) {
44344441
if (curr->getResults().isTuple()) {
44354442
info.shouldBeTrue(module.features.hasMultivalue(),
@@ -4474,7 +4481,7 @@ static void validateImports(Module& module, ValidationInfo& info) {
44744481
});
44754482
}
44764483

4477-
static void validateExports(Module& module, ValidationInfo& info) {
4484+
void validateExports(Module& module, ValidationInfo& info) {
44784485
for (auto& curr : module.exports) {
44794486
if (curr->kind == ExternalKind::Function) {
44804487
if (info.validateWeb) {
@@ -4543,7 +4550,7 @@ static void validateExports(Module& module, ValidationInfo& info) {
45434550
}
45444551
}
45454552

4546-
static void validateGlobals(Module& module, ValidationInfo& info) {
4553+
void validateGlobals(Module& module, ValidationInfo& info) {
45474554
std::unordered_set<Global*> seen;
45484555
ModuleUtils::iterDefinedGlobals(module, [&](Global* curr) {
45494556
info.shouldBeTrue(curr->type.getFeatures() <= module.features,
@@ -4589,7 +4596,7 @@ static void validateGlobals(Module& module, ValidationInfo& info) {
45894596
}
45904597
}
45914598

4592-
static void validateMemories(Module& module, ValidationInfo& info) {
4599+
void validateMemories(Module& module, ValidationInfo& info) {
45934600
if (module.memories.size() > 1) {
45944601
info.shouldBeTrue(
45954602
module.features.hasMultiMemory(),
@@ -4624,7 +4631,7 @@ static void validateMemories(Module& module, ValidationInfo& info) {
46244631
}
46254632
}
46264633

4627-
static void validateDataSegments(Module& module, ValidationInfo& info) {
4634+
void validateDataSegments(Module& module, ValidationInfo& info) {
46284635
for (auto& segment : module.dataSegments) {
46294636
if (segment->isPassive) {
46304637
info.shouldBeTrue(
@@ -4655,7 +4662,7 @@ static void validateDataSegments(Module& module, ValidationInfo& info) {
46554662
}
46564663
}
46574664

4658-
static void validateTables(Module& module, ValidationInfo& info) {
4665+
void validateTables(Module& module, ValidationInfo& info) {
46594666
FunctionValidator validator(module, &info);
46604667

46614668
if (!module.features.hasReferenceTypes()) {
@@ -4764,7 +4771,7 @@ static void validateTables(Module& module, ValidationInfo& info) {
47644771
}
47654772
}
47664773

4767-
static void validateTags(Module& module, ValidationInfo& info) {
4774+
void validateTags(Module& module, ValidationInfo& info) {
47684775
if (!module.tags.empty()) {
47694776
info.shouldBeTrue(
47704777
module.features.hasExceptionHandling(),
@@ -4797,7 +4804,7 @@ static void validateTags(Module& module, ValidationInfo& info) {
47974804
}
47984805
}
47994806

4800-
static void validateStart(Module& module, ValidationInfo& info) {
4807+
void validateStart(Module& module, ValidationInfo& info) {
48014808
// start
48024809
if (module.start.is()) {
48034810
auto func = module.getFunctionOrNull(module.start);
@@ -4813,7 +4820,6 @@ static void validateStart(Module& module, ValidationInfo& info) {
48134820
}
48144821
}
48154822

4816-
namespace {
48174823
template<typename T, typename U>
48184824
void validateModuleMap(Module& module,
48194825
ValidationInfo& info,
@@ -4839,9 +4845,8 @@ void validateModuleMap(Module& module,
48394845
// TODO: Also check there is nothing extraneous in the map, but that would
48404846
// require inspecting private fields of Module.
48414847
}
4842-
} // anonymous namespace
48434848

4844-
static void validateModuleMaps(Module& module, ValidationInfo& info) {
4849+
void validateModuleMaps(Module& module, ValidationInfo& info) {
48454850
// Module maps should be up to date.
48464851
validateModuleMap(
48474852
module, info, module.exports, &Module::getExportOrNull, "Export");
@@ -4866,14 +4871,16 @@ static void validateModuleMaps(Module& module, ValidationInfo& info) {
48664871
module, info, module.tables, &Module::getTableOrNull, "Table");
48674872
}
48684873

4869-
static void validateFeatures(Module& module, ValidationInfo& info) {
4874+
void validateFeatures(Module& module, ValidationInfo& info) {
48704875
if (module.features.hasGC()) {
48714876
info.shouldBeTrue(module.features.hasReferenceTypes(),
48724877
module.features,
48734878
"--enable-gc requires --enable-reference-types");
48744879
}
48754880
}
48764881

4882+
} // namespace
4883+
48774884
// TODO: If we want the validator to be part of libwasm rather than libpasses,
48784885
// then Using PassRunner::getPassDebug causes a circular dependence. We should
48794886
// fix that, perhaps by moving some of the pass infrastructure into libsupport.

0 commit comments

Comments
 (0)