ilya-biryukov wrote: > To make this testable, maybe we can refactor > `clang::runWithSufficientStackSpace` a little bit to make `DesiredStackSize` > and `isStackNearlyExhausted::SufficientStack` configurable.
Maybe... As long as we only use this in tests. However, for this particular case, we could also maybe track deserialization depth (similar to template instantiation depth). I actually landed a patch internally that increased the stack size and it immediately started crashing on [another test]( https://github.com/llvm/llvm-project/blob/e5054fb5c660cb81d1f96498e39662914a92a167/clang-tools-extra/clangd/test/infinite-instantiation.test). I suspect that what happened there is that increasing the stack size causes us to have more recursive calls before we issue a warning and start a new thread. In turn, those recursive calls may lead to stack overflow before we get to the next call of runWithSufficientStackSpace. I am not saying it's a bad idea to add this for testing purposes, but given how fragile this approach is, I think we should not provide configuration knobs to users there. At least everyone sees crashes at roughly the same points right now (although variation is still present because depending on inlining decisions stacks sizes might vary even with the same code and same limits). https://github.com/llvm/llvm-project/pull/79875 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
