Skip to content

Commit cd05ade

Browse files
bd1976brisbd1976bris
authored
Add a "don't override" mapping for -fvisibility-from-dllstorageclass (#74629)
`-fvisibility-from-dllstorageclass` allows for overriding the visibility of globals from their DLL storage class. The visibility to apply can be customised for the different classes of globals via a set of dependent options that specify the mapping values: - `-fvisibility-dllexport=<value>` - `-fvisibility-nodllstorageclass=<value>` - `-fvisibility-externs-dllimport=<value>` - `-fvisibility-externs-nodllstorageclass=<value>` Currently, one of the existing LLVM visibilities, `hidden`, `protected`, `default`, can be used as a mapping value. This change adds a new mapping value: `keep`, which specifies that the visibility should not be overridden for that class of globals. The behaviour of `-fvisibility-from-dllstorageclass` is otherwise unchanged and existing uses of this set of options will be unaffected. The background to this change is that currently the PS4 and PS5 compilers effectively ignore visibility - dllimport/export is the supported method for export control in C/C++ source code. Now, we would like to support visibility attributes and options in our frontend, in addition to dllimport/export. To support this, we will override the visibility of globals with explicit dllimport/export annotations but use the `keep` setting for globals which do not have an explicit dllimport/export. There are also some minor improvements to the existing options: - Make the `LANGOPS` `BENIGN` as they don't involve the AST. - Correct/clarify the help text for the options.
1 parent 2bfa5ca commit cd05ade

File tree

6 files changed

+160
-49
lines changed

6 files changed

+160
-49
lines changed

‎clang/include/clang/Basic/LangOptions.def‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -359,16 +359,16 @@ BENIGN_ENUM_LANGOPT(TypeVisibilityMode, Visibility, 3, DefaultVisibility,
359359
"default visibility for types [-ftype-visibility]")
360360
LANGOPT(SetVisibilityForExternDecls, 1, 0,
361361
"apply global symbol visibility to external declarations without an explicit visibility")
362-
LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
363-
"set the visiblity of globals from their DLL storage class [-fvisibility-from-dllstorageclass]")
364-
ENUM_LANGOPT(DLLExportVisibility, Visibility, 3, DefaultVisibility,
365-
"visibility for functions and variables with dllexport annotations [-fvisibility-from-dllstorageclass]")
366-
ENUM_LANGOPT(NoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
367-
"visibility for functions and variables without an explicit DLL storage class [-fvisibility-from-dllstorageclass]")
368-
ENUM_LANGOPT(ExternDeclDLLImportVisibility, Visibility, 3, DefaultVisibility,
369-
"visibility for external declarations with dllimport annotations [-fvisibility-from-dllstorageclass]")
370-
ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
371-
"visibility for external declarations without an explicit DLL storage class [-fvisibility-from-dllstorageclass]")
362+
BENIGN_LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
363+
"override the visibility of globals based on their final DLL storage class [-fvisibility-from-dllstorageclass]")
364+
BENIGN_ENUM_LANGOPT(DLLExportVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Default,
365+
"how to adjust the visibility for functions and variables with dllexport annotations [-fvisibility-dllexport]")
366+
BENIGN_ENUM_LANGOPT(NoDLLStorageClassVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Hidden,
367+
"how to adjust the visibility for functions and variables without an explicit DLL storage class [-fvisibility-nodllstorageclass]")
368+
BENIGN_ENUM_LANGOPT(ExternDeclDLLImportVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Default,
369+
"how to adjust the visibility for external declarations with dllimport annotations [-fvisibility-externs-dllimport]")
370+
BENIGN_ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Hidden,
371+
"how to adjust the visibility for external declarations without an explicit DLL storage class [-fvisibility-externs-nodllstorageclass]")
372372
BENIGN_LANGOPT(SemanticInterposition , 1, 0, "semantic interposition")
373373
BENIGN_LANGOPT(HalfNoSemanticInterposition, 1, 0,
374374
"Like -fno-semantic-interposition but don't use local aliases")

‎clang/include/clang/Basic/LangOptions.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,17 @@ class LangOptions : public LangOptionsBase {
381381
All,
382382
};
383383

