[clang] [llvm] Fix c-index-test when compiling llvm to a dylib (PR #219092)
Oliver Hunt via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 26 21:53:00 PDT 2026
https://github.com/ojhunt updated https://github.com/llvm/llvm-project/pull/219092
>From f8dd3cc4655aea723061cb5d92dbef335b284c57 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oliver at apple.com>
Date: Tue, 25 Aug 2026 13:25:02 -0600
Subject: [PATCH] Fix c-index-test when compiling llvm to a dylib
CrashRecoveryContext fails to fully protect against the crashing
thread holding a lock, similarly it does not handle a stack allocated
TimerGroup on a torn down stack frame (CrashRecoveryContext uses
setjmp/longjmp so does not run lock or TimerGroup destructors).
This PR makes the CRC more robust to these failures by resetting the
lock state on a crash, and by evicting any known dead TimerGroups
from the timer list.
---
clang/tools/driver/driver.cpp | 15 ++--
llvm/include/llvm/Support/Timer.h | 16 ++++-
llvm/lib/Object/OffloadBundle.cpp | 15 ++--
llvm/lib/Support/CrashRecoveryContext.cpp | 3 +
llvm/lib/Support/Timer.cpp | 72 +++++++++++++++++++-
llvm/unittests/Support/CrashRecoveryTest.cpp | 26 +++++++
6 files changed, 125 insertions(+), 22 deletions(-)
diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index d4d913a8977a4..dae19ecb00df0 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -33,7 +33,6 @@
#include "llvm/Option/ArgList.h"
#include "llvm/Option/OptTable.h"
#include "llvm/Option/Option.h"
-#include "llvm/Support/BuryPointer.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/ErrorHandling.h"
@@ -455,16 +454,10 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
*C, *FailingCommand))
Res = 1;
- if (!UseNewCC1Process && IsCrash) {
- // When crashing in -fintegrated-cc1 mode, bury the timer pointers, because
- // the internal linked list might point to already released stack frames.
- llvm::BuryPointer(llvm::TimerGroup::acquireTimerGlobals());
- } else {
- // If any timers were active but haven't been destroyed yet, print their
- // results now. This happens in -disable-free mode.
- llvm::TimerGroup::printAll(llvm::errs());
- llvm::TimerGroup::clearAll();
- }
+ // If any timers were active but haven't been destroyed yet, print their
+ // results now. This happens in -disable-free mode.
+ llvm::TimerGroup::printAll(llvm::errs());
+ llvm::TimerGroup::clearAll();
#ifdef _WIN32
// Exit status should not be negative on Win32, unless abnormal termination.
diff --git a/llvm/include/llvm/Support/Timer.h b/llvm/include/llvm/Support/Timer.h
index 097eaf3422ca3..a0cfdac95bcfc 100644
--- a/llvm/include/llvm/Support/Timer.h
+++ b/llvm/include/llvm/Support/Timer.h
@@ -260,9 +260,19 @@ class TimerGroup {
/// global constructors and destructors.
LLVM_ABI static void constructForStatistics();
- /// This makes the timer globals unmanaged, and lets the user manage the
- /// lifetime.
- LLVM_ABI static void *acquireTimerGlobals();
+ /// When a crash occurs CrashRecoveryContext needs to ensure that the process
+ /// can recover from two failure modes:
+ /// 1. A crash occuring under RunSafelyOnNewStack or similar APIs. This can
+ /// lead to a lock being held by a now terminated thread; and
+ /// 2. A crash occuring while a stack allocated timer or timergroup was live.
+ /// The first case leads to an eventual deadlock due to the global timer lock
+ /// being held, the latter leads to corruption of the timergroup list due to
+ /// the presence of dangling references.
+ /// To fix this CrashRecoveryContext calls recoverFromCrash. This resets the
+ /// global lock as the only possible holder would be a now dead thread or
+ /// stackframe, and uses \p StackBoundary to determine which locks are now
+ /// dead and should be evicted.
+ LLVM_ABI static void recoverFromCrash(uintptr_t StackBoundary);
private:
friend class Timer;
diff --git a/llvm/lib/Object/OffloadBundle.cpp b/llvm/lib/Object/OffloadBundle.cpp
index 428fe24f33d5e..f94cadfff6584 100644
--- a/llvm/lib/Object/OffloadBundle.cpp
+++ b/llvm/lib/Object/OffloadBundle.cpp
@@ -26,8 +26,11 @@
using namespace llvm;
using namespace llvm::object;
-static TimerGroup OffloadBundlerTimerGroup("Offload Bundler Timer Group",
- "Timer group for offload bundler");
+static TimerGroup &offloadBundlerTimerGroup() {
+ static TimerGroup Group("Offload Bundler Timer Group",
+ "Timer group for offload bundler");
+ return Group;
+}
// Returns the on-disk size recorded in the compressed offload bundle header at
// the start of \p Blob, or std::nullopt if the header carries no size field.
@@ -344,7 +347,7 @@ CompressedOffloadBundle::compress(compression::Params P,
if (!compression::zstd::isAvailable() && !compression::zlib::isAvailable())
return createStringError("compression not supported.");
Timer HashTimer("Hash Calculation Timer", "Hash calculation time",
- OffloadBundlerTimerGroup);
+ offloadBundlerTimerGroup());
if (VerboseStream)
HashTimer.startTimer();
MD5 Hash;
@@ -360,7 +363,7 @@ CompressedOffloadBundle::compress(compression::Params P,
reinterpret_cast<const uint8_t *>(Input.getBuffer().data()),
Input.getBuffer().size());
Timer CompressTimer("Compression Timer", "Compression time",
- OffloadBundlerTimerGroup);
+ offloadBundlerTimerGroup());
if (VerboseStream)
CompressTimer.startTimer();
compression::compress(P, BufferUint8, CompressedBuffer);
@@ -576,7 +579,7 @@ CompressedOffloadBundle::decompress(const MemoryBuffer &Input,
auto StoredHash = Normalized.Hash;
Timer DecompressTimer("Decompression Timer", "Decompression time",
- OffloadBundlerTimerGroup);
+ offloadBundlerTimerGroup());
if (VerboseStream)
DecompressTimer.startTimer();
@@ -598,7 +601,7 @@ CompressedOffloadBundle::decompress(const MemoryBuffer &Input,
// Recalculate MD5 hash for integrity check.
Timer HashRecalcTimer("Hash Recalculation Timer", "Hash recalculation time",
- OffloadBundlerTimerGroup);
+ offloadBundlerTimerGroup());
HashRecalcTimer.startTimer();
MD5 Hash;
MD5::MD5Result Result;
diff --git a/llvm/lib/Support/CrashRecoveryContext.cpp b/llvm/lib/Support/CrashRecoveryContext.cpp
index 493ba951fd3e1..7d6720fc45de2 100644
--- a/llvm/lib/Support/CrashRecoveryContext.cpp
+++ b/llvm/lib/Support/CrashRecoveryContext.cpp
@@ -10,7 +10,9 @@
#include "llvm/Config/llvm-config.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ExitCodes.h"
+#include "llvm/Support/ProgramStack.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/Timer.h"
#include "llvm/Support/thread.h"
#include <cassert>
#include <mutex>
@@ -433,6 +435,7 @@ bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
CRCI->ValidJumpBuffer = true;
if (setjmp(CRCI->JumpBuffer) != 0) {
+ TimerGroup::recoverFromCrash(llvm::getStackPointer());
return false;
}
}
diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index b08f5083e00a8..92197a114de1c 100644
--- a/llvm/lib/Support/Timer.cpp
+++ b/llvm/lib/Support/Timer.cpp
@@ -23,9 +23,11 @@
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/Process.h"
+#include "llvm/Support/ProgramStack.h"
#include "llvm/Support/Signposts.h"
#include "llvm/Support/raw_ostream.h"
#include <limits>
+#include <new>
#include <optional>
#if HAVE_UNISTD_H
@@ -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();
+ new (&Lock) sys::SmartMutex<true>();
+
+ sys::SmartScopedLock<true> L(Lock);
+
+ // Drop any dead stack allocated timer groups. We have to be careful
+ // to avoid ever trying to use or dereference any of these pointers until
+ // we have verified that they are still in a live part of the stack.
+ // We have to compare to StackBoundary rather than local stack position
+ // as our own stack frame may be one of the frames responsible for overwriting
+ // the dangling TimerGroups we're evicting.
+ auto IsTimerGroupInDeadFrame = [&](const void *Candidate) {
+ if (stackGrowsDown())
+ return (uintptr_t)Candidate < StackBoundary;
+ return (uintptr_t)Candidate > StackBoundary;
+ };
+
+ TimerGroup **PrevGroupSlot = &TimerGroupList;
+ TimerGroup *TG = TimerGroupList;
+ while (TG) {
+ if (IsTimerGroupInDeadFrame(TG)) {
+ *PrevGroupSlot = nullptr;
+ break;
+ }
+
+ Timer **PrevTimerSlot = &TG->FirstTimer;
+ Timer *T = TG->FirstTimer;
+ while (T) {
+ if (IsTimerGroupInDeadFrame(T)) {
+ *PrevTimerSlot = nullptr;
+ break;
+ }
+ PrevTimerSlot = &T->Next;
+ T = T->Next;
+ }
+
+ PrevGroupSlot = &TG->Next;
+ TG = TG->Next;
+ }
+}
+
void TimerGroup::removeTimer(Timer &T) {
sys::SmartScopedLock<true> L(timerLock());
@@ -571,8 +641,6 @@ void TimerGroup::constructForStatistics() {
ManagedTimerGlobals->initDeferred();
}
-void *TimerGroup::acquireTimerGlobals() { return ManagedTimerGlobals.claim(); }
-
static bool isTimerGlobalsConstructed() {
return ManagedTimerGlobals.isConstructed();
}
diff --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp
index ceafba5b36f11..bfafa494e0c9a 100644
--- a/llvm/unittests/Support/CrashRecoveryTest.cpp
+++ b/llvm/unittests/Support/CrashRecoveryTest.cpp
@@ -14,6 +14,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/Timer.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Host.h"
#include "llvm/TargetParser/Triple.h"
@@ -47,6 +48,31 @@ TEST(CrashRecoveryTest, Basic) {
EXPECT_FALSE(CrashRecoveryContext().RunSafely(llvmTrap));
}
+// Mirrors clang's `#pragma clang __debug crash`, which keeps a Timer live on
+// the stack across a deliberate crash (clang/lib/Lex/Pragma.cpp). Recovery
+// runs no destructors, so without TimerGroup::recoverFromCrash() this leaves
+// a dangling entry in the shared, default TimerGroup's list.
+static void crashWithLiveTimer() {
+ llvm::Timer T("crash-recovery-test", "timer live during crash");
+ llvm::TimeRegion R(&T);
+ llvmTrap();
+}
+
+TEST(CrashRecoveryTest, TimerReuseAfterCrash) {
+ llvm::CrashRecoveryContext::Enable();
+ EXPECT_FALSE(CrashRecoveryContext().RunSafely(crashWithLiveTimer));
+
+ // An unrelated, later Timer on the same (default) group must construct and
+ // destroy without hanging.
+ {
+ llvm::Timer T("crash-recovery-test-2", "unrelated later timer");
+ T.startTimer();
+ T.stopTimer();
+ }
+
+ llvm::CrashRecoveryContext::Disable();
+}
+
struct IncrementGlobalCleanup : CrashRecoveryContextCleanup {
IncrementGlobalCleanup(CrashRecoveryContext *CRC)
: CrashRecoveryContextCleanup(CRC) {}
More information about the llvm-commits
mailing list