[clang] Fix crash 'Cannot get layout of forward declarations' during CTU static analysis #156056
+55
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…uring 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.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) ChangesWhen a type is imported with Full diff: https://github.com/llvm/llvm-project/pull/156056.diff 2 Files Affected:
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 6299efaf6bbfc..d2ea60896094f 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 (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();
+ }
+
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..6d95fd5686cb4
--- /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.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 \
+// 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;
+}
|
|
I'll take one of the bots offline to test your change today and let you know how it goes. |
|
This is still failing on my MacOS bot: |
dyung
reviewed
Aug 29, 2025
| // 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 |
There was a problem hiding this comment.
From this page it might not be able to make a portable command line that replaces the file in-place with no backup that works on linux and MacOS.
If you are okay with creating an extra file in the test directory, one of the suggestions on that page seems to work:
// RUN: sed -i='' -e 's/$/.ast/' %t/externalDefMap.txt
This creates a file externalDefMap.txt=, but does seem to work on linux and MacOS.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.