Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "don't override" mapping for -fvisibility-from-dllstorageclass #74629

Merged
merged 5 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions clang/include/clang/Basic/LangOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -357,16 +357,16 @@ BENIGN_ENUM_LANGOPT(TypeVisibilityMode, Visibility, 3, DefaultVisibility,
"default visibility for types [-ftype-visibility]")
LANGOPT(SetVisibilityForExternDecls, 1, 0,
"apply global symbol visibility to external declarations without an explicit visibility")
LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
"set the visiblity of globals from their DLL storage class [-fvisibility-from-dllstorageclass]")
ENUM_LANGOPT(DLLExportVisibility, Visibility, 3, DefaultVisibility,
"visibility for functions and variables with dllexport annotations [-fvisibility-from-dllstorageclass]")
ENUM_LANGOPT(NoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
"visibility for functions and variables without an explicit DLL storage class [-fvisibility-from-dllstorageclass]")
ENUM_LANGOPT(ExternDeclDLLImportVisibility, Visibility, 3, DefaultVisibility,
"visibility for external declarations with dllimport annotations [-fvisibility-from-dllstorageclass]")
ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, Visibility, 3, HiddenVisibility,
"visibility for external declarations without an explicit DLL storage class [-fvisibility-from-dllstorageclass]")
BENIGN_LANGOPT(VisibilityFromDLLStorageClass, 1, 0,
"override the visibility of globals based on their final DLL storage class [-fvisibility-from-dllstorageclass]")
BENIGN_ENUM_LANGOPT(DLLExportVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Default,
"how to adjust the visibility for functions and variables with dllexport annotations [-fvisibility-dllexport]")
BENIGN_ENUM_LANGOPT(NoDLLStorageClassVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Hidden,
"how to adjust the visibility for functions and variables without an explicit DLL storage class [-fvisibility-nodllstorageclass]")
BENIGN_ENUM_LANGOPT(ExternDeclDLLImportVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Default,
"how to adjust the visibility for external declarations with dllimport annotations [-fvisibility-externs-dllimport]")
BENIGN_ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, VisibilityFromDLLStorageClassKinds, 3, VisibilityFromDLLStorageClassKinds::Hidden,
"how to adjust the visibility for external declarations without an explicit DLL storage class [-fvisibility-externs-nodllstorageclass]")
BENIGN_LANGOPT(SemanticInterposition , 1, 0, "semantic interposition")
BENIGN_LANGOPT(HalfNoSemanticInterposition, 1, 0,
"Like -fno-semantic-interposition but don't use local aliases")
Expand Down
11 changes: 11 additions & 0 deletions clang/include/clang/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,17 @@ class LangOptions : public LangOptionsBase {
All,
};

enum class VisibilityFromDLLStorageClassKinds {
/// Keep the IR-gen assigned visibility.
Keep,
/// Override the IR-gen assigned visibility with default visibility.
Default,
/// Override the IR-gen assigned visibility with hidden visibility.
Hidden,
/// Override the IR-gen assigned visibility with protected visibility.
Protected,
};

