GetAllCppNames: no names for a translation unit, and an intermittent crash in decls_begin()

Summary

Cpp::GetAllCppNames(Cpp::GetGlobalScope(), names) does not enumerate a translation unit. On a TU holding roughly 15,000 declarations it either returns an empty set or crashes the process inside clang::DeclContext::decls_begin(). Same call, same interpreter state: one run returned empty, the next faulted — so this looks like undefined behaviour rather than a missing null check.

Separately, and from code reading only, the TagDecl branch of the same function advances its iterator before the loop's end check.

Environment

CppInterOp main (321657d). Both functions quoted below are identical at that commit to the build I hit this on.
clang-repl backend, in-process, LLVM 21.1, Linux x86-64.
Driven through cppyy (visible in the stack frames), after the interpreter has included a large header closure.

1. Translation unit yields nothing, or crashes

# after the interpreter has parsed a large header closure
names = cppyy.gbl.std.set['std::string']()
cppyy.gbl.Cpp.GetAllCppNames(cppyy.gbl.Cpp.GetGlobalScope(), names)
print(names.size())      # 0, for a TU with ~15k declarations

Re-running the identical script, the same call faults instead:

#4  clang::DeclContext::getParentASTContext() const                  DeclBase.cpp
#5  clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const
#6  clang::DeclContext::decls_begin() const
#7  Cpp::GetAllCppNames(Cpp::ConstDeclRef, std::set<std::string>&)   lib/CppInterOp/CppInterOp.cpp
#10 Cpp::JitCall::Invoke(void*, Cpp::JitCall::ArgList, void*) const  include/CppInterOp/CppInterOpTypes.h
#11 WrapperCall(Cpp::FuncRef, unsigned long, void*, void*, void*)    clingwrapper/src/clingwrapper.cxx:1099
#12 CPyCppyy::VoidExecutor::Execute(...)                             src/Executors.cxx:435

The DeclRef is non-null, and GetGlobalScope() itself returned successfully.

One candidate cause: GetGlobalScope does not return the translation unit declaration — it returns a redeclaration-chain lookup from it (lib/CppInterOp/CppInterOp.cpp:995):

DeclRef GetGlobalScope() {
  return INTEROP_RETURN(
      getSema().getASTContext().getTranslationUnitDecl()->getFirstDecl());
}

GetAllCppNames then dyn_cast_or_null<TranslationUnitDecl>s that result and calls castToDeclContext(...)->decls_begin() on it (lib/CppInterOp/CppInterOp.cpp:5394). If getFirstDecl() does not yield the TranslationUnitDecl here, the DeclContext cast is operating on the wrong object, which would account for both the empty result and the fault in getParentASTContext(). I have not confirmed that under a debug build.

I should also flag the alternative: enumerating a clang-repl TU lexically may simply not be expected to work across partial translation units, in which case the empty result is by design and only the crash is a bug. Happy to be told that is the case.

2. TagDecl branch advances before checking end

Code reading only; I have not constructed a repro for this one.

