From 20ce9d38f32be62c85d953ae3a4479e7159bc9fb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:03:55 -0700 Subject: [PATCH 1/8] start --- src/wasm/wasm-validator.cpp | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index e8a4033ef9e..ab731d823ac 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3908,7 +3908,7 @@ static void validateTags(Module& module, ValidationInfo& info) { } } -static void validateModule(Module& module, ValidationInfo& info) { +static void validateStart(Module& module, ValidationInfo& info) { // start if (module.start.is()) { auto func = module.getFunctionOrNull(module.start); @@ -3924,6 +3924,36 @@ static void validateModule(Module& module, ValidationInfo& info) { } } +namespace { +template +void validateModuleMap(Module& module, ValidationInfo& info, T& list, U method, const std::string& kind) { + // The things in the list should be accessible using the get* APIs, which uses + // the lookup maps. + for (auto& item : list) { + if (!((module.*method)(item->name))) { + info.fail(kind + " must be found (use updateMaps)", + item->name, + nullptr); + } + } + + // TODO: Also check there is nothing extraneous in the map, but that would + // require inspecting private fields of Module. +} +} // anonymous namespace + +static void validateModuleMaps(Module& module, ValidationInfo& info) { + // Module maps should be up to date. + validateModuleMap(module, info, module.exports, &Module::getExportOrNull, "Export"); + validateModuleMap(module, info, module.functions, &Module::getFunctionOrNull, "Function"); + validateModuleMap(module, info, module.globals, &Module::getGlobalOrNull, "Global"); + validateModuleMap(module, info, module.tags, &Module::getTagOrNull, "tag"); + validateModuleMap(module, info, module.elementSegments, &Module::getElementSegmentOrNull, "elementSegment"); + validateModuleMap(module, info, module.memories, &Module::getMemoryOrNull, "Memory"); + validateModuleMap(module, info, module.dataSegments, &Module::getDataSegmentOrNull, "DataSegment"); + validateModuleMap(module, info, module.tables, &Module::getTableOrNull, "Table"); +} + static void validateFeatures(Module& module, ValidationInfo& info) { if (module.features.hasGC()) { info.shouldBeTrue(module.features.hasReferenceTypes(), @@ -4004,7 +4034,8 @@ bool WasmValidator::validate(Module& module, Flags flags) { validateDataSegments(module, info); validateTables(module, info); validateTags(module, info); - validateModule(module, info); + validateStart(module, info); + validateModuleMaps(module, info); validateFeatures(module, info); if (info.closedWorld) { validateClosedWorldInterface(module, info); From 1d6dde2fc6261a78d9496c4fcf5403536a4cc4ac Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:04:08 -0700 Subject: [PATCH 2/8] work --- src/wasm/wasm-validator.cpp | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index ab731d823ac..871efeda55d 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3926,14 +3926,16 @@ static void validateStart(Module& module, ValidationInfo& info) { namespace { template -void validateModuleMap(Module& module, ValidationInfo& info, T& list, U method, const std::string& kind) { +void validateModuleMap(Module& module, + ValidationInfo& info, + T& list, + U method, + const std::string& kind) { // The things in the list should be accessible using the get* APIs, which uses // the lookup maps. for (auto& item : list) { if (!((module.*method)(item->name))) { - info.fail(kind + " must be found (use updateMaps)", - item->name, - nullptr); + info.fail(kind + " must be found (use updateMaps)", item->name, nullptr); } } @@ -3944,14 +3946,27 @@ void validateModuleMap(Module& module, ValidationInfo& info, T& list, U method, static void validateModuleMaps(Module& module, ValidationInfo& info) { // Module maps should be up to date. - validateModuleMap(module, info, module.exports, &Module::getExportOrNull, "Export"); - validateModuleMap(module, info, module.functions, &Module::getFunctionOrNull, "Function"); - validateModuleMap(module, info, module.globals, &Module::getGlobalOrNull, "Global"); + validateModuleMap( + module, info, module.exports, &Module::getExportOrNull, "Export"); + validateModuleMap( + module, info, module.functions, &Module::getFunctionOrNull, "Function"); + validateModuleMap( + module, info, module.globals, &Module::getGlobalOrNull, "Global"); validateModuleMap(module, info, module.tags, &Module::getTagOrNull, "tag"); - validateModuleMap(module, info, module.elementSegments, &Module::getElementSegmentOrNull, "elementSegment"); - validateModuleMap(module, info, module.memories, &Module::getMemoryOrNull, "Memory"); - validateModuleMap(module, info, module.dataSegments, &Module::getDataSegmentOrNull, "DataSegment"); - validateModuleMap(module, info, module.tables, &Module::getTableOrNull, "Table"); + validateModuleMap(module, + info, + module.elementSegments, + &Module::getElementSegmentOrNull, + "elementSegment"); + validateModuleMap( + module, info, module.memories, &Module::getMemoryOrNull, "Memory"); + validateModuleMap(module, + info, + module.dataSegments, + &Module::getDataSegmentOrNull, + "DataSegment"); + validateModuleMap( + module, info, module.tables, &Module::getTableOrNull, "Table"); } static void validateFeatures(Module& module, ValidationInfo& info) { From 195d5585857dc6c8f8462b7d6c8c9ec289ee8124 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:07:31 -0700 Subject: [PATCH 3/8] bettr --- src/wasm/wasm-validator.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 871efeda55d..e8a950751be 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3929,13 +3929,21 @@ template void validateModuleMap(Module& module, ValidationInfo& info, T& list, - U method, + U getter, const std::string& kind) { - // The things in the list should be accessible using the get* APIs, which uses - // the lookup maps. + // Given a list of module elements (like exports or globals), see that we can + // get the items using the getter (getExportorNull, etc.). The getter uses the + // lookup map internally, so this validates that they contain all items in + // the list. for (auto& item : list) { - if (!((module.*method)(item->name))) { + auto* ptr = (module.*getter)(item->name); + if (!ptr) { info.fail(kind + " must be found (use updateMaps)", item->name, nullptr); + } else { + info.shouldBeEqual(item->name, + ptr->name, + item, + "getter must return the correct item"); } } From a16b5d63bd5877b47cedd04078911d0efab83d08 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:10:13 -0700 Subject: [PATCH 4/8] bettr --- src/wasm/wasm-validator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index e8a950751be..159fb90865b 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3942,7 +3942,7 @@ void validateModuleMap(Module& module, } else { info.shouldBeEqual(item->name, ptr->name, - item, + item->name, "getter must return the correct item"); } } From fc3061aedc2924ef370f4bb379eafd04711acacf Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:33:22 -0700 Subject: [PATCH 5/8] fix --- src/tools/fuzzing/fuzzing.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index f4f059deb43..5ae8eb47d01 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -225,12 +225,12 @@ void TranslateToFuzzReader::setupMemory() { segment->memory = memory->name; segment->offset = builder.makeConst(int32_t(0)); segment->setName(Name::fromInt(0), false); - wasm.dataSegments.push_back(std::move(segment)); auto num = upTo(USABLE_MEMORY * 2); for (size_t i = 0; i < num; i++) { auto value = upTo(512); - wasm.dataSegments[0]->data.push_back(value >= 256 ? 0 : (value & 0xff)); + segment->data.push_back(value >= 256 ? 0 : (value & 0xff)); } + wasm.addDataSegment(std::move(segment)); } } From c4f3913c29fe891b1dd99c4fbc31c4c8c1b2d697 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 11:50:08 -0700 Subject: [PATCH 6/8] fix name collision --- src/tools/fuzzing/fuzzing.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 5ae8eb47d01..57c7ec6b069 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -224,7 +224,8 @@ void TranslateToFuzzReader::setupMemory() { auto segment = builder.makeDataSegment(); segment->memory = memory->name; segment->offset = builder.makeConst(int32_t(0)); - segment->setName(Name::fromInt(0), false); + segment->setName(Names::getValidDataSegmentName(wasm, Name::fromInt(0)), + false); auto num = upTo(USABLE_MEMORY * 2); for (size_t i = 0; i < num; i++) { auto value = upTo(512); From 617412e0b74718265e38a86376c7a338ab7761a9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 14:11:57 -0700 Subject: [PATCH 7/8] Update src/wasm/wasm-validator.cpp Co-authored-by: Heejin Ahn --- src/wasm/wasm-validator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index 159fb90865b..e86448e1285 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3965,7 +3965,7 @@ static void validateModuleMaps(Module& module, ValidationInfo& info) { info, module.elementSegments, &Module::getElementSegmentOrNull, - "elementSegment"); + "ElementSegment"); validateModuleMap( module, info, module.memories, &Module::getMemoryOrNull, "Memory"); validateModuleMap(module, From cc68a61b3ec3bb45a9748d4ac3b815702b76b82c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 23 May 2024 14:12:43 -0700 Subject: [PATCH 8/8] another --- src/wasm/wasm-validator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp index e86448e1285..689872a196e 100644 --- a/src/wasm/wasm-validator.cpp +++ b/src/wasm/wasm-validator.cpp @@ -3960,7 +3960,7 @@ static void validateModuleMaps(Module& module, ValidationInfo& info) { module, info, module.functions, &Module::getFunctionOrNull, "Function"); validateModuleMap( module, info, module.globals, &Module::getGlobalOrNull, "Global"); - validateModuleMap(module, info, module.tags, &Module::getTagOrNull, "tag"); + validateModuleMap(module, info, module.tags, &Module::getTagOrNull, "Tag"); validateModuleMap(module, info, module.elementSegments,