enum class StrictFlexArraysLevelKind {
/// Any trailing array member is a FAM.
Default = 0,
Expand Down
23 changes: 14 additions & 9 deletions clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -3821,27 +3821,32 @@ def dA : Flag<["-"], "dA">, Alias<fverbose_asm>;
defm visibility_from_dllstorageclass : BoolFOption<"visibility-from-dllstorageclass",
LangOpts<"VisibilityFromDLLStorageClass">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option],
"Set the visibility of symbols in the generated code from their DLL storage class">,
"Override the visibility of globals based on their final DLL storage class.">,
NegFlag<SetFalse>>;
class MarshallingInfoVisibilityFromDLLStorage<KeyPathAndMacro kpm, code default>
: MarshallingInfoEnum<kpm, default>,
Values<"keep,hidden,protected,default">,
NormalizedValuesScope<"LangOptions::VisibilityFromDLLStorageClassKinds">,
NormalizedValues<["Keep", "Hidden", "Protected", "Default"]> {}
def fvisibility_dllexport_EQ : Joined<["-"], "fvisibility-dllexport=">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"The visibility for dllexport definitions [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibility<LangOpts<"DLLExportVisibility">, "DefaultVisibility">,
HelpText<"The visibility for dllexport definitions. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"DLLExportVisibility">, "Default">,
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
def fvisibility_nodllstorageclass_EQ : Joined<["-"], "fvisibility-nodllstorageclass=">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"The visibility for definitions without an explicit DLL export class [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibility<LangOpts<"NoDLLStorageClassVisibility">, "HiddenVisibility">,
HelpText<"The visibility for definitions without an explicit DLL storage class. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"NoDLLStorageClassVisibility">, "Hidden">,
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
def fvisibility_externs_dllimport_EQ : Joined<["-"], "fvisibility-externs-dllimport=">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"The visibility for dllimport external declarations [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibility<LangOpts<"ExternDeclDLLImportVisibility">, "DefaultVisibility">,
HelpText<"The visibility for dllimport external declarations. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"ExternDeclDLLImportVisibility">, "Default">,
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
def fvisibility_externs_nodllstorageclass_EQ : Joined<["-"], "fvisibility-externs-nodllstorageclass=">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"The visibility for external declarations without an explicit DLL dllstorageclass [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibility<LangOpts<"ExternDeclNoDLLStorageClassVisibility">, "HiddenVisibility">,
HelpText<"The visibility for external declarations without an explicit DLL storage class. If Keep is specified the visibility is not adjusted [-fvisibility-from-dllstorageclass]">,
MarshallingInfoVisibilityFromDLLStorage<LangOpts<"ExternDeclNoDLLStorageClassVisibility">, "Hidden">,
ShouldParseIf<fvisibility_from_dllstorageclass.KeyPath>;
def fvisibility_EQ : Joined<["-"], "fvisibility=">, Group<f_Group>,
Visibility<[ClangOption, CC1Option]>,
Expand Down
81 changes: 54 additions & 27 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -721,43 +721,70 @@ void InstrProfStats::reportDiagnostics(DiagnosticsEngine &Diags,
}
}

static std::optional<llvm::GlobalValue::VisibilityTypes>
getLLVMVisibility(clang::LangOptions::VisibilityFromDLLStorageClassKinds K) {
// Map to LLVM visibility.
switch (K) {
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Keep:
return std::nullopt;
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Default:
return llvm::GlobalValue::DefaultVisibility;
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Hidden:
return llvm::GlobalValue::HiddenVisibility;
case clang::LangOptions::VisibilityFromDLLStorageClassKinds::Protected:
return llvm::GlobalValue::ProtectedVisibility;
}
llvm_unreachable("unknown option value!");
}

void setLLVMVisibility(llvm::GlobalValue &GV,
std::optional<llvm::GlobalValue::VisibilityTypes> V) {
if (!V)
return;

// Reset DSO locality before setting the visibility. This removes
// any effects that visibility options and annotations may have
// had on the DSO locality. Setting the visibility will implicitly set
// appropriate globals to DSO Local; however, this will be pessimistic
// w.r.t. to the normal compiler IRGen.
GV.setDSOLocal(false);
GV.setVisibility(*V);
}

