fix Issue 18026 - Stack overflow in dmd/dtemplate.d:6248, TemplateIns…

…tance::needsCodegen()
dlang · Jan 7, 2023 · 24c08e8 · 24c08e8
1 parent 0f542f9
commit 24c08e8
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 27 deletions.
diff --git a/compiler/src/dmd/dtemplate.d b/compiler/src/dmd/dtemplate.d
@@ -6264,24 +6264,31 @@ extern (C++) class TemplateInstance : ScopeDsymbol
         if (global.params.allInst)
         {
             // Do codegen if there is an instantiation from a root module, to maximize link-ability.
+            static ThreeState needsCodegenAllInst(TemplateInstance tithis, TemplateInstance tinst)
+            {
+                // Do codegen if `this` is instantiated from a root module.
+                if (tithis.minst && tithis.minst.isRoot())
+                    return ThreeState.yes;
 
-            // Do codegen if `this` is instantiated from a root module.
-            if (minst && minst.isRoot())
-                return true;
+                // Do codegen if the ancestor needs it.
+                if (tinst && tinst.needsCodegen())
+                {
+                    tithis.minst = tinst.minst; // cache result
+                    assert(tithis.minst);
+                    assert(tithis.minst.isRoot());
+                    return ThreeState.yes;
+                }
+                return ThreeState.none;
+            }
 
-            // Do codegen if the ancestor needs it.
-            if (tinst && tinst.needsCodegen())
-            {
-                minst = tinst.minst; // cache result
-                assert(minst);
-                assert(minst.isRoot());
+            if (needsCodegenAllInst(this, tinst) == ThreeState.yes)
                 return true;
-            }
 
             // Do codegen if a sibling needs it.
-            if (tnext)
+            for (; tnext; tnext = tnext.tnext)
             {
-                if (tnext.needsCodegen())
+                const needsCodegen = needsCodegenAllInst(tnext, tnext.tinst);
+                if (needsCodegen == ThreeState.yes)
                 {
                     minst = tnext.minst; // cache result
                     assert(minst);
@@ -6293,6 +6300,8 @@ extern (C++) class TemplateInstance : ScopeDsymbol
                     minst = tnext.minst; // cache result from non-speculative sibling
                     return false;
                 }
+                else if (needsCodegen != ThreeState.none)
+                    break;
             }
 
             // Elide codegen because there's no instantiation from any root modules.
@@ -6317,31 +6326,38 @@ extern (C++) class TemplateInstance : ScopeDsymbol
              * => Elide codegen if there is at least one instantiation from a non-root module
              *    which doesn't import any root modules.
              */
-
-            // If the ancestor isn't speculative,
-            // 1. do codegen if the ancestor needs it
-            // 2. elide codegen if the ancestor doesn't need it (non-root instantiation of ancestor incl. subtree)
-            if (tinst)
+            static ThreeState needsCodegenRootOnly(TemplateInstance tithis, TemplateInstance tinst)
             {
-                const needsCodegen = tinst.needsCodegen(); // sets tinst.minst
-                if (tinst.minst) // not speculative
+                // If the ancestor isn't speculative,
+                // 1. do codegen if the ancestor needs it
+                // 2. elide codegen if the ancestor doesn't need it (non-root instantiation of ancestor incl. subtree)
+                if (tinst)
                 {
-                    minst = tinst.minst; // cache result
-                    return needsCodegen;
+                    const needsCodegen = tinst.needsCodegen(); // sets tinst.minst
+                    if (tinst.minst) // not speculative
+                    {
+                        tithis.minst = tinst.minst; // cache result
+                        return needsCodegen ? ThreeState.yes : ThreeState.no;
+                    }
                 }
+
+                // Elide codegen if `this` doesn't need it.
+                if (tithis.minst && !tithis.minst.isRoot() && !tithis.minst.rootImports())
+                    return ThreeState.no;
+
+                return ThreeState.none;
             }
 
-            // Elide codegen if `this` doesn't need it.
-            if (minst && !minst.isRoot() && !minst.rootImports())
-                return false;
+            if (const needsCodegen = needsCodegenRootOnly(this, tinst))
+                return needsCodegen == ThreeState.yes ? true : false;
 
             // Elide codegen if a (non-speculative) sibling doesn't need it.
-            if (tnext)
+            for (; tnext; tnext = tnext.tnext)
             {
-                const needsCodegen = tnext.needsCodegen(); // sets tnext.minst
+                const needsCodegen = needsCodegenRootOnly(tnext, tnext.tinst); // sets tnext.minst
                 if (tnext.minst) // not speculative
                 {
-                    if (!needsCodegen)
+                    if (needsCodegen == ThreeState.no)
                     {
                         minst = tnext.minst; // cache result
                         assert(!minst.isRoot() && !minst.rootImports());
@@ -6352,6 +6368,8 @@ extern (C++) class TemplateInstance : ScopeDsymbol
                         minst = tnext.minst; // cache result from non-speculative sibling
                         return true;
                     }
+                    else if (needsCodegen != ThreeState.none)
+                        break;
                 }
             }
 

diff --git a/compiler/test/compilable/test18026.d b/compiler/test/compilable/test18026.d
@@ -0,0 +1,12 @@
+// https://issues.dlang.org/show_bug.cgi?id=18026
+bool f(T)(T x)
+{
+    return false;
+}
+
+static foreach(i; 0..60000)
+{
+    static if(f(i))
+    {
+    }
+}