Author: Hubert Tong Date: 2022-03-24T20:17:49-04:00 New Revision: ce21c926f8efe969717e21e3ae6c5a3246b3d455
URL: https://github.com/llvm/llvm-project/commit/ce21c926f8efe969717e21e3ae6c5a3246b3d455 DIFF: https://github.com/llvm/llvm-project/commit/ce21c926f8efe969717e21e3ae6c5a3246b3d455.diff LOG: [Clang] Work with multiple pragmas weak before definition 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. Reviewed By: aaron.ballman, cebowleratibm Differential Revision: https://reviews.llvm.org/D121927 Co-authored-by: Rachel Craik <rcr...@ca.ibm.com> Co-authored-by: Jamie Schmeiser <schme...@ca.ibm.com> Added: clang/test/PCH/pragma-weak-functional.c clang/test/PCH/pragma-weak-functional.h Modified: clang/docs/ReleaseNotes.rst 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 Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7dd43cb01a8ea..b268d0f8c20d8 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -78,6 +78,10 @@ Bug Fixes - No longer crash when specifying a variably-modified parameter type in a function with the ``naked`` attribute. This fixes `Issue 50541 <https://github.com/llvm/llvm-project/issues/50541>`_. +- 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 <https://github.com/llvm/llvm-project/issues/28985>`_. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f95308275688e..4c44ee3cc9313 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1072,10 +1072,20 @@ class Sema final { } }; - /// WeakUndeclaredIdentifiers - Identifiers contained in - /// \#pragma weak before declared. rare. may alias another - /// identifier, declared or undeclared - llvm::MapVector<IdentifierInfo *, WeakInfo> 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< + WeakInfo, llvm::SmallVector<WeakInfo, 1u>, + llvm::SmallDenseSet<WeakInfo, 2u, WeakInfo::DenseMapInfoByAliasOnly>>> + WeakUndeclaredIdentifiers; /// ExtnameUndeclaredIdentifiers - Identifiers contained in /// \#pragma redefine_extname before declared. Used in Solaris system headers @@ -10175,7 +10185,7 @@ class Sema final { 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 index 434393677d42d..877b47d2474ea 100644 --- 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,32 @@ class IdentifierInfo; /// Captures information about a \#pragma weak directive. class WeakInfo { - IdentifierInfo *alias; // alias (optional) - SourceLocation loc; // for diagnostics - bool used; // identifier later declared? + const IdentifierInfo *alias = nullptr; // alias (optional) + SourceLocation loc; // for diagnostics public: - WeakInfo() - : alias(nullptr), loc(SourceLocation()), used(false) {} - WeakInfo(IdentifierInfo *Alias, SourceLocation Loc) - : alias(Alias), loc(Loc), used(false) {} - inline IdentifierInfo * getAlias() const { return alias; } + WeakInfo() = default; + WeakInfo(const IdentifierInfo *Alias, SourceLocation Loc) + : 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 DenseMapInfoByAliasOnly + : private llvm::DenseMapInfo<const IdentifierInfo *> { + 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 DenseMapInfo::getHashValue(W.getAlias()); + } + static bool isEqual(const WeakInfo &LHS, const WeakInfo &RHS) { + return DenseMapInfo::isEqual(LHS.getAlias(), RHS.getAlias()); + } + }; }; } // end namespace clang diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index fa09281f2f0e5..b0e47c0e54aa5 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -929,7 +929,7 @@ void Sema::LoadExternalWeakUndeclaredIdentifiers() { SmallVector<std::pair<IdentifierInfo *, WeakInfo>, 4> WeakIDs; ExternalSource->ReadWeakUndeclaredIdentifiers(WeakIDs); for (auto &WeakID : WeakIDs) - WeakUndeclaredIdentifiers.insert(WeakID); + (void)WeakUndeclaredIdentifiers[WeakID.first].insert(WeakID.second); } @@ -1180,19 +1180,21 @@ void Sema::ActOnEndOfTranslationUnit() { // 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 && diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8d46d6061bda1..5c262063a6aef 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -18707,9 +18707,7 @@ void Sema::ActOnPragmaWeakID(IdentifierInfo* Name, 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)); } } @@ -18727,8 +18725,7 @@ void Sema::ActOnPragmaWeakAlias(IdentifierInfo* Name, 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); } } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index b99a970061c0c..6788c92ab9828 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -9070,9 +9070,7 @@ NamedDecl *Sema::DeclClonePragmaWeak(NamedDecl *ND, const IdentifierInfo *II, /// 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()); @@ -9099,23 +9097,25 @@ void Sema::ProcessPragmaWeak(Scope *S, Decl *D) { // 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); } } } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index e6b6e1fb51857..67b3a822df182 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3307,7 +3307,7 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, 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 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, 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 @@ void ASTReader::ReadWeakUndeclaredIdentifiers( = 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 index 01381b0bd6e01..16c25a00b4461 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -4559,13 +4559,14 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, // 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 index 4fe8886a009f0..b210f6922fed6 100644 --- 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 @@ void __a1(void) {} __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.c b/clang/test/PCH/pragma-weak-functional.c new file mode 100644 index 0000000000000..6f39c2fcadf8e --- /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}} diff --git a/clang/test/PCH/pragma-weak-functional.h b/clang/test/PCH/pragma-weak-functional.h new file mode 100644 index 0000000000000..30178daf41be8 --- /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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
