[clang] [clang] Avoid caching linkage before redeclaration merging (PR #221476)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Vedran Miletić (vedranmiletic)

<details>
<summary>Changes</summary>

Linkage queries made while a declaration is still being formed can cache a result based only on a previous declaration. Compute linkage without caching in these paths, and defer applying #pragma redefine_extname until after redeclaration merging.

Fixes #<!-- -->112737
Fixes #<!-- -->173464

Assisted-by: Codex (OpenAI ChatGPT)

---
Full diff: https://github.com/llvm/llvm-project/pull/221476.diff


5 Files Affected:

- (modified) clang/include/clang/AST/Decl.h (+3) 
- (modified) clang/lib/AST/Decl.cpp (+6) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+24-13) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+2-1) 
- (added) clang/test/SemaCXX/redecl-linkage-before-merge.cpp (+13) 


``````````diff
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index eafeeecac7794..ccae943ed4b6e 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -422,6 +422,9 @@ class NamedDecl : public Decl {
   /// those.
   Linkage getLinkageInternal() const;
 
+  /// Compute this entity's linkage without caching it.
+  Linkage computeLinkageInternal() const;
+
   /// Get the linkage from a semantic point of view. Entities in
   /// anonymous namespaces are external (in c++98).
   Linkage getFormalLinkage() const;
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index a620e9f211ca6..78225f877dca3 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1188,6 +1188,12 @@ Linkage NamedDecl::getLinkageInternal() const {
       .getLinkage();
 }
 
+Linkage NamedDecl::computeLinkageInternal() const {
+  return LinkageComputer{}
+      .computeLVForDecl(this, LVComputationKind::forLinkageOnly())
+      .getLinkage();
+}
+
 static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) {
   // FIXME: Handle isModulePrivate.
   switch (D->getModuleOwnershipKind()) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2451baaf94703..b12c2c8858409 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10670,19 +10670,6 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     StringLiteral *SE = cast<StringLiteral>(E);
     NewFD->addAttr(
         AsmLabelAttr::Create(Context, SE->getString(), SE->getStrTokenLoc(0)));
-  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
-    llvm::MapVector<IdentifierInfo *, AsmLabelAttr *>::iterator I =
-        ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
-    if (I != ExtnameUndeclaredIdentifiers.end()) {
-      if (isDeclExternC(NewFD)) {
-        NewFD->addAttr(I->second);
-        ExtnameUndeclaredIdentifiers.erase(I);
-      } else if (NewFD->getDeclContext()
-                     ->getRedeclContext()
-                     ->isTranslationUnit())
-        Diag(NewFD->getLocation(), diag::warn_redefine_extname_not_applied)
-            << /*Variable*/0 << NewFD;
-    }
   }
 
   // Copy the parameter declarations from the declarator D to the function
@@ -10850,6 +10837,26 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     }
   }
 
+  // This linkage query must happen after merging previous declarations.
+  auto ApplyPragmaRedefineExtname = [&] {
+    if (D.getAsmLabel() || ExtnameUndeclaredIdentifiers.empty())
+      return;
+
+    auto I = ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
+    if (I == ExtnameUndeclaredIdentifiers.end())
+      return;
+
+    if (isDeclExternC(NewFD)) {
+      NewFD->addAttr(I->second);
+      ExtnameUndeclaredIdentifiers.erase(I);
+    } else if (NewFD->getDeclContext()
+                   ->getRedeclContext()
+                   ->isTranslationUnit()) {
+      Diag(NewFD->getLocation(), diag::warn_redefine_extname_not_applied)
+          << /*Variable*/ 0 << NewFD;
+    }
+  };
+
   if (!getLangOpts().CPlusPlus) {
     // Perform semantic checking on the function declaration.
     if (!NewFD->isInvalidDecl() && NewFD->isMain())
@@ -10869,6 +10876,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
             Previous.getResultKind() != LookupResultKind::FoundOverloaded) &&
            "previous declaration set still overloaded");
 
+    ApplyPragmaRedefineExtname();
+
     // Diagnose no-prototype function declarations with calling conventions that
     // don't support variadic calls. Only do this in C and do it after merging
     // possibly prototyped redeclarations.
@@ -11038,6 +11047,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
             Previous.getResultKind() != LookupResultKind::FoundOverloaded) &&
            "previous declaration set still overloaded");
 
+    ApplyPragmaRedefineExtname();
+
     NamedDecl *PrincipalDecl = (FunctionTemplate
                                 ? cast<NamedDecl>(FunctionTemplate)
                                 : NewFD);
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 106605962171b..c096c1a467cb4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1809,7 +1809,8 @@ static void handleAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     }
   } else {
     const auto *VD = cast<VarDecl>(D);
-    if (VD->isThisDeclarationADefinition() && VD->isExternallyVisible()) {
+    if (VD->isThisDeclarationADefinition() &&
+        isExternallyVisible(VD->computeLinkageInternal())) {
       S.Diag(AL.getLoc(), diag::err_alias_is_definition) << VD << 0;
       return;
     }
diff --git a/clang/test/SemaCXX/redecl-linkage-before-merge.cpp b/clang/test/SemaCXX/redecl-linkage-before-merge.cpp
new file mode 100644
index 0000000000000..b9d8d05f124ca
--- /dev/null
+++ b/clang/test/SemaCXX/redecl-linkage-before-merge.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+// https://github.com/llvm/llvm-project/issues/173464
+// A linkage query before redeclaration merging must not cache a result that
+// depends on a previous declaration.
+typedef float v4f __attribute__((vector_size(16)));
+extern const v4f kAlias;
+const v4f kAlias __attribute__((alias("kOne")));
+
+// https://github.com/llvm/llvm-project/issues/112737
+#pragma redefine_extname foo_cpp bar_cpp
+static int foo_cpp(); // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_cpp'}}
+extern int foo_cpp() { return 1; } // expected-warning {{#pragma redefine_extname is applicable to external C declarations only; not applied to function 'foo_cpp'}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/221476