diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -65,6 +65,10 @@ fixes `Issue 53044 `_. - Allow `-Wno-gnu` to silence GNU extension diagnostics for pointer arithmetic diagnostics. Fixes `Issue 54444 `_. +- Allow multiple ``#pragma weak`` directives to name the same undeclared (if an + alias, target) identifier instead of only processing one such ``#pragma weak`` + per identifier. + Fixes `Issue 28985 `_. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1072,10 +1072,20 @@ } }; - /// WeakUndeclaredIdentifiers - Identifiers contained in - /// \#pragma weak before declared. rare. may alias another - /// identifier, declared or undeclared - llvm::MapVector WeakUndeclaredIdentifiers; + /// WeakUndeclaredIdentifiers - Identifiers contained in \#pragma weak before + /// declared. Rare. May alias another identifier, declared or undeclared. + /// + /// For aliases, the target identifier is used as a key for eventual + /// processing when the target is declared. For the single-identifier form, + /// the sole identifier is used as the key. Each entry is a `SetVector` + /// (ordered by parse order) of aliases (identified by the alias name) in case + /// of multiple aliases to the same undeclared identifier. + llvm::MapVector< + IdentifierInfo *, + llvm::SetVector, + llvm::SmallDenseSet< + WeakInfo, 2u, WeakInfo::DenseMapInfoByAliasNameOnly>>> + WeakUndeclaredIdentifiers; /// ExtnameUndeclaredIdentifiers - Identifiers contained in /// \#pragma redefine_extname before declared. Used in Solaris system headers @@ -10177,7 +10187,7 @@ NamedDecl *DeclClonePragmaWeak(NamedDecl *ND, const IdentifierInfo *II, SourceLocation Loc); - void DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, WeakInfo &W); + void DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, const WeakInfo &W); /// ActOnPragmaWeakID - Called on well formed \#pragma weak ident. void ActOnPragmaWeakID(IdentifierInfo* WeakName, diff --git a/clang/include/clang/Sema/Weak.h b/clang/include/clang/Sema/Weak.h --- a/clang/include/clang/Sema/Weak.h +++ b/clang/include/clang/Sema/Weak.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_SEMA_WEAK_H #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/DenseMapInfo.h" namespace clang { @@ -22,22 +23,44 @@ /// Captures information about a \#pragma weak directive. class WeakInfo { - IdentifierInfo *alias; // alias (optional) - SourceLocation loc; // for diagnostics - bool used; // identifier later declared? + IdentifierInfo *alias = nullptr; // alias (optional) + SourceLocation loc; // for diagnostics public: - WeakInfo() - : alias(nullptr), loc(SourceLocation()), used(false) {} + WeakInfo() = default; WeakInfo(IdentifierInfo *Alias, SourceLocation Loc) - : alias(Alias), loc(Loc), used(false) {} - inline IdentifierInfo * getAlias() const { return alias; } + : alias(Alias), loc(Loc) {} + inline const IdentifierInfo *getAlias() const { return alias; } inline SourceLocation getLocation() const { return loc; } - void setUsed(bool Used=true) { used = Used; } - inline bool getUsed() { return used; } - bool operator==(WeakInfo RHS) const { - return alias == RHS.getAlias() && loc == RHS.getLocation(); - } - bool operator!=(WeakInfo RHS) const { return !(*this == RHS); } + bool operator==(WeakInfo RHS) const = delete; + bool operator!=(WeakInfo RHS) const = delete; + + struct DenseMapInfoByAliasNameOnly : private llvm::DenseMapInfo { + static inline WeakInfo getEmptyKey() { + return WeakInfo(DenseMapInfo::getEmptyKey(), + SourceLocation()); + } + static inline WeakInfo getTombstoneKey() { + return WeakInfo(DenseMapInfo::getTombstoneKey(), + SourceLocation()); + } + static unsigned getHashValue(const WeakInfo &W) { + return W.getAlias() ? DenseMapInfo::getHashValue(W.getAlias()->getName()) + : DenseMapInfo::getHashValue(""); + } + static bool isEqual(const WeakInfo &LHS, const WeakInfo &RHS) { + if (LHS.getAlias() == RHS.getAlias()) + return true; + if (!LHS.getAlias() || !RHS.getAlias()) + return false; + if (LHS.getAlias() == getEmptyKey().getAlias() || + RHS.getAlias() == getEmptyKey().getAlias()) + return false; + if (LHS.getAlias() == getTombstoneKey().getAlias() || + RHS.getAlias() == getTombstoneKey().getAlias()) + return false; + return LHS.getAlias()->getName() == RHS.getAlias()->getName(); + } + }; }; } // end namespace clang diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -929,7 +929,7 @@ SmallVector, 4> WeakIDs; ExternalSource->ReadWeakUndeclaredIdentifiers(WeakIDs); for (auto &WeakID : WeakIDs) - WeakUndeclaredIdentifiers.insert(WeakID); + (void)WeakUndeclaredIdentifiers[WeakID.first].insert(WeakID.second); } @@ -1180,19 +1180,21 @@ // Check for #pragma weak identifiers that were never declared LoadExternalWeakUndeclaredIdentifiers(); - for (auto WeakID : WeakUndeclaredIdentifiers) { - if (WeakID.second.getUsed()) + for (const auto &WeakIDs : WeakUndeclaredIdentifiers) { + if (WeakIDs.second.empty()) continue; - Decl *PrevDecl = LookupSingleName(TUScope, WeakID.first, SourceLocation(), + Decl *PrevDecl = LookupSingleName(TUScope, WeakIDs.first, SourceLocation(), LookupOrdinaryName); if (PrevDecl != nullptr && !(isa(PrevDecl) || isa(PrevDecl))) - Diag(WeakID.second.getLocation(), diag::warn_attribute_wrong_decl_type) - << "'weak'" << ExpectedVariableOrFunction; + for (const auto &WI : WeakIDs.second) + Diag(WI.getLocation(), diag::warn_attribute_wrong_decl_type) + << "'weak'" << ExpectedVariableOrFunction; else - Diag(WeakID.second.getLocation(), diag::warn_weak_identifier_undeclared) - << WeakID.first; + for (const auto &WI : WeakIDs.second) + Diag(WI.getLocation(), diag::warn_weak_identifier_undeclared) + << WeakIDs.first; } if (LangOpts.CPlusPlus11 && diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -18708,9 +18708,7 @@ if (PrevDecl) { PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc, AttributeCommonInfo::AS_Pragma)); } else { - (void)WeakUndeclaredIdentifiers.insert( - std::pair - (Name, WeakInfo((IdentifierInfo*)nullptr, NameLoc))); + (void)WeakUndeclaredIdentifiers[Name].insert(WeakInfo(nullptr, NameLoc)); } } @@ -18728,8 +18726,7 @@ if (NamedDecl *ND = dyn_cast(PrevDecl)) DeclApplyPragmaWeak(TUScope, ND, W); } else { - (void)WeakUndeclaredIdentifiers.insert( - std::pair(AliasName, W)); + (void)WeakUndeclaredIdentifiers[AliasName].insert(W); } } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -9062,9 +9062,7 @@ /// DeclApplyPragmaWeak - A declaration (maybe definition) needs \#pragma weak /// applied to it, possibly with an alias. -void Sema::DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, WeakInfo &W) { - if (W.getUsed()) return; // only do this once - W.setUsed(true); +void Sema::DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, const WeakInfo &W) { if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...)) IdentifierInfo *NDId = ND->getIdentifier(); NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation()); @@ -9091,23 +9089,25 @@ // It's valid to "forward-declare" #pragma weak, in which case we // have to do this. LoadExternalWeakUndeclaredIdentifiers(); - if (!WeakUndeclaredIdentifiers.empty()) { - NamedDecl *ND = nullptr; - if (auto *VD = dyn_cast(D)) - if (VD->isExternC()) - ND = VD; - if (auto *FD = dyn_cast(D)) - if (FD->isExternC()) - ND = FD; - if (ND) { - if (IdentifierInfo *Id = ND->getIdentifier()) { - auto I = WeakUndeclaredIdentifiers.find(Id); - if (I != WeakUndeclaredIdentifiers.end()) { - WeakInfo W = I->second; - DeclApplyPragmaWeak(S, ND, W); - WeakUndeclaredIdentifiers[Id] = W; - } - } + if (WeakUndeclaredIdentifiers.empty()) + return; + NamedDecl *ND = nullptr; + if (auto *VD = dyn_cast(D)) + if (VD->isExternC()) + ND = VD; + if (auto *FD = dyn_cast(D)) + if (FD->isExternC()) + ND = FD; + if (!ND) + return; + if (IdentifierInfo *Id = ND->getIdentifier()) { + auto I = WeakUndeclaredIdentifiers.find(Id); + if (I != WeakUndeclaredIdentifiers.end()) { + auto &WeakInfos = I->second; + for (const auto &W : WeakInfos) + DeclApplyPragmaWeak(S, ND, W); + std::remove_reference_t EmptyWeakInfos; + WeakInfos.swap(EmptyWeakInfos); } } } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3307,7 +3307,7 @@ break; case WEAK_UNDECLARED_IDENTIFIERS: - if (Record.size() % 4 != 0) + if (Record.size() % 3 != 0) return llvm::createStringError(std::errc::illegal_byte_sequence, "invalid weak identifiers record"); @@ -3322,8 +3322,7 @@ WeakUndeclaredIdentifiers.push_back( getGlobalIdentifierID(F, Record[I++])); WeakUndeclaredIdentifiers.push_back( - ReadSourceLocation(F, Record, I).getRawEncoding()); - WeakUndeclaredIdentifiers.push_back(Record[I++]); + ReadSourceLocation(F, Record, I).getRawEncoding()); } break; @@ -8396,11 +8395,9 @@ = DecodeIdentifierInfo(WeakUndeclaredIdentifiers[I++]); IdentifierInfo *AliasId = DecodeIdentifierInfo(WeakUndeclaredIdentifiers[I++]); - SourceLocation Loc - = SourceLocation::getFromRawEncoding(WeakUndeclaredIdentifiers[I++]); - bool Used = WeakUndeclaredIdentifiers[I++]; + SourceLocation Loc = + SourceLocation::getFromRawEncoding(WeakUndeclaredIdentifiers[I++]); WeakInfo WI(AliasId, Loc); - WI.setUsed(Used); WeakIDs.push_back(std::make_pair(WeakId, WI)); } WeakUndeclaredIdentifiers.clear(); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4559,13 +4559,14 @@ // entire table, since later PCH files in a PCH chain are only interested in // the results at the end of the chain. RecordData WeakUndeclaredIdentifiers; - for (auto &WeakUndeclaredIdentifier : SemaRef.WeakUndeclaredIdentifiers) { - IdentifierInfo *II = WeakUndeclaredIdentifier.first; - WeakInfo &WI = WeakUndeclaredIdentifier.second; - AddIdentifierRef(II, WeakUndeclaredIdentifiers); - AddIdentifierRef(WI.getAlias(), WeakUndeclaredIdentifiers); - AddSourceLocation(WI.getLocation(), WeakUndeclaredIdentifiers); - WeakUndeclaredIdentifiers.push_back(WI.getUsed()); + for (const auto &WeakUndeclaredIdentifierList : + SemaRef.WeakUndeclaredIdentifiers) { + const IdentifierInfo *const II = WeakUndeclaredIdentifierList.first; + for (const auto &WI : WeakUndeclaredIdentifierList.second) { + AddIdentifierRef(II, WeakUndeclaredIdentifiers); + AddIdentifierRef(WI.getAlias(), WeakUndeclaredIdentifiers); + AddSourceLocation(WI.getLocation(), WeakUndeclaredIdentifiers); + } } // Build a record containing all of the ext_vector declarations. diff --git a/clang/test/CodeGen/pragma-weak.c b/clang/test/CodeGen/pragma-weak.c --- a/clang/test/CodeGen/pragma-weak.c +++ b/clang/test/CodeGen/pragma-weak.c @@ -17,6 +17,10 @@ // CHECK-DAG: @mix2 = weak{{.*}} alias void (), void ()* @__mix2 // CHECK-DAG: @a1 = weak{{.*}} alias void (), void ()* @__a1 // CHECK-DAG: @xxx = weak{{.*}} alias void (), void ()* @__xxx +// CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias4 = weak{{.*}} alias void (), void ()* @undecfunc @@ -136,6 +140,15 @@ __attribute((pure,noinline,const)) void __xxx(void) { } // CHECK: void @__xxx() [[RN:#[0-9]+]] +///////////// PR28611: Try multiple aliases of same undeclared symbol or alias +#pragma weak undecfunc_alias1 = undecfunc +#pragma weak undecfunc_alias1 = undecfunc // Try specifying same alias/target pair a second time. +#pragma weak undecfunc_alias3 = undecfunc_alias2 // expected-warning {{alias will always resolve to undecfunc}} +#pragma weak undecfunc_alias4 = undecfunc_alias2 // expected-warning {{alias will always resolve to undecfunc}} +#pragma weak undecfunc_alias2 = undecfunc +void undecfunc_alias2(void); +void undecfunc(void) { } + ///////////// PR10878: Make sure we can call a weak alias void SHA512Pad(void *context) {} #pragma weak SHA384Pad = SHA512Pad diff --git a/clang/test/PCH/pragma-weak-functional.h b/clang/test/PCH/pragma-weak-functional.h new file mode 100644 --- /dev/null +++ b/clang/test/PCH/pragma-weak-functional.h @@ -0,0 +1,6 @@ +// Header for PCH test pragma-weak-functional.c + +#pragma weak undecfunc_alias2 = undecfunc +#pragma weak undecfunc_alias4 = undecfunc_alias1 +#pragma weak undecfunc_alias3 = undecfunc_alias1 +#pragma weak undecfunc_alias1 = undecfunc diff --git a/clang/test/PCH/pragma-weak-functional.c b/clang/test/PCH/pragma-weak-functional.c new file mode 100644 --- /dev/null +++ b/clang/test/PCH/pragma-weak-functional.c @@ -0,0 +1,17 @@ +// Test this without pch. +// RUN: %clang_cc1 -include %S/pragma-weak-functional.h %s -verify -emit-llvm -o - | FileCheck %s + +// Test with pch. +// RUN: %clang_cc1 -x c-header -emit-pch -o %t %S/pragma-weak-functional.h +// RUN: %clang_cc1 -include-pch %t %s -verify -emit-llvm -o - | FileCheck %s + +// CHECK-DAG: @undecfunc_alias1 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias2 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias3 = weak{{.*}} alias void (), void ()* @undecfunc +// CHECK-DAG: @undecfunc_alias4 = weak{{.*}} alias void (), void ()* @undecfunc + +///////////// PR28611: Try multiple aliases of same undeclared symbol or alias +void undecfunc_alias1(void); +void undecfunc(void) { } +// expected-warning@pragma-weak-functional.h:4 {{alias will always resolve to undecfunc}} +// expected-warning@pragma-weak-functional.h:5 {{alias will always resolve to undecfunc}}