[Clang] Work with multiple pragmas weak before definition

[hubert.reinterpretcast]

Summary

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 <[e-mail]>
Co-authored-by: Jamie Schmeiser <[e-mail]>

[cebowleratibm]

Makes sense to me.

clang/include/clang/Sema/Weak.h

34–63

For my selfish education: what is the purpose of marking this deleted rather than actually deleting it?

clang/test/CodeGen/pragma-weak.c

144

small suggestion for test: repeat this line so that you cover the case where you're collecting identical WeakInfos to the same AliasName.

[hubert.reinterpretcast]

clang/include/clang/Sema/Weak.h

34–63

Removing this should be fine (I will make the change). Didn't want to delete the operator functions (immediately) though: downstream consumers might end up calling a different function, which warrants some scrutiny.

[hubert.reinterpretcast]

• Address review comments: Remove deleted non-operator member functions; add extra line to test

[aaron.ballman]

Can you also add a release note for the fix?

clang/include/clang/Sema/Sema.h

1074–1075

Should probably update this comment as the type just got much weirder than before.

clang/include/clang/Sema/Weak.h

19–20

25–29

32

Would it be onerous to make the return type be const IdentifierInfo * given that the function is const? (If it is, just ignore the suggestion -- I love adding const correctness where we can get it basically for free.)

61

Previously we cared about the source locations being the same. But... is there a situation where the aliases are the same but the source locations are different? I can't think of any, so perhaps that's worth an assert that the locations match?

[hubert.reinterpretcast]

clang/include/clang/Sema/Weak.h

61

We didn't really case about the source locations being the same. The removed operators were just dead code (featuring such things as compare-by-pointer-value). I have verified that splitting the deletion of them to a separate patch does not cause any build or LIT check-all failures. I can commit that part as an NFC patch first if you prefer.

Also, the aliases can be the same with different source locations. The location comes from where the #pragma weak is (from the parser by way of ActOnPragmaWeakAlias). That duplicates are ignored (meaning we only diagnose one instance) is mentioned in the patch description.

It is already the status quo that having the same alias specified with different targets (at least those not yet declared) neither produces a diagnostic nor operates by "last one wins". Instead, Clang prefers the first declared target and GCC just emits both and leaves it to the assembler to figure out (https://godbolt.org/z/EasK375j3).

[hubert.reinterpretcast]

clang/include/clang/Sema/Weak.h

32

It's not free but I can post an NFC patch to add the const through. On the chain of calls that const needs to be added to, the pointer eventually feeds a DeclarationName (which already takes const IdentifierInfo *).

[hubert.reinterpretcast]

• Address review comments: Use default member init in WeakInfo

[hubert.reinterpretcast]

clang/include/clang/Sema/Weak.h

32

https://reviews.llvm.org/D122261 has the NFC patch.

[hubert.reinterpretcast]

• Address review comments: Add release notes, expand comments

[hubert.reinterpretcast]

• Address review comments: Return const from getAlias()

[hubert.reinterpretcast]

@aaron.ballman, I believe I have responded to all of the comments.

[aaron.ballman]

LGTM, thank you for the fix!

clang/include/clang/Sema/Weak.h

61

Ah, thank you for the explanation, that makes sense to me.

[hubert.reinterpretcast]

clang/include/clang/Sema/Weak.h

47

The fact that WeakUndeclaredIdentifiers is keyed by IdentifierInfo * must mean that it is safe to use pointer comparison. This is further bolstered by IdentifierInfo having only private or deleted constructors (with IdentifierTable being a friend class).

I will adjust to make this class simply forward to DenseMapInfo<const IdentifierInfo *> with the alias members.

[aaron.ballman]

clang/include/clang/Sema/Weak.h

47

SGTM!

[hubert.reinterpretcast]

• Adjust per observation: Use DenseMapInfo for the alias pointer value

clang/docs/ReleaseNotes.rst

- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
   wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array
   size expression. This was fixed and ``::getArraySize()`` will now always
   either return ``None`` or a ``llvm::Optional`` wrapping a valid ``Expr*``.
   This fixes `Issue 53742 <https://github.com/llvm/llvm-project/issues/53742>`_.
 - We now ignore full expressions when traversing cast subexpressions. This
   fixes `Issue 53044 <https://github.com/llvm/llvm-project/issues/53044>`_.
 - Allow `-Wno-gnu` to silence GNU extension diagnostics for pointer arithmetic
   diagnostics. Fixes `Issue 54444 <https://github.com/llvm/llvm-project/issues/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 <https://github.com/llvm/llvm-project/issues/28985>`_.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - ``-Wliteral-range`` will warn on floating-point equality comparisons with
   constants that are not representable in a casted value. For example,
   ``(float) f == 0.1`` is always false.
 
 Non-comprehensive list of changes in this release

clang/include/clang/Sema/Sema.h

~SynthesizedFunctionScope() {
       if (PushedCodeSynthesisContext)
         S.popCodeSynthesisContext();
       if (auto *FD = dyn_cast<FunctionDecl>(S.CurContext))
         FD->setWillHaveBody(false);
       S.PopExpressionEvaluationContext();
       S.PopFunctionScopeInfo();
     }
   };
 
