diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -644,6 +644,9 @@ Expected FindFunctionTemplateSpecialization( FunctionDecl *FromFD); + + // Copy the inheritable attributes of the FunctionDecl `Old` into `New. + void CopyInheritedAttributes(FunctionDecl *Old, FunctionDecl *New); }; template @@ -3286,6 +3289,7 @@ auto *Recent = const_cast( 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()) { + if (!New->getAttr()) { + AnalyzerNoReturnAttr *NewAttr = OldAttr->clone(Importer.ToContext); + NewAttr->setInherited(true); + New->addAttr(NewAttr); + } + } + if (Old->isNoReturn() && !New->isNoReturn()) { + const FunctionType *NewType = cast(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, diff --git a/clang/test/Analysis/Inputs/ctu-attr-noreturn-other.c b/clang/test/Analysis/Inputs/ctu-attr-noreturn-other.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt b/clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/Inputs/ctu-attr-noreturn.externalDefMap.txt @@ -0,0 +1 @@ +c:@F@ff ctu-attr-noreturn-other.c.ast diff --git a/clang/test/Analysis/ctu-attr-noreturn.c b/clang/test/Analysis/ctu-attr-noreturn.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/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().match(FromTU, functionDecl(hasName("f"))); + ASSERT_TRUE(From2->hasAttrs()); + ASSERT_TRUE(From2->getAttr()); + auto *From1 = From2->getPreviousDecl(); + ASSERT_TRUE(From1->hasAttrs()); + ASSERT_TRUE(From1->getAttr()); + auto *From0 = From1->getPreviousDecl(); + ASSERT_FALSE(From0->hasAttrs()); + ASSERT_FALSE(From0->getAttr()); + + auto *Imported = Import(From2, Lang_C); + EXPECT_TRUE(Imported->hasAttrs()); + EXPECT_TRUE(Imported->getAttr()); + EXPECT_EQ(Imported->getAttrs().size(), 1u); + EXPECT_TRUE(Imported->getPreviousDecl()->hasAttrs()); + EXPECT_TRUE(Imported->getPreviousDecl()->getAttr()); +} + +TEST_P(ImportAttrs, InheritedNoreturnShouldBeImported) { + Decl *ToTU = getToTuDecl( + R"( + void f(); + )", + Lang_C); + auto *To = + FirstDeclMatcher().match(ToTU, functionDecl(hasName("f"))); + ASSERT_FALSE(cast(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().match(FromTU, functionDecl(hasName("f"))); + ASSERT_TRUE(cast(From2->getType())->getNoReturnAttr()); + auto *From1 = From2->getPreviousDecl(); + ASSERT_TRUE(cast(From1->getType())->getNoReturnAttr()); + auto *From0 = From1->getPreviousDecl(); + ASSERT_FALSE(cast(From0->getType())->getNoReturnAttr()); + + auto *Imported = Import(From2, Lang_C); + EXPECT_TRUE(cast(Imported->getType())->getNoReturnAttr()); + EXPECT_TRUE(cast(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().match( + FromTU, functionDecl(hasName("f"))); + + auto *To0 = + FirstDeclMatcher().match(ToTU, functionDecl(hasName("f"))); + ASSERT_TRUE(To0->hasAttrs()); + ASSERT_TRUE(To0->getAttr()); + auto *To1 = + LastDeclMatcher().match(ToTU, functionDecl(hasName("f"))); + ASSERT_TRUE(To1->hasAttrs()); + ASSERT_TRUE(To1->getAttr()); + ASSERT_TRUE(To1->getAttr()->isInherited()); + + // Should have an Inherited attribute. + auto *Imported = Import(From, Lang_C); + EXPECT_TRUE(Imported->hasAttrs()); + ASSERT_TRUE(Imported->getAttr()); + EXPECT_TRUE(Imported->getAttr()->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().match( + FromTU, functionDecl(hasName("f"))); + + auto *To0 = + FirstDeclMatcher().match(ToTU, functionDecl(hasName("f"))); + ASSERT_TRUE(cast(To0->getType())->getNoReturnAttr()); + auto *To1 = + LastDeclMatcher().match(ToTU, functionDecl(hasName("f"))); + ASSERT_TRUE(cast(To1->getType())->getNoReturnAttr()); + + // Should have an Inherited attribute as part of the type. + auto *Imported = Import(From, Lang_C); + EXPECT_TRUE(cast(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