https://github.com/balazske created https://github.com/llvm/llvm-project/pull/156056
When a type is imported with `ASTImporter`, the "original declaration" of the type is imported. In some cases this is not the definition (of the class). Before the fix the definition was only imported if there was an other reference to it in the AST to import. This is not always the case (like in the added test case), if not the definition was missing in the "To" AST which can cause the assertion later. From 7da69b65a6cb614023c0cd22a01b1693ce622401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 26 Aug 2025 10:20:42 +0200 Subject: [PATCH 1/3] [clang] Fix of a crash 'Cannot get layout of forward declarations!' during CTU static analysis When a type is imported with `ASTImporter`, the "original declaration" of the type is imported. In some cases this is not the definition (of the class). Before the fix the definition was only imported if there was an other reference to it in the AST to import. This is not always the case (like in the added test case), if not the definition was missing in the "To" AST which can cause the assertion later. --- clang/lib/AST/ASTImporter.cpp | 13 +++++- .../ctu-import-type-decl-definition.c | 43 +++++++++++++++++++ clang/unittests/AST/ASTImporterTest.cpp | 3 +- 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/ctu-import-type-decl-definition.c diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 6299efaf6bbfc..b4c9cf699fd9c 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1740,10 +1740,21 @@ ExpectedType ASTNodeImporter::VisitDeducedTemplateSpecializationType( } ExpectedType ASTNodeImporter::VisitTagType(const TagType *T) { - Expected<TagDecl *> ToDeclOrErr = import(T->getOriginalDecl()); + TagDecl *DeclForType = T->getOriginalDecl(); + Expected<TagDecl *> ToDeclOrErr = import(DeclForType); if (!ToDeclOrErr) return ToDeclOrErr.takeError(); + // If there is a definition of the 'OriginalDecl', it should be imported to + // have all information for the type in the "To" AST. (In rare cases no other + // reference may exist to the definition and it would not be imported + // otherwise.) + if (TagDecl *DefDecl = DeclForType->getDefinition()) { + Expected<TagDecl *> ToDefDeclOrErr = import(DefDecl); + if (!ToDefDeclOrErr) + return ToDefDeclOrErr.takeError(); + } + if (T->isCanonicalUnqualified()) return Importer.getToContext().getCanonicalTagType(*ToDeclOrErr); diff --git a/clang/test/Analysis/ctu-import-type-decl-definition.c b/clang/test/Analysis/ctu-import-type-decl-definition.c new file mode 100644 index 0000000000000..4610ff8268555 --- /dev/null +++ b/clang/test/Analysis/ctu-import-type-decl-definition.c @@ -0,0 +1,43 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -emit-pch -o %t/import.ast %t/import.c + +// RUN: %clang_cc1 -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.c + +//--- main.c + +// expected-no-diagnostics + +typedef struct X_s X_t; +unsigned long f_import(struct X_s *xPtr); + +static void freeWriteFileResources(struct X_s *xPtr) { + f_import(xPtr); +} + +//--- import.c + +typedef struct Y_s Y_t; + +struct Y_s { +}; + +struct X_s { + Y_t y; +}; + +unsigned long f_import(struct X_s *xPtr) { + if (xPtr != 0) { + } + return 0; +} + +//--- externalDefMap.txt +13:c:@F@f_import import.ast diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 5badbd7d65e48..138d0a68ad1a4 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -10025,7 +10025,8 @@ struct ImportTemplateParmDeclDefaultValue EXPECT_EQ(ToD->getPreviousDecl(), ToDInherited); } else { EXPECT_EQ(FromD, FromDInherited->getPreviousDecl()); - EXPECT_EQ(ToD, ToDInherited->getPreviousDecl()); + // The order becomes reversed by the import process. + EXPECT_EQ(ToD->getPreviousDecl(), ToDInherited); } } From 32d39c2bf0936791dd08fdaf54976b200b683d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 26 Aug 2025 16:57:40 +0200 Subject: [PATCH 2/3] only import definition if 'isUsed' is true --- clang/lib/AST/ASTImporter.cpp | 12 ++++++------ .../test/Analysis/ctu-import-type-decl-definition.c | 8 ++++---- clang/unittests/AST/ASTImporterTest.cpp | 3 +-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index b4c9cf699fd9c..d2ea60896094f 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1745,12 +1745,12 @@ ExpectedType ASTNodeImporter::VisitTagType(const TagType *T) { if (!ToDeclOrErr) return ToDeclOrErr.takeError(); - // If there is a definition of the 'OriginalDecl', it should be imported to - // have all information for the type in the "To" AST. (In rare cases no other - // reference may exist to the definition and it would not be imported - // otherwise.) - if (TagDecl *DefDecl = DeclForType->getDefinition()) { - Expected<TagDecl *> ToDefDeclOrErr = import(DefDecl); + if (DeclForType->isUsed()) { + // If there is a definition of the 'OriginalDecl', it should be imported to + // have all information for the type in the "To" AST. (In some cases no + // other reference may exist to the definition decl and it would not be + // imported otherwise.) + Expected<TagDecl *> ToDefDeclOrErr = import(DeclForType->getDefinition()); if (!ToDefDeclOrErr) return ToDefDeclOrErr.takeError(); } diff --git a/clang/test/Analysis/ctu-import-type-decl-definition.c b/clang/test/Analysis/ctu-import-type-decl-definition.c index 4610ff8268555..9855c9d8f5729 100644 --- a/clang/test/Analysis/ctu-import-type-decl-definition.c +++ b/clang/test/Analysis/ctu-import-type-decl-definition.c @@ -2,7 +2,10 @@ // RUN: mkdir -p %t // RUN: split-file %s %t -// RUN: %clang_cc1 -emit-pch -o %t/import.ast %t/import.c +// RUN: %clang_cc1 -emit-pch -o %t/import.c.ast %t/import.c + +// RUN: %clang_extdef_map -- -x c %t/import.c >> %t/externalDefMap.txt +// RUN: sed -i 's/$/.ast/' %t/externalDefMap.txt // RUN: %clang_cc1 -analyze \ // RUN: -analyzer-checker=core \ @@ -38,6 +41,3 @@ unsigned long f_import(struct X_s *xPtr) { } return 0; } - -//--- externalDefMap.txt -13:c:@F@f_import import.ast diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 138d0a68ad1a4..5badbd7d65e48 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -10025,8 +10025,7 @@ struct ImportTemplateParmDeclDefaultValue EXPECT_EQ(ToD->getPreviousDecl(), ToDInherited); } else { EXPECT_EQ(FromD, FromDInherited->getPreviousDecl()); - // The order becomes reversed by the import process. - EXPECT_EQ(ToD->getPreviousDecl(), ToDInherited); + EXPECT_EQ(ToD, ToDInherited->getPreviousDecl()); } } From 4482f726d4913d5297af6eff4cc19d9c871d7840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Fri, 29 Aug 2025 18:18:24 +0200 Subject: [PATCH 3/3] fix 'sed' command problem --- clang/test/Analysis/ctu-import-type-decl-definition.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/ctu-import-type-decl-definition.c b/clang/test/Analysis/ctu-import-type-decl-definition.c index 9855c9d8f5729..6d95fd5686cb4 100644 --- a/clang/test/Analysis/ctu-import-type-decl-definition.c +++ b/clang/test/Analysis/ctu-import-type-decl-definition.c @@ -5,7 +5,7 @@ // RUN: %clang_cc1 -emit-pch -o %t/import.c.ast %t/import.c // RUN: %clang_extdef_map -- -x c %t/import.c >> %t/externalDefMap.txt -// RUN: sed -i 's/$/.ast/' %t/externalDefMap.txt +// RUN: sed -i'' 's/$/.ast/' %t/externalDefMap.txt // RUN: %clang_cc1 -analyze \ // RUN: -analyzer-checker=core \ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits