================
@@ -328,6 +330,74 @@ TimerGroup::~TimerGroup() {
   unlink();
 }
 
+// ***REVIEWER***: I really don't know if this is warranted? I'm unaware of any
+// major environment where the stack grows up these days, but there's a
+// difference between me being unaware of such, and it not actually existing or
+// being a supported host platform.
+__attribute__((noinline)) static void
+stackGrowsDownResult(uintptr_t BaseStackPtr, bool *Result) {
+  *Result = llvm::getStackPointer() < BaseStackPtr;
+}
+__attribute__((noinline)) static bool
+stackGrowsDownInner(uintptr_t BaseStackPtr) {
+  // We use this to force a non-zero-sized stack frame, and we force it to live
+  // by using it to return the growth direction;
+  bool StackGrowsDown = false;
+  stackGrowsDownResult(BaseStackPtr, &StackGrowsDown);
+  return StackGrowsDown;
+}
+static bool stackGrowsDown() {
+  static bool GrowsDown = stackGrowsDownInner(llvm::getStackPointer());
+  return GrowsDown;
+}
+
+void TimerGroup::recoverFromCrash(uintptr_t StackBoundary) {
+  if (!isTimerGlobalsConstructed())
+    return;
+
+  // Reset the global timer group lock. We cannot call the destructor as it may
+  // currently be held by a dead thread, so we simply reinitialize in place.
+  sys::SmartMutex<true> &Lock = timerLock();
----------------
AaronBallman wrote:

> Right, the problem is that we have tests that assert that this is possible.
> I hit this with c-index-test test cases that were explicitly testing for 
> recovery in this path.

That test file 
(https://github.com/llvm/llvm-project/blob/main/clang/test/Index/crash-recovery-modules.m)
 has needed quite a few opt-outs over the years. I suspect the test might be 
invalid (though it may have been more reasonable back in 2012 when it was 
added).

> My reason for this hideous approach was entirely that the existing code 
> already seemed to think that this was a recoverable state.

I think "recoverable" in terms of `CrashRecoveryContext` means "we can recover 
to the point of doing a graceful exit" and not "we can recover and just carry 
on as though nothing bad happened". We have this comment on `RunSafely()`:

> Clients should make as little assumptions as possible about the program state 
> when RunSafely has returned false.

I can't tell whether the c-index-test seems to have the second one in mind, 
though. It's actually a bit confused:

https://github.com/llvm/llvm-project/blob/519a7ffc164d1b2c4e29750eb8e43c435d43de44/clang/test/Index/crash-recovery-modules.m#L7

implies it's trying to cause a crash, but it does `-DCRASH` and not 
`-DLIBCLANG_CRASH` and so... I don't think it should crash?

https://github.com/llvm/llvm-project/pull/219092
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to