https://github.com/guillem-bartrina-sonarsource updated https://github.com/llvm/llvm-project/pull/214009
>From fbcfc6f383e1126e62357fe0f992e0b6cbad806d Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Thu, 30 Jul 2026 22:37:15 +0200 Subject: [PATCH 1/3] [ASTImporter] Link FunctionDecl into its DeclContext before importing its body FunctionDecl was the only Decl kind that deferred addDeclToContexts() until after its body was imported; every other kind with dependent content -- RecordDecl, EnumDecl, BindingDecl, etc. -- registers itself immediately after creation, before its members/definition are imported. This left a function invisible to name lookup in its own DeclContext while its body was still being built, so a lambda whose call operator body re-entered the same import (e.g. because it referenced a self-referencing global) could build a second, independent LambdaExpr for the same closure and trip the "Missing lambda call operator!" assertion in getLambdaCallOperatorHelper(), since name lookup could not find the call operator until its own body import had completed. Add a unit test and a CTU regression test reproducing the crash. --- clang/lib/AST/ASTImporter.cpp | 10 +- .../self-referential-lambda-import.cpp | 68 ++++++++++++++ clang/unittests/AST/ASTImporterTest.cpp | 93 ++++++++++--------- 3 files changed, 127 insertions(+), 44 deletions(-) create mode 100644 clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 3ad71a223903c..03ebf02fdcf5b 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -4378,6 +4378,14 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { FromCXXMethod)) return std::move(Err); + // Make the function visible in its DeclContext's lookup table *before* + // importing its body. Every other Decl kind with dependent content + // (RecordDecl, EnumDecl, BindingDecl, ...) registers itself via + // addDeclToContexts() immediately after creation, before importing its + // members/definition. FunctionDecl used to be the exception, deferring + // this until after ImportFunctionDeclBody() below. + addDeclToContexts(D, ToFunction); + if (D->doesThisDeclarationHaveABody()) { Error Err = ImportFunctionDeclBody(D, ToFunction); @@ -4399,8 +4407,6 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // FIXME: Other bits to merge? - addDeclToContexts(D, ToFunction); - // Import the rest of the chain. I.e. import all subsequent declarations. for (++RedeclIt; RedeclIt != Redecls.end(); ++RedeclIt) { ExpectedDecl ToRedeclOrErr = import(*RedeclIt); diff --git a/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp b/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp new file mode 100644 index 0000000000000..d7053c6e84bc4 --- /dev/null +++ b/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp @@ -0,0 +1,68 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// Pathological case: a global variable of class type (`selfRef`) is +// initialized with a lambda that captures a reference to the variable +// itself, and the class's constructor template closes back over the +// lambda's own closure type through a namespace-qualified variable +// template argument. Importing `selfRef`'s type forces the whole class -- +// including the constructor template's body -- to be imported before +// `selfRef` itself is registered in the ASTImporter's Decl map. Resolving +// the "ns::" qualifier inside that constructor pulls in the closure type +// again and force-imports it as a complete RecordDecl, reaching the +// lambda's call operator body, which references `selfRef` again. Since +// `selfRef` isn't mapped yet, this re-enters its import and builds a +// second, independent LambdaExpr for the same closure while the call +// operator's own body import is still in flight. That second +// LambdaExpr::Create() looks up the call operator via name lookup, but the +// operator isn't visible in its DeclContext until its body import +// completes, tripping the "Missing lambda call operator!" assertion. + +// RUN: %clang_cc1 -std=c++20 -fpch-instantiate-templates -emit-pch -o %t/api.cpp.ast %t/api.cpp + +// RUN: %clang_extdef_map %t/api.cpp -- -std=c++20 > %t/externalDefMap.tmp.txt +// On windows, absolute paths generated by extdef_map are not recognized, +// so CSA prepends the workdir path to them. Force relative paths to work +// around this issue. +// RUN: sed -e 's| .*api\.cpp| api.cpp.ast|' \ +// RUN: %t/externalDefMap.tmp.txt > %t/externalDefMap.txt + +// RUN: %clang_cc1 -std=c++20 -analyze \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config display-ctu-progress=true \ +// RUN: -analyzer-config ctu-dir=%t \ +// RUN: -verify %t/main.cpp + +//--- main.cpp + +// expected-no-diagnostics + +void trigger(); + +void entrypoint() { + trigger(); +} + +//--- api.cpp + +namespace ns { + +template <typename> constexpr bool always_false = false; + +} + +class SelfReferencingWrapper { +public: + // Naming "ns::always_false<Callback>" here, with Callback bound to the + // lambda's closure type below, force-imports that closure as a complete + // RecordDecl while resolving the "ns::" qualifier. + template <class Callback> SelfReferencingWrapper(Callback callback) { + ns::always_false<Callback>; + } + // The lambda's body references `selfRef`, re-entering import of this + // same VarDecl before it has been registered as imported. +} selfRef { []{ (void)selfRef; } }; + +void trigger() { selfRef; } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 503f5da8af90f..21a10f1623347 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6504,10 +6504,9 @@ TEST_P(ErrorHandlingTest, ErrorHappensBeforeCreatingANewNode) { EXPECT_EQ(OptErr->Error, ASTImportError::NameConflict); } -// Check a case when a new AST node is created but not linked to the AST before +// Check a case when a new AST node is created and linked to the AST before // encountering the error. -TEST_P(ErrorHandlingTest, - ErrorHappensAfterCreatingTheNodeButBeforeLinkingThatToTheAST) { +TEST_P(ErrorHandlingTest, ErrorHappensAfterNodeIsCreatedAndLinked) { TranslationUnitDecl *FromTU = getTuDecl( std::string("void foo() { ") + ErroneousStmt + " }", Lang_CXX03); auto *FromFoo = FirstDeclMatcher<FunctionDecl>().match( @@ -6517,10 +6516,10 @@ TEST_P(ErrorHandlingTest, EXPECT_FALSE(ImportedFoo); TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); - // Created, but not linked. + // Created and linked. EXPECT_EQ( DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("foo"))), - 0u); + 1u); ASTImporter *Importer = findFromTU(FromFoo)->Importer.get(); std::optional<ASTImportError> OptErr = @@ -6529,43 +6528,6 @@ TEST_P(ErrorHandlingTest, EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct); } -// Check a case when a new AST node is created and linked to the AST before -// encountering the error. The error is set for the counterpart of the nodes in -// the "from" context. -TEST_P(ErrorHandlingTest, ErrorHappensAfterNodeIsCreatedAndLinked) { - TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( - void f(); - void f() { )") + ErroneousStmt + R"( } - )", - Lang_CXX03); - auto *FromProto = FirstDeclMatcher<FunctionDecl>().match( - FromTU, functionDecl(hasName("f"))); - auto *FromDef = - LastDeclMatcher<FunctionDecl>().match(FromTU, functionDecl(hasName("f"))); - FunctionDecl *ImportedProto = Import(FromProto, Lang_CXX03); - EXPECT_FALSE(ImportedProto); // Could not import. - // However, we created two nodes in the AST. 1) the fwd decl 2) the - // definition. The definition is not added to its DC, but the fwd decl is - // there. - TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl(); - EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))), - 1u); - // Match the fwd decl. - auto *ToProto = - FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))); - EXPECT_TRUE(ToProto); - // An error is set to the counterpart in the "from" context both for the fwd - // decl and the definition. - ASTImporter *Importer = findFromTU(FromProto)->Importer.get(); - std::optional<ASTImportError> OptErr = - Importer->getImportDeclErrorIfAny(FromProto); - ASSERT_TRUE(OptErr); - EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct); - OptErr = Importer->getImportDeclErrorIfAny(FromDef); - ASSERT_TRUE(OptErr); - EXPECT_EQ(OptErr->Error, ASTImportError::UnsupportedConstruct); -} - // An error should be set for a class if we cannot import one member. TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( @@ -9310,6 +9272,53 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportRecursiveFieldInitializer1) { // EXPECT_TRUE(ToA->field_begin()->getInClassInitializer()); } +TEST_P(ASTImporterOptionSpecificTestBase, + ImportSelfReferencingGlobalWithLambdaInTemplateArg) { + // A global variable of class type is initialized with a lambda that + // captures a reference to the variable itself. The class's constructor + // template names a variable template specialization whose (dependent) + // argument is the lambda's own closure type, reached again as a class + // template argument. + // + // Importing the variable's declared type forces importing the whole + // class definition (including the constructor template's body) before + // the VarDecl itself is registered in the ASTImporter's Decl map (see + // VisitVarDecl(), which imports D->getType() before calling + // GetImportedOrCreateDecl()). Resolving the "ns::" qualifier inside that + // constructor pulls in the whole namespace, including the concrete + // specialization that closes back over the lambda's closure type; that + // closure gets force-imported (ImportDeclContext(ForceImport=true) for a + // complete RecordDecl), which reaches the lambda's call operator body, + // which references the same self-referencing global again -- and since + // it isn't mapped yet, this re-enters VisitVarDecl() for the same source + // Decl and rebuilds a second, independent LambdaExpr for the same + // closure while its call operator is still being imported into it. + // LambdaExpr::Create() -> CXXRecordDecl::getLambdaCallOperator() then + // performs a name lookup that fails, because the call operator isn't + // visible in its DeclContext until after its own body import completes + // (see addDeclToContexts() in VisitFunctionDecl()) -- tripping the + // "Missing lambda call operator!" assertion in + // getLambdaCallOperatorHelper() (DeclCXX.cpp). + const char *Code = + R"( + namespace ns { + template <typename> constexpr bool always_false = false; + } + class SelfReferencingWrapper { + public: + template <class Callback> SelfReferencingWrapper(Callback callback) { + ns::always_false<Callback>; + } + } selfRef { []{ (void)selfRef; } }; + void trigger() { selfRef; } + )"; + Decl *FromTU = getTuDecl(Code, Lang_CXX20); + auto *FromFunc = FirstDeclMatcher<FunctionDecl>().match( + FromTU, functionDecl(hasName("trigger"))); + auto *ToFunc = Import(FromFunc, Lang_CXX20); + EXPECT_TRUE(ToFunc); +} + TEST_P(ASTImporterOptionSpecificTestBase, isNewDecl) { Decl *FromTU = getTuDecl( R"( >From 436ac6b3c526d61f8aee1b13aad505a361a2b8cf Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:43:20 +0200 Subject: [PATCH 2/3] clean up --- clang/lib/AST/ASTImporter.cpp | 6 +-- .../self-referential-lambda-import.cpp | 4 +- clang/unittests/AST/ASTImporterTest.cpp | 37 ++++++++----------- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 03ebf02fdcf5b..7415ac98cf5b1 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -4379,11 +4379,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { return std::move(Err); // Make the function visible in its DeclContext's lookup table *before* - // importing its body. Every other Decl kind with dependent content - // (RecordDecl, EnumDecl, BindingDecl, ...) registers itself via - // addDeclToContexts() immediately after creation, before importing its - // members/definition. FunctionDecl used to be the exception, deferring - // this until after ImportFunctionDeclBody() below. + // importing its body. addDeclToContexts(D, ToFunction); if (D->doesThisDeclarationHaveABody()) { diff --git a/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp b/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp index d7053c6e84bc4..331dd237a1d55 100644 --- a/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp +++ b/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp @@ -15,9 +15,7 @@ // `selfRef` isn't mapped yet, this re-enters its import and builds a // second, independent LambdaExpr for the same closure while the call // operator's own body import is still in flight. That second -// LambdaExpr::Create() looks up the call operator via name lookup, but the -// operator isn't visible in its DeclContext until its body import -// completes, tripping the "Missing lambda call operator!" assertion. +// LambdaExpr::Create() looks up the call operator via name lookup. // RUN: %clang_cc1 -std=c++20 -fpch-instantiate-templates -emit-pch -o %t/api.cpp.ast %t/api.cpp diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 21a10f1623347..d7a8d7f6f8d2b 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -9277,28 +9277,23 @@ TEST_P(ASTImporterOptionSpecificTestBase, // A global variable of class type is initialized with a lambda that // captures a reference to the variable itself. The class's constructor // template names a variable template specialization whose (dependent) - // argument is the lambda's own closure type, reached again as a class - // template argument. + // argument is the lambda's own closure type, which is reached again as a + // class template argument. // - // Importing the variable's declared type forces importing the whole - // class definition (including the constructor template's body) before - // the VarDecl itself is registered in the ASTImporter's Decl map (see - // VisitVarDecl(), which imports D->getType() before calling - // GetImportedOrCreateDecl()). Resolving the "ns::" qualifier inside that - // constructor pulls in the whole namespace, including the concrete - // specialization that closes back over the lambda's closure type; that - // closure gets force-imported (ImportDeclContext(ForceImport=true) for a - // complete RecordDecl), which reaches the lambda's call operator body, - // which references the same self-referencing global again -- and since - // it isn't mapped yet, this re-enters VisitVarDecl() for the same source - // Decl and rebuilds a second, independent LambdaExpr for the same - // closure while its call operator is still being imported into it. - // LambdaExpr::Create() -> CXXRecordDecl::getLambdaCallOperator() then - // performs a name lookup that fails, because the call operator isn't - // visible in its DeclContext until after its own body import completes - // (see addDeclToContexts() in VisitFunctionDecl()) -- tripping the - // "Missing lambda call operator!" assertion in - // getLambdaCallOperatorHelper() (DeclCXX.cpp). + // Importing the variable's declared type forces the import of the entire + // class definition before the VarDecl itself is registered. + // Resolving the "ns::" qualifier inside the constructor pulls in + // the entire namespace, including the concrete specialization that + // refers back to the lambda's closure type. When that closure is + // force-imported, the process reaches the lambda's call operator body, + // which references the same self-referencing global variable again. + // Because the variable isn't mapped yet, the importer revisits it and + // rebuilds a second, independent LambdaExpr for the same closure while + // its call operator is still being imported into it. + // + // Finally, the sequence LambdaExpr::Create() -> + // CXXRecordDecl::getLambdaCallOperator() performs a name lookup on the + // lambda closure that is currently being constructed. const char *Code = R"( namespace ns { >From 9fe216a736d5aae4b6de205e6557a17a47b95517 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:55:11 +0200 Subject: [PATCH 3/3] format --- clang/unittests/AST/ASTImporterTest.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index d7a8d7f6f8d2b..2702da1a0f443 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -9286,13 +9286,13 @@ TEST_P(ASTImporterOptionSpecificTestBase, // the entire namespace, including the concrete specialization that // refers back to the lambda's closure type. When that closure is // force-imported, the process reaches the lambda's call operator body, - // which references the same self-referencing global variable again. - // Because the variable isn't mapped yet, the importer revisits it and - // rebuilds a second, independent LambdaExpr for the same closure while + // which references the same self-referencing global variable again. + // Because the variable isn't mapped yet, the importer revisits it and + // rebuilds a second, independent LambdaExpr for the same closure while // its call operator is still being imported into it. - // - // Finally, the sequence LambdaExpr::Create() -> - // CXXRecordDecl::getLambdaCallOperator() performs a name lookup on the + // + // Finally, the sequence LambdaExpr::Create() -> + // CXXRecordDecl::getLambdaCallOperator() performs a name lookup on the // lambda closure that is currently being constructed. const char *Code = R"( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