-  /// 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::DenseMapInfoByAliasNameOnly>>>
+      WeakUndeclaredIdentifiers;
 
   /// ExtnameUndeclaredIdentifiers - Identifiers contained in
   /// \#pragma redefine_extname before declared.  Used in Solaris system headers
   /// to define functions that occur in multiple standards to call the version
   /// in the currently selected standard.
   llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*> ExtnameUndeclaredIdentifiers;
 
 
void ActOnPragmaUnused(const Token &Identifier,
                          SourceLocation PragmaLoc);
 
   /// ActOnPragmaVisibility - Called on well formed \#pragma GCC visibility... .
   void ActOnPragmaVisibility(const IdentifierInfo* VisType,
                              SourceLocation PragmaLoc);
 
   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,
                          SourceLocation PragmaLoc,
                          SourceLocation WeakNameLoc);
 
   /// ActOnPragmaRedefineExtname - Called on well formed
   /// \#pragma redefine_extname oldname newname.

clang/include/clang/Sema/Weak.h

 //  information about the target of a #pragma weak directive.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_SEMA_WEAK_H
 #define LLVM_CLANG_SEMA_WEAK_H
 
 #include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseMapInfo.h"
 
 namespace clang {
 
 class IdentifierInfo;
 
 /// 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 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 = 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("");
     }
-  bool operator!=(WeakInfo RHS) const { return !(*this == RHS); }
+    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
 
 #endif // LLVM_CLANG_SEMA_WEAK_H

clang/lib/Sema/Sema.cpp

 
 void Sema::LoadExternalWeakUndeclaredIdentifiers() {
   if (!ExternalSource)
     return;
 
   SmallVector<std::pair<IdentifierInfo *, WeakInfo>, 4> WeakIDs;
   ExternalSource->ReadWeakUndeclaredIdentifiers(WeakIDs);
   for (auto &WeakID : WeakIDs)
-    WeakUndeclaredIdentifiers.insert(WeakID);
+    (void)WeakUndeclaredIdentifiers[WeakID.first].insert(WeakID.second);
 }
 
 
 typedef llvm::DenseMap<const CXXRecordDecl*, bool> RecordCompleteMap;
 
 /// Returns true, if all methods and nested classes of the given
 /// CXXRecordDecl are defined in this translation unit.
 ///
if (TUKind == TU_Prefix) {
     // Translation unit prefixes don't need any of the checking below.
     if (!PP.isIncrementalProcessingEnabled())
       TUScope = nullptr;
     return;
   }
 
   // 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)
+      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 &&
       !Diags.isIgnored(diag::warn_delegating_ctor_cycle, SourceLocation()))
     CheckDelegatingCtorCycles();
 
   if (!Diags.hasErrorOccurred()) {
     if (ExternalSource)

clang/lib/Sema/SemaDecl.cpp

 void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
                              SourceLocation PragmaLoc,
                              SourceLocation NameLoc) {
   Decl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName);
 
   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));
   }
 }
 
 void Sema::ActOnPragmaWeakAlias(IdentifierInfo* Name,
                                 IdentifierInfo* AliasName,
                                 SourceLocation PragmaLoc,
                                 SourceLocation NameLoc,
                                 SourceLocation AliasNameLoc) {
   Decl *PrevDecl = LookupSingleName(TUScope, AliasName, AliasNameLoc,
                                     LookupOrdinaryName);
   WeakInfo W = WeakInfo(Name, NameLoc);
 
   if (PrevDecl && (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl))) {
     if (!PrevDecl->hasAttr<AliasAttr>())
       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);
   }
 }
 
 Decl *Sema::getObjCDeclContext() const {
   return (dyn_cast_or_null<ObjCContainerDecl>(CurContext));
 }
 
 Sema::FunctionEmissionStatus Sema::getEmissionStatus(FunctionDecl *FD,

clang/lib/Sema/SemaDeclAttr.cpp

if (auto *FD = dyn_cast<FunctionDecl>(ND)) {
     if (VD->getQualifier())
       cast<VarDecl>(NewD)->setQualifierInfo(VD->getQualifierLoc());
   }
   return NewD;
 }
 
 /// 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());
     NewD->addAttr(
         AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
     NewD->addAttr(WeakAttr::CreateImplicit(Context, W.getLocation(),
                                            AttributeCommonInfo::AS_Pragma));
     WeakTopLevelDecl.push_back(NewD);
ND->addAttr(WeakAttr::CreateImplicit(Context, W.getLocation(),
                                          AttributeCommonInfo::AS_Pragma));
   }
 }
 
 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;
