[clang] ce21c92 - [Clang] Work with multiple pragmas weak before definition
Hubert Tong via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 24 17:20:09 PDT 2022
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 <rcraik at ca.ibm.com>
Co-authored-by: Jamie Schmeiser <schmeise at 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 at pragma-weak-functional.h:4 {{alias will always resolve to undecfunc}}
+// expected-warning at 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
More information about the cfe-commits
mailing list