[llvm-commits] [llvm] r160256 - in /llvm/trunk: lib/Transforms/Instrumentation/AddressSanitizer.cpp test/Instrumentation/AddressSanitizer/basic.ll

[Chandler Carruth]

Author: chandlerc
Date: Mon Jul 16 05:01:02 2012
New Revision: 160256

URL: http://llvm.org/viewvc/llvm-project?rev=160256&view=rev
Log:
Revert r160254 temporarily.

It turns out that ASan relied on the at-the-end block insertion order to
(purely by happenstance) disable some LLVM optimizations, which in turn
start firing when the ordering is made more "normal". These
optimizations in turn merge many of the instrumentation reporting calls
which breaks the return address based error reporting in ASan.

We're looking at several different options for fixing this.

Modified:
    llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
    llvm/trunk/test/Instrumentation/AddressSanitizer/basic.ll

Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=160256&r1=160255&r2=160256&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Mon Jul 16 05:01:02 2012
@@ -230,17 +230,17 @@
 // Returns the ThenBlock's terminator.
 static BranchInst *splitBlockAndInsertIfThen(Value *Cmp) {
   Instruction *SplitBefore = cast<Instruction>(Cmp)->getNextNode();
-
-  // Create three basic blocks, with the middle block empty, by splitting twice.
   BasicBlock *Head = SplitBefore->getParent();
-  BasicBlock *Then = Head->splitBasicBlock(SplitBefore);
-  BasicBlock *Tail = Then->splitBasicBlock(SplitBefore);
-
+  BasicBlock *Tail = Head->splitBasicBlock(SplitBefore);
   TerminatorInst *HeadOldTerm = Head->getTerminator();
-  IRBuilder<>(HeadOldTerm).CreateCondBr(Cmp, Then, Tail);
-  HeadOldTerm->eraseFromParent();
+  LLVMContext &C = Head->getParent()->getParent()->getContext();
+  BasicBlock *ThenBlock = BasicBlock::Create(C, "", Head->getParent());
+  BranchInst *HeadNewTerm =
+    BranchInst::Create(/*ifTrue*/ThenBlock, /*ifFalse*/Tail, Cmp);
+  ReplaceInstWithInst(HeadOldTerm, HeadNewTerm);
 
-  return cast<BranchInst>(Then->getTerminator());
+  BranchInst *CheckTerm = BranchInst::Create(Tail, ThenBlock);
+  return CheckTerm;
 }
 
 Value *AddressSanitizer::memToShadow(Value *Shadow, IRBuilder<> &IRB) {
@@ -387,28 +387,28 @@
   Value *Cmp = IRB.CreateICmpNE(ShadowValue, CmpVal);
 
   Instruction *CheckTerm = splitBlockAndInsertIfThen(Cmp);
-  IRB.SetInsertPoint(CheckTerm);
+  IRBuilder<> IRB2(CheckTerm);
 
   size_t Granularity = 1 << MappingScale;
   if (TypeSize < 8 * Granularity) {
     // Addr & (Granularity - 1)
-    Value *LastAccessedByte = IRB.CreateAnd(
+    Value *LastAccessedByte = IRB2.CreateAnd(
         AddrLong, ConstantInt::get(IntptrTy, Granularity - 1));
     // (Addr & (Granularity - 1)) + size - 1
     if (TypeSize / 8 > 1)
-      LastAccessedByte = IRB.CreateAdd(
+      LastAccessedByte = IRB2.CreateAdd(
           LastAccessedByte, ConstantInt::get(IntptrTy, TypeSize / 8 - 1));
     // (uint8_t) ((Addr & (Granularity-1)) + size - 1)
-    LastAccessedByte = IRB.CreateIntCast(
+    LastAccessedByte = IRB2.CreateIntCast(
         LastAccessedByte, IRB.getInt8Ty(), false);
     // ((uint8_t) ((Addr & (Granularity-1)) + size - 1)) >= ShadowValue
-    Value *Cmp2 = IRB.CreateICmpSGE(LastAccessedByte, ShadowValue);
+    Value *Cmp2 = IRB2.CreateICmpSGE(LastAccessedByte, ShadowValue);
 
     CheckTerm = splitBlockAndInsertIfThen(Cmp2);
-    IRB.SetInsertPoint(CheckTerm);
   }
 
-  Instruction *Crash = generateCrashCode(IRB, AddrLong, IsWrite, TypeSize);
+  IRBuilder<> IRB1(CheckTerm);
+  Instruction *Crash = generateCrashCode(IRB1, AddrLong, IsWrite, TypeSize);
   Crash->setDebugLoc(OrigIns->getDebugLoc());
   ReplaceInstWithInst(CheckTerm, new UnreachableInst(*C));
 }

Modified: llvm/trunk/test/Instrumentation/AddressSanitizer/basic.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/basic.ll?rev=160256&r1=160255&r2=160256&view=diff
==============================================================================
--- llvm/trunk/test/Instrumentation/AddressSanitizer/basic.ll (original)
+++ llvm/trunk/test/Instrumentation/AddressSanitizer/basic.ll Mon Jul 16 05:01:02 2012
@@ -16,6 +16,11 @@
 ; CHECK:   icmp ne i8
 ; CHECK:   br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
 ;
+; The actual load comes next because ASan adds the last instrumentation block
+; to the end of the function.
+; CHECK:   %tmp1 = load i32* %a
+; CHECK:   ret i32 %tmp1
+;
 ; First instrumentation block refines the shadow test.
 ; CHECK:   and i64 %[[LOAD_ADDR]], 7
 ; CHECK:   add i64 %{{.*}}, 3
@@ -23,13 +28,9 @@
 ; CHECK:   icmp sge i8 %{{.*}}, %[[LOAD_SHADOW]]
 ; CHECK:   br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
 ;
-; Second instrumentation block reports the error.
+; Final instrumentation block reports the error.
 ; CHECK:   call void @__asan_report_load4(i64 %[[LOAD_ADDR]]) noreturn
 ; CHECK:   unreachable
-;
-; Finally the instrumented load.
-; CHECK:   %tmp1 = load i32* %a
-; CHECK:   ret i32 %tmp1
 
 entry:
   %tmp1 = load i32* %a
@@ -47,6 +48,11 @@
 ; CHECK:   icmp ne i8
 ; CHECK:   br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
 ;
+; The actual store comes next because ASan adds the last instrumentation block
+; to the end of the function.
+; CHECK:   store i32 42, i32* %a
+; CHECK:   ret void
+;
 ; First instrumentation block refines the shadow test.
 ; CHECK:   and i64 %[[STORE_ADDR]], 7
 ; CHECK:   add i64 %{{.*}}, 3
@@ -54,13 +60,9 @@
 ; CHECK:   icmp sge i8 %{{.*}}, %[[STORE_SHADOW]]
 ; CHECK:   br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
 ;
-; Second instrumentation block reports the error.
+; Final instrumentation block reports the error.
 ; CHECK:   call void @__asan_report_store4(i64 %[[STORE_ADDR]]) noreturn
 ; CHECK:   unreachable
-;
-; Finally the instrumented store.
-; CHECK:   store i32 42, i32* %a
-; CHECK:   ret void
 
 entry:
   store i32 42, i32* %a