diff --git a/clang/include/clang/AST/ODRDiagsEmitter.h b/clang/include/clang/AST/ODRDiagsEmitter.h --- a/clang/include/clang/AST/ODRDiagsEmitter.h +++ b/clang/include/clang/AST/ODRDiagsEmitter.h @@ -116,6 +116,17 @@ const VarDecl *FirstVD, const VarDecl *SecondVD) const; + /// Diagnose ODR mismatch in attributes between two declarations. + /// + /// Returns true if found a mismatch and diagnosed it. If a diagnosed entity + /// \p FirstDecl is nested, like a field in a struct, \p FirstContainer should + /// point to the parent. Otherwise, for top-level decls \p FirstContainer must + /// be null. + bool diagnoseSubMismatchAttr(const NamedDecl *FirstContainer, + StringRef FirstModule, StringRef SecondModule, + const NamedDecl *FirstDecl, + const NamedDecl *SecondDecl) const; + private: DiagnosticsEngine &Diags; const ASTContext &Context; diff --git a/clang/include/clang/AST/ODRHash.h b/clang/include/clang/AST/ODRHash.h --- a/clang/include/clang/AST/ODRHash.h +++ b/clang/include/clang/AST/ODRHash.h @@ -25,6 +25,7 @@ namespace clang { +class Attr; class Decl; class IdentifierInfo; class NestedNameSpecifier; @@ -86,11 +87,18 @@ void AddTemplateArgument(TemplateArgument TA); void AddTemplateParameterList(const TemplateParameterList *TPL); + // Process attributes attached to a decl that is being hashed. + void AddAttrs(const Decl *D); + void AddAttr(const Attr *A); + // Save booleans until the end to lower the size of data to process. void AddBoolean(bool value); static bool isDeclToBeProcessed(const Decl* D, const DeclContext *Parent); + static void getHashableAttrs(const Decl *D, + SmallVectorImpl &HashableAttrs); + private: void AddDeclarationNameImpl(DeclarationName Name); }; diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -848,6 +848,22 @@ "%ordinal2 element %3 has different initializer|" "}1">; +def err_module_odr_violation_attribute : Error< + "%q0 has different definitions in different modules; first difference is " + "%select{definition in module '%2'|defined here}1 found" + "%select{" + "%select{| %5 with}4 %select{no attribute|attribute %7}6|" + "%select{| %5 with}4 attribute '%6'" + "}3">; +def note_module_odr_violation_attribute : Note< + "but in '%0' found" + "%select{" + "%select{| %3 with}2 %select{no attribute|attribute %5}4|" + "%select{| %3 with}2 different attribute argument '%4'" + "}1">; + +def note_attribute_specified_here : Note<"attribute specified here">; + def err_module_odr_violation_mismatch_decl_unknown : Error< "%q0 %select{with definition in module '%2'|defined here}1 has different " "definitions in different modules; first difference is this " diff --git a/clang/lib/AST/ODRDiagsEmitter.cpp b/clang/lib/AST/ODRDiagsEmitter.cpp --- a/clang/lib/AST/ODRDiagsEmitter.cpp +++ b/clang/lib/AST/ODRDiagsEmitter.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "clang/AST/ODRDiagsEmitter.h" +#include "clang/AST/Attr.h" #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/ODRHash.h" @@ -148,6 +149,10 @@ } } + if (diagnoseSubMismatchAttr(FirstRecord, FirstModule, SecondModule, + FirstField, SecondField)) + return true; + return false; } @@ -188,6 +193,10 @@ DiagNote(TypedefType) << IsTypeAlias << SecondName << SecondType; return true; } + + if (diagnoseSubMismatchAttr(FirstRecord, FirstModule, SecondModule, FirstTD, + SecondTD)) + return true; return false; } @@ -263,6 +272,92 @@ DiagNote(VarConstexpr) << SecondName << SecondIsConstexpr; return true; } + + if (diagnoseSubMismatchAttr(FirstRecord, FirstModule, SecondModule, FirstVD, + SecondVD)) + return true; + return false; +} + +bool ODRDiagsEmitter::diagnoseSubMismatchAttr( + const NamedDecl *FirstContainer, StringRef FirstModule, + StringRef SecondModule, const NamedDecl *FirstDecl, + const NamedDecl *SecondDecl) const { + llvm::SmallVector FirstAttrs; + llvm::SmallVector SecondAttrs; + ODRHash::getHashableAttrs(FirstDecl, FirstAttrs); + ODRHash::getHashableAttrs(SecondDecl, SecondAttrs); + if (FirstAttrs.empty() && SecondAttrs.empty()) + return false; + + enum ODRAttributeDifference { + AttributeKind, + AttributeArguments, + }; + + auto ComputeAttrODRHash = [](const Attr *A) { + ODRHash Hasher; + Hasher.AddAttr(A); + return Hasher.CalculateHash(); + }; + auto FullAttributeText = [](const Attr *A, const ASTContext &Ctx) { + std::string FullText; + llvm::raw_string_ostream OutputStream(FullText); + A->printPretty(OutputStream, Ctx.getPrintingPolicy()); + return OutputStream.str(); + }; + auto DiagError = [FirstContainer, FirstDecl, FirstModule, + this](ODRAttributeDifference DiffType) { + return Diag(FirstDecl->getLocation(), + diag::err_module_odr_violation_attribute) + << (FirstContainer ? FirstContainer : FirstDecl) + << FirstModule.empty() << FirstModule << DiffType + << (FirstContainer != nullptr) << FirstDecl; + }; + auto DiagNote = [FirstContainer, SecondDecl, SecondModule, + this](ODRAttributeDifference DiffType) { + return Diag(SecondDecl->getLocation(), + diag::note_module_odr_violation_attribute) + << SecondModule << DiffType << (FirstContainer != nullptr) + << SecondDecl; + }; + auto DiagNoteAttrLoc = [this](const Attr *A) { + if (A) + Diag(A->getLocation(), diag::note_attribute_specified_here) + << A->getRange(); + }; + + const Attr *LHS = nullptr; + const Attr *RHS = nullptr; + unsigned NumFirstAttrs = FirstAttrs.size(); + unsigned NumSecondAttrs = SecondAttrs.size(); + unsigned MaxNumAttrs = std::max(NumFirstAttrs, NumSecondAttrs); + for (unsigned I = 0; I < MaxNumAttrs; ++I) { + if (I < NumFirstAttrs) + LHS = FirstAttrs[I]; + if (I < NumSecondAttrs) + RHS = SecondAttrs[I]; + + if (!LHS || !RHS || LHS->getKind() != RHS->getKind()) { + DiagError(AttributeKind) + << (LHS != nullptr) << LHS << FirstDecl->getSourceRange(); + DiagNoteAttrLoc(LHS); + DiagNote(AttributeKind) + << (RHS != nullptr) << RHS << SecondDecl->getSourceRange(); + DiagNoteAttrLoc(RHS); + return true; + } + if (ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)) { + DiagError(AttributeArguments) + << FullAttributeText(LHS, Context) << LHS->getRange(); + DiagNoteAttrLoc(LHS); + DiagNote(AttributeArguments) + << FullAttributeText(RHS, Context) << RHS->getRange(); + DiagNoteAttrLoc(RHS); + return true; + } + } + return false; } @@ -568,6 +663,10 @@ } } + if (diagnoseSubMismatchAttr(/*FirstContainer=*/nullptr, FirstModule, + SecondModule, FirstRecord, SecondRecord)) + return true; + auto PopulateHashes = [](DeclHashes &Hashes, const RecordDecl *Record, const DeclContext *DC) { for (const Decl *D : Record->decls()) { @@ -944,6 +1043,10 @@ } } + if (diagnoseSubMismatchAttr(FirstRecord, FirstModule, SecondModule, + FirstMethod, SecondMethod)) + return true; + // Compute the hash of the method as if it has no body. auto ComputeCXXMethodODRHash = [](const CXXMethodDecl *D) { ODRHash Hasher; @@ -1391,6 +1494,10 @@ "Undiagnosed parameter difference."); } + if (diagnoseSubMismatchAttr(/*FirstContainer=*/nullptr, FirstModule, + SecondModule, FirstFunction, SecondFunction)) + return true; + // If no error has been generated before now, assume the problem is in // the body and generate a message. DiagError(FirstFunction->getLocation(), FirstFunction->getSourceRange(), @@ -1472,6 +1579,10 @@ } } + if (diagnoseSubMismatchAttr(/*FirstContainer=*/nullptr, FirstModule, + SecondModule, FirstEnum, SecondEnum)) + return true; + // Compare enum constants. using DeclHashes = llvm::SmallVector, 4>; diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp --- a/clang/lib/AST/ODRHash.cpp +++ b/clang/lib/AST/ODRHash.cpp @@ -14,6 +14,7 @@ #include "clang/AST/ODRHash.h" +#include "clang/AST/Attr.h" #include "clang/AST/DeclVisitor.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/StmtVisitor.h" @@ -280,6 +281,11 @@ Inherited::Visit(D); } + void VisitDecl(const Decl *D) { + Hash.AddAttrs(D); + Inherited::VisitDecl(D); + } + void VisitNamedDecl(const NamedDecl *D) { Hash.AddDeclarationName(D->getDeclName()); Inherited::VisitNamedDecl(D); @@ -464,6 +470,60 @@ } } +void ODRHash::getHashableAttrs(const Decl *D, + SmallVectorImpl &HashableAttrs) { + HashableAttrs.clear(); + if (!D->hasAttrs()) + return; + + llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs), + [](const Attr *A) { return !A->isImplicit(); }); +} + +void ODRHash::AddAttrs(const Decl *D) { + llvm::SmallVector Attrs; + getHashableAttrs(D, Attrs); + ID.AddInteger(Attrs.size()); + for (const Attr *A : Attrs) + AddAttr(A); +} + +void ODRHash::AddAttr(const Attr *A) { + ID.AddInteger(A->getKind()); + + // FIXME: This should be auto-generated as part of Attr.td + switch (A->getKind()) { + case attr::Aligned: { + auto *M = cast(A); + ID.AddBoolean(M->isAlignmentExpr()); + if (M->isAlignmentExpr()) { + Expr *AlignmentExpr = M->getAlignmentExpr(); + ID.AddBoolean(AlignmentExpr); + if (AlignmentExpr) + AddStmt(AlignmentExpr); + } else { + ID.AddString(M->getAlignmentType()->getType().getAsString()); + } + break; + } + case attr::AlignValue: { + auto *M = cast(A); + Expr *AlignmentExpr = M->getAlignment(); + ID.AddBoolean(AlignmentExpr); + if (AlignmentExpr) + AddStmt(AlignmentExpr); + break; + } + case attr::EnumExtensibility: { + auto *M = cast(A); + ID.AddInteger(M->getExtensibility()); + break; + } + default: + break; + } +} + void ODRHash::AddSubDecl(const Decl *D) { assert(D && "Expecting non-null pointer."); @@ -650,6 +710,8 @@ for (const TemplateArgument &TA : List.asArray()) AddTemplateArgument(TA); } + + AddAttrs(D); } namespace { diff --git a/clang/test/Modules/odr_hash.cpp b/clang/test/Modules/odr_hash.cpp --- a/clang/test/Modules/odr_hash.cpp +++ b/clang/test/Modules/odr_hash.cpp @@ -3637,6 +3637,228 @@ #endif } // namespace DependentSizedExtVector +namespace Attributes { +#if defined(FIRST) +struct __attribute__((packed)) PackingPresence { + char x; + long y; +}; +#elif defined(SECOND) +struct PackingPresence { + char x; + long y; +}; +#else +PackingPresence testPackingAttributePresence; +// expected-error@first.h:* {{'Types::Attributes::PackingPresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'packed'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found no attribute}} +#endif + +#if defined(FIRST) +struct AlignmentPresence { + char x; + alignas(8) char y; +}; +struct DifferentAlignmentKeywords { + char x; + char __attribute__((aligned(8))) y; +}; +struct DifferentAlignmentValues { + char x; + alignas(4) char y; +}; +struct AlignmentWithTypedefType { + char x; + alignas(float) char y; +}; +#elif defined(SECOND) +struct AlignmentPresence { + char x; + char y; +}; +struct DifferentAlignmentKeywords { + char x; + alignas(8) char y; +}; +struct DifferentAlignmentValues { + char x; + alignas(8) char y; +}; +typedef float MyFloat; +struct AlignmentWithTypedefType { + char x; + alignas(MyFloat) char y; +}; +#else +AlignmentPresence testAlignmentAttributePresence; +// expected-error@first.h:* {{'Types::Attributes::AlignmentPresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute 'alignas'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'y' with no attribute}} +DifferentAlignmentKeywords testDifferentAlignmentKeywords; // expected no errors as different keywords have the same meaning +DifferentAlignmentValues testDifferentAlignments; +// expected-error@first.h:* {{'Types::Attributes::DifferentAlignmentValues' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute ' alignas(4)'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'y' with different attribute argument ' alignas(8)'}} +// expected-note@second.h:* {{attribute specified here}} +AlignmentWithTypedefType testAligningAsTheSameTypeButDifferentName; +// expected-error@first.h:* {{'Types::Attributes::AlignmentWithTypedefType' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute ' alignas(alignof(float))'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'y' with different attribute argument ' alignas(alignof(MyFloat))'}} +// expected-note@second.h:* {{attribute specified here}} +#endif + +#if defined(FIRST) +struct AttributeOnMethod { + int test(int x, int y); +}; +struct AttributeOnStaticField { + __attribute__((unused)) static int x; +}; +#elif defined(SECOND) +struct AttributeOnMethod { + __attribute__((optnone)) int test(int x, int y); +}; +struct AttributeOnStaticField { + static int x; +}; +#else +AttributeOnMethod testAttributeDifferenceOnMethods; +// expected-error@first.h:* {{'Types::Attributes::AttributeOnMethod' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'test' with no attribute}} +// expected-note@second.h:* {{but in 'SecondModule' found 'test' with attribute 'optnone'}} +// expected-note@second.h:* {{attribute specified here}} +AttributeOnStaticField testAttributeDifferenceOnStaticFields; +// expected-error@first.h:* {{'Types::Attributes::AttributeOnStaticField' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'x' with attribute 'unused'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'x' with no attribute}} +#endif + +#if defined(FIRST) +struct __attribute__((packed)) __attribute__((aligned(8))) AttributeOrder { + char x; + long y; +}; +#elif defined(SECOND) +struct __attribute__((aligned(8))) __attribute__((packed)) AttributeOrder { + char x; + long y; +}; +#else +AttributeOrder testOrderOfAttributes; +// expected-error@first.h:* {{'Types::Attributes::AttributeOrder' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'packed'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found attribute 'aligned'}} +// expected-note@second.h:* {{attribute specified here}} +#endif + +#if defined(FIRST) +__attribute__((naked)) void testFunctionAttr(void) {} +#elif defined(SECOND) +void testFunctionAttr(void) {} +#else +auto testFunctionAttrVar = testFunctionAttr; +// expected-error@second.h:* {{'Types::Attributes::testFunctionAttr' has different definitions in different modules; first difference is definition in module 'SecondModule' found no attribute}} +// expected-note@first.h:* {{but in 'FirstModule' found attribute 'naked'}} +// expected-note@first.h:* {{attribute specified here}} +#endif + +#if defined(FIRST) +enum __attribute__((enum_extensibility(open))) EnumExtensibilityPresence { + kEnumExtensibilityPresence0, +}; +enum __attribute__((enum_extensibility(open))) EnumExtensibilityValues { + kEnumExtensibilityValues0, +}; +#elif defined(SECOND) +enum EnumExtensibilityPresence { + kEnumExtensibilityPresence0, +}; +enum __attribute__((enum_extensibility(closed))) EnumExtensibilityValues { + kEnumExtensibilityValues0, +}; +#else +EnumExtensibilityPresence testEnumAttrPresence; +// expected-error@first.h:* {{'Types::Attributes::EnumExtensibilityPresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'enum_extensibility'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found no attribute}} +EnumExtensibilityValues testEnumAttrValue; +// expected-error@first.h:* {{'Types::Attributes::EnumExtensibilityValues' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute ' __attribute__((enum_extensibility("open")))'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found different attribute argument ' __attribute__((enum_extensibility("closed")))'}} +// expected-note@second.h:* {{attribute specified here}} +#endif + +#if defined(FIRST) +struct TypedefAttributePresence { + typedef double *AlignedDoublePtr __attribute__((align_value(64))); +}; +struct TypedefDifferentAttributeValue { + typedef double *AlignedDoublePtr __attribute__((align_value(64))); +}; +#elif defined(SECOND) +struct TypedefAttributePresence { + typedef double *AlignedDoublePtr; +}; +struct TypedefDifferentAttributeValue { + typedef double *AlignedDoublePtr __attribute__((align_value(32))); +}; +#else +TypedefAttributePresence testTypedefAttrPresence; +// expected-error@first.h:* {{'Types::Attributes::TypedefAttributePresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'AlignedDoublePtr' with attribute 'align_value'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' with no attribute}} +TypedefDifferentAttributeValue testDifferentArgumentsInTypedefAttribute; +// expected-error@first.h:* {{'Types::Attributes::TypedefDifferentAttributeValue' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'AlignedDoublePtr' with attribute ' __attribute__((align_value(64)))'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' with different attribute argument ' __attribute__((align_value(32)))'}} +// expected-note@second.h:* {{attribute specified here}} +#endif + +#if defined(FIRST) +#define PACKED __attribute__((packed)) +struct PACKED AttributeInMacro { + char x; + long y; +}; + +#pragma clang attribute push (__attribute__((abi_tag("a"))), apply_to = any(record(unless(is_union)))) +struct AttributeInPragma { char x; }; +#pragma clang attribute pop + +struct __attribute__((packed)) AttributeInherited; +struct AttributeInherited { + char x; + long y; +}; +#elif defined(SECOND) +struct AttributeInMacro { + char x; + long y; +}; + +struct AttributeInPragma { char x; }; + +struct AttributeInherited { + char x; + long y; +}; +#else +AttributeInMacro testDiagnosticWhenAttributeIsInMacro; +// expected-error@first.h:* {{'Types::Attributes::AttributeInMacro' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'packed'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-text-not-enforceable@first.h:* {{expanded from macro 'PACKED'}} +// expected-note@second.h:* {{but in 'SecondModule' found no attribute}} +AttributeInPragma testDiagnosticShowsPragmaLocation; +// expected-error@first.h:* {{'Types::Attributes::AttributeInPragma' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'abi_tag'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found no attribute}} +AttributeInherited testShowLocationOnFwdDecl; +// expected-error@first.h:* {{'Types::Attributes::AttributeInherited' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'packed'}} +// expected-note@first.h:* {{attribute specified here}} +// expected-note@second.h:* {{but in 'SecondModule' found no attribute}} +#endif +} // namespace Attributes + namespace InjectedClassName { #if defined(FIRST) struct Invalid {