From cd269b5a2ddcd9aa25f51a9be9677403d8ebd511 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 3 Apr 2025 13:26:15 -0700 Subject: [PATCH 1/3] [NFC] Introduce HeapTypeDef and use it for printing Now that we have exact heap types, non-abstract heap types no longer correspond 1:1 with heap type definitions. We previously used a single type, `HeapType`, to represent both heap types and heap type definitions. Now that `HeapType` can represent multiple heap types corresponding to the same definition, there is a new class of potential bugs in which code that expects `HeapType` values to map 1:1 with heap type definitions observes both an exact and inexact heap type for the same definition. To eliminate this class of bugs, introduce a new type, `HeapTypeDef`, whose values do correspond 1:1 with heap type definitions. `HeapTypeDef` is a subclass of `HeapType`, so it supports all the same functionality, but it cannot represent exact heap types because its constructor clears the exact bit. As an initial proof-of-concept, use HeapTypeDef in the type printing machinery, which only cares about heap type names corresponding to heap type definitions. Future PRs will use HeapTypeDef in more places. --- src/wasm-type-printing.h | 16 +++++++++------- src/wasm-type.h | 17 ++++++++++++++++- src/wasm.h | 4 ++-- src/wasm/wasm-type.cpp | 6 +++++- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/wasm-type-printing.h b/src/wasm-type-printing.h index 11afde88212..5ecd8fc0b57 100644 --- a/src/wasm-type-printing.h +++ b/src/wasm-type-printing.h @@ -32,19 +32,21 @@ namespace wasm { // ability to use the generator as a function to print Types and HeapTypes to // streams. template struct TypeNameGeneratorBase { - TypeNames getNames(HeapType type) { + TypeNames getNames(HeapTypeDef type) { static_assert(&TypeNameGeneratorBase::getNames != &Subclass::getNames, "Derived class must implement getNames"); WASM_UNREACHABLE("Derived class must implement getNames"); } - HeapType::Printed operator()(HeapType type) { - return type.print( - [&](HeapType ht) { return static_cast(this)->getNames(ht); }); + HeapType::Printed operator()(HeapTypeDef type) { + return type.print([&](HeapTypeDef ht) { + return static_cast(this)->getNames(ht); + }); } Type::Printed operator()(Type type) { - return type.print( - [&](HeapType ht) { return static_cast(this)->getNames(ht); }); + return type.print([&](HeapTypeDef ht) { + return static_cast(this)->getNames(ht); + }); } }; @@ -60,7 +62,7 @@ struct DefaultTypeNameGenerator // Cached names for types that have already been seen. std::unordered_map nameCache; - TypeNames getNames(HeapType type); + TypeNames getNames(HeapTypeDef type); }; // Generates names based on the indices of types in some collection, falling diff --git a/src/wasm-type.h b/src/wasm-type.h index 1eb1f9fe86e..b3fdf08f31a 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -50,6 +50,7 @@ void destroyAllTypesForTestingPurposesOnly(); // data. class Type; class HeapType; +class HeapTypeDef; class RecGroup; struct Signature; struct Continuation; @@ -73,7 +74,7 @@ struct TypeNames { }; // Used to generate HeapType names. -using HeapTypeNameGenerator = std::function; +using HeapTypeNameGenerator = std::function; // The type used for interning IDs in the public interfaces of Type and // HeapType. @@ -294,6 +295,16 @@ class HeapType { std::string toString() const; }; +// Like `HeapType`, but used to represent heap type definitions and abstract +// heap types rather than arbitrary heap types. Use this whenever it would be a +// category error to use an exact heap type. +class HeapTypeDef : public HeapType { +public: + // Allow implicit conversions from HeapType. + constexpr HeapTypeDef(HeapType type) : HeapType(type.with(Inexact)) {} + constexpr HeapTypeDef() = default; +}; + class Type { // The `id` uniquely represents each type, so type equality is just a // comparison of the ids. The basic types are packed at the bottom of the @@ -1007,6 +1018,10 @@ template<> class hash { public: size_t operator()(const wasm::HeapType&) const; }; +template<> class hash { +public: + size_t operator()(const wasm::HeapTypeDef&) const; +}; template<> class hash { public: size_t operator()(const wasm::RecGroup&) const; diff --git a/src/wasm.h b/src/wasm.h index e4429c699ff..e3979e21975 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2415,8 +2415,8 @@ class Module { // Module name, if specified. Serves a documentary role only. Name name; - std::unordered_map typeNames; - std::unordered_map typeIndices; + std::unordered_map typeNames; + std::unordered_map typeIndices; MixedArena allocator; diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 6fc43c2e091..4bb988fad2e 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -1369,7 +1369,7 @@ size_t RecGroup::size() const { } } -TypeNames DefaultTypeNameGenerator::getNames(HeapType type) { +TypeNames DefaultTypeNameGenerator::getNames(HeapTypeDef type) { auto [it, inserted] = nameCache.insert({type, {}}); if (inserted) { // Generate a new name for this type we have not previously seen. @@ -2768,6 +2768,10 @@ size_t hash::operator()(const wasm::HeapType& heapType) const { return wasm::hash(heapType.getID()); } +size_t hash::operator()(const wasm::HeapTypeDef& def) const { + return wasm::hash(def.getID()); +} + size_t hash::operator()(const wasm::RecGroup& group) const { return wasm::hash(group.getID()); } From f67f7365d0d21f3c9fcd8155f36fd930e033c0e4 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 3 Apr 2025 14:00:33 -0700 Subject: [PATCH 2/3] fix assertion and subclasses --- src/passes/Print.cpp | 2 +- src/tools/wasm-fuzz-types.cpp | 2 +- src/wasm-type-printing.h | 21 +++++++++++++++------ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp index 90eecef2eef..b68921e198e 100644 --- a/src/passes/Print.cpp +++ b/src/passes/Print.cpp @@ -198,7 +198,7 @@ struct PrintSExpression : public UnifiedExpressionVisitor { } } - TypeNames getNames(HeapType type) { + TypeNames getNames(HeapTypeDef type) { if (parent.currModule) { if (auto it = parent.currModule->typeNames.find(type); it != parent.currModule->typeNames.end()) { diff --git a/src/tools/wasm-fuzz-types.cpp b/src/tools/wasm-fuzz-types.cpp index 7ba341e09df..b53d5c0b5b8 100644 --- a/src/tools/wasm-fuzz-types.cpp +++ b/src/tools/wasm-fuzz-types.cpp @@ -97,7 +97,7 @@ void Fuzzer::printTypes(const std::vector& types) { std::cout << "Built " << types.size() << " types:\n"; struct FatalTypeNameGenerator : TypeNameGeneratorBase { - TypeNames getNames(HeapType type) { + TypeNames getNames(HeapTypeDef type) { Fatal() << "trying to print unknown heap type"; } } fatalGenerator; diff --git a/src/wasm-type-printing.h b/src/wasm-type-printing.h index 5ecd8fc0b57..f1002f0a463 100644 --- a/src/wasm-type-printing.h +++ b/src/wasm-type-printing.h @@ -32,10 +32,9 @@ namespace wasm { // ability to use the generator as a function to print Types and HeapTypes to // streams. template struct TypeNameGeneratorBase { + TypeNameGeneratorBase() { assertValidUsage(); } + TypeNames getNames(HeapTypeDef type) { - static_assert(&TypeNameGeneratorBase::getNames != - &Subclass::getNames, - "Derived class must implement getNames"); WASM_UNREACHABLE("Derived class must implement getNames"); } HeapType::Printed operator()(HeapTypeDef type) { @@ -48,6 +47,16 @@ template struct TypeNameGeneratorBase { return static_cast(this)->getNames(ht); }); } + +private: + constexpr void assertValidUsage() { + // Check that the subclass provides `getNames` with the correct type. + using Self = TypeNameGeneratorBase; + static_assert( + static_cast(&Self::getNames) != + static_cast(&Subclass::getNames), + "Derived class must implement getNames"); + } }; // Generates names like "func.0", "struct.1", "array.2", etc. Struct fields are @@ -73,7 +82,7 @@ struct IndexedTypeNameGenerator : TypeNameGeneratorBase> { DefaultTypeNameGenerator defaultGenerator; FallbackGenerator& fallback; - std::unordered_map names; + std::unordered_map names; template IndexedTypeNameGenerator(T& types, @@ -88,7 +97,7 @@ struct IndexedTypeNameGenerator IndexedTypeNameGenerator(T& types, const std::string& prefix = "") : IndexedTypeNameGenerator(types, defaultGenerator, prefix) {} - TypeNames getNames(HeapType type) { + TypeNames getNames(HeapTypeDef type) { if (auto it = names.find(type); it != names.end()) { return it->second; } else { @@ -119,7 +128,7 @@ struct ModuleTypeNameGenerator std::enable_if_t>* = nullptr) : ModuleTypeNameGenerator(wasm, defaultGenerator) {} - TypeNames getNames(HeapType type) { + TypeNames getNames(HeapTypeDef type) { if (auto it = wasm.typeNames.find(type); it != wasm.typeNames.end()) { return it->second; } From 9ebb943c4ee547f1cf2aeef2f4ba8c545de5d27d Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 3 Apr 2025 14:42:45 -0700 Subject: [PATCH 3/3] exclude old gcc from static assertion --- src/wasm-type-printing.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/wasm-type-printing.h b/src/wasm-type-printing.h index f1002f0a463..e977a07188b 100644 --- a/src/wasm-type-printing.h +++ b/src/wasm-type-printing.h @@ -50,12 +50,14 @@ template struct TypeNameGeneratorBase { private: constexpr void assertValidUsage() { +#if !defined(__GNUC__) || __GNUC__ >= 14 // Check that the subclass provides `getNames` with the correct type. using Self = TypeNameGeneratorBase; static_assert( static_cast(&Self::getNames) != static_cast(&Subclass::getNames), "Derived class must implement getNames"); +#endif } };