hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: jamieschmeiser, aaron.ballman, chandlerc, cebowleratibm, w2yehia. Herald added a project: All. hubert.reinterpretcast requested review of this revision. Herald added a project: clang.
Update `WeakUndeclaredIdentifiers` to hold a collection of weak aliases per identifier instead of only one. This also allows the "used" state to be removed from `WeakInfo` because it is really only there as an alternative to removing processed map entries, and we can represent that using an empty set now. The serialization code is updated for the removal of the field. Additionally, a PCH test is added for the new functionality. The records are grouped by the "target" identifier, which was already being used as a key for lookup purposes. We also store only one record per alias name; combined, this means that diagnostics are grouped by the "target" and limited to one per alias (which should be acceptable). Fixes PR28611. Fixes llvm/llvm-project#28985. Co-authored-by: Rachel Craik <rcr...@ca.ibm.com> Co-authored-by: Jamie Schmeiser <schme...@ca.ibm.com> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121927 Files: clang/include/clang/Sema/Sema.h clang/include/clang/Sema/Weak.h clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/test/CodeGen/pragma-weak.c clang/test/PCH/pragma-weak-functional.c clang/test/PCH/pragma-weak-functional.h
Index: clang/test/PCH/pragma-weak-functional.h =================================================================== --- /dev/null +++ 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 Index: clang/test/PCH/pragma-weak-functional.c =================================================================== --- /dev/null +++ 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}} Index: clang/test/CodeGen/pragma-weak.c =================================================================== --- clang/test/CodeGen/pragma-weak.c +++ 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,14 @@ __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_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 Index: clang/lib/Serialization/ASTWriter.cpp =================================================================== --- clang/lib/Serialization/ASTWriter.cpp +++ clang/lib/Serialization/ASTWriter.cpp @@ -4561,13 +4561,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. Index: clang/lib/Serialization/ASTReader.cpp =================================================================== --- clang/lib/Serialization/ASTReader.cpp +++ 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(); Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ 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<VarDecl>(D)) - if (VD->isExternC()) - ND = VD; - if (auto *FD = dyn_cast<FunctionDecl>(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<VarDecl>(D)) + if (VD->isExternC()) + ND = VD; + if (auto *FD = dyn_cast<FunctionDecl>(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<decltype(WeakInfos)> EmptyWeakInfos; + WeakInfos.swap(EmptyWeakInfos); } } } Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -18714,9 +18714,7 @@ if (PrevDecl) { PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc, AttributeCommonInfo::AS_Pragma)); } else { - (void)WeakUndeclaredIdentifiers.insert( - std::pair<IdentifierInfo*,WeakInfo> - (Name, WeakInfo((IdentifierInfo*)nullptr, NameLoc))); + (void)WeakUndeclaredIdentifiers[Name].insert(WeakInfo(nullptr, NameLoc)); } } @@ -18734,8 +18732,7 @@ if (NamedDecl *ND = dyn_cast<NamedDecl>(PrevDecl)) DeclApplyPragmaWeak(TUScope, ND, W); } else { - (void)WeakUndeclaredIdentifiers.insert( - std::pair<IdentifierInfo*,WeakInfo>(AliasName, W)); + (void)WeakUndeclaredIdentifiers[AliasName].insert(W); } } Index: clang/lib/Sema/Sema.cpp =================================================================== --- clang/lib/Sema/Sema.cpp +++ clang/lib/Sema/Sema.cpp @@ -923,7 +923,7 @@ SmallVector<std::pair<IdentifierInfo *, WeakInfo>, 4> WeakIDs; ExternalSource->ReadWeakUndeclaredIdentifiers(WeakIDs); for (auto &WeakID : WeakIDs) - WeakUndeclaredIdentifiers.insert(WeakID); + (void)WeakUndeclaredIdentifiers[WeakID.first].insert(WeakID.second); } @@ -1174,19 +1174,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<FunctionDecl>(PrevDecl) || isa<VarDecl>(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 && Index: clang/include/clang/Sema/Weak.h =================================================================== --- clang/include/clang/Sema/Weak.h +++ clang/include/clang/Sema/Weak.h @@ -16,6 +16,8 @@ #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/DenseMapInfo.h" + namespace clang { class IdentifierInfo; @@ -24,20 +26,44 @@ class WeakInfo { IdentifierInfo *alias; // alias (optional) SourceLocation loc; // for diagnostics - bool used; // identifier later declared? public: - WeakInfo() - : alias(nullptr), loc(SourceLocation()), used(false) {} + WeakInfo() : alias(nullptr), loc(SourceLocation()) {} WeakInfo(IdentifierInfo *Alias, SourceLocation Loc) - : alias(Alias), loc(Loc), used(false) {} - inline IdentifierInfo * getAlias() const { return alias; } + : alias(Alias), loc(Loc) {} + inline 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); } + void setUsed(bool Used = true) = delete; + inline bool getUsed() const = delete; + bool operator==(WeakInfo RHS) const = delete; + bool operator!=(WeakInfo RHS) const = delete; + + struct DenseMapInfoByAliasNameOnly : private llvm::DenseMapInfo<StringRef> { + static inline WeakInfo getEmptyKey() { + return WeakInfo(DenseMapInfo<IdentifierInfo *>::getEmptyKey(), + SourceLocation()); + } + static inline WeakInfo getTombstoneKey() { + return WeakInfo(DenseMapInfo<IdentifierInfo *>::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 Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -1075,7 +1075,12 @@ /// WeakUndeclaredIdentifiers - Identifiers contained in /// \#pragma weak before declared. rare. may alias another /// identifier, declared or undeclared - llvm::MapVector<IdentifierInfo *, WeakInfo> WeakUndeclaredIdentifiers; + llvm::MapVector< + IdentifierInfo *, + llvm::SetVector<WeakInfo, llvm::SmallVector<WeakInfo, 1u>, + llvm::SmallDenseSet< + WeakInfo, 2u, WeakInfo::DenseMapInfoByAliasNameOnly>>> + WeakUndeclaredIdentifiers; /// ExtnameUndeclaredIdentifiers - Identifiers contained in /// \#pragma redefine_extname before declared. Used in Solaris system headers @@ -10177,7 +10182,7 @@ NamedDecl *DeclClonePragmaWeak(NamedDecl *ND, 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,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits