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

@@ -59,6 +59,7 @@
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangStandard.h"
 #include "clang/Basic/Linkage.h"
 #include "clang/Basic/OperatorKinds.h"
@@ -402,6 +403,94 @@ static void ForceCodeGen(Decl* D, compat::Interpreter& I) {
 #endif
 }
 
+// Force emission of a variable's definition by synthesizing 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 anchor is a dummy global
+// initialized with the variable's address, built directly through Sema (the
+// AST that parsing "&var" would produce — no source-text round-trip), so
+// the definition flows through the regular deferred-decl path and leaves no
+// used-list residue. Returns false when the odr-use cannot be built — the
+// caller falls back to ForceCodeGen.
+// FIXME: Remove the synthesized-anchor mechanism once clang grows a direct
+// interpreter emission API for a GlobalDecl (the emitUsed hardening it
+// depends on landed via llvm/llvm-project#210959):
+#if CLANG_VERSION_MAJOR >= 24
+#warning "Revisit EmitVariableViaOdrUse: a direct-emission API may exist now"
+#endif
+static bool EmitVariableViaOdrUse(compat::Interpreter& I, VarDecl* VD) {
+  // A reference cannot conjure a definition that does not exist: the dummy
+  // would carry an unresolvable symbol into the JIT (Emscripten's dynamic
+  // loader rejects the whole module over it, and the stale entry then
+  // breaks every later load in the process). The UsedAttr fallback defers
+  // harmlessly for definition-less declarations.
+  VarDecl* Def = VD->getDefinition();
+  if (!Def)
+    return false;
+
+  Sema& S = I.getCI()->getSema();
+  ASTContext& C = S.getASTContext();
+
+  // Open the synthesizing region before ANY Sema work: odr-use marking in
+  // BuildDeclRefExpr can trigger an immediate static-member instantiation,
+  // and cling's DeclCollector aborts on instantiation callbacks that arrive
+  // outside an active transaction. (Inert under clang-repl — TODO destructor
+  // in Compatibility.h.)
+  compat::SynthesizingCodeRAII RAII(&I);
+
+  // Build '&VD' the way the parser would; the address-of marks Def
+  // odr-used, which is what schedules the deferred definition.
+  ExprResult Ref = S.BuildDeclRefExpr(Def, Def->getType().getNonReferenceType(),
+                                      VK_LValue, SourceLocation());
+  if (Ref.isInvalid())
+    return false;
+  ExprResult AddrOf =
+      S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, Ref.get());
+  if (AddrOf.isInvalid())
+    return false;
+
+  // Anchor the odr-use in an external-linkage dummy the JIT must emit; the
+  // initializer's implicit conversion to 'const void*' is Sema-checked, so
+  // exotic types fail over to the fallback instead of asserting.
+  // FIXME: The synthesized nodes are parked in the TU for the rest of the
+  // session; move them under a scratch area whose AST nodes get deallocated
+  // after emission.
+  static unsigned Counter = 0;
+  std::string DummyName = "__cppinterop_odr_use_v" + std::to_string(Counter++);
+  QualType Ty = C.getPointerType(C.VoidTy.withConst());
+  TranslationUnitDecl* TU = C.getTranslationUnitDecl();
+  VarDecl* Dummy = VarDecl::Create(C, TU, SourceLocation(), SourceLocation(),
+                                   &C.Idents.get(DummyName), Ty,
+                                   C.getTrivialTypeSourceInfo(Ty), SC_None);
+  S.AddInitializerToDecl(Dummy, AddrOf.get(), /*DirectInit=*/false);
+  if (Dummy->isInvalidDecl())
+    return false;
+  TU->addDecl(Dummy);
+
+  // Under cling the RAII's transaction commit (at scope exit) triggers
+  // emission of everything collected above plus this dummy.
+  I.getCI()->getASTConsumer().HandleTopLevelDecl(DeclGroupRef(Dummy));
+#ifndef CPPINTEROP_USE_CLING
+  // FIXME: Parsing an empty string is the only way to flush incremental
+  // CodeGen for a decl handed straight to the consumer — the state-reset
+  // bug SynthesizingCodeRAII's clang-repl destructor should eventually
+  // own. Drop this when it does.
+  auto GeneratedPTU = I.Parse("");
+  if (!GeneratedPTU) {
+    llvm::consumeError(GeneratedPTU.takeError());
+    return false;
+  }
+  if (auto Err = I.Execute(*GeneratedPTU)) {
+    llvm::consumeError(std::move(Err));
+    return false;
+  }
+#endif
+  return true;
+}
+
 #define DEBUG_TYPE "jitcall"
 bool JitCall::AreArgumentsValid(void* result, ArgList args, void* self,
                                 size_t nary) const {
@@ -2806,7 +2895,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 +5085,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,16 +1,23 @@
 #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 "llvm/Support/Error.h"
+
 #include "gtest/gtest.h"
-#include <string>
 
+#include <array>
 #include <cstddef>
+#include <string>
+#include <utility>
 
 using namespace TestUtils;
 using namespace llvm;
@@ -376,6 +383,147 @@ TYPED_TEST(CPPINTEROP_TEST_MODE,
   EXPECT_FALSE(Cpp::GetVariableOffset(var2));
 }
 
+TYPED_TEST(CPPINTEROP_TEST_MODE,
+           VariableReflection_GetVariableOffset_NoStaleUsedHandle) {
+#ifdef __EMSCRIPTEN__
+  // The stale-handle crash this test pins is native-JIT mechanics (ORC
+  // freeing materialized IR), and a single failed module load poisons
+  // Emscripten's process-global dynamic-linking state for every test that
+  // follows (issue #1071) — not worth the fragility for a native-only
+  // scenario.
+  GTEST_SKIP() << "Stale-used-handle scenario is native-JIT specific";
+#endif
+  // 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; the
+  // per-index sources are assembled at compile time by stringification.
+  struct SweepCase {
+    const char* decl;
+    const char* cls;
+    const char* var;
+    const char* probe;
+    const char* use;
+  };
+#define SWEEP_CASE(n)                                                          \
+  {                                                                            \
+    "struct Sweep" #n " { inline static int v" #n " = " #n "; };", "Sweep" #n, \
+        "v" #n, "template <> struct Sweep" #n "<int>;",                        \
+        "int use" #n " = Sweep" #n "::v" #n ";"                                \
+  }
+  constexpr std::array<SweepCase, 8> Sweeps = {
+      {SWEEP_CASE(0), SWEEP_CASE(1), SWEEP_CASE(2), SWEEP_CASE(3),
+       SWEEP_CASE(4), SWEEP_CASE(5), SWEEP_CASE(6), SWEEP_CASE(7)}};
+#undef SWEEP_CASE
+  for (const SweepCase& S : Sweeps) {
+    Cpp::Declare(S.decl);
+    Cpp::DeclRef k = Cpp::GetNamed(S.cls);
+    ASSERT_TRUE(k);
+    Cpp::DeclRef v = Cpp::GetNamed(S.var, k);
+    ASSERT_TRUE(v);
+    EXPECT_TRUE(Cpp::GetVariableOffset(v));
+    Cpp::Declare(S.probe, /*silent=*/true); // expected parse failure
+    Cpp::Declare(S.use);
+  }
+  Cpp::Declare("int sweep_done = 1;");
+  EXPECT_TRUE(Cpp::GetNamed("sweep_done"));
+
+  // Template-specialization members take the odr-use route too — the
+  // Sema-built address-of needs no source spelling of the specialization,
+  // and the caller has already instantiated the definition.
+  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));
+  EXPECT_FALSE(Cpp::unwrap<Decl>(member)->hasAttr<clang::UsedAttr>());
+
+  // Anonymous-namespace members cannot be named from a fresh chunk of
+  // source; they bail out of the odr-use route the same way.
+  Cpp::Declare(R"(
+    namespace {
+      struct AnonNsStatic {
+        inline static int member = 9;
+      };
+    }
+  )");
+  Cpp::DeclRef anon_klass = Cpp::GetNamed("AnonNsStatic");
+  ASSERT_TRUE(anon_klass);
+  Cpp::DeclRef anon_member = Cpp::GetNamed("member", anon_klass);
+  ASSERT_TRUE(anon_member);
+  EXPECT_TRUE(Cpp::GetVariableOffset(anon_member));
+#ifndef CPPINTEROP_USE_CLING
+  EXPECT_TRUE(Cpp::unwrap<Decl>(anon_member)->hasAttr<clang::UsedAttr>());
+#endif
+
+  // A linkage-spec block is transparent for naming purposes and stays on
+  // the odr-use route.
+  Cpp::Declare(R"(
+    extern "C++" {
+      struct CxxLinkStatic {
+        inline static int member = 11;
+      };
+    }
+  )");
+  Cpp::DeclRef link_klass = Cpp::GetNamed("CxxLinkStatic");
+  ASSERT_TRUE(link_klass);
+  Cpp::DeclRef link_member = Cpp::GetNamed("member", link_klass);
+  ASSERT_TRUE(link_member);
+  EXPECT_TRUE(Cpp::GetVariableOffset(link_member));
+  EXPECT_FALSE(Cpp::unwrap<Decl>(link_member)->hasAttr<clang::UsedAttr>());
+}
+
 #define CODE                                                                   \
   class BaseA {                                                                \
   public:                                                                      \