[PATCH] D128223: [clang] Linkage of static locals may require inspecting visibility

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm created this revision.
cebowleratibm added a reviewer: daltenty.
Herald added a project: All.
cebowleratibm requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An assertion failure was encountered after https://reviews.llvm.org/D126340:

llvm/clang/lib/AST/Decl.cpp:1510: clang::LinkageInfo clang::LinkageComputer::getLVForDecl(const clang::NamedDecl *, clang::LVComputationKind): Assertion `D->getCachedLinkage() == LV.getLinkage()' failed.

See chromium bug <https://bugs.chromium.org/p/chromium/issues/detail?id=1331274#c2> for a reproducer.

Note D126340 <https://reviews.llvm.org/D126340> was reverted and re-landed.  The original problem can still be reproduced using the appropriate options as outlined in the new test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128223

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGenCXX/linkage-static-local-crash.cpp


Index: clang/test/CodeGenCXX/linkage-static-local-crash.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/linkage-static-local-crash.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix -mdefault-visibility-export-mapping=explicit -fvisibility-inlines-hidden -emit-llvm %s -o - | FileCheck %s
+
+struct C {
+  template <typename> 
+  __attribute__((__visibility__("hidden"))) 
+  static int& f() {
+    static int g = 42;
+    return g;
+  }
+};
+
+template<typename T>
+void foo(T i) { C::f<T>(); }
+
+void bar() {
+  foo([]{});
+}
+
+// CHECK: @"_ZZN1C1fIZ3barvE3$_0EERivE1g" = internal global i32 42, align 4
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1349,19 +1349,24 @@
     // If a function is hidden by -fvisibility-inlines-hidden option and
     // is not explicitly attributed as a hidden function,
     // we should not make static local variables in the function hidden.
-    LV = getLVForDecl(FD, computation);
-    if (isa<VarDecl>(D) && useInlineVisibilityHidden(FD) &&
-        !LV.isVisibilityExplicit() &&
-        !Context.getLangOpts().VisibilityInlinesHiddenStaticLocalVar) {
+    bool mayBeVisible = 
+        isa<VarDecl>(D) && useInlineVisibilityHidden(FD) &&
+        !Context.getLangOpts().VisibilityInlinesHiddenStaticLocalVar;
+    LVComputationKind nestedComputation =
+        mayBeVisible
+            ? LVComputationKind(computation.getExplicitVisibilityKind())
+            : computation;
+    LV = getLVForDecl(FD, nestedComputation);
+    if (mayBeVisible && !LV.isVisibilityExplicit()) {
       assert(cast<VarDecl>(D)->isStaticLocal());
       // If this was an implicitly hidden inline method, check again for
       // explicit visibility on the parent class, and use that for static locals
       // if present.
       if (const auto *MD = dyn_cast<CXXMethodDecl>(FD))
-        LV = getLVForDecl(MD->getParent(), computation);
+        LV = getLVForDecl(MD->getParent(), nestedComputation);
       if (!LV.isVisibilityExplicit()) {
         Visibility globalVisibility =
-            computation.isValueVisibility()
+            nestedComputation.isValueVisibility()
                 ? Context.getLangOpts().getValueVisibilityMode()
                 : Context.getLangOpts().getTypeVisibilityMode();
         return LinkageInfo(VisibleNoLinkage, globalVisibility,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128223.438464.patch
Type: text/x-patch
Size: 2489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220620/53715c14/attachment.bin>