Fix infinite recursion of generic class types specialized inside unin…

…stantiated scopes
MikePopoloski · Nov 24, 2024 · 5399d34 · 5399d34
1 parent fed2ddf
commit 5399d34
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 2 deletions.
diff --git a/include/slang/ast/symbols/ClassSymbols.h b/include/slang/ast/symbols/ClassSymbols.h
@@ -250,6 +250,7 @@ class SLANG_EXPORT GenericClassDefSymbol : public Symbol {
     using SpecMap = flat_hash_map<detail::ClassSpecializationKey, const Type*,
                                   detail::ClassSpecializationHasher>;
     mutable SpecMap specMap;
+    mutable SpecMap uninstantiatedSpecMap;
     mutable std::optional<const Type*> defaultSpecialization;
     mutable const ForwardingTypedefSymbol* firstForward = nullptr;
     mutable uint32_t recursionDepth = 0;

diff --git a/source/ast/symbols/ClassSymbols.cpp b/source/ast/symbols/ClassSymbols.cpp
@@ -999,6 +999,8 @@ const Type* GenericClassDefSymbol::getSpecializationImpl(
     detail::ClassSpecializationKey key(*this, paramValues.copy(comp), typeParams.copy(comp));
     if (auto it = specMap.find(key); it != specMap.end())
         return it->second;
+    if (auto it = uninstantiatedSpecMap.find(key); it != uninstantiatedSpecMap.end())
+        return it->second;
 
     // Not found, so this is a new entry. Fill in its members and store the
     // specialization for later lookup. If we have a specialization function,
@@ -1008,8 +1010,15 @@ const Type* GenericClassDefSymbol::getSpecializationImpl(
     else
         classType->populate(*scope, getSyntax()->as<ClassDeclarationSyntax>());
 
-    if (!forceInvalidParams && !context.scope->isUninstantiated())
-        specMap.emplace(key, classType);
+    if (!forceInvalidParams) {
+        // If we're in an uninstantiated scope we save this specialization
+        // in a separate map so that we don't try to elaborate it further
+        // and potentially cause spurious errors to be issued.
+        if (context.scope->isUninstantiated())
+            uninstantiatedSpecMap.emplace(key, classType);
+        else
+            specMap.emplace(key, classType);
+    }
 
     return classType;
 }

diff --git a/tests/unittests/ast/ClassTests.cpp b/tests/unittests/ast/ClassTests.cpp
@@ -3541,3 +3541,17 @@ endmodule
     REQUIRE(diags.size() == 1);
     CHECK(diags[0].code == diag::VirtualVisibilityMismatch);
 }
+
+TEST_CASE("Generic class size computation stack overflow regress") {
+    auto tree = SyntaxTree::fromText(R"(
+interface I;
+    class G #(type l);
+        G #(int) g2;
+    endclass
+endinterface
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
+}
diff --git a/tests/unittests/ast/MemberTests.cpp b/tests/unittests/ast/MemberTests.cpp
@@ -2412,4 +2412,5 @@ alias;
 
     Compilation compilation;
     compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
 }