+  if (!ND)
+    return;
+      auto &WeakInfos = I->second;
+      for (const auto &W : WeakInfos)
+      std::remove_reference_t<decltype(WeakInfos)> EmptyWeakInfos;
+      WeakInfos.swap(EmptyWeakInfos);
     }
   }
 }
 
 /// ProcessDeclAttributes - Given a declarator (PD) with attributes indicated in
 /// it, apply them to D.  This is a bit tricky because PD can have attributes
 /// specified in many different places, and we need to find and apply them all.
 void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {

clang/lib/Serialization/ASTReader.cpp

case UNUSED_FILESCOPED_DECLS:
       break;
 
     case DELEGATING_CTORS:
       for (unsigned I = 0, N = Record.size(); I != N; ++I)
         DelegatingCtorDecls.push_back(getGlobalDeclID(F, Record[I]));
       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");
 
       // FIXME: Ignore weak undeclared identifiers from non-original PCH
       // files. This isn't the way to do it :)
       WeakUndeclaredIdentifiers.clear();
 
       // Translate the weak, undeclared identifiers into global IDs.
       for (unsigned I = 0, N = Record.size(); I < N; /* in loop */) {
         WeakUndeclaredIdentifiers.push_back(
           getGlobalIdentifierID(F, Record[I++]));
         WeakUndeclaredIdentifiers.push_back(
           getGlobalIdentifierID(F, Record[I++]));
         WeakUndeclaredIdentifiers.push_back(
             ReadSourceLocation(F, Record, I).getRawEncoding());
-        WeakUndeclaredIdentifiers.push_back(Record[I++]);
       }
       break;
 
     case SELECTOR_OFFSETS: {
       F.SelectorOffsets = (const uint32_t *)Blob.data();
       F.LocalNumSelectors = Record[0];
       unsigned LocalBaseSelectorID = Record[1];
       F.BaseSelectorID = getTotalNumSelectors();
void ASTReader::ReadWeakUndeclaredIdentifiers(
   if (WeakUndeclaredIdentifiers.empty())
     return;
 
   for (unsigned I = 0, N = WeakUndeclaredIdentifiers.size(); I < N; /*none*/) {
     IdentifierInfo *WeakId
       = 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();
 }
 
 void ASTReader::ReadUsedVTables(SmallVectorImpl<ExternalVTableUse> &VTables) {
   for (unsigned Idx = 0, N = VTableUses.size(); Idx < N; /* In loop */) {
     ExternalVTableUse VT;

clang/lib/Serialization/ASTWriter.cpp

ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   RecordData DelegatingCtorDecls;
   if (!isModule)
     AddLazyVectorDecls(*this, SemaRef.DelegatingCtorDecls, DelegatingCtorDecls);
 
   // Write the set of weak, undeclared identifiers. We always write the
   // 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;
+  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);
-    WeakUndeclaredIdentifiers.push_back(WI.getUsed());
+    }
   }
 
   // Build a record containing all of the ext_vector declarations.
   RecordData ExtVectorDecls;
   AddLazyVectorDecls(*this, SemaRef.ExtVectorDecls, ExtVectorDecls);
 
   // Build a record containing all of the VTable uses information.
   RecordData VTableUses;

clang/test/CodeGen/pragma-weak.c

 // CHECK-DAG: @foo2 = weak{{.*}} alias void (), void ()* @__foo2
 // CHECK-DAG: @stutter = weak{{.*}} alias void (), void ()* @__stutter
 // CHECK-DAG: @stutter2 = weak{{.*}} alias void (), void ()* @__stutter2
 // CHECK-DAG: @declfirst = weak{{.*}} alias void (), void ()* @__declfirst
 // CHECK-DAG: @declfirstattr = weak{{.*}} alias void (), void ()* @__declfirstattr
 // 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
 
 
 
 // CHECK-LABEL: define weak{{.*}} void @weakdef()
 
 
 #pragma weak weakvar
 int weakvar;

 #pragma weak a1 = __a1
 void __a1(void) {}
 // CHECK: define{{.*}} void @__a1() [[NI:#[0-9]+]]
 
 #pragma weak xxx = __xxx
 __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
 void PR10878(void) { SHA384Pad(0); }
 // CHECK: call void @SHA384Pad(i8* noundef null)
 
 
 // PR14046: Parse #pragma weak in function-local context

clang/test/PCH/pragma-weak-functional.h

+// 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

clang/test/PCH/pragma-weak-functional.c

+// 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}}