[PATCH] D53052: [AST] Use -fvisibility value when ignoring -fv-i-h* inline static locals

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: hans, thakis.
Herald added a subscriber: eraman.

In r340386 we added code to give static locals in inline functions
default visibility. Instead, we should use the "default" visibility
passed on the command line, which could be hidden or protected, as GCC
does.

Most codebases probably only use either -fvisibility=hidden or
-fvisibility-inlines-hidden, since marking inlines hidden should be a
no-op if everything was already going to be hidden, but it looks like
Chromium uses both.

Fixes PR39236


https://reviews.llvm.org/D53052

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGenCXX/visibility-inlines-hidden-staticvar.cpp


Index: clang/test/CodeGenCXX/visibility-inlines-hidden-staticvar.cpp
===================================================================
--- clang/test/CodeGenCXX/visibility-inlines-hidden-staticvar.cpp
+++ clang/test/CodeGenCXX/visibility-inlines-hidden-staticvar.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 -fvisibility-inlines-hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck -check-prefixes=CHECK-NO-VIH %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 -fvisibility hidden -fvisibility-inlines-hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s --check-prefix=CHECK-VIS-HIDDEN
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c++11 -fvisibility protected -fvisibility-inlines-hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s --check-prefix=CHECK-VIS-PROTECTED
 
 // When a function is hidden due to -fvisibility-inlines-hidden option, static local variables of the function should not be hidden by the option.
 
@@ -29,6 +31,31 @@
 // CHECK-NO-VIH-DAG: define linkonce_odr hidden i32 @_Z18inline_hidden_funcv()
 // CHECK-NO-VIH-DAG: define linkonce_odr i32 @_Z19inline_default_funcv()
 
+// CHECK-VIS-HIDDEN-DAG: @_ZZ4funcvE3var = internal global i32 0
+// CHECK-VIS-HIDDEN-DAG: @_ZZ11hidden_funcvE3var = internal global i32 0
+// CHECK-VIS-HIDDEN-DAG: @_ZZ12default_funcvE3var = internal global i32 0
+// CHECK-VIS-HIDDEN-DAG: @_ZZ11inline_funcvE3var = linkonce_odr hidden global i32 0, comdat
+// CHECK-VIS-HIDDEN-DAG: @_ZZ18inline_hidden_funcvE3var = linkonce_odr hidden global i32 0, comdat
+// CHECK-VIS-HIDDEN-DAG: @_ZZ19inline_default_funcvE3var = linkonce_odr global i32 0, comdat
+// CHECK-VIS-HIDDEN-DAG: define hidden i32 @_Z4funcv()
+// CHECK-VIS-HIDDEN-DAG: define hidden i32 @_Z11hidden_funcv()
+// CHECK-VIS-HIDDEN-DAG: define i32 @_Z12default_funcv()
+// CHECK-VIS-HIDDEN-DAG: define linkonce_odr hidden i32 @_Z11inline_funcv()
+// CHECK-VIS-HIDDEN-DAG: define linkonce_odr hidden i32 @_Z18inline_hidden_funcv()
+// CHECK-VIS-HIDDEN-DAG: define linkonce_odr i32 @_Z19inline_default_funcv()
+
+// CHECK-VIS-PROTECTED-DAG: @_ZZ4funcvE3var = internal global i32 0
+// CHECK-VIS-PROTECTED-DAG: @_ZZ11hidden_funcvE3var = internal global i32 0
+// CHECK-VIS-PROTECTED-DAG: @_ZZ12default_funcvE3var = internal global i32 0
+// CHECK-VIS-PROTECTED-DAG: @_ZZ11inline_funcvE3var = linkonce_odr protected global i32 0, comdat
+// CHECK-VIS-PROTECTED-DAG: @_ZZ18inline_hidden_funcvE3var = linkonce_odr hidden global i32 0, comdat
+// CHECK-VIS-PROTECTED-DAG: @_ZZ19inline_default_funcvE3var = linkonce_odr global i32 0, comdat
+// CHECK-VIS-PROTECTED-DAG: define protected i32 @_Z4funcv()
+// CHECK-VIS-PROTECTED-DAG: define hidden i32 @_Z11hidden_funcv()
+// CHECK-VIS-PROTECTED-DAG: define i32 @_Z12default_funcv()
+// CHECK-VIS-PROTECTED-DAG: define linkonce_odr hidden i32 @_Z11inline_funcv()
+// CHECK-VIS-PROTECTED-DAG: define linkonce_odr hidden i32 @_Z18inline_hidden_funcv()
+// CHECK-VIS-PROTECTED-DAG: define linkonce_odr i32 @_Z19inline_default_funcv()
 
 int func(void) {
   static int var = 0;
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1262,14 +1262,17 @@
         !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.
-    if (isa<VarDecl>(D) && useInlineVisibilityHidden(FD) &&
+    // If this is a static local inside a function which is marked hidden by
+    // -fvisibility-inlines-hidden, use the default value visibility for the
+    // static local, but not the function itself. If the function has an
+    // explicit visibility attribute, use that instead.
+    if (isa<VarDecl>(D) &&
         !(!hasExplicitVisibilityAlready(computation) &&
-          getExplicitVisibility(FD, computation))) {
+          getExplicitVisibility(FD, computation)) &&
+        useInlineVisibilityHidden(FD)) {
       assert(cast<VarDecl>(D)->isStaticLocal());
-      return LinkageInfo(VisibleNoLinkage, DefaultVisibility, false);
+      return LinkageInfo(VisibleNoLinkage,
+                         Context.getLangOpts().getValueVisibilityMode(), false);
     }
 
     LV = getLVForDecl(FD, computation);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53052.168894.patch
Type: text/x-patch
Size: 4608 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181009/7fb10f6e/attachment.bin>