[clang][HeuristicResolver] Additional hardening against an infinite loop in simplifyType()

[HighCommander4]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/126690.diff

1 Files Affected:

• (modified) clang/lib/Sema/HeuristicResolver.cpp (+5-1)

diff --git a/clang/lib/Sema/HeuristicResolver.cpp b/clang/lib/Sema/HeuristicResolver.cpp
index 3cbf33dcdced38..adce403412f689 100644
--- a/clang/lib/Sema/HeuristicResolver.cpp
+++ b/clang/lib/Sema/HeuristicResolver.cpp
@@ -258,7 +258,11 @@ QualType HeuristicResolverImpl::simplifyType(QualType Type, const Expr *E,
     }
     return T;
   };
-  while (!Current.Type.isNull()) {
+  // As an additional protection against infinite loops, bound the number of
+  // simplification steps.
+  size_t StepCount = 0;
+  const size_t MaxSteps = 64;
+  while (!Current.Type.isNull() && StepCount++ < MaxSteps) {
     TypeExprPair New = SimplifyOneStep(Current);
     if (New.Type == Current.Type)
       break;

[HighCommander4]

This is not needed to fix #126536 (#126689 does that), it's a hedge against additional bugs that may be lurking that cause infinite loops.

I'm open to suggestions as to whether this is a good idea. It may hide some bugs, but the symptoms of those bugs are much less severe (incorrect or failed heuristic resolution of a dependent name in a template) than an infinite loop.

[kadircet]

what about landing this one, and cherry picking it into the 20 release. then we can land #126689 and revert this one. giving us the next release cycle to vet the underlying change?

[kadircet]

going to land this one, since review of #126689 can take longer and this should enable us to unblock our releases. I am happy to revert afterwards.

[kadircet]

oops I wasn't looking at the target branch :( let me cherry-pick this into main

[HighCommander4]

and cherry picking it into the 20 release

The regressing change, which introduced the simplifyType() function, isn't present on the llvm 20 branch (it landed a bit after the branch cut). So there should not be a need to cherry-pick the fix onto the llvm 20 branch either.