[llvm] r187827 - Change the linkage of these global values to 'internal'.
Kostya Serebryany
kcc at google.com
Mon Nov 18 00:03:45 PST 2013
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131118/48846e01/attachment.html>
More information about the llvm-commits
mailing list