[clang] [clang][ASTImporter] Invalidate ImportedTypes cache on Decl import failure (PR #214008)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 5 06:19:38 PDT 2026
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 <guillem.bartrina at sonarsource.com>
Date: Wed, 29 Jul 2026 15:22:39 +0200
Subject: [PATCH 1/3] [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); // expected-warning at isolate.cpp: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 <guillem.bartrina at sonarsource.com>
Date: Wed, 5 Aug 2026 15:11:23 +0200
Subject: [PATCH 2/3] 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));
>From 38015cd6fe3241b7a8f71287ce2954d6764aa0f4 Mon Sep 17 00:00:00 2001
From: guillem-bartrina-sonarsource <guillem.bartrina at sonarsource.com>
Date: Wed, 5 Aug 2026 15:19:15 +0200
Subject: [PATCH 3/3] clean up lit test
---
.../regression/lambda-import-corruption.cpp | 35 +++++--------------
1 file changed, 8 insertions(+), 27 deletions(-)
diff --git a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp
index 0ecc0a6c03662..ce9a1e355bf1f 100644
--- a/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp
+++ b/clang/test/Analysis/ctu/regression/lambda-import-corruption.cpp
@@ -2,19 +2,16 @@
// 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
+// Pathological case: a lambda's closure is created as a Decl before its
+// members are imported. If a member unrelated to the eventual failure
+// (e.g. the implicit copy constructor) imports successfully first, that
+// success permanently caches the closure's type as "imported", before the
+// member that actually fails is even reached. That cache was 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
+// subsequently needs the same type (e.g. the DeclRefExpr inside
+// `decltype(func(...))` on `rudolf`, below) silently got the half-built
// closure back instead of a clean failure, producing an inconsistent node
-// that crashes downstream.
+// that crashed.
// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t/api.cpp.ast %t/api.cpp
@@ -50,14 +47,6 @@ void entrypoint() {
import_api(0); // expected-warning at isolate.cpp:20 {{Division by zero}}
}
-void trigger1() {
- trigger_api();
-}
-
-void trigger2() {
- trigger_isolate();
-}
-
//--- api.cpp
template <class> int declval();
@@ -82,10 +71,6 @@ void import_api(int v) {
import_isolate(v);
}
-void trigger_api() {
- ns::rudolf<void>(0); // fails to import
-}
-
//--- isolate.cpp
template <class> int declval();
@@ -108,7 +93,3 @@ 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
-}
More information about the cfe-commits
mailing list