384+
enum class VisibilityFromDLLStorageClassKinds {
385+
/// Keep the IR-gen assigned visibility.
386+
Keep,
387+
/// Override the IR-gen assigned visibility with default visibility.
388+
Default,
389+
/// Override the IR-gen assigned visibility with hidden visibility.
390+
Hidden,
391+
/// Override the IR-gen assigned visibility with protected visibility.
392+
Protected,
393+
};
394+
384395
enum class StrictFlexArraysLevelKind {
385396
/// Any trailing array member is a FAM.
386397
Default = 0,

‎clang/include/clang/Driver/Options.td‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3862,27 +3862,32 @@ def dA : Flag<["-"], "dA">, Alias<fverbose_asm>;
38623862
defm visibility_from_dllstorageclass : BoolFOption<"visibility-from-dllstorageclass",
38633863
LangOpts<"VisibilityFromDLLStorageClass">, DefaultFalse,
38643864
PosFlag<SetTrue, [], [ClangOption, CC1Option],
3865-
"Set the visibility of symbols in the generated code from their DLL storage class">,
3865+
"Override the visibility of globals based on their final DLL storage class.">,
38663866
NegFlag<SetFalse>>;
3867+
class MarshallingInfoVisibilityFromDLLStorage<KeyPathAndMacro kpm, code default>
3868+
: MarshallingInfoEnum<kpm, default>,
3869+
Values<"keep,hidden,protected,default">,
3870+
NormalizedValuesScope<"LangOptions::VisibilityFromDLLStorageClassKinds">,
3871+
NormalizedValues<["Keep", "Hidden", "Protected", "Default"]> {}
38673872
def fvisibility_dllexport_EQ : Joined<["-"], "fvisibility-dllexport=">, Group<f_Group>,
38683873
Visibility<[ClangOption, CC1Option]>,
3869-
HelpText<"The visibility for dllexport definitions [-fvisibility-from-dllstorageclass]">,
3870-
MarshallingInfoVisibility<LangOpts<"DLLExportVisibility">, "DefaultVisibility">,
3874+
HelpText<"The visibility for dllexport definitions. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
3875+
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"DLLExportVisibility">, "Default">,
38713876
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
38723877
def fvisibility_nodllstorageclass_EQ : Joined<["-"], "fvisibility-nodllstorageclass=">, Group<f_Group>,
38733878
Visibility<[ClangOption, CC1Option]>,
3874-
HelpText<"The visibility for definitions without an explicit DLL export class [-fvisibility-from-dllstorageclass]">,
3875-
MarshallingInfoVisibility<LangOpts<"NoDLLStorageClassVisibility">, "HiddenVisibility">,
3879+
HelpText<"The visibility for definitions without an explicit DLL storage class. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
3880+
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"NoDLLStorageClassVisibility">, "Hidden">,
38763881
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
38773882
def fvisibility_externs_dllimport_EQ : Joined<["-"], "fvisibility-externs-dllimport=">, Group<f_Group>,
38783883
Visibility<[ClangOption, CC1Option]>,
3879-
HelpText<"The visibility for dllimport external declarations [-fvisibility-from-dllstorageclass]">,
3880-
MarshallingInfoVisibility<LangOpts<"ExternDeclDLLImportVisibility">, "DefaultVisibility">,
3884+
HelpText<"The visibility for dllimport external declarations. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
3885+
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"ExternDeclDLLImportVisibility">, "Default">,
38813886
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
38823887
def fvisibility_externs_nodllstorageclass_EQ : Joined<["-"], "fvisibility-externs-nodllstorageclass=">, Group<f_Group>,
38833888
Visibility<[ClangOption, CC1Option]>,
3884-
HelpText<"The visibility for external declarations without an explicit DLL dllstorageclass [-fvisibility-from-dllstorageclass]">,
3885-
MarshallingInfoVisibility<LangOpts<"ExternDeclNoDLLStorageClassVisibility">, "HiddenVisibility">,
3889+
HelpText<"The visibility for external declarations without an explicit DLL storage class. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
3890+
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"ExternDeclNoDLLStorageClassVisibility">, "Hidden">,
38863891
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
38873892
def fvisibility_EQ : Joined<["-"], "fvisibility=">, Group<f_Group>,
38883893
Visibility<[ClangOption, CC1Option]>,

‎clang/lib/CodeGen/CodeGenModule.cpp‎

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -722,43 +722,70 @@ void InstrProfStats::reportDiagnostics(DiagnosticsEngine &Diags,
722722
}
723723
}
724724

