[llvm] r196973 - [asan] Fix the coverage.cc test broken by r196939

[Kostya Serebryany]

On Wed, Dec 11, 2013 at 1:49 AM, Reid Kleckner <reid at kleckner.net> wrote:

> Author: rnk
> Date: Tue Dec 10 15:49:28 2013
> New Revision: 196973
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196973&view=rev
> Log:
> [asan] Fix the coverage.cc test broken by r196939
>
Thanks!
This seems to have fixed (or somehow avoided) a miscopmpile in
http://llvm.org/bugs/show_bug.cgi?id=17907


>
> It was failing because ASan was adding all of the following to one
> function:
> - dynamic alloca
> - stack realignment
> - inline asm
>
> This patch avoids making the static alloca dynamic when coverage is
> used.
>

Good catch!


> ASan should probably not be inserting empty inline asm blobs to inhibit
> duplicate tail elimination.
>

We didn't find any alternative yet. And anyway, inline asm may come from
other places.
E.g. we extensively use it to break compiler optimizations in asan tests.


>
> Modified:
>     llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>
> Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=196973&r1=196972&r2=196973&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
> (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Tue Dec
> 10 15:49:28 2013
> @@ -1167,7 +1167,19 @@ bool AddressSanitizer::maybeInsertAsanIn
>  //  b) collect usage statistics to help improve Clang coverage design.
>  bool AddressSanitizer::InjectCoverage(Function &F) {
>    if (!ClCoverage) return false;
> -  IRBuilder<> IRB(F.getEntryBlock().getFirstInsertionPt());
> +
> +  // Skip static allocas at the top of the entry block so they don't
> become
> +  // dynamic when we split the block.  If we used our optimized stack
> layout,
> +  // then there will only be one alloca and it will come first.
> +  BasicBlock &Entry = F.getEntryBlock();
> +  BasicBlock::iterator IP = Entry.getFirstInsertionPt(), BE = Entry.end();
> +  for (; IP != BE; ++IP) {
> +    AllocaInst *AI = dyn_cast<AllocaInst>(IP);
>

Is there a guarantee that the Allocas  are the first insns after
getFirstInsertionPt?
(I know it is usually true).

+    if (!AI || !AI->isStaticAlloca())
> +      break;
> +  }
> +
> +  IRBuilder<> IRB(IP);
>    Type *Int8Ty = IRB.getInt8Ty();
>    GlobalVariable *Guard = new GlobalVariable(
>        *F.getParent(), Int8Ty, false, GlobalValue::PrivateLinkage,
>
>
> _______________________________________________
> 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/20131211/a3d62da6/attachment.html>