<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div>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. <br><br>-bw</div><div><br>On Nov 18, 2013, at 12:03 AM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Nov 18, 2013 at 11:53 AM, Bill Wendling <span dir="ltr"><<a href="mailto:isanbard@gmail.com" target="_blank">isanbard@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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… </blockquote>
<div><br></div><div>We do worry about code size a lot. </div><div>asan is used at a large scale and so 1% of code size translates into some significant $$. </div><div>Besides, certain environments limit the code size due to resource constraints and even 1% code size increase may trigger a failure.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Certainly you’re not shipping with the sanitizer code in there?<br></blockquote><div><br></div><div><br></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-bw<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Nov 15, 2013, at 1:43 AM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
<br>
> Bill, Alex,<br>
><br>
> This change makes the asan-private globals to stay in the symbol<br>
> table, which costs code size:<br>
><br>
> % cat <a href="http://a.cc">a.cc</a><br>
> int x, y, z;<br>
> int main() { }<br>
> % clang -fsanitize=address  <a href="http://a.cc">a.cc</a> && nm a.out | grep asan_gen<br>
> 000000000046f178 r __asan_gen_<br>
> 000000000046724b r __asan_gen_1<br>
> 000000000046d48a r __asan_gen_2<br>
> 000000000046f17d r __asan_gen_3<br>
><br>
> E.g. for chrome's content_shell the size increase is 1.5% (6Mb), which is huge.<br>
><br>
> Is this CL a proper fix, or just a workaround for a linker bug on Mac?<br>
> If this is due to Mac-specific linker bug, can we have InternalLinkage<br>
> on Mac and PrivateLinkage on Linux?<br>
> Anyway, Alex, if this is not something we can fix quickly please file<br>
> a bug -- we need to get the binary size back.<br>
><br>
> --kcc<br>
><br>
><br>
> On Thu, Aug 8, 2013 at 3:19 PM, Alexander Potapenko <<a href="mailto:glider@google.com">glider@google.com</a>> wrote:<br>
>> Looks like your commit has fixed<br>
>> <a href="https://code.google.com/p/address-sanitizer/issues/detail?id=171" target="_blank">https://code.google.com/p/address-sanitizer/issues/detail?id=171</a>. Yay!<br>
>><br>
>> On Wed, Aug 7, 2013 at 2:52 AM, Bill Wendling <<a href="mailto:isanbard@gmail.com">isanbard@gmail.com</a>> wrote:<br>
>>> Author: void<br>
>>> Date: Tue Aug  6 17:52:42 2013<br>
>>> New Revision: 187827<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=187827&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=187827&view=rev</a><br>
>>> Log:<br>
>>> Change the linkage of these global values to 'internal'.<br>
>>><br>
>>> The globals being generated here were given the 'private' linkage type. However,<br>
>>> this caused them to end up in different sections with the wrong prefix. E.g.,<br>
>>> they would be in the __TEXT,__const section with an 'L' prefix instead of an 'l'<br>
>>> (lowercase ell) prefix.<br>
>>><br>
>>> The problem is that the linker will eat a literal label with 'L'. If a weak<br>
>>> symbol is then placed into the __TEXT,__const section near that literal, then it<br>
>>> cannot distinguish between the literal and the weak symbol.<br>
>>><br>
>>> Part of the problems here was introduced because the address sanitizer converted<br>
>>> some C strings into constant initializers with trailing nuls. (Thus putting them<br>
>>> in the __const section with the wrong prefix.) The others were variables that<br>
>>> the address sanitizer created but simply had the wrong linkage type.<br>
>>><br>
>>> Modified:<br>
>>>    llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
>>>    llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll<br>
>>><br>
>>> Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=187827&r1=187826&r2=187827&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=187827&r1=187826&r2=187827&view=diff</a><br>

>>> ==============================================================================<br>
>>> --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp (original)<br>
>>> +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Tue Aug  6 17:52:42 2013<br>
>>> @@ -547,11 +547,11 @@ static size_t TypeSizeToSizeIndex(uint32<br>
>>>   return Res;<br>
>>> }<br>
>>><br>
>>> -// Create a constant for Str so that we can pass it to the run-time lib.<br>
>>> +// \brief Create a constant for Str so that we can pass it to the run-time lib.<br>
>>> static GlobalVariable *createPrivateGlobalForString(Module &M, StringRef Str) {<br>
>>>   Constant *StrConst = ConstantDataArray::getString(M.getContext(), Str);<br>
>>>   GlobalVariable *GV = new GlobalVariable(M, StrConst->getType(), true,<br>
>>> -                            GlobalValue::PrivateLinkage, StrConst,<br>
>>> +                            GlobalValue::InternalLinkage, StrConst,<br>
>>>                             kAsanGenPrefix);<br>
>>>   GV->setUnnamedAddr(true);  // Ok to merge these.<br>
>>>   GV->setAlignment(1);  // Strings may not be merged w/o setting align 1.<br>
>>> @@ -961,8 +961,11 @@ bool AddressSanitizerModule::runOnModule<br>
>>>     GlobalVariable *Name = createPrivateGlobalForString(M, G->getName());<br>
>>><br>
>>>     // Create a new global variable with enough space for a redzone.<br>
>>> +    GlobalValue::LinkageTypes Linkage = G->getLinkage();<br>
>>> +    if (G->isConstant() && Linkage == GlobalValue::PrivateLinkage)<br>
>>> +      Linkage = GlobalValue::InternalLinkage;<br>
>>>     GlobalVariable *NewGlobal = new GlobalVariable(<br>
>>> -        M, NewTy, G->isConstant(), G->getLinkage(),<br>
>>> +        M, NewTy, G->isConstant(), Linkage,<br>
>>>         NewInitializer, "", G, G->getThreadLocalMode());<br>
>>>     NewGlobal->copyAttributesFrom(G);<br>
>>>     NewGlobal->setAlignment(MinRZ);<br>
>>> @@ -995,7 +998,7 @@ bool AddressSanitizerModule::runOnModule<br>
>>><br>
>>>   ArrayType *ArrayOfGlobalStructTy = ArrayType::get(GlobalStructTy, n);<br>
>>>   GlobalVariable *AllGlobals = new GlobalVariable(<br>
>>> -      M, ArrayOfGlobalStructTy, false, GlobalVariable::PrivateLinkage,<br>
>>> +      M, ArrayOfGlobalStructTy, false, GlobalVariable::InternalLinkage,<br>
>>>       ConstantArray::get(ArrayOfGlobalStructTy, Initializers), "");<br>
>>><br>
>>>   // Create calls for poisoning before initializers run and unpoisoning after.<br>
>>><br>
>>> Modified: llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll<br>
>>> URL: <a href="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" target="_blank">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</a><br>

>>> ==============================================================================<br>
>>> --- llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll (original)<br>
>>> +++ llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-instrument-internal-globals.ll Tue Aug  6 17:52:42 2013<br>
>>> @@ -16,5 +16,5 @@ declare void @_Z3fooPi(i32*)<br>
>>> ; We create one global string constant for the stack frame above.<br>
>>> ; It should have unnamed_addr and align 1.<br>
>>> ; Make sure we don't create any other global constants.<br>
>>> -; CHECK: = private unnamed_addr constant{{.*}}align 1<br>
>>> -; CHECK-NOT: = private unnamed_addr constant<br>
>>> +; CHECK: = internal unnamed_addr constant{{.*}}align 1<br>
>>> +; CHECK-NOT: = internal unnamed_addr constant<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
>><br>
>> --<br>
>> Alexander Potapenko<br>
>> Software Engineer<br>
>> Google Moscow<br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>
</div></blockquote></body></html>