llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/155375.diff


3 Files Affected:

- (modified) clang/lib/AST/ASTImporter.cpp (+12-1) 
- (added) clang/test/Analysis/ctu-import-type-decl-definition.c (+43) 
- (modified) clang/unittests/AST/ASTImporterTest.cpp (+2-1) 


``````````diff
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 15df9af889386..49bdc591dda92 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -10027,7 +10027,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);
     }
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/155375
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to