725+
static std::optional<llvm::GlobalValue::VisibilityTypes>
726+
getLLVMVisibility(clang::LangOptions::VisibilityFromDLLStorageClassKinds K) {
727+
// Map to LLVM visibility.
728+
switch (K) {
729+
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Keep:
730+
return std::nullopt;
731+
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Default:
732+
return llvm::GlobalValue::DefaultVisibility;
733+
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Hidden:
734+
return llvm::GlobalValue::HiddenVisibility;
735+
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Protected:
736+
return llvm::GlobalValue::ProtectedVisibility;
737+
}
738+
llvm_unreachable("unknown option value!");
739+
}
740+
741+
void setLLVMVisibility(llvm::GlobalValue &GV,
742+
std::optional<llvm::GlobalValue::VisibilityTypes> V) {
743+
if (!V)
744+
return;
745+
746+
// Reset DSO locality before setting the visibility. This removes
747+
// any effects that visibility options and annotations may have
748+
// had on the DSO locality. Setting the visibility will implicitly set
749+
// appropriate globals to DSO Local; however, this will be pessimistic
750+
// w.r.t. to the normal compiler IRGen.
751+
GV.setDSOLocal(false);
752+
GV.setVisibility(*V);
753+
}
754+
725755
static void setVisibilityFromDLLStorageClass(const clang::LangOptions &LO,
726756
llvm::Module &M) {
727757
if (!LO.VisibilityFromDLLStorageClass)
728758
return;
729759

730-
llvm::GlobalValue::VisibilityTypes DLLExportVisibility =
731-
CodeGenModule::GetLLVMVisibility(LO.getDLLExportVisibility());
732-
llvm::GlobalValue::VisibilityTypes NoDLLStorageClassVisibility =
733-
CodeGenModule::GetLLVMVisibility(LO.getNoDLLStorageClassVisibility());
734-
llvm::GlobalValue::VisibilityTypes ExternDeclDLLImportVisibility =
735-
CodeGenModule::GetLLVMVisibility(LO.getExternDeclDLLImportVisibility());
736-
llvm::GlobalValue::VisibilityTypes ExternDeclNoDLLStorageClassVisibility =
737-
CodeGenModule::GetLLVMVisibility(
738-
LO.getExternDeclNoDLLStorageClassVisibility());
760+
std::optional<llvm::GlobalValue::VisibilityTypes> DLLExportVisibility =
761+
getLLVMVisibility(LO.getDLLExportVisibility());
762+
763+
std::optional<llvm::GlobalValue::VisibilityTypes>
764+
NoDLLStorageClassVisibility =
765+
getLLVMVisibility(LO.getNoDLLStorageClassVisibility());
766+
767+
std::optional<llvm::GlobalValue::VisibilityTypes>
768+
ExternDeclDLLImportVisibility =
769+
getLLVMVisibility(LO.getExternDeclDLLImportVisibility());
770+
771+
std::optional<llvm::GlobalValue::VisibilityTypes>
772+
ExternDeclNoDLLStorageClassVisibility =
773+
getLLVMVisibility(LO.getExternDeclNoDLLStorageClassVisibility());
739774

740775
for (llvm::GlobalValue &GV : M.global_values()) {
741776
if (GV.hasAppendingLinkage() || GV.hasLocalLinkage())
742777
continue;
743778

744-
// Reset DSO locality before setting the visibility. This removes
745-
// any effects that visibility options and annotations may have
746-
// had on the DSO locality. Setting the visibility will implicitly set
747-
// appropriate globals to DSO Local; however, this will be pessimistic
748-
// w.r.t. to the normal compiler IRGen.
749-
GV.setDSOLocal(false);
750-
751-
if (GV.isDeclarationForLinker()) {
752-
GV.setVisibility(GV.getDLLStorageClass() ==
753-
llvm::GlobalValue::DLLImportStorageClass
754-
? ExternDeclDLLImportVisibility
755-
: ExternDeclNoDLLStorageClassVisibility);
756-
} else {
757-
GV.setVisibility(GV.getDLLStorageClass() ==
758-
llvm::GlobalValue::DLLExportStorageClass
759-
? DLLExportVisibility
760-
: NoDLLStorageClassVisibility);
761-
}
779+
if (GV.isDeclarationForLinker())
780+
setLLVMVisibility(GV, GV.getDLLStorageClass() ==
781+
llvm::GlobalValue::DLLImportStorageClass
782+
? ExternDeclDLLImportVisibility
783+
: ExternDeclNoDLLStorageClassVisibility);
784+
else
785+
setLLVMVisibility(GV, GV.getDLLStorageClass() ==
786+
llvm::GlobalValue::DLLExportStorageClass
787+
? DLLExportVisibility
788+
: NoDLLStorageClassVisibility);
762789

763790
GV.setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
764791
}

‎clang/test/CodeGenCXX/visibility-dllstorageclass.cpp‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// REQUIRES: x86-registered-target
22

3-
// Test that -fvisibility-from-dllstorageclass maps DLL storage class to visibility
4-
// and that it overrides the effect of visibility options and annotations.
3+
//// Test that -fvisibility-from-dllstorageclass maps DLL storage class to visibility
4+
//// and that it overrides the effect of visibility options and annotations.
55

66
// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
77
// RUN: -fvisibility=hidden \
@@ -32,12 +32,35 @@
3232
// RUN: -x c++ %s -S -emit-llvm -o - | \
3333
// RUN: FileCheck %s --check-prefixes=ALL_DEFAULT
3434

35+
// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
36+
// RUN: -fvisibility=hidden \
37+
// RUN: -fapply-global-visibility-to-externs \
38+
// RUN: -fvisibility-from-dllstorageclass \
39+
// RUN: -fvisibility-dllexport=keep \
40+
// RUN: -fvisibility-nodllstorageclass=keep \
41+
// RUN: -fvisibility-externs-dllimport=keep \
42+
// RUN: -fvisibility-externs-nodllstorageclass=keep \
43+
// RUN: -x c++ %s -S -emit-llvm -o - | \
44+
// RUN: FileCheck %s --check-prefixes=ALL_KEEP
45+
46+
//// Show that omitting -fvisibility-from-dllstorageclass causes the other options to be ignored.
47+
// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
48+
// RUN: -fvisibility=hidden \
49+
// RUN: -fapply-global-visibility-to-externs \
50+
// RUN: -fvisibility-dllexport=protected \
51+
// RUN: -fvisibility-nodllstorageclass=protected \
52+
// RUN: -fvisibility-externs-dllimport=protected \
53+
// RUN: -fvisibility-externs-nodllstorageclass=protected \
54+
// RUN: -x c++ %s -S -emit-llvm -o - | \
55+
// RUN: FileCheck %s --check-prefixes=ALL_KEEP
56+
3557
// Local
3658
static void l() {}
3759
void use_locals(){l();}
3860
// DEFAULTS-DAG: define internal void @_ZL1lv()
3961
// EXPLICIT-DAG: define internal void @_ZL1lv()
4062
// ALL_DEFAULT-DAG: define internal void @_ZL1lv()
63+
// ALL_KEEP-DAG: define internal void @_ZL1lv()
4164

4265
// Function
4366
void f() {}
@@ -48,6 +71,8 @@ void __declspec(dllexport) exported_f() {}
4871
// EXPLICIT-DAG: define hidden void @_Z10exported_fv()
4972
// ALL_DEFAULT-DAG: define void @_Z1fv()
5073
// ALL_DEFAULT-DAG: define void @_Z10exported_fv()
74+
// ALL_KEEP-DAG: define hidden void @_Z1fv()
75+
// ALL_KEEP-DAG: define hidden void @_Z10exported_fv()
5176

5277
// Variable
5378
int d = 123;
@@ -58,6 +83,8 @@ __declspec(dllexport) int exported_d = 123;
5883
// EXPLICIT-DAG: @exported_d = hidden global
5984
// ALL_DEFAULT-DAG: @d = global
6085
// ALL_DEFAULT-DAG: @exported_d = global
86+
// ALL_KEEP-DAG: @d = hidden global
87+
// ALL_KEEP-DAG: @exported_d = hidden global
6188

6289
// Alias
6390
extern "C" void aliased() {}
@@ -69,6 +96,8 @@ void __declspec(dllexport) a_exported() __attribute__((alias("aliased")));
6996
// EXPLICIT-DAG: @_Z10a_exportedv = hidden alias
7097
// ALL_DEFAULT-DAG: @_Z1av = alias
7198
// ALL_DEFAULT-DAG: @_Z10a_exportedv = alias
99+
// ALL_KEEP-DAG: @_Z1av = hidden alias
100+
// ALL_KEEP-DAG: @_Z10a_exportedv = dso_local alias
72101

73102
// Declaration
74103
extern void e();
@@ -79,6 +108,8 @@ extern void __declspec(dllimport) imported_e();
79108
// EXPLICIT-DAG: declare hidden void @_Z10imported_ev()
80109
// ALL_DEFAULT-DAG: declare void @_Z1ev()
81110
// ALL_DEFAULT-DAG: declare void @_Z10imported_ev()
111+
// ALL_KEEP-DAG: declare hidden void @_Z1ev()
112+
// ALL_KEEP-DAG: declare void @_Z10imported_ev()
82113

83114
// Weak Declaration
84115
__attribute__((weak))
@@ -91,6 +122,8 @@ extern void __declspec(dllimport) imported_w();
91122
// EXPLICIT-DAG: declare extern_weak hidden void @_Z10imported_wv()
92123
// ALL_DEFAULT-DAG: declare extern_weak void @_Z1wv()
93124
// ALL_DEFAULT-DAG: declare extern_weak void @_Z10imported_wv()
125+
// ALL_KEEP-DAG: declare extern_weak hidden void @_Z1wv()
126+
// ALL_KEEP-DAG: declare extern_weak void @_Z10imported_wv()
94127

95128
void use_declarations(){e(); imported_e(); w(); imported_w();}
96129

@@ -101,11 +134,14 @@ struct __attribute__((type_visibility("protected"))) t {
101134
};
102135
void t::foo() {}
103136
// DEFAULTS-DAG: @_ZTV1t = hidden unnamed_addr constant
137+
// ALL_KEEP-DAG: @_ZTV1t = protected unnamed_addr constant
104138

105139
int v __attribute__ ((__visibility__ ("protected"))) = 123;
106140
// DEFAULTS-DAG: @v = hidden global
141+
// ALL_KEEP-DAG: @v = protected global
107142

108143
#pragma GCC visibility push(protected)
109144
int p = 345;
110145
#pragma GCC visibility pop
111146
// DEFAULTS-DAG: @p = hidden global
147+
// ALL_KEEP-DAG: @p = protected global

‎clang/test/Driver/visibility-dllstorageclass.c‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Check behaviour of -fvisibility-from-dllstorageclass options
1+
//// Check behaviour of -fvisibility-from-dllstorageclass options
22

33
// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
44
// RUN: -Werror -S -### %s 2>&1 | \
@@ -85,3 +85,35 @@
8585
// ALL-SAME: "-fvisibility-nodllstorageclass=protected"
8686
// ALL-SAME: "-fvisibility-externs-dllimport=hidden"
8787
// ALL-SAME: "-fvisibility-externs-nodllstorageclass=protected"
88+
89+
//// Test that "keep" can be specified, which means that the visibility of
90+
//// the matching globals will not be adjusted.
91+
92+
// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
93+
// RUN: -fvisibility-from-dllstorageclass \
94+
// RUN: -fvisibility-dllexport=keep \
95+
// RUN: -fvisibility-nodllstorageclass=keep \
96+
// RUN: -fvisibility-externs-dllimport=keep \
97+
// RUN: -fvisibility-externs-nodllstorageclass=keep \
98+
// RUN: -Werror -S -### %s 2>&1 | \
99+
// RUN: FileCheck %s --check-prefixes=KEEP
100+
101+
// KEEP: "-fvisibility-from-dllstorageclass"
102+
// KEEP-SAME: "-fvisibility-dllexport=keep"
103+
// KEEP-SAME: "-fvisibility-nodllstorageclass=keep"
104+
// KEEP-SAME: "-fvisibility-externs-dllimport=keep"
105+
// KEEP-SAME: "-fvisibility-externs-nodllstorageclass=keep"
106+
107+
// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
108+
// RUN: -fvisibility-from-dllstorageclass \
109+
// RUN: -fvisibility-dllexport=default \
110+
// RUN: -fvisibility-dllexport=keep \
111+
// RUN: -fvisibility-nodllstorageclass=default \
112+
// RUN: -fvisibility-nodllstorageclass=keep \
113+
// RUN: -fvisibility-externs-dllimport=default \
114+
// RUN: -fvisibility-externs-dllimport=keep \
115+
// RUN: -fvisibility-externs-nodllstorageclass=default \
116+
// RUN: -fvisibility-externs-nodllstorageclass=keep \
117+
// RUN: -Werror -S -### %s 2>&1 | \
118+
// RUN: FileCheck %s --check-prefixes=KEEP
119+

0 commit comments

Comments
 (0)