static void setVisibilityFromDLLStorageClass(const clang::LangOptions &LO,
llvm::Module &M) {
if (!LO.VisibilityFromDLLStorageClass)
return;

llvm::GlobalValue::VisibilityTypes DLLExportVisibility =
CodeGenModule::GetLLVMVisibility(LO.getDLLExportVisibility());
llvm::GlobalValue::VisibilityTypes NoDLLStorageClassVisibility =
CodeGenModule::GetLLVMVisibility(LO.getNoDLLStorageClassVisibility());
llvm::GlobalValue::VisibilityTypes ExternDeclDLLImportVisibility =
CodeGenModule::GetLLVMVisibility(LO.getExternDeclDLLImportVisibility());
llvm::GlobalValue::VisibilityTypes ExternDeclNoDLLStorageClassVisibility =
CodeGenModule::GetLLVMVisibility(
LO.getExternDeclNoDLLStorageClassVisibility());
std::optional<llvm::GlobalValue::VisibilityTypes> DLLExportVisibility =
getLLVMVisibility(LO.getDLLExportVisibility());

std::optional<llvm::GlobalValue::VisibilityTypes>
NoDLLStorageClassVisibility =
getLLVMVisibility(LO.getNoDLLStorageClassVisibility());

std::optional<llvm::GlobalValue::VisibilityTypes>
ExternDeclDLLImportVisibility =
getLLVMVisibility(LO.getExternDeclDLLImportVisibility());

std::optional<llvm::GlobalValue::VisibilityTypes>
ExternDeclNoDLLStorageClassVisibility =
getLLVMVisibility(LO.getExternDeclNoDLLStorageClassVisibility());

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

// Reset DSO locality before setting the visibility. This removes
// any effects that visibility options and annotations may have
// had on the DSO locality. Setting the visibility will implicitly set
// appropriate globals to DSO Local; however, this will be pessimistic
// w.r.t. to the normal compiler IRGen.
GV.setDSOLocal(false);

if (GV.isDeclarationForLinker()) {
GV.setVisibility(GV.getDLLStorageClass() ==
llvm::GlobalValue::DLLImportStorageClass
? ExternDeclDLLImportVisibility
: ExternDeclNoDLLStorageClassVisibility);
} else {
GV.setVisibility(GV.getDLLStorageClass() ==
llvm::GlobalValue::DLLExportStorageClass
? DLLExportVisibility
: NoDLLStorageClassVisibility);
}
if (GV.isDeclarationForLinker())
setLLVMVisibility(GV, GV.getDLLStorageClass() ==
llvm::GlobalValue::DLLImportStorageClass
? ExternDeclDLLImportVisibility
: ExternDeclNoDLLStorageClassVisibility);
else
setLLVMVisibility(GV, GV.getDLLStorageClass() ==
llvm::GlobalValue::DLLExportStorageClass
? DLLExportVisibility
: NoDLLStorageClassVisibility);

GV.setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
}
Expand Down
39 changes: 37 additions & 2 deletions clang/test/CodeGenCXX/visibility-dllstorageclass.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// REQUIRES: x86-registered-target

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

// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -fvisibility=hidden \
Expand Down Expand Up @@ -32,12 +32,34 @@
// RUN: -x c++ %s -S -emit-llvm -o - | \
// RUN: FileCheck %s --check-prefixes=ALL_DEFAULT

// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -fvisibility=hidden \
// RUN: -fapply-global-visibility-to-externs \
// RUN: -fvisibility-from-dllstorageclass \
// RUN: -fvisibility-dllexport=keep \
// RUN: -fvisibility-nodllstorageclass=keep \
// RUN: -fvisibility-externs-dllimport=keep \
// RUN: -fvisibility-externs-nodllstorageclass=keep \
// RUN: -x c++ %s -S -emit-llvm -o - | \
// RUN: FileCheck %s --check-prefixes=ALL_KEEP

// RUN: %clang_cc1 -triple x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -fvisibility=hidden \
// RUN: -fapply-global-visibility-to-externs \
// RUN: -fvisibility-dllexport=protected \
// RUN: -fvisibility-nodllstorageclass=protected \
// RUN: -fvisibility-externs-dllimport=protected \
// RUN: -fvisibility-externs-nodllstorageclass=protected \
// RUN: -x c++ %s -S -emit-llvm -o - | \
// RUN: FileCheck %s --check-prefixes=ALL_KEEP

// Local
static void l() {}
void use_locals(){l();}
// DEFAULTS-DAG: define internal void @_ZL1lv()
// EXPLICIT-DAG: define internal void @_ZL1lv()
// ALL_DEFAULT-DAG: define internal void @_ZL1lv()
// ALL_KEEP-DAG: define internal void @_ZL1lv()

// Function
void f() {}
Expand All @@ -48,6 +70,8 @@ void __declspec(dllexport) exported_f() {}
// EXPLICIT-DAG: define hidden void @_Z10exported_fv()
// ALL_DEFAULT-DAG: define void @_Z1fv()
// ALL_DEFAULT-DAG: define void @_Z10exported_fv()
// ALL_KEEP-DAG: define hidden void @_Z1fv()
// ALL_KEEP-DAG: define hidden void @_Z10exported_fv()

// Variable
int d = 123;
Expand All @@ -58,6 +82,8 @@ __declspec(dllexport) int exported_d = 123;
// EXPLICIT-DAG: @exported_d = hidden global
// ALL_DEFAULT-DAG: @d = global
// ALL_DEFAULT-DAG: @exported_d = global
// ALL_KEEP-DAG: @d = hidden global
// ALL_KEEP-DAG: @exported_d = hidden global

