[clang] Cached linkage assertion for static locals of static function

[cebowleratibm]

Summary

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.

[daltenty]

LGTM. Doing the inner computation while observing visibility seems like the right thing to do in this case.

clang/lib/AST/Decl.cpp

1377

Just adding my own note of explanation, since this stuff can be a little hard to follow.

This inner linkage query needs to observe visibility in order to get a consistent result, even if the calling query (i.e. getLinkageInternal in this case) has passed in a LVComputationKind indicating we don't need to for the LocalDecl. If we don't, we may get results inconsistent with the cached value for the enclosing FunctionDecl, hence the assert.

[cebowleratibm]

I believe the fix for a52d151f9dde7 inadvertently exposed a code path where by the linkage of a static local of a static function, which would otherwise return LinkageInfo::none() may now return VisibleNoLinkage depending on the incoming computation argument.

I don't think a static local of a function with no linkage should ever return VisibleNoLinkage so I've added the appropriate guard such that the linkage evaluation with return no linkage regardless of the computation argument.

[daltenty]

New revision LGTM, VisibleNoLinkage is typical reserved for types from inline functions so it doesn't seem sensible to return. Looking at the C++ standard, I'm not even convinced this is guaranteed to be the same object if the function doesn't have external linkage, so I think the guard is sensible.

clang/lib/AST/Decl.cpp

if (const auto *BD = dyn_cast<BlockDecl>(OuterD)) {
     if (!FD->isInlined() &&
         !isTemplateInstantiation(FD->getTemplateSpecializationKind()))
       return LinkageInfo::none();
 
     // 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) &&
+    if (isExternallyVisible(LV.getLinkage()) &&
+        isa<VarDecl>(D) && useInlineVisibilityHidden(FD) &&
         !LV.isVisibilityExplicit() &&
         !Context.getLangOpts().VisibilityInlinesHiddenStaticLocalVar) {
       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);
       if (!LV.isVisibilityExplicit()) {
         Visibility globalVisibility =
             computation.isValueVisibility()
                 ? Context.getLangOpts().getValueVisibilityMode()
                 : Context.getLangOpts().getTypeVisibilityMode();

clang/test/CodeGenCXX/linkage-static-local-crash.cpp

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