https://github.com/guillem-bartrina-sonarsource updated https://github.com/llvm/llvm-project/pull/214008
>From 315175d6c1f863b90e6983f238f71c32e8a67432 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 29 Jul 2026 15:22:39 +0200 Subject: [PATCH 1/2] [ASTImporter] Invalidate ImportedTypes cache on Decl import failure A TagDecl's type can get cached as successfully imported in ASTImporter::ImportedTypes before the Decl's own import fails, if a member referencing the type (e.g. an implicit copy constructor) is imported first. That stale entry was never invalidated, so later references to the same type -- directly, or via structural-equivalence comparisons on lambda closures -- could silently resolve to a half-built Decl and crash instead of failing cleanly. Add a unit test and a CTU regression test reproducing the crash. --- clang/lib/AST/ASTImporter.cpp | 6 + .../regression/lambda-import-corruption.cpp | 114 ++++++++++++++++++ clang/unittests/AST/ASTImporterTest.cpp | 36 ++++++ 3 files changed, 156 insertions(+) create mode 100644 clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 3ad71a223903c..0b85636a06598 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10038,6 +10038,12 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { auto *ToD = CreatedToD; ImportedDecls.erase(Pos); + // Also scrub the imported type mapping, if applicable. Import(Type*) can + // cache a type mapping to a declaration that ultimately fails. + if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) + if (const Type *FromTy = getFromContext().getCanonicalTagType(FromTD).getTypePtr()) + ImportedTypes.erase(FromTy); + // ImportedDecls and ImportedFromDecls are not symmetric. It may happen // (e.g. with namespaces) that several decls from the 'from' context are // mapped to the same decl in the 'to' context. If we removed entries diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp new file mode 100644 index 0000000000000..0ecc0a6c03662 --- /dev/null +++ b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp @@ -0,0 +1,114 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// Pathological case: a lambda's closure (the anonymous class that +// implements its operator()) is created as a Decl before its members are +// imported. If a member unrelated to the eventual failure -- e.g. the +// implicit copy constructor, whose parameter type is `const ClosureType&` +// -- imports successfully first, that success permanently caches the +// closure's type as "imported" in ASTImporter::ImportedTypes, before the +// member that actually fails (here, operator(), due to an unsupported +// trailing requires-clause) is even reached. That cache is never +// invalidated when the closure's own import later fails, so anything that +// subsequently needs the same type (the DeclRefExpr inside +// `decltype(func(...))` on `rudolf`, below) silently gets the half-built +// closure back instead of a clean failure, producing an inconsistent node +// that crashes downstream. + + +// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/api.cpp.ast %t/api.cpp +// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/isolate.cpp.ast %t/isolate.cpp + +// RUN: %clang_extdef_map %t/api.cpp -- -std=c++20 > %t/externalDefMap.tmp.txt +// RUN: %clang_extdef_map %t/isolate.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|' -e 's| .*isolate\.cpp| isolate.cpp.ast|' \ +// RUN: %t/externalDefMap.tmp.txt > %t/externalDefMap.txt + +// RUN: %clang_analyze_cc1 -std=c++20 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config ctu-dir=%t \ +// RUN: -verify %t/main.cpp + +//--- main.cpp + +namespace ns { + +inline constexpr auto func = []<class T>(const T p) {}; + +} + +void import_api(int v); +void trigger_api(); +void trigger_isolate(); + +void entrypoint() { + import_api(0); // [email protected]:20 {{Division by zero}} +} + +void trigger1() { + trigger_api(); +} + +void trigger2() { + trigger_isolate(); +} + +//--- api.cpp + +template <class> int declval(); + +namespace ns { +int import_ns; + +// This closure fails to import: its call operator's trailing +// requires-clause has no importer support. +inline constexpr auto func = []<class T>(const T p) requires requires { 0; } {}; + +// The DeclRefExpr for `func` in this decltype independently re-resolves +// the closure's type after `func` itself was merged away above. +template <class K> decltype(func(declval<K>())) rudolf(int v); + +} // namespace ns + +void import_isolate(int v); + +void import_api(int v) { + (void)ns::import_ns; + import_isolate(v); +} + +void trigger_api() { + ns::rudolf<void>(0); // fails to import +} + +//--- isolate.cpp + +template <class> int declval(); + +namespace ns { +int import_ns; + +constexpr auto func = []<class T>(const T p) requires requires { 0; } {}; + +// Structural equivalence of the return type accesses the closure's +// definition through its type -- an access that assumes the closure is +// intact. +template <class K> decltype(func(declval<K>())) rudolf(int v) { // no-crash + (void)(42 / v); +} + +} // namespace ns + +void import_isolate(int v) { + (void)ns::import_ns; + (void)(42 / v); // raises "Division by zero" +} + +void trigger_isolate() { + ns::rudolf<void>(0); // fails to import +} diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 503f5da8af90f..038b86edb40fd 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6608,6 +6608,42 @@ TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { EXPECT_FALSE(ImportedOK); } +// A member whose signature refers back to the enclosing class (e.g. a +// copy constructor's `const Self&` parameter) can succeed and cache the +// class's *type* before a later, failing member causes the class's own +// Decl import to fail as a whole. Check that this doesn't leave a stale, +// "successfully imported" entry for the class's type behind: any later, +// independent request to import that type must also fail, not silently +// hand back the half-built class. +TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { + TranslationUnitDecl *FromTU = getTuDecl(std::string(R"( + class X { + void ok(const X &) {} // Succeeds; imports X's own type + // as a side effect, before X's + // own import is known to fail. + void bad() { )") + ErroneousStmt + R"( } // Fails to import. + }; + )", + Lang_CXX03); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + + CXXRecordDecl *ImportedX = Import(FromX, Lang_CXX03); + EXPECT_FALSE(ImportedX); // X itself fails to import. + + // The bug: without the fix, a later, independent request to import X's + // type silently succeeds, returning the half-built X as if nothing had + // gone wrong, because ASTImporter::ImportedTypes was never scrubbed + // when X's own Decl import failed. + ASTImporter *Importer = findFromTU(FromX)->Importer.get(); + const Type *FromXTy = FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); + ASSERT_TRUE(FromXTy); + Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); + EXPECT_FALSE(static_cast<bool>(ToTyOrErr)); + if (!ToTyOrErr) + llvm::consumeError(ToTyOrErr.takeError()); +} + // Check that an error propagates to the dependent AST nodes. // In the below code it means that an error in X should propagate to A. // And even to F since the containing A is erroneous. >From 58467bc1704e632749ab49029fbd55f84e0c1a22 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 15:11:23 +0200 Subject: [PATCH 2/2] format --- clang/lib/AST/ASTImporter.cpp | 3 ++- clang/unittests/AST/ASTImporterTest.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 0b85636a06598..ffaca93b5d2ac 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -10041,7 +10041,8 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { // Also scrub the imported type mapping, if applicable. Import(Type*) can // cache a type mapping to a declaration that ultimately fails. if (const auto *FromTD = dyn_cast<TagDecl>(FromD)) - if (const Type *FromTy = getFromContext().getCanonicalTagType(FromTD).getTypePtr()) + if (const Type *FromTy = + getFromContext().getCanonicalTagType(FromTD).getTypePtr()) ImportedTypes.erase(FromTy); // ImportedDecls and ImportedFromDecls are not symmetric. It may happen diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 038b86edb40fd..d5fd9ec1d17d2 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6636,7 +6636,8 @@ TEST_P(ErrorHandlingTest, ImportedTypeCacheIsInvalidatedOnFailure) { // gone wrong, because ASTImporter::ImportedTypes was never scrubbed // when X's own Decl import failed. ASTImporter *Importer = findFromTU(FromX)->Importer.get(); - const Type *FromXTy = FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); + const Type *FromXTy = + FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); ASSERT_TRUE(FromXTy); Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); EXPECT_FALSE(static_cast<bool>(ToTyOrErr)); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
