[clang] [clang] Prevent stack overflow on deeply nested template arguments (PR #224393)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> > I don't have strong opinions about the approach taken here, but it seems like we're punishing the simple case of a single level of templates in order to better diagnose more pathological cases.
> 
> Just a short brain-storming:
> 
> As I understand, the current StackHandler.runWithSufficientStackSpace() does not differentiate the recursion level at the moment : the same checks are done for 1 as well as for 1000 levels of recursion.

Correct.

> What if the StackHandler would track the recursion level and do something like:
> 
> ```
> if (recursion_level < threshold_level) [[ likely ]] {
>   F(); // just recurse normaly
> } else {
>   // the current logic
>   // if (isStackNearlyExhausted()) [[ unlikely ]]  {...} else {...}
> }
> ```
`> 
> What we want to achieve with this:
> 
>     * up to some small (threshold) number of recursions just recurse normally ( by assuming we will never exceed stack limit on this recursion level)
> 
>     * for deeper recursions start doing more checks.
> 
>     * different functions may have different thresholds
> 
> 
> Some rough idea: https://godbolt.org/z/ovvMY46zT
> 
> Of course, measurements would have to confirm benefits of such an approach, if any at all.
> 
> But for the case it's positive, it would cover all scenarios: cheap for small recursion depth and robust otherwise.
> 
> Or formulated differently: Is there a way to make this robust but cheap for the normal usage (zero-cost principle) ?
> 
> Just an idea.

That's an interesting idea and might be a workable general solution. I think eliminating the recursion via tail calls is likely still a better approach when we can achieve it though. We have other problems beyond just stack resource limits; for example, the more memory pressure we apply the fewer recursive templates we can instantiate due to running into out of memory scenarios. By reducing stack usage, we also help with that kind of thing. However, I think there's even more low-hanging fruit for that situation like paying attention to stack frame sizes (I've seen some *gigantic* stack frames when I was last poking around with this issue). But for the times when we just can't get tail calls to work, having a smarter `runWithSufficientStackSpace()` is attractive enough to be worth exploring.

Now, whether we need to do that as part of this PR or not is a whole different question. Given the performance behavior @zyn0217 posted about, maybe it's fine to move forward with this as-is. I think Clang two-stage uses enough templates to be a reasonable proxy for the scenario I was worried we'd be slowing down.

https://github.com/llvm/llvm-project/pull/224393