Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

[erik.pilkington]

Summary

This effectively disables https://reviews.llvm.org/D50968 on Darwin, and provides a command line flag to opt into/out of this behaviour. This change is needed to compile certain Apple headers correctly.

rdar://47688592

[jyknight]

But this (re-)breaks the functionality of -fvisibility-inlines-hidden on Darwin. That seems bad? I'd've liked to see more of an explanation as to why this was considered a necessary breakage.

[MaskRay]

Agreed. This feels like a workaround. Can the offending projects be fixed?

[arphaman]

Hey, thanks for following up on this PR. I've done some more digging and I think we can remove this Darwin-specific workaround in the near future. I'm hoping to provide an update in the next few weeks.

[sberg]

In D86881#2777808, @arphaman wrote:

Hey, thanks for following up on this PR. I've done some more digging and I think we can remove this Darwin-specific workaround in the near future. I'm hoping to provide an update in the next few weeks.

The Darwin -fvisibility-inlines-hidden-static-local-var hack is still present on Clang 16 trunk. Any update on this?

clang/include/clang/Basic/LangOptions.def

 BENIGN_LANGOPT(ModulesCodegen , 1, 0, "Modules code generation")
 BENIGN_LANGOPT(ModulesDebugInfo , 1, 0, "Modules debug info")
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd records in a simple form")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline C++ methods")
+BENIGN_LANGOPT(VisibilityInlinesHiddenStaticLocalVar, 1, 0,
+               "hidden visibility for static local variables in inline C++ "
+               "methods when -fvisibility-inlines hidden is enabled")
 LANGOPT(GlobalAllocationFunctionVisibilityHidden , 1, 0, "hidden visibility for global operator new and delete declaration")
 BENIGN_LANGOPT(ParseUnknownAnytype, 1, 0, "__unknown_anytype")
 BENIGN_LANGOPT(DebuggerSupport , 1, 0, "debugger support")
 BENIGN_LANGOPT(DebuggerCastResultToId, 1, 0, "for 'po' in the debugger, cast the result to id if it is of unknown type")
 BENIGN_LANGOPT(DebuggerObjCLiteral , 1, 0, "debugger Objective-C literals and subscripting support")
 
 BENIGN_LANGOPT(SpellChecking , 1, 1, "spell-checking")
 LANGOPT(SinglePrecisionConstants , 1, 0, "treating double-precision floating point constants as single precision constants")

clang/include/clang/Driver/Options.td

 def fverbose_asm : Flag<["-"], "fverbose-asm">, Group<f_Group>,
   HelpText<"Generate verbose assembly output">;
 def dA : Flag<["-"], "dA">, Alias<fverbose_asm>;
 def fvisibility_EQ : Joined<["-"], "fvisibility=">, Group<f_Group>,
   HelpText<"Set the default symbol visibility for all global declarations">, Values<"hidden,default">;
 def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">, Group<f_Group>,
   HelpText<"Give inline C++ member functions hidden visibility by default">,
   Flags<[CC1Option]>;
+def fvisibility_inlines_hidden_static_local_var :
+  Flag<["-"], "fvisibility-inlines-hidden-static-local-var">, Group<f_Group>,
+  HelpText<"When -fvisibility-inlines-hidden is enabled, static variables in "
+           "inline C++ member functions will also be given hidden visibility "
+           "by default">,
+  Flags<[CC1Option]>;
+def fno_visibility_inlines_hidden_static_local_var :
+  Flag<["-"], "fno-visibility-inlines-hidden-static-local-var">, Group<f_Group>,
+  HelpText<"Disables -fvisibility-inlines-hidden-static-local-var "
+           "(this is the default on non-darwin targets)">,
+  Flags<[CC1Option]>;
 def fvisibility_ms_compat : Flag<["-"], "fvisibility-ms-compat">, Group<f_Group>,
   HelpText<"Give global types 'default' visibility and global functions and "
            "variables 'hidden' visibility by default">;
 def fvisibility_global_new_delete_hidden : Flag<["-"], "fvisibility-global-new-delete-hidden">, Group<f_Group>,
   HelpText<"Give global C++ operator new and delete declarations hidden visibility">, Flags<[CC1Option]>;
 defm whole_program_vtables : OptInFFlag<"whole-program-vtables",
   "Enables whole-program vtable optimization. Requires -flto", "", "", [CoreOption]>;
 defm split_lto_unit : OptInFFlag<"split-lto-unit",

clang/lib/AST/Decl.cpp

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) &&
-        !LV.isVisibilityExplicit()) {
+        !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 =

clang/lib/Driver/ToolChains/Clang.cpp

if (A->getOption().matches(options::OPT_fvisibility_EQ)) {
       CmdArgs.push_back("-fvisibility");
       CmdArgs.push_back("hidden");
       CmdArgs.push_back("-ftype-visibility");
       CmdArgs.push_back("default");
     }
   }
 
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
+  Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden_static_local_var,
+                           options::OPT_fno_visibility_inlines_hidden_static_local_var);
   Args.AddLastArg(CmdArgs, options::OPT_fvisibility_global_new_delete_hidden);
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
   // Forward -f (flag) options which we can pass directly.
   Args.AddLastArg(CmdArgs, options::OPT_femit_all_decls);
   Args.AddLastArg(CmdArgs, options::OPT_fheinous_gnu_extensions);
   Args.AddLastArg(CmdArgs, options::OPT_fdigraphs, options::OPT_fno_digraphs);

clang/lib/Driver/ToolChains/Darwin.cpp

