[llvm] r187827 - Change the linkage of these global values to 'internal'.

[Bill Wendling]

This isn’t a workaround for a bug in the linker. As far as I know, this is a proper fix. To be honest, I don’t think we should worry too much about size when implementing an analysis on the code… Certainly you’re not shipping with the sanitizer code in there?

-bw

On Nov 15, 2013, at 1:43 AM, Kostya Serebryany <kcc at google.com> wrote:

> Bill, Alex,
> 
> This change makes the asan-private globals to stay in the symbol
> table, which costs code size:
> 
> % cat a.cc
> int x, y, z;
> int main() { }
> % clang -fsanitize=address  a.cc && nm a.out | grep asan_gen
> 000000000046f178 r __asan_gen_
> 000000000046724b r __asan_gen_1
> 000000000046d48a r __asan_gen_2
> 000000000046f17d r __asan_gen_3
> 
> E.g. for chrome's content_shell the size increase is 1.5% (6Mb), which is huge.
> 
> Is this CL a proper fix, or just a workaround for a linker bug on Mac?
> If this is due to Mac-specific linker bug, can we have InternalLinkage
> on Mac and PrivateLinkage on Linux?
> Anyway, Alex, if this is not something we can fix quickly please file
> a bug -- we need to get the binary size back.
> 
> --kcc
> 
> 
> On Thu, Aug 8, 2013 at 3:19 PM, Alexander Potapenko <glider at google.com> wrote:
>> Looks like your commit has fixed
>> https://code.google.com/p/address-sanitizer/issues/detail?id=171. Yay!
>> 
>> On Wed, Aug 7, 2013 at 2:52 AM, Bill Wendling <isanbard at gmail.com> wrote:
>>> Author: void
>>> Date: Tue Aug  6 17:52:42 2013
>>> New Revision: 187827
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=187827&view=rev
>>> Log:
>>> Change the linkage of these global values to 'internal'.
>>> 
>>> The globals being generated here were given the 'private' linkage type. However,
>>> this caused them to end up in different sections with the wrong prefix. E.g.,
>>> they would be in the __TEXT,__const section with an 'L' prefix instead of an 'l'
>>> (lowercase ell) prefix.
>>> 
>>> The problem is that the linker will eat a literal label with 'L'. If a weak
>>> symbol is then placed into the __TEXT,__const section near that literal, then it
>>> cannot distinguish between the literal and the weak symbol.
>>> 
>>> Part of the problems here was introduced because the address sanitizer converted
>>> some C strings into constant initializers with trailing nuls. (Thus putting them
>>> in the __const section with the wrong prefix.) The others were variables that
>>> the address sanitizer created but simply had the wrong linkage type.
>>> 
>>> Modified:
>>>    llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>    llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll
>>> 
>>> Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=187827&r1=187826&r2=187827&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Tue Aug  6 17:52:42 2013
>>> @@ -547,11 +547,11 @@ static size_t TypeSizeToSizeIndex(uint32
>>>   return Res;
>>> }
>>> 
>>> -// Create a constant for Str so that we can pass it to the run-time lib.
>>> +// \brief Create a constant for Str so that we can pass it to the run-time lib.
>>> static GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str) {
>>>   Constant *StrConst = ConstantDataArray::getString(M.getContext(), Str);
>>>   GlobalVariable *GV = new GlobalVariable(M, StrConst->getType(), true,
>>> -                            GlobalValue::PrivateLinkage, StrConst,
>>> +                            GlobalValue::InternalLinkage, StrConst,
>>>                             kAsanGenPrefix);
>>>   GV->setUnnamedAddr(true);  // Ok to merge these.
>>>   GV->setAlignment(1);  // Strings may not be merged w/o setting align 1.
>>> @@ -961,8 +961,11 @@ bool AddressSanitizerModule::runOnModule
>>>     GlobalVariable *Name = createPrivateGlobalForString(M, G->getName());
>>> 
>>>     // Create a new global variable with enough space for a redzone.
>>> +    GlobalValue::LinkageTypes Linkage = G->getLinkage();
>>> +    if (G->isConstant() && Linkage == GlobalValue::PrivateLinkage)
>>> +      Linkage = GlobalValue::InternalLinkage;
>>>     GlobalVariable *NewGlobal = new GlobalVariable(
>>> -        M, NewTy, G->isConstant(), G->getLinkage(),
>>> +        M, NewTy, G->isConstant(), Linkage,
>>>         NewInitializer, "", G, G->getThreadLocalMode());
>>>     NewGlobal->copyAttributesFrom(G);
>>>     NewGlobal->setAlignment(MinRZ);
>>> @@ -995,7 +998,7 @@ bool AddressSanitizerModule::runOnModule
>>> 
>>>   ArrayType *ArrayOfGlobalStructTy = ArrayType::get(GlobalStructTy, n);
>>>   GlobalVariable *AllGlobals = new GlobalVariable(
>>> -      M, ArrayOfGlobalStructTy, false, GlobalVariable::PrivateLinkage,
>>> +      M, ArrayOfGlobalStructTy, false, GlobalVariable::InternalLinkage,
>>>       ConstantArray::get(ArrayOfGlobalStructTy, Initializers), "");
>>> 
>>>   // Create calls for poisoning before initializers run and unpoisoning after.
>>> 
>>> Modified: llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll?rev=187827&r1=187826&r2=187827&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll (original)
>>> +++ llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll Tue Aug  6 17:52:42 2013
>>> @@ -16,5 +16,5 @@ declare void @_Z3fooPi(i32*)
>>> ; We create one global string constant for the stack frame above.
>>> ; It should have unnamed_addr and align 1.
>>> ; Make sure we don't create any other global constants.
>>> -; CHECK: = private unnamed_addr constant{{.*}}align 1
>>> -; CHECK-NOT: = private unnamed_addr constant
>>> +; CHECK: = internal unnamed_addr constant{{.*}}align 1
>>> +; CHECK-NOT: = internal unnamed_addr constant
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> 
>> --
>> Alexander Potapenko
>> Software Engineer
>> Google Moscow
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits