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,90 @@ 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.
+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();
+
+  // 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.
+  // FIXME: Remove this whole 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 "Check whether clang's direct-emission API landed; if so, replace \
+EmitVariableViaOdrUse's synthesized dummy with it."
+#endif
+  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 triggers emission; under
+  // clang-repl it is still a stub (TODO destructor in Compatibility.h) and
+  // only annotates the synthesizing region.
+  compat::SynthesizingCodeRAII RAII(&I);
+  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 +2891,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 +5081,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,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.
+  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 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:                                                                      \