[clang][CodeGen] Skip deleted globals in emitUsed#210959
Conversation
A global on the llvm.used/llvm.compiler.used list can be deleted before the module is released: CodeGen erases an unreferenced GlobalValue without RAUW when the same mangled name is redefined with a different type (GetOrCreateLLVMGlobal), which nulls the WeakTrackingVH on the used list. In whole-TU compilation the error suppresses Release(), but the incremental interpreter clears the diagnostic and keeps building the same module, so the next successful parse crashes in emitUsed dereferencing the dead handle. Skip null handles when building the used array. (clang-repl repro included as a lit test.) Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
llvmorg-github-actions
Bot
added
clang
|
@llvm/pr-subscribers-clang-codegen Author: Emery Conrad (conrade-ctc) ChangesA global on the In whole-TU compilation this is unobservable because the accompanying Out-of-tree incremental clients hit the same crash through other routes that delete an emitted used global before module release (e.g. CppInterOp's force-emission of reflection queries, see compiler-research/CppInterOp#1068). Fix: skip null handles when building the used array. The repro above is included as a lit test; 🤖 Done with the help of Claude Code (Fable 5, human in the loop) Full diff: https://github.com/llvm/llvm-project/pull/210959.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 78627047b19ad..ead29559ced51 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3701,13 +3701,16 @@ static void emitUsed(CodeGenModule &CGM, StringRef Name,
if (List.empty())
return;
- // Convert List to what ConstantArray needs.
- SmallVector<llvm::Constant*, 8> UsedArray;
- UsedArray.resize(List.size());
- for (unsigned i = 0, e = List.size(); i != e; ++i) {
- UsedArray[i] =
- llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(
- cast<llvm::Constant>(&*List[i]), CGM.Int8PtrTy);
+ // Convert List to what ConstantArray needs. A used global may have been
+ // deleted after it was added to the list (e.g. when its home module keeps
+ // accumulating declarations after an erroneous incremental parse), leaving
+ // a null value handle behind; skip those entries.
+ SmallVector<llvm::Constant *, 8> UsedArray;
+ UsedArray.reserve(List.size());
+ for (const llvm::WeakTrackingVH &VH : List) {
+ if (llvm::Value *V = VH)
+ UsedArray.push_back(llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(
+ cast<llvm::Constant>(V), CGM.Int8PtrTy));
}
if (UsedArray.empty())
diff --git a/clang/test/Interpreter/used-global-after-error.cpp b/clang/test/Interpreter/used-global-after-error.cpp
new file mode 100644
index 0000000000000..fc1043042d5cc
--- /dev/null
+++ b/clang/test/Interpreter/used-global-after-error.cpp
@@ -0,0 +1,15 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+
+// A cast<> of a null WeakTrackingVH used to crash emitUsed when a global on
+// the llvm.used list was deleted before the module was released: the
+// duplicate definition below is diagnosed by CodeGen, which then replaces the
+// used global's unreferenced GV with a differently-typed one; the failed
+// parse keeps the module alive, and the next successful parse finalizes it.
+extern "C" int printf(const char *, ...);
+__attribute__((used)) int a asm("sym") = 1; float b asm("sym") = 2.0f;
+auto r1 = printf("ok = %d\n", 42);
+// CHECK: ok = 42
+
+%quit
|
|
@llvm/pr-subscribers-clang Author: Emery Conrad (conrade-ctc) ChangesA global on the In whole-TU compilation this is unobservable because the accompanying Out-of-tree incremental clients hit the same crash through other routes that delete an emitted used global before module release (e.g. CppInterOp's force-emission of reflection queries, see compiler-research/CppInterOp#1068). Fix: skip null handles when building the used array. The repro above is included as a lit test; 🤖 Done with the help of Claude Code (Fable 5, human in the loop) Full diff: https://github.com/llvm/llvm-project/pull/210959.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 78627047b19ad..ead29559ced51 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3701,13 +3701,16 @@ static void emitUsed(CodeGenModule &CGM, StringRef Name,
if (List.empty())
return;
- // Convert List to what ConstantArray needs.
- SmallVector<llvm::Constant*, 8> UsedArray;
- UsedArray.resize(List.size());
- for (unsigned i = 0, e = List.size(); i != e; ++i) {
- UsedArray[i] =
- llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(
- cast<llvm::Constant>(&*List[i]), CGM.Int8PtrTy);
+ // Convert List to what ConstantArray needs. A used global may have been
+ // deleted after it was added to the list (e.g. when its home module keeps
+ // accumulating declarations after an erroneous incremental parse), leaving
+ // a null value handle behind; skip those entries.
+ SmallVector<llvm::Constant *, 8> UsedArray;
+ UsedArray.reserve(List.size());
+ for (const llvm::WeakTrackingVH &VH : List) {
+ if (llvm::Value *V = VH)
+ UsedArray.push_back(llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(
+ cast<llvm::Constant>(V), CGM.Int8PtrTy));
}
if (UsedArray.empty())
diff --git a/clang/test/Interpreter/used-global-after-error.cpp b/clang/test/Interpreter/used-global-after-error.cpp
new file mode 100644
index 0000000000000..fc1043042d5cc
--- /dev/null
+++ b/clang/test/Interpreter/used-global-after-error.cpp
@@ -0,0 +1,15 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+
+// A cast<> of a null WeakTrackingVH used to crash emitUsed when a global on
+// the llvm.used list was deleted before the module was released: the
+// duplicate definition below is diagnosed by CodeGen, which then replaces the
+// used global's unreferenced GV with a differently-typed one; the failed
+// parse keeps the module alive, and the next successful parse finalizes it.
+extern "C" int printf(const char *, ...);
+__attribute__((used)) int a asm("sym") = 1; float b asm("sym") = 2.0f;
+auto r1 = printf("ok = %d\n", 42);
+// CHECK: ok = 42
+
+%quit
|
|
@vgvassilev, here's the simpler of the two PRs discussed in compiler-research/CppInterOp#1068 |
2 participants
A global on the
llvm.used/llvm.compiler.usedlist can be deleted before the module is released: CodeGen erases an unreferencedGlobalValuewithout RAUW when the same mangled name is redefined with a different type (GetOrCreateLLVMGlobal), which nulls theWeakTrackingVHon the used list — value handles are not uses, so theuse_empty()guard does not protect them.In whole-TU compilation this is unobservable because the accompanying
err_duplicate_mangled_namesuppressesRelease(). The incremental interpreter, however, clears the diagnostic state after the failed parse and keeps building the same module, so the next successful parse runsRelease()and crashes inemitUseddereferencing the dead handle:Out-of-tree incremental clients hit the same crash through other routes that delete an emitted used global before module release (e.g. CppInterOp's force-emission of reflection queries, see compiler-research/CppInterOp#1068).
Fix: skip null handles when building the used array. The repro above is included as a lit test;
clang/test/Interpreter,clang/test/CodeGen, andclang/test/CodeGenCXXpass locally.🤖 Done with the help of Claude Code (Fable 5, human in the loop)