if (const auto* TD = dyn_cast_or_null<TagDecl>(D)) {
    DC = clang::TagDecl::castToDeclContext(TD);
    decl = DC->decls_begin();
    decl++;                          // runs before the loop's decls_end() check
} else if (...) {
...
for (/* decl set above */; decl != DC->decls_end(); decl++) {

The decl++ skips the injected class name, but nothing establishes that the context is non-empty first. Where decls_begin() == decls_end(), this advances a past-the-end iterator.

Why it matters

I am trying to enumerate the declarations a header closure defines from the interpreter's own AST, so that a second parse of the same closure with libclang can be dropped. GetAllCppNames is the only bulk enumerator in the API, so either a working TU-level enumeration path, or confirmation that it is out of scope along with a pointer to the intended approach, would be very helpful.

@vgvassilev

@vgvassilev for viz!

I built main (321657d) against LLVM 21.1 and reduced both items to standalone unit tests. Root cause found for the first one, and one correction to my original report.

Correction

The translation-unit case does not crash in the minimal repro — it silently returns the wrong declarations. The crash I originally reported came from the empty-tag path (item 2), which does reproduce deterministically. Apologies for conflating them.

Root cause: `GetGlobalScope()` returns the first PTU's `TranslationUnitDecl`

TranslationUnitDecl is redeclarable, and clang-repl creates one per partial translation unit. GetGlobalScope() does:

getSema().getASTContext().getTranslationUnitDecl()->getFirstDecl()

which walks that chain back to the initial PTU — the one holding only the compiler's predefined builtins, created before any user code is parsed. Added to ScopeReflectionTest.cpp:

TYPED_TEST(CPPINTEROP_TEST_MODE, ScopeReflection_GetAllCppNamesTranslationUnit) {
  std::vector<Decl*> Decls;
  std::string code = R"(
    class A { int a; };
    namespace N { class B { int b; }; }
    void myfunc() {}
  )";

  GetAllTopLevelDecls(code, Decls);

  std::set<std::string> names;
  Cpp::GetAllCppNames(Cpp::GetGlobalScope(), names);

  EXPECT_FALSE(names.empty());
  EXPECT_TRUE(names.count("A"));
  EXPECT_TRUE(names.count("N"));
  EXPECT_TRUE(names.count("myfunc"));
}

Fails identically under both InProcessJIT and OutOfProcessJIT. EXPECT_FALSE(names.empty()) passes; the three name checks fail. Dumping the set:

TU name: __NSConstantString
TU name: __builtin_ms_va_list
TU name: __builtin_va_list
TU name: __int128_t
TU name: __uint128_t
TU name count: 5

Comparing against the current TU in the same test confirms it:

ASTContext& C = Interp->getCI()->getASTContext();
auto* TU = C.getTranslationUnitDecl();
std::set<std::string> current;
Cpp::GetAllCppNames(TU, current);

getFirstDecl()==current TU: 0
current TU name count: 3 hasA=1 hasN=1 hasMyfunc=1

So the enumeration logic is fine; it is being handed the wrong TranslationUnitDecl.

Suggested fix

GetGlobalScope()'s getFirstDecl() looks deliberate — it gives a stable handle for the global scope, which matters because the current TU changes as PTUs are added, and scope handles get compared for identity. So rather than changing GetGlobalScope, the narrower fix is in GetAllCppNames: when the decl is a TranslationUnitDecl, union the declarations across the redeclaration chain rather than only the one node.

} else if (const auto* TUD = dyn_cast_or_null<TranslationUnitDecl>(D)) {
  for (const auto* R : TUD->redecls())
    for (const auto* Child : cast<DeclContext>(R)->decls())
      if (const auto* ND = dyn_cast_or_null<NamedDecl>(Child))
        names.insert(ND->getNameAsString());
  return INTEROP_VOID_RETURN();
}

Happy to send that as a PR with the test if the approach looks right to you. If instead you would rather GetGlobalScope() track the most recent TU, say so and I will do it that way — I avoided it only because of the handle-stability concern.

Item 2 confirmed: crash on an empty tag

An enum with no enumerators is an empty DeclContext, so decls_begin() == decls_end() and the unconditional decl++ steps past the end. This segfaults the test process deterministically:

TYPED_TEST(CPPINTEROP_TEST_MODE, ScopeReflection_GetAllCppNamesEmptyTag) {
  std::vector<Decl*> Decls;
  std::string code = "enum class Empty {};";

  GetAllTopLevelDecls(code, Decls);

  std::set<std::string> names;
  Cpp::GetAllCppNames(Decls[0], names);

  EXPECT_TRUE(names.empty());
}

The existing ScopeReflection_GetAllCppNames test never hits this because its tag cases are all CXXRecordDecls, which always contain at least the injected class name, so decls_begin() is never equal to decls_end() there. Guarding the increment fixes it:

if (const auto* TD = dyn_cast_or_null<TagDecl>(D)) {
  DC = clang::TagDecl::castToDeclContext(TD);
  decl = DC->decls_begin();
  if (decl != DC->decls_end())
    decl++;  // skip the injected class name
}

Just to confirm -- you don't have any pch/pcms attached right? If that's the case then perhaps we can look at the PTU iterators in upstream clang-repl..

no pch/pcms right now... isn't the pcm question orthogonal?

in working to optimize our pathway right now, first step would be to create a new api that exposes more than the names out of CppInterOp (hence i stumbled on this while checking what apis there might already be and what content is accesible with minor changes), then I was going to consider pch options beyond that.

do you mean that it might be better to push the basic iterator functionality upstream to iterate over the PTUs for efficiency?

