[PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: include/clang/Basic/Stack.h:42
+                                          llvm::function_ref<void()> Fn) {
+    if (LLVM_UNLIKELY(isStackNearlyExhausted())) {
+      runWithSufficientStackSpaceSlow(Diag, Fn);
----------------
For LLVM_THREADS_DISABLED builds used by Chromium today, maybe we should skip this completely?


================
Comment at: lib/Basic/Stack.cpp:23
+  return __builtin_frame_address(0);
+#else
+  char CharOnStack = 0;
----------------
I think `_AddressOfReturnAddress()` ifdef _MSC_VER would be the proper way to ask MSVC for a reasonable stack pointer, if we're worried about returning the address of a local.


================
Comment at: lib/Basic/Stack.cpp:53-54
+  // If the stack pointer has a surprising value, we do not understand this
+  // stack usage scheme. (Perhaps the target allocates new stack regions on
+  // demand for us.) Don't try to guess what's going on.
+  if (StackUsage > DesiredStackSize)
----------------
So, for example, ASan with UAR, where frames are heap allocated. I suppose in that case we will go down the __builtin_frame_address path, though.


================
Comment at: test/SemaTemplate/stack-exhaustion.cpp:10
+template<typename ...T> struct tuple {};
+template<typename ...T> auto f(tuple<T...> t) -> decltype(f(tuple<T...>(t))) {} // expected-error {{exceeded maximum depth}}
+void g() { f(tuple<int, int>()); }
----------------
This test seems like it could be fragile. If threads are disabled, it will probably crash.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361