================
@@ -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