if (SDKInfo) {
     llvm::raw_string_ostream OS(Arg);
     OS << "-target-sdk-version=" << SDKInfo->getVersion();
     CC1Args.push_back(DriverArgs.MakeArgString(OS.str()));
   }
 
   // Enable compatibility mode for NSItemProviderCompletionHandler in
   // Foundation/NSItemProvider.h.
   CC1Args.push_back("-fcompatibility-qualified-id-block-type-checking");
+
+  // Give static local variables in inline functions hidden visibility when
+  // -fvisibility-inlines-hidden is enabled.
+  if (!DriverArgs.getLastArgNoClaim(
+          options::OPT_fvisibility_inlines_hidden_static_local_var,
+          options::OPT_fno_visibility_inlines_hidden_static_local_var))
+    CC1Args.push_back("-fvisibility-inlines-hidden-static-local-var");
 }
 
 DerivedArgList *
 Darwin::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
                       Action::OffloadKind DeviceOffloadKind) const {
   // First get the generic Apple args, before moving onto Darwin-specific ones.
   DerivedArgList *DAL =
       MachO::TranslateArgs(Args, BoundArch, DeviceOffloadKind);

clang/lib/Frontend/CompilerInvocation.cpp

if (Arg *typeVisOpt = Args.getLastArg(OPT_ftype_visibility)) {
     Opts.setTypeVisibilityMode(parseVisibility(typeVisOpt, Args, Diags));
   } else {
     Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
   if (Args.hasArg(OPT_fvisibility_inlines_hidden))
     Opts.InlineVisibilityHidden = 1;
 
+  if (Args.hasArg(OPT_fvisibility_inlines_hidden_static_local_var))
+    Opts.VisibilityInlinesHiddenStaticLocalVar = 1;
+
   if (Args.hasArg(OPT_fvisibility_global_new_delete_hidden))
     Opts.GlobalAllocationFunctionVisibilityHidden = 1;
 
   if (Args.hasArg(OPT_fapply_global_visibility_to_externs))
     Opts.SetVisibilityForExternDecls = 1;
 
   if (Args.hasArg(OPT_ftrapv)) {
     Opts.setSignedOverflowBehavior(LangOptions::SOB_Trapping);

clang/test/CodeGenCXX/visibility-inlines-hidden-static-local-var.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fvisibility-inlines-hidden -fvisibility-inlines-hidden-static-local-var %s -emit-llvm -o - | FileCheck %s
+
+#define used __attribute__((used))
+
+used inline void f1() {
+  // CHECK: @_ZZ2f1vE6f1_var = linkonce_odr hidden global i32 0
+  static int f1_var = 0;
+}
+
+__attribute__((visibility("default")))
+used inline void f2() {
+  // CHECK: @_ZZ2f2vE6f2_var = linkonce_odr global i32 0
+  static int f2_var = 0;
+}
+
+struct S {
+  used void f3() {
+    // CHECK: @_ZZN1S2f3EvE6f3_var = linkonce_odr hidden global i32 0
+    static int f3_var = 0;
+  }
+
+  void f6();
+  void f7();
+};
+
+used void f4() {
+  // CHECK: @_ZZ2f4vE6f4_var = internal global i32 0
+  static int f4_var = 0;
+}
+
+__attribute__((visibility("default")))
+used void f5() {
+  // CHECK: @_ZZ2f5vE6f5_var = internal global i32 0
+  static int f5_var = 0;
+}
+
+used void S::f6() {
+  // CHECK: @_ZZN1S2f6EvE6f6_var = internal global i32 0
+  static int f6_var = 0;
+}
+
+used inline void S::f7() {
+  // CHECK: @_ZZN1S2f7EvE6f7_var = linkonce_odr hidden global i32 0
+  static int f7_var = 0;
+}
+
+
+struct __attribute__((visibility("default"))) S2 {
+  used void f8() {
+    // CHECK: @_ZZN2S22f8EvE6f8_var = linkonce_odr hidden global i32 0
+    static int f8_var = 0;
+  }
+};

clang/test/Driver/darwin-objc-options.m

 // Don't crash with an unexpected target triple.
 // RUN: %clang -target i386-apple-ios7 -S -### %s
 
 // Add -fcompatibility-qualified-id-block-type-checking only on Darwin.
 // RUN: %clang -target x86_64-apple-darwin10 -### %s 2>&1 | FileCheck --check-prefix=DARWIN_COMPATIBILITY %s
 // RUN: %clang -target x86_64-linux-gnu -### %s 2>&1 | FileCheck --check-prefix=OTHER_COMPATIBILITY %s
 // DARWIN_COMPATIBILITY: -fcompatibility-qualified-id-block-type-checking
 // OTHER_COMPATIBILITY-NOT: -fcompatibility-qualified-id-block-type-checking
+
+// Add -fvisibility-inlines-hidden-static-local-var on Darwin.
+// RUN: %clang -target x86_64-apple-darwin10 -### %s 2>&1 | FileCheck --check-prefix=DARWIN_INLINES_HIDDEN %s
+// RUN: %clang -target x86_64-apple-darwin10 -fno-visibility-inlines-hidden-static-local-var -### %s 2>&1 | FileCheck --check-prefix=DARWIN_INLINES_HIDDEN_EXPLICIT_NO %s
+// RUN: %clang -target x86_64-linux-gnu -### %s 2>&1 | FileCheck --check-prefix=NO_DARWIN_INLINES_HIDDEN %s
+// DARWIN_INLINES_HIDDEN: -fvisibility-inlines-hidden-static-local-var
+// DARWIN_INLINES_HIDDEN_EXPLICIT_NO-NOT: -fvisibility-inlines-hidden-static-local-var
+// DARWIN_INLINES_HIDDEN_EXPLICIT_NO: -fno-visibility-inlines-hidden-static-local-var
+// NO_DARWIN_INLINES_HIDDEN-NOT: -fvisibility-inlines-hidden-static-local-var