martong created this revision.
martong added reviewers: a_sidorin, balazske, dkrupp, vbridgers.
Herald added subscribers: cfe-commits, teemperor, gamesh411, Szelethus,
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
This way some CTU false positives are eliminated.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D68634
Files:
clang/lib/AST/ASTImporter.cpp
clang/test/Analysis/Inputs/ctu-attr-noreturn-other.c
clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt
clang/test/Analysis/ctu-attr-noreturn.c
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5783,6 +5783,134 @@
2u);
}
+struct ImportAttrs : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportAttrs, InheritedAnalyzerNoreturnShouldBeImported) {
+ getToTuDecl(
+ R"(
+ void f();
+ )",
+ Lang_C);
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(); /* From0 */
+ void f() __attribute__((analyzer_noreturn)); /* From1 */
+ void f(); /* From2 */
+ )",
+ Lang_C, "input0.c");
+
+ auto *From2 =
+ LastDeclMatcher<FunctionDecl>().match(FromTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(From2->hasAttrs());
+ ASSERT_TRUE(From2->getAttr<AnalyzerNoReturnAttr>());
+ auto *From1 = From2->getPreviousDecl();
+ ASSERT_TRUE(From1->hasAttrs());
+ ASSERT_TRUE(From1->getAttr<AnalyzerNoReturnAttr>());
+ auto *From0 = From1->getPreviousDecl();
+ ASSERT_FALSE(From0->hasAttrs());
+ ASSERT_FALSE(From0->getAttr<AnalyzerNoReturnAttr>());
+
+ auto *Imported = Import(From2, Lang_C);
+ EXPECT_TRUE(Imported->hasAttrs());
+ EXPECT_TRUE(Imported->getAttr<AnalyzerNoReturnAttr>());
+ EXPECT_EQ(Imported->getAttrs().size(), 1u);
+ EXPECT_TRUE(Imported->getPreviousDecl()->hasAttrs());
+ EXPECT_TRUE(Imported->getPreviousDecl()->getAttr<AnalyzerNoReturnAttr>());
+}
+
+TEST_P(ImportAttrs, InheritedNoreturnShouldBeImported) {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ void f();
+ )",
+ Lang_C);
+ auto *To =
+ FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ ASSERT_FALSE(cast<FunctionType>(To->getType())->getNoReturnAttr());
+
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f(); /* From0 */
+ void f() __attribute__((noreturn)); /* From1 */
+ void f(); /* From2 */
+ )",
+ Lang_C, "input0.cc");
+
+ auto *From2 =
+ LastDeclMatcher<FunctionDecl>().match(FromTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(cast<FunctionType>(From2->getType())->getNoReturnAttr());
+ auto *From1 = From2->getPreviousDecl();
+ ASSERT_TRUE(cast<FunctionType>(From1->getType())->getNoReturnAttr());
+ auto *From0 = From1->getPreviousDecl();
+ ASSERT_FALSE(cast<FunctionType>(From0->getType())->getNoReturnAttr());
+
+ auto *Imported = Import(From2, Lang_C);
+ EXPECT_TRUE(cast<FunctionType>(Imported->getType())->getNoReturnAttr());
+ EXPECT_TRUE(cast<FunctionType>(Imported->getPreviousDecl()->getType())
+ ->getNoReturnAttr());
+}
+
+TEST_P(ImportAttrs, ImportShouldInheritExistingInheritableAttr) {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ void f() __attribute__((analyzer_noreturn));
+ void f();
+ )",
+ Lang_C);
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f();
+ )",
+ Lang_C, "input0.c");
+
+ auto *From = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+
+ auto *To0 =
+ FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(To0->hasAttrs());
+ ASSERT_TRUE(To0->getAttr<AnalyzerNoReturnAttr>());
+ auto *To1 =
+ LastDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(To1->hasAttrs());
+ ASSERT_TRUE(To1->getAttr<AnalyzerNoReturnAttr>());
+ ASSERT_TRUE(To1->getAttr<AnalyzerNoReturnAttr>()->isInherited());
+
+ // Should have an Inherited attribute.
+ auto *Imported = Import(From, Lang_C);
+ EXPECT_TRUE(Imported->hasAttrs());
+ ASSERT_TRUE(Imported->getAttr<AnalyzerNoReturnAttr>());
+ EXPECT_TRUE(Imported->getAttr<AnalyzerNoReturnAttr>()->isInherited());
+}
+
+TEST_P(ImportAttrs, ImportShouldInheritExistingNoreturnTypeSpec) {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ void f() __attribute__((noreturn));
+ void f();
+ )",
+ Lang_C);
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f();
+ )",
+ Lang_C, "input0.c");
+
+ auto *From = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+
+ auto *To0 =
+ FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(cast<FunctionType>(To0->getType())->getNoReturnAttr());
+ auto *To1 =
+ LastDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ ASSERT_TRUE(cast<FunctionType>(To1->getType())->getNoReturnAttr());
+
+ // Should have an Inherited attribute as part of the type.
+ auto *Imported = Import(From, Lang_C);
+ EXPECT_TRUE(cast<FunctionType>(Imported->getType())->getNoReturnAttr());
+}
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions, );
@@ -5841,5 +5969,8 @@
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ConflictingDeclsWithLiberalStrategy,
DefaultTestValuesForRunOptions, );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportAttrs,
+ DefaultTestValuesForRunOptions, );
+
} // end namespace ast_matchers
} // end namespace clang
Index: clang/test/Analysis/ctu-attr-noreturn.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/ctu-attr-noreturn.c
@@ -0,0 +1,34 @@
+// Test that imported function decls inherit attributes from existing functions
+// in the "to" ctx and this way false positives are eliminated.
+//
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN: -emit-pch -o %t/ctudir2/ctu-attr-noreturn-other.c.ast \
+// RUN: %S/Inputs/ctu-attr-noreturn-other.c
+// RUN: cp %S/Inputs/ctu-attr-noreturn.externalDefMap.txt \
+// RUN: %t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze \
+// RUN: -analyzer-checker=core,debug.ExprInspection \
+// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN: -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN: -verify %s
+
+// expected-no-diagnostics
+
+__attribute__((noreturn)) void fatal(void);
+
+typedef struct {
+ int a;
+ int b;
+} Data;
+
+void ff(Data* data);
+
+int caller(void) {
+ Data d;
+ ff(&d);
+ int i = (int)d.b;// If the import of inherited attrs fail then a bug is
+ // reported here: "Assigned value is garbage or undefined".
+ return i;
+}
Index: clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt
===================================================================
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt
@@ -0,0 +1 @@
+c:@F@ff ctu-attr-noreturn-other.c.ast
Index: clang/test/Analysis/Inputs/ctu-attr-noreturn-other.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-attr-noreturn-other.c
@@ -0,0 +1,16 @@
+int cond(void);
+void fatal(void); // Should inherit noreturn when imported.
+
+typedef struct {
+ int a;
+ int b;
+} Data;
+
+void ff(Data* data) {
+ if (cond()) {
+ fatal();
+ return;
+ }
+ data->a = 0;
+ data->b = 0;
+}
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -644,6 +644,9 @@
Expected<FunctionDecl *> FindFunctionTemplateSpecialization(
FunctionDecl *FromFD);
+
+ // Copy the inheritable attributes of the FunctionDecl `Old` into `New.
+ void CopyInheritedAttributes(FunctionDecl *Old, FunctionDecl *New);
};
template <typename InContainerTy>
@@ -3286,6 +3289,7 @@
auto *Recent = const_cast<FunctionDecl *>(
FoundByLookup->getMostRecentDecl());
ToFunction->setPreviousDecl(Recent);
+ CopyInheritedAttributes(Recent, ToFunction);
// FIXME Probably we should merge exception specifications. E.g. In the
// "To" context the existing function may have exception specification with
// noexcept-unevaluated, while the newly imported function may have an
@@ -7834,6 +7838,28 @@
return ImportErrors;
}
+// This implementation is inspired by Sema::mergeDeclAttributes.
+void ASTNodeImporter::CopyInheritedAttributes(FunctionDecl *Old,
+ FunctionDecl *New) {
+ assert(&Old->getASTContext() == &New->getASTContext() &&
+ "Should copy attrs only from decl in the same context!");
+ if (AnalyzerNoReturnAttr *OldAttr = Old->getAttr<AnalyzerNoReturnAttr>()) {
+ if (!New->getAttr<AnalyzerNoReturnAttr>()) {
+ AnalyzerNoReturnAttr *NewAttr = OldAttr->clone(Importer.ToContext);
+ NewAttr->setInherited(true);
+ New->addAttr(NewAttr);
+ }
+ }
+ if (Old->isNoReturn() && !New->isNoReturn()) {
+ const FunctionType *NewType = cast<FunctionType>(New->getType());
+ FunctionType::ExtInfo NewTypeInfo = NewType->getExtInfo();
+ NewType = Importer.ToContext.adjustFunctionType(
+ NewType, NewTypeInfo.withNoReturn(true));
+ auto Quals = New->getType().getQualifiers().getAsOpaqueValue();
+ New->setType(QualType(NewType, Quals));
+ }
+}
+
ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
bool MinimalImport,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