#4  clang::DeclContext::getParentASTContext() const                  DeclBase.cpp
#5  clang::DeclContext::LoadLexicalDeclsFromExternalStorage() const

This hinted in some PCH/PCH related issue but it might be used for other things too.

I am thinking about either broken iterators across PTUs upstream or we store something in a vector that relocates its storage or similar...

do you mean that it might be better to push the basic iterator functionality upstream to iterate over the PTUs for efficiency?

I guess so -- these api invariants need to work regardless of what we do. What you requested is valid and the clang api should not crash.

Following up on the pch/pcm question, and I need to withdraw the fix I proposed above — it would make things worse. Details, and a root cause reproduced in clang with no CppInterOp involved.

Withdrawing the suggested TU fix

I suggested unioning TUD->redecls() in GetAllCppNames's translation-unit branch. Don't take that: the redeclaration chain retains declarations that Undo has withdrawn, so unioning it would report stale names after any undo. That trades wrong-results-for-builtins for wrong-results-including-things-the-user-removed.

Root cause: Undo does not unlink the partial translation unit

IncrementalParser::CleanUpPTU clears the StoredDeclsMap and the C IdResolver chain, but leaves the TranslationUnitDecl and its decls() intact, as the source notes:

  // FIXME: We should de-allocate MostRecentTU
  for (Decl *D : MostRecentTU->decls()) {

So name lookup correctly stops finding a withdrawn declaration while a lexical walk of the chain still yields it. Reproduced as a clang unittest (clang/unittests/Interpreter/InterpreterTest.cpp), failing on current main:

TEST_F(InterpreterTest, UndoLeavesDeclsInTranslationUnitChain) {
  std::unique_ptr<Interpreter> Interp = createInterpreter();

  cantFail(Interp->Parse("struct Kept {};"));
  cantFail(Interp->Parse("struct Withdrawn {};"));
  cantFail(Interp->Undo());

  std::set<std::string> Names;
  TranslationUnitDecl *TU = Interp->getASTContext().getTranslationUnitDecl();
  for (auto *R : TU->redecls())
    for (auto *D : cast<DeclContext>(R)->decls())
      if (auto *ND = dyn_cast<NamedDecl>(D))
        Names.insert(ND->getNameAsString());

  EXPECT_TRUE(Names.count("Kept"));
  EXPECT_FALSE(Names.count("Withdrawn"));   // fails: Actual: true
}

This is why your instinct about the PTU iterators is the right one — the live PTU list is a sound enumeration source and the AST redeclaration chain is not. Interpreter::PTUs and InitPTUSize are both private with no accessor today, which is the only thing standing in the way.

On the crash, and the pch question

No pch or pcm attached, confirmed. And the FIXME above actually argues against my original dangling-pointer theory: if the TU is never deallocated, Undo by itself should not be producing freed memory. So I now think the intermittent crash in my first report has a different cause than the stale-decl problem, and I have not root-caused it yet. The crash banner reported three active interpreters in that process, so interpreter lifetime is my next line of investigation; I will report back rather than speculate further here.

Worth separating the three things, since only one of them is now clear-cut:

Empty-tag crash — unambiguous, fix is the guarded decl++, unaffected by any of the above. Happy to send that as a small PR whenever you want it.
TU enumeration returning only the initial unit's builtins — real, but the right fix depends on whether enumeration should go through the PTU list; your call on where that lives.
Intermittent crash under a large closure — not yet explained, investigating.

Is a guard inside CppInterOp acceptable to you as an interim measure for (2)? Cpp::Undo is CppInterOp's own entry point, so it could record withdrawn TranslationUnitDecls and skip them while enumerating. It is only sound while every undo goes through CppInterOp, so it is a workaround rather than a fix — I would mark it as such and reference this issue. Otherwise I am happy to wait for a clang-side accessor, or to help with the FIXME if you would rather fix it at the root.

I think we can do what we usually do: a) reproduce with clang-repl either with a test in clang/test or clang/unittest; b) provide an upstream fix; c) add a workaround in CppInterOp which will go away with the next release of clang.

Can you make sure that Cpp::Undo actually ran and broke the invariants? I still think there is something wrong with the iterators since the PTUs are redeclarables and when we iterate we have to collect all information from every PTU, just like we do for namespace partitions.

Your (a)/(b)/(c) works for us.

Can you make sure that Cpp::Undo actually ran and broke the invariants?

