Author: guillem-bartrina-sonarsource Date: 2026-08-14T10:35:33+02:00 New Revision: a451685c2e4a9cf2a85c74f08b00db3e8c30dd9c
URL: https://github.com/llvm/llvm-project/commit/a451685c2e4a9cf2a85c74f08b00db3e8c30dd9c DIFF: https://github.com/llvm/llvm-project/commit/a451685c2e4a9cf2a85c74f08b00db3e8c30dd9c.diff LOG: [clang][ASTImporter] Remove type from ImportedTypes cache on Decl import failure (#214008) 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. If this type entry is not removed it can result in reference to the failed TagDecl (for example at structural-equivalence comparisons on a lambda closure that is shown in the added test). With the fix the type is removed from ImportedTypes which fixes crashes related to accessing an incompletely imported AST node. Added: clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp Modified: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 5be9ce780aec5..01c5385ff5bb5 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -9820,6 +9820,16 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { auto *ToD = CreatedToD; ImportedDecls.erase(Pos); + // Remove the imported type mapping as well. + // The imported type can point to a declaration that failed to import + // later. + 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 @@ -9873,6 +9883,14 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) { break; PrevFromDi = FromDi; setImportDeclError(FromDi, ErrOut); + + if (const auto *FromTDi = dyn_cast<TagDecl>(FromDi)) { + if (const Type *FromTyi = + getFromContext().getCanonicalTagType(FromTDi).getTypePtr()) { + ImportedTypes.erase(FromTyi); + } + } + //FIXME Should we remove these Decls from ImportedDecls? // Set the error for the mapped to Decl, which is in the "to" context. auto Ii = ImportedDecls.find(FromDi); diff --git a/clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp b/clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp new file mode 100644 index 0000000000000..eed5bd6362a3f --- /dev/null +++ b/clang/test/Analysis/ctu/invalid-lambda-type-equivalence.cpp @@ -0,0 +1,83 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// 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 + +// Check that importing 'api' and then 'isolate' does not cause crash. + +namespace ns { + +inline constexpr auto func = []<class T>(const T p) {}; + +} + +void import_api(int v); + +void entrypoint() { + import_api(0); // [email protected]:20 {{Division by zero}} +} + +//--- 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); +} + +//--- 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" +} diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 503f5da8af90f..f3b4c9ca7fa9a 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -6608,6 +6608,42 @@ TEST_P(ErrorHandlingTest, ErrorIsPropagatedFromMemberToClass) { EXPECT_FALSE(ImportedOK); } +// Check that the imported types, and not only the decls, are invalidated +// (removed from ImportedTypes) upon an import failure. It can happen, for +// instance with a member whose signature refers back to the enclosing class, +// that the type is successfully imported and pointing to the decl being +// imported, but that the decl import then fails further on. +// The decl mapping is correctly invalidated, but if the connected type is not +// invalidated as well, the half-built decl (which unavoidably remains +// in the 'To' AST) could be accessed through the type during later operations, +// like structural equivalence checks. +TEST_P(ErrorHandlingTest, ImportedTypeMappingIsInvalidatedOnFailure) { + 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); + // Class X fails to import + EXPECT_FALSE(ImportedX); + + ASTImporter *Importer = findFromTU(FromX)->Importer.get(); + const Type *FromXTy = + FromTU->getASTContext().getCanonicalTagType(FromX)->getTypePtr(); + ASSERT_TRUE(FromXTy); + Expected<const Type *> ToTyOrErr = Importer->Import(FromXTy); + // And its type should fail to import as well + EXPECT_TRUE(ToTyOrErr.errorIsA<clang::ASTImportError>()); + 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. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
