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
 }
 
+static 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 {
@@ -2806,7 +2845,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));
@@ -4989,7 +5035,7 @@ class clangSilent {
 };
 } // namespace
 
-int Declare(compat::Interpreter& I, const char* code, bool silent) {
+static int Declare(compat::Interpreter& I, const char* code, bool silent) {
   // Trap diagnostics on both paths: I.declare's rc is 0 even when
   // Parse recovered from emitted errors, so callers need the trap to
   // distinguish "parsed cleanly" from "parsed with errors".

unittests/CppInterOp/VariableReflectionTest.cpp

@@ -1,14 +1,20 @@
 #include "Utils.h"
 
+#include "../../lib/CppInterOp/Unwrap.h"
+
 #include "CppInterOp/CppInterOp.h"
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Sema/Sema.h"
 
-#include "gtest/gtest.h"
+#include "llvm/Support/Error.h"
+
 #include <string>
+#include <utility>
+#include "gtest/gtest.h"
 
 #include <cstddef>
 
@@ -376,6 +382,110 @@ TYPED_TEST(CPPINTEROP_TEST_MODE,
   EXPECT_FALSE(Cpp::GetVariableOffset(var2));
 }
 
+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>());
+#ifndef CPPINTEROP_USE_CLING
+  // 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. (PTU/TheModule is
+  // clang::Interpreter surface; cling covers this path via the UsedAttr
+  // assert above.)
+  {
+    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"));
+#endif // !CPPINTEROP_USE_CLING
+  // 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);
+    std::string decl = "struct Sweep";
+    decl += n;
+    decl += " { inline static int v";
+    decl += n;
+    decl += " = ";
+    decl += n;
+    decl += "; };";
+    Cpp::Declare(decl.c_str());
+    Cpp::DeclRef k = Cpp::GetNamed("Sweep" + n);
+    ASSERT_TRUE(k);
+    Cpp::DeclRef v = Cpp::GetNamed("v" + n, k);
+    ASSERT_TRUE(v);
+    EXPECT_TRUE(Cpp::GetVariableOffset(v));
+    std::string probe = "template <> struct Sweep";
+    probe += n;
+    probe += "<int>;";
+    Cpp::Declare(probe.c_str(), /*silent=*/true); // expected parse failure
+    std::string use = "int use";
+    use += n;
+    use += " = Sweep";
+    use += n;
+    use += "::v";
+    use += n;
+    use += ";";
+    Cpp::Declare(use.c_str());
+  }
+  Cpp::Declare("int sweep_done = 1;");
+  EXPECT_TRUE(Cpp::GetNamed("sweep_done"));
+
+  // Template-specialization spellings bail out of the odr-use route
+  // (printed non-type arguments do not reliably round-trip as source) and
+  // keep the UsedAttr fallback.
+  Cpp::Declare(R"(
+    template <typename T> struct TmplStatic {
+      inline static int member = 7;
+    };
+  )");
+  Cpp::DeclRef tmpl = Cpp::GetNamed("TmplStatic");
+  EXPECT_TRUE(tmpl);
+  ASTContext& C = Interp->getCI()->getASTContext();
+  std::vector<Cpp::TemplateArgInfo> template_args = {
+      {C.IntTy.getAsOpaquePtr()}};
+  Cpp::DeclRef inst = Cpp::InstantiateTemplate(tmpl, template_args);
+  EXPECT_TRUE(inst);
+  Cpp::DeclRef member = Cpp::GetNamed("member", inst);
+  EXPECT_TRUE(member);
+  EXPECT_TRUE(Cpp::GetVariableOffset(member));
+#ifndef CPPINTEROP_USE_CLING
+  // cling's getAddressOfGlobal can emit on demand, resolving the address
+  // before the forced-emission gate is reached; only clang-repl reliably
+  // takes the UsedAttr fallback here.
+  EXPECT_TRUE(Cpp::unwrap<Decl>(member)->hasAttr<clang::UsedAttr>());
+#endif
+}
+
 #define CODE                                                                   \
   class BaseA {                                                                \
   public:                                                                      \