// Alias
extern "C" void aliased() {}
Expand All @@ -69,6 +95,8 @@ void __declspec(dllexport) a_exported() __attribute__((alias("aliased")));
// EXPLICIT-DAG: @_Z10a_exportedv = hidden alias
// ALL_DEFAULT-DAG: @_Z1av = alias
// ALL_DEFAULT-DAG: @_Z10a_exportedv = alias
// ALL_KEEP-DAG: @_Z1av = hidden alias
// ALL_KEEP-DAG: @_Z10a_exportedv = dso_local alias

// Declaration
extern void e();
Expand All @@ -79,6 +107,8 @@ extern void __declspec(dllimport) imported_e();
// EXPLICIT-DAG: declare hidden void @_Z10imported_ev()
// ALL_DEFAULT-DAG: declare void @_Z1ev()
// ALL_DEFAULT-DAG: declare void @_Z10imported_ev()
// ALL_KEEP-DAG: declare hidden void @_Z1ev()
// ALL_KEEP-DAG: void @_Z10imported_ev()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this line looks like it would allow any visibility (missing declare and whatever might be between that and the void)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Thanks the declare is missing here. I have added it back.


// Weak Declaration
__attribute__((weak))
Expand All @@ -91,6 +121,8 @@ extern void __declspec(dllimport) imported_w();
// EXPLICIT-DAG: declare extern_weak hidden void @_Z10imported_wv()
// ALL_DEFAULT-DAG: declare extern_weak void @_Z1wv()
// ALL_DEFAULT-DAG: declare extern_weak void @_Z10imported_wv()
// ALL_KEEP-DAG: declare extern_weak hidden void @_Z1wv()
// ALL_KEEP-DAG: declare extern_weak void @_Z10imported_wv()

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

Expand All @@ -101,11 +133,14 @@ struct __attribute__((type_visibility("protected"))) t {
};
void t::foo() {}
// DEFAULTS-DAG: @_ZTV1t = hidden unnamed_addr constant
// ALL_KEEP-DAG: @_ZTV1t = protected unnamed_addr constant

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

#pragma GCC visibility push(protected)
int p = 345;
#pragma GCC visibility pop
// DEFAULTS-DAG: @p = hidden global
// ALL_KEEP-DAG: @p = protected global
34 changes: 33 additions & 1 deletion clang/test/Driver/visibility-dllstorageclass.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Check behaviour of -fvisibility-from-dllstorageclass options
//// Check behaviour of -fvisibility-from-dllstorageclass options

// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -Werror -S -### %s 2>&1 | \
Expand Down Expand Up @@ -85,3 +85,35 @@
// ALL-SAME: "-fvisibility-nodllstorageclass=protected"
// ALL-SAME: "-fvisibility-externs-dllimport=hidden"
// ALL-SAME: "-fvisibility-externs-nodllstorageclass=protected"

//// Test that "keep" can be specified, which means that the visibility of
//// the matching globals will not be adjusted.

// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -fvisibility-from-dllstorageclass \
// RUN: -fvisibility-dllexport=keep \
// RUN: -fvisibility-nodllstorageclass=keep \
// RUN: -fvisibility-externs-dllimport=keep \
// RUN: -fvisibility-externs-nodllstorageclass=keep \
// RUN: -Werror -S -### %s 2>&1 | \
// RUN: FileCheck %s --check-prefixes=KEEP

// KEEP: "-fvisibility-from-dllstorageclass"
// KEEP-SAME: "-fvisibility-dllexport=keep"
// KEEP-SAME: "-fvisibility-nodllstorageclass=keep"
// KEEP-SAME: "-fvisibility-externs-dllimport=keep"
// KEEP-SAME: "-fvisibility-externs-nodllstorageclass=keep"

// RUN: %clang -target x86_64-unknown-windows-itanium -fdeclspec \
// RUN: -fvisibility-from-dllstorageclass \
// RUN: -fvisibility-dllexport=default \
// RUN: -fvisibility-dllexport=keep \
// RUN: -fvisibility-nodllstorageclass=default \
// RUN: -fvisibility-nodllstorageclass=keep \
// RUN: -fvisibility-externs-dllimport=default \
// RUN: -fvisibility-externs-dllimport=keep \
// RUN: -fvisibility-externs-nodllstorageclass=default \
// RUN: -fvisibility-externs-nodllstorageclass=keep \
// RUN: -Werror -S -### %s 2>&1 | \
// RUN: FileCheck %s --check-prefixes=KEEP