It did not. cppyy never calls Cpp::Undo — no references in clingwrapper or CPyCppyy — so no undo ran in the session that crashed. The Undo finding is separate: I induced it deliberately in a test, and it is not the cause of the original crash.

That crash root-caused elsewhere, and not to clang: a std::vector<DeclRef> out-parameter does not survive the Python boundary. Elements read back in Python fault on first dereference, while the same declarations are fine in C++ — resolving their names inside the library returns all of them, and a gtest iterating the identical vector passes. Single DeclRef values cross fine. Still narrowing whether that is cppyy-wide for any small struct or specific to DeclRef.

the PTUs are redeclarables and when we iterate we have to collect all information from every PTU

Agreed, and that is precisely the first bug: GetAllCppNames only ever sees the single node GetGlobalScope() hands it, and getFirstDecl() makes that the first PTU — the one holding just the predefined builtins. Walking TU->redecls() and collecting from each returns everything; a 200-PTU test doing that, and resolving every declaration's context and qualified name, passes.

Starting with the empty-tag decls_end() crash, which is independent of all of the above.

FYI, posted this issue to llvm: llvm/llvm-project#213230

Conclusions, now that the pieces separate cleanly.

The intermittent crash was not CppInterOp. ConstDeclRef has both ConstDeclRef(const void*) and ConstDeclRef(DeclRef), and CPyCppyy resolved between them by declaration order, so a handle passed from Python arrived as the proxy's address and was reinterpreted as a clang::Decl*. The cause is that CPPMethod::GetPriority's void* penalty was unreachable — it sits behind an IsBuiltin check that no pointer spelling passes, and it matches "void*" where arguments are spelled "const void *". Fixed in compiler-research/CPyCppyy#225, regression test in compiler-research/cppyy#239.

Please disregard my earlier "vector out-parameter" framing: single handles and returns were equally affected, and the declarations were never corrupt.

With that fix, GetQualifiedName on a scope returns std (was empty) and GetAllCppNames(GetGlobalScope()) returns the 5 predefined builtins from Python (was 0), which isolates the two items that are genuinely here:

Translation-unit enumeration reports only the initial partial unit, because GetGlobalScope() returns getTranslationUnitDecl()->getFirstDecl(). Clang-side neighbour: Undo leaves withdrawn units in the redeclaration chain — [clang-repl] Undo leaves the withdrawn PTU's declarations reachable through the TranslationUnitDecl chain llvm/llvm-project#213230, fix in [clang-repl] Unlink the withdrawn partial translation unit on Undo llvm/llvm-project#213235.
The empty-tag crash in the TagDecl branch, unchanged; the guard will be pushed up in another PR shortly.

Navigation Menu

Repository navigation

GetAllCppNames: no names for a translation unit, and an intermittent crash in decls_begin() #1078

Summary

Environment

1. Translation unit yields nothing, or crashes

2. TagDecl branch advances before checking end

Why it matters

Correction

Root cause: `GetGlobalScope()` returns the first PTU's `TranslationUnitDecl`

Suggested fix

Item 2 confirmed: crash on an empty tag

Withdrawing the suggested TU fix

Root cause: Undo does not unlink the partial translation unit

On the crash, and the pch question

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

GetAllCppNames: no names for a translation unit, and an intermittent crash in decls_begin() #1078

Description

Summary

Environment

1. Translation unit yields nothing, or crashes

2. TagDecl branch advances before checking end

Why it matters

Activity

conrade-ctc commented on Jul 30, 2026

conrade-ctc commented on Jul 30, 2026

Correction

Root cause: GetGlobalScope() returns the first PTU's TranslationUnitDecl

Suggested fix

Item 2 confirmed: crash on an empty tag

vgvassilev commented on Jul 30, 2026

conrade-ctc commented on Jul 30, 2026

vgvassilev commented on Jul 30, 2026

vgvassilev commented on Jul 30, 2026

conrade-ctc commented on Jul 30, 2026

Withdrawing the suggested TU fix

Root cause: Undo does not unlink the partial translation unit

On the crash, and the pch question

vgvassilev commented on Jul 31, 2026

conrade-ctc commented on Jul 31, 2026

conrade-ctc commented on Jul 31, 2026

conrade-ctc commented on Jul 31, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Root cause: `GetGlobalScope()` returns the first PTU's `TranslationUnitDecl`