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

Alexander Potapenko glider at google.com
Mon Nov 18 01:59:40 PST 2013


I've opened http://llvm.org/bugs/show_bug.cgi?id=17976 to track this
issue, let us move the discussion there.
I'll copy the thread in a moment.

On Mon, Nov 18, 2013 at 1:39 PM, Bill Wendling <isanbard at gmail.com> wrote:
> Okay. What would need to happen is to figure out how to allow these symbols
> to be private and to be coalesced. You create a private global for this
> string with a trailing red zone. It's the initializing of the red zone with
> nuls that places it into the wrong section. If you could find a way to
> create this structure but retain the other attributes you need while keeping
> it in the correct section, then that'd be the way to go.
>
> -bw
>
> On Nov 18, 2013, at 12:03 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>
>
>
> On Mon, Nov 18, 2013 at 11:53 AM, Bill Wendling <isanbard at gmail.com> wrote:
>>
>> 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…
>
>
> We do worry about code size a lot.
> asan is used at a large scale and so 1% of code size translates into some
> significant $$.
> Besides, certain environments limit the code size due to resource
> constraints and even 1% code size increase may trigger a failure.
>
>>
>> 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
>>
>



-- 
Alexander Potapenko
Software Engineer
Google Moscow




More information about the llvm-commits mailing list