Emit discardable-ODR variables via odr-use, not UsedAttr, in GetVariableOffset

ForceCodeGen forces deferred emission by planting a permanent
__attribute__((used)) on the decl; every global emitted that way gets a
WeakTrackingVH in codegen's llvm.used list. When such a global is
deleted before emitUsed runs (ORC freeing a materialized PTU's IR), the
handle nulls and release-built clang dereferences it unchecked — a
process crash that accumulates with interpreter state rather than
tracing to any one declaration (large reflection sweeps died; per-type
bisection never converged, so the crash itself has no compact unit
repro — root-caused via gdb).

GetVariableOffset now emits GVA_DiscardableODR variables (the
inline/constexpr class the crash traced to) by declaring an
external-linkage odr-use of the qualified name instead: the definition
flows through the regular deferred-decl path and leaves no used-list
residue, no permanent attribute. Everything else keeps the UsedAttr
path, deliberately:

- Internal-linkage variables cannot be odr-used from a later PTU (the
  module-local symbol duplicates or goes missing); getting this wrong
  broke VariableReflection_GetVariableOffset (static int S) and cppyy's
  Lifeline::count lookup.
- Available-externally definitions would not be emitted by a mere
  reference.
- Template-specialization and anonymous/lambda spellings do not
  reliably round-trip as source (a printed LLONG_MIN non-type argument
  re-parses as an overflowing literal), and a parse-failing declaration
  poisons the incremental interpreter.

An interpreter-level alternative — Undo(1) on parse failure to drop the
failed PTU — was evaluated and rejected: cppyy depends on failed parses
leaving side-effect declarations (an explicit-instantiation probe of a
declared-but-undefined template is expected to fail and leave the
specialization decl behind; test_templates test32).

The dummy-name counter is deliberately plain, matching gWrapperSerial:
the interpreter API is caller-serialized. ForceCodeGen's function path
(GetFunctionAddress) still uses UsedAttr — same theoretical hazard,
never observed.

The test pins the mechanism: after an offset query on a
discardable-ODR static, the decl carries no UsedAttr and later PTU
modules carry no llvm.used, with a sweep-shaped regression net over
interleaved absorbed parse failures.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)

Author: Emery Conrad

lib/CppInterOp/CppInterOp.cpp

@@ -402,6 +402,45 @@ static void ForceCodeGen(Decl* D, compat::Interpreter& I) {
 #endif
 }
 
+int Declare(compat::Interpreter& I, const char* code, bool silent);
+
+// Force emission of a variable's definition by declaring an odr-use of it,
+// instead of ForceCodeGen's UsedAttr route. The UsedAttr route records the
+// emitted global in codegen's llvm.used list as a weak handle; the handle
+// can go null (global deleted) before a later incremental PTU's emitUsed
+// runs, and release-built clang dereferences it without checking — a
+// process crash that accumulates with interpreter state rather than
+// tracing to any one declaration. The odr-use emits the (discardable)
+// definition through the regular deferred-decl path and leaves no
+// used-list residue. Returns false when the variable cannot be named from
+// a fresh chunk of source (anonymous scopes, or printed names that fail to
+// parse) — the caller falls back to ForceCodeGen.
+static bool EmitVariableViaOdrUse(compat::Interpreter& I, VarDecl* VD) {
+  std::string name;
+  {
+    llvm::raw_string_ostream OS(name);
+    VD->printQualifiedName(OS);
+  }
+  if (name.find("(anonymous ") != std::string::npos ||
+      name.find("(unnamed ") != std::string::npos ||
+      name.find("(lambda ") != std::string::npos)
+    return false;
+  // Template-specialization spellings do not reliably round-trip as source
+  // (e.g. a printed LLONG_MIN non-type argument re-parses as an overflowing
+  // literal), and a parse-failing declaration poisons the incremental
+  // interpreter; keep those on the caller's UsedAttr path.
+  if (name.find('<') != std::string::npos)
+    return false;
+
+  // External linkage on the dummy keeps it — and therefore the referenced
+  // definition — from being discarded as unused internal state.
+  static unsigned Counter = 0;
+  std::string code = "namespace __cppinterop_odr_use { const void* __v" +
+                     std::to_string(Counter++) +
+                     " = (const void*)__builtin_addressof(::" + name + "); }";
+  return Declare(I, code.c_str(), /*silent=*/true) == 0;
+}
+
 #define DEBUG_TYPE "jitcall"
 bool JitCall::AreArgumentsValid(void* result, ArgList args, void* self,
                                 size_t nary) const {
@@ -2794,7 +2833,14 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D,
     }
     if (!address) {
       auto Linkage = C.GetGVALinkageForVariable(VD);
-      if (isDiscardableGVALinkage(Linkage))
+      // Odr-use emission only for discardable-ODR entities (inline/constexpr
+      // statics) — the class the used-list crash traced to. Internal-linkage
+      // variables cannot be odr-used from a later PTU (module-local symbol:
+      // the reference duplicates or misses the entity), and an
+      // available-externally definition would not be emitted by a mere
+      // reference; both stay on the stock UsedAttr path.
+      if (isDiscardableGVALinkage(Linkage) &&
+          (Linkage != GVA_DiscardableODR || !EmitVariableViaOdrUse(I, VD)))
         ForceCodeGen(VD, I);
     }
     auto VDAorErr = compat::getSymbolAddress(I, StringRef(mangledName));

unittests/CppInterOp/VariableReflectionTest.cpp

@@ -340,6 +340,68 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, VariableReflection_GetVariableOffset) {
   EXPECT_TRUE(Cpp::GetVariableOffset(var));
 }
 
+TYPED_TEST(CPPINTEROP_TEST_MODE,
+           VariableReflection_GetVariableOffset_NoStaleUsedHandle) {
+  // Emitting a discardable-ODR variable through the UsedAttr route records
+  // the global as a weak handle in codegen's llvm.used list. If the global
+  // is later replaced/erased (weak-def discard when a later PTU re-emits
+  // the same entity), the next PTU's emitUsed dereferences the nulled
+  // handle. The offset query must not leave used-list residue.
+  TestFixture::CreateInterpreter();
+  Cpp::Declare(R"(
+    struct UsedHandle {
+      inline static int probe = 3;
+    };
+  )");
+  Cpp::DeclRef klass = Cpp::GetNamed("UsedHandle");
+  EXPECT_TRUE(klass);
+  Cpp::DeclRef var = Cpp::GetNamed("probe", klass);
+  EXPECT_TRUE(var);
+  EXPECT_TRUE(Cpp::GetVariableOffset(var));
+  // ForceCodeGen's UsedAttr is planted permanently on the AST decl; the
+  // odr-use route must not.
+  EXPECT_FALSE(Cpp::unwrap<Decl>(var)->hasAttr<clang::UsedAttr>());
+  // The UsedAttr lives on the AST decl, so every later PTU that re-emits
+  // the entity re-adds it to that module's llvm.used — the residue the
+  // stale-handle crash grows from. Neither a module that re-emits the
+  // variable nor an unrelated one may carry llvm.used.
+  {
+    auto PTUOrErr = Interp->Parse("int consume_probe = UsedHandle::probe;");
+    ASSERT_TRUE(bool(PTUOrErr));
+    EXPECT_EQ(PTUOrErr->TheModule->getNamedGlobal("llvm.used"), nullptr);
+    if (auto Err = Interp->Execute(*PTUOrErr))
+      llvm::consumeError(std::move(Err));
+  }
+  {
+    auto PTUOrErr = Interp->Parse("int flush_ptu = 0;");
+    ASSERT_TRUE(bool(PTUOrErr));
+    EXPECT_EQ(PTUOrErr->TheModule->getNamedGlobal("llvm.used"), nullptr);
+    if (auto Err = Interp->Execute(*PTUOrErr))
+      llvm::consumeError(std::move(Err));
+  }
+  EXPECT_TRUE(Cpp::GetNamed("flush_ptu"));
+  // The crash this guards against was cumulative — many force-emitted
+  // statics plus JIT materialization cycles, interleaved with absorbed
+  // parse failures. Mimic that sweep shape as a regression net.
+  for (int i = 0; i < 8; ++i) {
+    std::string n = std::to_string(i);
+    Cpp::Declare(("struct Sweep" + n + " { inline static int v" + n + " = " +
+                  n + "; };")
+                     .c_str());
+    Cpp::DeclRef k = Cpp::GetNamed(("Sweep" + n).c_str());
+    ASSERT_TRUE(k);
+    Cpp::DeclRef v = Cpp::GetNamed(("v" + n).c_str(), k);
+    ASSERT_TRUE(v);
+    EXPECT_TRUE(Cpp::GetVariableOffset(v));
+    Cpp::Declare(("template <> struct Sweep" + n + "<int>;").c_str(),
+                 /*silent=*/true); // expected parse failure, absorbed
+    Cpp::Declare(
+        ("int use" + n + " = Sweep" + n + "::v" + n + ";").c_str());
+  }
+  Cpp::Declare("int sweep_done = 1;");
+  EXPECT_TRUE(Cpp::GetNamed("sweep_done"));
+}
+
 #define CODE                                                                   \
   class BaseA {                                                                \
   public:                                                                      \