llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: guillem-bartrina-sonarsource <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/214009.diff 3 Files Affected: - (modified) clang/lib/AST/ASTImporter.cpp (+4-2) - (added) clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp (+66) - (modified) clang/unittests/AST/ASTImporterTest.cpp (+46-42) ``````````diff diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 3ad71a223903c..7415ac98cf5b1 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -4378,6 +4378,10 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { FromCXXMethod)) return std::move(Err); + // Make the function visible in its DeclContext's lookup table *before* + // importing its body. + addDeclToContexts(D, ToFunction); + if (D->doesThisDeclarationHaveABody()) { Error Err = ImportFunctionDeclBody(D, ToFunction); @@ -4399,8 +4403,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..331dd237a1d55 --- /dev/null +++ b/clang/test/Analysis/ctu/regression/self-referential-lambda-import.cpp @@ -0,0 +1,66 @@ +// 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. + +// 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..2702da1a0f443 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,48 @@ 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, which is reached again as a + // class template argument. + // + // 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 { + 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"( `````````` </details> https://github.com/llvm/llvm-project/pull/214009 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
