[clang] 7eee2a2 - [IR] Don't allow DLL storage-class and local linkage
Ben Dunbobbin via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 29 16:26:14 PDT 2022
Author: Ben Dunbobbin
Date: 2022-09-30T00:26:01+01:00
New Revision: 7eee2a2d4401813cd485ae708e8cb0f94469e037
URL: https://github.com/llvm/llvm-project/commit/7eee2a2d4401813cd485ae708e8cb0f94469e037
DIFF: https://github.com/llvm/llvm-project/commit/7eee2a2d4401813cd485ae708e8cb0f94469e037.diff
LOG: [IR] Don't allow DLL storage-class and local linkage
Disallow this meaningless combination. Doing so simplifies analysis
of LLVM code w.r.t t DLL storage-class, and prevents mistakes with
DLL storage class.
- Change the assembler to reject DLL storage class on symbols with
local linkage.
- Change the bitcode reader to clear the DLL Storage class when the
linkage is local for auto-upgrading
- Update LangRef.
There is an existing restriction on non-default visibility and local
linkage which this is modelled on.
Differential Review: https://reviews.llvm.org/D134784
Added:
llvm/test/Assembler/dll-storage-class-local-linkage.ll
Modified:
clang/lib/CodeGen/CodeGenModule.cpp
llvm/docs/LangRef.rst
llvm/include/llvm/IR/GlobalValue.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/test/Transforms/GlobalOpt/alias-used-section.ll
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 8feb673e9393a..92920dae8111a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6013,10 +6013,13 @@ ConstantAddress CodeGenModule::GetAddrOfGlobalTemporary(
getModule(), Type, Constant, Linkage, InitialValue, Name.c_str(),
/*InsertBefore=*/nullptr, llvm::GlobalVariable::NotThreadLocal, TargetAS);
if (emitter) emitter->finalize(GV);
- setGVProperties(GV, VD);
- if (GV->getDLLStorageClass() == llvm::GlobalVariable::DLLExportStorageClass)
- // The reference temporary should never be dllexport.
- GV->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
+ // Don't assign dllimport or dllexport to local linkage globals.
+ if (!llvm::GlobalValue::isLocalLinkage(Linkage)) {
+ setGVProperties(GV, VD);
+ if (GV->getDLLStorageClass() == llvm::GlobalVariable::DLLExportStorageClass)
+ // The reference temporary should never be dllexport.
+ GV->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
+ }
GV->setAlignment(Align.getAsAlign());
if (supportsCOMDAT() && GV->isWeakForLinker())
GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 5b66ac0f76a64..1778a0a14f3b5 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -522,6 +522,9 @@ DLL storage class:
assembler and linker know it is externally referenced and must refrain from
deleting the symbol.
+A symbol with ``internal`` or ``private`` linkage cannot have a DLL storage
+class.
+
.. _tls_model:
Thread Local Storage Models
diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h
index 63c5d1b5f5c79..db290923dcf0c 100644
--- a/llvm/include/llvm/IR/GlobalValue.h
+++ b/llvm/include/llvm/IR/GlobalValue.h
@@ -275,7 +275,11 @@ class GlobalValue : public Constant {
bool hasDLLExportStorageClass() const {
return DllStorageClass == DLLExportStorageClass;
}
- void setDLLStorageClass(DLLStorageClassTypes C) { DllStorageClass = C; }
+ void setDLLStorageClass(DLLStorageClassTypes C) {
+ assert((!hasLocalLinkage() || C == DefaultStorageClass) &&
+ "local linkage requires DefaultStorageClass");
+ DllStorageClass = C;
+ }
bool hasSection() const { return !getSection().empty(); }
StringRef getSection() const;
@@ -524,8 +528,10 @@ class GlobalValue : public Constant {
}
void setLinkage(LinkageTypes LT) {
- if (isLocalLinkage(LT))
+ if (isLocalLinkage(LT)) {
Visibility = DefaultVisibility;
+ DllStorageClass = DefaultStorageClass;
+ }
Linkage = LT;
if (isImplicitDSOLocal())
setDSOLocal(true);
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 23445f4a71b9d..1b072f8451a06 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -967,6 +967,10 @@ static bool isValidVisibilityForLinkage(unsigned V, unsigned L) {
return !GlobalValue::isLocalLinkage((GlobalValue::LinkageTypes)L) ||
(GlobalValue::VisibilityTypes)V == GlobalValue::DefaultVisibility;
}
+static bool isValidDLLStorageClassForLinkage(unsigned S, unsigned L) {
+ return !GlobalValue::isLocalLinkage((GlobalValue::LinkageTypes)L) ||
+ (GlobalValue::DLLStorageClassTypes)S == GlobalValue::DefaultStorageClass;
+}
// If there was an explicit dso_local, update GV. In the absence of an explicit
// dso_local we keep the default value.
@@ -1020,6 +1024,10 @@ bool LLParser::parseAliasOrIFunc(const std::string &Name, LocTy NameLoc,
return error(NameLoc,
"symbol with local linkage must have default visibility");
+ if (!isValidDLLStorageClassForLinkage(DLLStorageClass, L))
+ return error(NameLoc,
+ "symbol with local linkage cannot have a DLL storage class");
+
Type *Ty;
LocTy ExplicitTypeLoc = Lex.getLoc();
if (parseType(Ty) ||
@@ -1207,6 +1215,10 @@ bool LLParser::parseGlobal(const std::string &Name, LocTy NameLoc,
return error(NameLoc,
"symbol with local linkage must have default visibility");
+ if (!isValidDLLStorageClassForLinkage(DLLStorageClass, Linkage))
+ return error(NameLoc,
+ "symbol with local linkage cannot have a DLL storage class");
+
unsigned AddrSpace;
bool IsConstant, IsExternallyInitialized;
LocTy IsExternallyInitializedLoc;
@@ -5656,6 +5668,10 @@ bool LLParser::parseFunctionHeader(Function *&Fn, bool IsDefine) {
return error(LinkageLoc,
"symbol with local linkage must have default visibility");
+ if (!isValidDLLStorageClassForLinkage(DLLStorageClass, Linkage))
+ return error(LinkageLoc,
+ "symbol with local linkage cannot have a DLL storage class");
+
if (!FunctionType::isValidReturnType(RetType))
return error(RetTypeLoc, "invalid function return type");
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index 0ed061497e951..bfce79c8deec3 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1298,6 +1298,9 @@ static FastMathFlags getDecodedFastMathFlags(unsigned Val) {
}
static void upgradeDLLImportExportLinkage(GlobalValue *GV, unsigned Val) {
+ // A GlobalValue with local linkage cannot have a DLL storage class.
+ if (GV->hasLocalLinkage())
+ return;
switch (Val) {
case 5: GV->setDLLStorageClass(GlobalValue::DLLImportStorageClass); break;
case 6: GV->setDLLStorageClass(GlobalValue::DLLExportStorageClass); break;
@@ -3764,10 +3767,14 @@ Error BitcodeReader::parseGlobalVarRecord(ArrayRef<uint64_t> Record) {
NewGV->setVisibility(Visibility);
NewGV->setUnnamedAddr(UnnamedAddr);
- if (Record.size() > 10)
- NewGV->setDLLStorageClass(getDecodedDLLStorageClass(Record[10]));
- else
+ if (Record.size() > 10) {
+ // A GlobalValue with local linkage cannot have a DLL storage class.
+ if (!NewGV->hasLocalLinkage()) {
+ NewGV->setDLLStorageClass(getDecodedDLLStorageClass(Record[10]));
+ }
+ } else {
upgradeDLLImportExportLinkage(NewGV, RawLinkage);
+ }
ValueList.push_back(NewGV, getVirtualTypeID(NewGV->getType(), TyID));
@@ -3928,10 +3935,14 @@ Error BitcodeReader::parseFunctionRecord(ArrayRef<uint64_t> Record) {
if (Record.size() > 10)
OperandInfo.Prologue = Record[10];
- if (Record.size() > 11)
- Func->setDLLStorageClass(getDecodedDLLStorageClass(Record[11]));
- else
+ if (Record.size() > 11) {
+ // A GlobalValue with local linkage cannot have a DLL storage class.
+ if (!Func->hasLocalLinkage()) {
+ Func->setDLLStorageClass(getDecodedDLLStorageClass(Record[11]));
+ }
+ } else {
upgradeDLLImportExportLinkage(Func, RawLinkage);
+ }
if (Record.size() > 12) {
if (unsigned ComdatID = Record[12]) {
@@ -4034,8 +4045,12 @@ Error BitcodeReader::parseGlobalIndirectSymbolRecord(
}
if (BitCode == bitc::MODULE_CODE_ALIAS ||
BitCode == bitc::MODULE_CODE_ALIAS_OLD) {
- if (OpNum != Record.size())
- NewGA->setDLLStorageClass(getDecodedDLLStorageClass(Record[OpNum++]));
+ if (OpNum != Record.size()) {
+ auto S = Record[OpNum++];
+ // A GlobalValue with local linkage cannot have a DLL storage class.
+ if (!NewGA->hasLocalLinkage())
+ NewGA->setDLLStorageClass(getDecodedDLLStorageClass(S));
+ }
else
upgradeDLLImportExportLinkage(NewGA, Linkage);
if (OpNum != Record.size())
diff --git a/llvm/test/Assembler/dll-storage-class-local-linkage.ll b/llvm/test/Assembler/dll-storage-class-local-linkage.ll
new file mode 100644
index 0000000000000..67429ef5eaa1d
--- /dev/null
+++ b/llvm/test/Assembler/dll-storage-class-local-linkage.ll
@@ -0,0 +1,88 @@
+;; Check that an error is emitted for local linkage symbols with DLL storage class.
+
+; RUN: rm -rf %t && split-file %s %t
+
+; RUN: not llvm-as %t/internal_function_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/internal_variable_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/internal_alias_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+
+; RUN: not llvm-as %t/private_function_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/private_variable_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/private_alias_dllexport.ll -o /dev/null 2>&1 | FileCheck %s
+
+; RUN: not llvm-as %t/internal_function_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/internal_variable_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/internal_alias_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+
+; RUN: not llvm-as %t/private_function_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/private_variable_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %t/private_alias_dllimport.ll -o /dev/null 2>&1 | FileCheck %s
+
+
+; CHECK: symbol with local linkage cannot have a DLL storage class
+
+
+;--- internal_function_dllexport.ll
+
+define internal dllexport void @function() {
+entry:
+ ret void
+}
+
+;--- internal_variable_dllexport.ll
+
+ at var = internal dllexport global i32 0
+
+;--- internal_alias_dllexport.ll
+
+ at global = global i32 0
+ at alias = internal dllexport alias i32, i32* @global
+
+;--- private_function_dllexport.ll
+
+define private dllexport void @function() {
+entry:
+ ret void
+}
+
+;--- private_variable_dllexport.ll
+
+ at var = private dllexport global i32 0
+
+;--- private_alias_dllexport.ll
+
+ at global = global i32 0
+ at alias = private dllexport alias i32, i32* @global
+
+
+;--- internal_function_dllimport.ll
+
+define internal dllimport void @function() {
+entry:
+ ret void
+}
+
+;--- internal_variable_dllimport.ll
+
+ at var = internal dllimport global i32 0
+
+;--- internal_alias_dllimport.ll
+
+ at global = global i32 0
+ at alias = internal dllimport alias i32, i32* @global
+
+;--- private_function_dllimport.ll
+
+define private dllimport void @function() {
+entry:
+ ret void
+}
+
+;--- private_variable_dllimport.ll
+
+ at var = private dllimport global i32 0
+
+;--- private_alias_dllimport.ll
+
+ at global = global i32 0
+ at alias = private dllimport alias i32, i32* @global
diff --git a/llvm/test/Transforms/GlobalOpt/alias-used-section.ll b/llvm/test/Transforms/GlobalOpt/alias-used-section.ll
index 389d77ae0a339..e0ef39d8f67bd 100644
--- a/llvm/test/Transforms/GlobalOpt/alias-used-section.ll
+++ b/llvm/test/Transforms/GlobalOpt/alias-used-section.ll
@@ -1,8 +1,8 @@
; RUN: opt -S -passes=globalopt < %s | FileCheck %s
@_Z17in_custom_section = internal global i8 42, section "CUSTOM"
- at in_custom_section = internal dllexport alias i8, i8* @_Z17in_custom_section
+ at in_custom_section = internal alias i8, i8* @_Z17in_custom_section
-; CHECK: @in_custom_section = internal dllexport global i8 42, section "CUSTOM"
+; CHECK: @in_custom_section = internal global i8 42, section "CUSTOM"
@llvm.used = appending global [1 x i8*] [i8* @in_custom_section], section "llvm.metadata"
More information about the cfe-commits
mailing list