From 76c17d324ca877420e4be98638ef15a62b2efa4e Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Wed, 7 May 2014 22:57:20 +0000 Subject: [PATCH] IR: Don't allow non-default visibility on local linkage Visibilities of `hidden` and `protected` are meaningless for symbols with local linkage. - Change the assembler to reject non-default visibility on symbols with local linkage. - Change the bitcode reader to auto-upgrade `hidden` and `protected` to `default` when the linkage is local. - Update LangRef. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@208263 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.rst | 3 + lib/AsmParser/LLParser.cpp | 17 ++++ lib/Bitcode/Reader/BitcodeReader.cpp | 13 ++- test/Assembler/internal-hidden-alias.ll | 6 ++ test/Assembler/internal-hidden-function.ll | 7 ++ test/Assembler/internal-hidden-variable.ll | 4 + test/Assembler/internal-protected-alias.ll | 6 ++ test/Assembler/internal-protected-function.ll | 7 ++ test/Assembler/internal-protected-variable.ll | 4 + test/Assembler/private-hidden-alias.ll | 6 ++ test/Assembler/private-hidden-function.ll | 7 ++ test/Assembler/private-hidden-variable.ll | 4 + test/Assembler/private-protected-alias.ll | 6 ++ test/Assembler/private-protected-function.ll | 7 ++ test/Assembler/private-protected-variable.ll | 4 + .../local-linkage-default-visibility.3.4.ll | 79 ++++++++++++++++++ ...local-linkage-default-visibility.3.4.ll.bc | Bin 0 -> 924 bytes test/LTO/keep-used-puts-during-instcombine.ll | 4 +- .../GlobalOpt/alias-used-section.ll | 4 +- test/Transforms/GlobalOpt/atexit.ll | 2 +- test/Transforms/MergeFunc/crash.ll | 14 ++-- .../MergeFunc/inttoptr-address-space.ll | 6 +- test/Transforms/MergeFunc/inttoptr.ll | 14 ++-- 23 files changed, 199 insertions(+), 25 deletions(-) create mode 100644 test/Assembler/internal-hidden-alias.ll create mode 100644 test/Assembler/internal-hidden-function.ll create mode 100644 test/Assembler/internal-hidden-variable.ll create mode 100644 test/Assembler/internal-protected-alias.ll create mode 100644 test/Assembler/internal-protected-function.ll create mode 100644 test/Assembler/internal-protected-variable.ll create mode 100644 test/Assembler/private-hidden-alias.ll create mode 100644 test/Assembler/private-hidden-function.ll create mode 100644 test/Assembler/private-hidden-variable.ll create mode 100644 test/Assembler/private-protected-alias.ll create mode 100644 test/Assembler/private-protected-function.ll create mode 100644 test/Assembler/private-protected-variable.ll create mode 100644 test/Bitcode/local-linkage-default-visibility.3.4.ll create mode 100644 test/Bitcode/local-linkage-default-visibility.3.4.ll.bc diff --git a/docs/LangRef.rst b/docs/LangRef.rst index a5055f6605d..77433ab1422 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -440,6 +440,9 @@ styles: defining module will bind to the local symbol. That is, the symbol cannot be overridden by another module. +A symbol with ``internal`` or ``private`` linkage must have ``default`` +visibility. + .. _namedtypes: DLL Storage Classes diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 56422393e46..32037df958b 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -622,6 +622,11 @@ bool LLParser::ParseStandaloneMetadata() { return false; } +static bool isValidVisibilityForLinkage(unsigned V, unsigned L) { + return !GlobalValue::isLocalLinkage((GlobalValue::LinkageTypes)L) || + (GlobalValue::VisibilityTypes)V == GlobalValue::DefaultVisibility; +} + /// ParseAlias: /// ::= GlobalVar '=' OptionalVisibility OptionalDLLStorageClass 'alias' /// OptionalLinkage Aliasee @@ -646,6 +651,10 @@ bool LLParser::ParseAlias(const std::string &Name, LocTy NameLoc, if(!GlobalAlias::isValidLinkage(Linkage)) return Error(LinkageLoc, "invalid linkage type for alias"); + if (!isValidVisibilityForLinkage(Visibility, L)) + return Error(LinkageLoc, + "symbol with local linkage must have default visibility"); + Constant *Aliasee; LocTy AliaseeLoc = Lex.getLoc(); if (Lex.getKind() != lltok::kw_bitcast && @@ -714,6 +723,10 @@ bool LLParser::ParseAlias(const std::string &Name, LocTy NameLoc, bool LLParser::ParseGlobal(const std::string &Name, LocTy NameLoc, unsigned Linkage, bool HasLinkage, unsigned Visibility, unsigned DLLStorageClass) { + if (!isValidVisibilityForLinkage(Visibility, Linkage)) + return Error(NameLoc, + "symbol with local linkage must have default visibility"); + unsigned AddrSpace; bool IsConstant, UnnamedAddr, IsExternallyInitialized; GlobalVariable::ThreadLocalMode TLM; @@ -3014,6 +3027,10 @@ bool LLParser::ParseFunctionHeader(Function *&Fn, bool isDefine) { return Error(LinkageLoc, "invalid function linkage type"); } + if (!isValidVisibilityForLinkage(Visibility, Linkage)) + return Error(LinkageLoc, + "symbol with local linkage must have default visibility"); + if (!FunctionType::isValidReturnType(RetType)) return Error(RetTypeLoc, "invalid function return type"); diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 1b2cf76f3e6..d0ce237ee67 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -1856,7 +1856,9 @@ error_code BitcodeReader::ParseModule(bool Resume) { Section = SectionTable[Record[5]-1]; } GlobalValue::VisibilityTypes Visibility = GlobalValue::DefaultVisibility; - if (Record.size() > 6) + // Local linkage must have default visibility. + if (Record.size() > 6 && !GlobalValue::isLocalLinkage(Linkage)) + // FIXME: Change to an error if non-default in 4.0. Visibility = GetDecodedVisibility(Record[6]); GlobalVariable::ThreadLocalMode TLM = GlobalVariable::NotThreadLocal; @@ -1922,7 +1924,10 @@ error_code BitcodeReader::ParseModule(bool Resume) { return Error(InvalidID); Func->setSection(SectionTable[Record[6]-1]); } - Func->setVisibility(GetDecodedVisibility(Record[7])); + // Local linkage must have default visibility. + if (!Func->hasLocalLinkage()) + // FIXME: Change to an error if non-default in 4.0. + Func->setVisibility(GetDecodedVisibility(Record[7])); if (Record.size() > 8 && Record[8]) { if (Record[8]-1 > GCTable.size()) return Error(InvalidID); @@ -1964,7 +1969,9 @@ error_code BitcodeReader::ParseModule(bool Resume) { GlobalAlias *NewGA = new GlobalAlias(Ty, GetDecodedLinkage(Record[2]), "", nullptr, TheModule); // Old bitcode files didn't have visibility field. - if (Record.size() > 3) + // Local linkage must have default visibility. + if (Record.size() > 3 && !NewGA->hasLocalLinkage()) + // FIXME: Change to an error if non-default in 4.0. NewGA->setVisibility(GetDecodedVisibility(Record[3])); if (Record.size() > 4) NewGA->setDLLStorageClass(GetDecodedDLLStorageClass(Record[4])); diff --git a/test/Assembler/internal-hidden-alias.ll b/test/Assembler/internal-hidden-alias.ll new file mode 100644 index 00000000000..660514bb185 --- /dev/null +++ b/test/Assembler/internal-hidden-alias.ll @@ -0,0 +1,6 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@global = global i32 0 + +@alias = hidden alias internal i32* @global +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/internal-hidden-function.ll b/test/Assembler/internal-hidden-function.ll new file mode 100644 index 00000000000..193ed7c2891 --- /dev/null +++ b/test/Assembler/internal-hidden-function.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +define internal hidden void @function() { +; CHECK: symbol with local linkage must have default visibility +entry: + ret void +} diff --git a/test/Assembler/internal-hidden-variable.ll b/test/Assembler/internal-hidden-variable.ll new file mode 100644 index 00000000000..eddd06758a0 --- /dev/null +++ b/test/Assembler/internal-hidden-variable.ll @@ -0,0 +1,4 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@var = internal hidden global i32 0 +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/internal-protected-alias.ll b/test/Assembler/internal-protected-alias.ll new file mode 100644 index 00000000000..d78582684c5 --- /dev/null +++ b/test/Assembler/internal-protected-alias.ll @@ -0,0 +1,6 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@global = global i32 0 + +@alias = protected alias internal i32* @global +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/internal-protected-function.ll b/test/Assembler/internal-protected-function.ll new file mode 100644 index 00000000000..944cb75eec4 --- /dev/null +++ b/test/Assembler/internal-protected-function.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +define internal protected void @function() { +; CHECK: symbol with local linkage must have default visibility +entry: + ret void +} diff --git a/test/Assembler/internal-protected-variable.ll b/test/Assembler/internal-protected-variable.ll new file mode 100644 index 00000000000..df02275bac7 --- /dev/null +++ b/test/Assembler/internal-protected-variable.ll @@ -0,0 +1,4 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@var = internal protected global i32 0 +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/private-hidden-alias.ll b/test/Assembler/private-hidden-alias.ll new file mode 100644 index 00000000000..58be92a34f2 --- /dev/null +++ b/test/Assembler/private-hidden-alias.ll @@ -0,0 +1,6 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@global = global i32 0 + +@alias = hidden alias private i32* @global +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/private-hidden-function.ll b/test/Assembler/private-hidden-function.ll new file mode 100644 index 00000000000..dd73f0413b9 --- /dev/null +++ b/test/Assembler/private-hidden-function.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +define private hidden void @function() { +; CHECK: symbol with local linkage must have default visibility +entry: + ret void +} diff --git a/test/Assembler/private-hidden-variable.ll b/test/Assembler/private-hidden-variable.ll new file mode 100644 index 00000000000..ce6bfa9bae6 --- /dev/null +++ b/test/Assembler/private-hidden-variable.ll @@ -0,0 +1,4 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@var = private hidden global i32 0 +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/private-protected-alias.ll b/test/Assembler/private-protected-alias.ll new file mode 100644 index 00000000000..a72c248f0b0 --- /dev/null +++ b/test/Assembler/private-protected-alias.ll @@ -0,0 +1,6 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@global = global i32 0 + +@alias = protected alias private i32* @global +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Assembler/private-protected-function.ll b/test/Assembler/private-protected-function.ll new file mode 100644 index 00000000000..5dbb420a825 --- /dev/null +++ b/test/Assembler/private-protected-function.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +define private protected void @function() { +; CHECK: symbol with local linkage must have default visibility +entry: + ret void +} diff --git a/test/Assembler/private-protected-variable.ll b/test/Assembler/private-protected-variable.ll new file mode 100644 index 00000000000..c4458f5b3f6 --- /dev/null +++ b/test/Assembler/private-protected-variable.ll @@ -0,0 +1,4 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +@var = private protected global i32 0 +; CHECK: symbol with local linkage must have default visibility diff --git a/test/Bitcode/local-linkage-default-visibility.3.4.ll b/test/Bitcode/local-linkage-default-visibility.3.4.ll new file mode 100644 index 00000000000..45a7b1213a8 --- /dev/null +++ b/test/Bitcode/local-linkage-default-visibility.3.4.ll @@ -0,0 +1,79 @@ +; RUN: llvm-dis < %s.bc | FileCheck %s + +; local-linkage-default-visibility.3.4.ll.bc was generated by passing this file +; to llvm-as-3.4. The test checks that LLVM upgrades visibility of symbols +; with local linkage to default visibility. + +@default.internal.var = internal global i32 0 +; CHECK: @default.internal.var = internal global i32 0 + +@hidden.internal.var = internal hidden global i32 0 +; CHECK: @hidden.internal.var = internal global i32 0 + +@protected.internal.var = internal protected global i32 0 +; CHECK: @protected.internal.var = internal global i32 0 + +@default.private.var = private global i32 0 +; CHECK: @default.private.var = private global i32 0 + +@hidden.private.var = private hidden global i32 0 +; CHECK: @hidden.private.var = private global i32 0 + +@protected.private.var = private protected global i32 0 +; CHECK: @protected.private.var = private global i32 0 + +@global = global i32 0 + +@default.internal.alias = alias internal i32* @global +; CHECK: @default.internal.alias = alias internal i32* @global + +@hidden.internal.alias = hidden alias internal i32* @global +; CHECK: @hidden.internal.alias = alias internal i32* @global + +@protected.internal.alias = protected alias internal i32* @global +; CHECK: @protected.internal.alias = alias internal i32* @global + +@default.private.alias = alias private i32* @global +; CHECK: @default.private.alias = alias private i32* @global + +@hidden.private.alias = hidden alias private i32* @global +; CHECK: @hidden.private.alias = alias private i32* @global + +@protected.private.alias = protected alias private i32* @global +; CHECK: @protected.private.alias = alias private i32* @global + +define internal void @default.internal() { +; CHECK: define internal void @default.internal +entry: + ret void +} + +define internal hidden void @hidden.internal() { +; CHECK: define internal void @hidden.internal +entry: + ret void +} + +define internal protected void @protected.internal() { +; CHECK: define internal void @protected.internal +entry: + ret void +} + +define private void @default.private() { +; CHECK: define private void @default.private +entry: + ret void +} + +define private hidden void @hidden.private() { +; CHECK: define private void @hidden.private +entry: + ret void +} + +define private protected void @protected.private() { +; CHECK: define private void @protected.private +entry: + ret void +} diff --git a/test/Bitcode/local-linkage-default-visibility.3.4.ll.bc b/test/Bitcode/local-linkage-default-visibility.3.4.ll.bc new file mode 100644 index 0000000000000000000000000000000000000000..6e49f7e365b7bf457b04a49a22aa602092a22e49 GIT binary patch literal 924 zcmd6l&rcIU6vyBGVz=!AyWO?6v5;L_2!|dvEf_E5WU7!5~BJoHFRydw~@i69Aq=)oA#hMNfoFJ9EymPq{%oMh%r-uupHzB74RV&k*S z0#pIOvnTfm6sKD=M+Pn#}a@W#b}4kNSTM^Gx?Q;mT%TtN%R`|qQ}Y@oH#?K zR6o|8y8~BR!$~O=R$*wbr4uTcV`#1$Q=0*SHY%UiNO%q^+B#JdNB}>T@Zr#e?f}R~ zNI;K4S$2W}KLCg7uQ@2ys!B!%33EUxTLTpb6LGv9N>j?k`xEOHQyd)E$RRu;<001z zE>~tIBnq93wy%l--Y6AaUDtQW#3qAs3kZG@#Ta7-w~vEUA{Kv?=(Dzpnn^Fr(=)5n z>&C>T7dLg--&C>U>#N7OH zR-zGmS^~I%;$@{-$41^&ukvGu@ril=T?4NKvdp7scQ z@5Wb0<{NCY74FGzJxLi>YlM*kxmjsT?WtROK