[PATCH] D11339: [asan] Improve moving of non-instrumented allocas

[Kuba Brecka]

kubabrecka created this revision.
kubabrecka added subscribers: llvm-commits, samsonov, kcc, glider, zaks.anna, friss, gribozavr.

In r242510, non-instrumented allocas are now moved into the first basic block.  This patch limits that to:

1) Only move allocas that were originally in the first basic block.
2) Only move allocas that are present *after* the first instrumented one (i.e. only move allocas up).

A testcase was updated to show behavior in these two cases.  Without the patch, an alloca could be moved down, and could cause an invalid IR.

http://reviews.llvm.org/D11339

Files:
  lib/Transforms/Instrumentation/AddressSanitizer.cpp
  test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll

Index: test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll
===================================================================
--- test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll
+++ test/Instrumentation/AddressSanitizer/debug_info_noninstrumented_alloca.ll
@@ -9,18 +9,31 @@
 
 define i32 @foo() sanitize_address {
 entry:
+  ; Won't be instrumented because of asan-skip-promotable-allocas.
+  %non_instrumented1 = alloca i32, align 4
+
   ; Regular alloca, will get instrumented (forced by the ptrtoint below).
   %instrumented = alloca i32, align 4
 
   ; Won't be instrumented because of asan-skip-promotable-allocas.
-  %non_instrumented = alloca i32, align 4
-  store i32 0, i32* %non_instrumented, align 4
-  %value = load i32, i32* %non_instrumented, align 4
+  %non_instrumented2 = alloca i32, align 4
+
+  br label %bb0
+
+bb0:
+  ; Won't be instrumented because of asan-skip-promotable-allocas.
+  %non_instrumented3 = alloca i32, align 4
+
+  %ptr = ptrtoint i32* %instrumented to i32
+  br label %bb1
 
-  %ptr = ptrtoint i32* %instrumented to i64
-  ret i32 %value
+bb1:
+  ret i32 %ptr
 }
 
 ; CHECK: entry:
-; CHECK: %non_instrumented = alloca i32, align 4
+; CHECK: %non_instrumented1 = alloca i32, align 4
+; CHECK: %non_instrumented2 = alloca i32, align 4
 ; CHECK: load i32, i32* @__asan_option_detect_stack_use_after_return
+; CHECK: bb0:
+; CHECK: %non_instrumented3 = alloca i32, align 4
Index: lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -527,7 +528,7 @@
   ShadowMapping Mapping;
 
   SmallVector<AllocaInst *, 16> AllocaVec;
-  SmallVector<AllocaInst *, 16> NonInstrumentedStaticAllocaVec;
+  SmallSetVector<AllocaInst *, 16> NonInstrumentedStaticAllocaVec;
   SmallVector<Instruction *, 8> RetVec;
   unsigned StackAlignment;
 
@@ -629,7 +630,7 @@
   /// \brief Collect Alloca instructions we want (and can) handle.
   void visitAllocaInst(AllocaInst &AI) {
     if (!ASan.isInterestingAlloca(AI)) {
-      if (AI.isStaticAlloca()) NonInstrumentedStaticAllocaVec.push_back(&AI);
+      if (AI.isStaticAlloca()) NonInstrumentedStaticAllocaVec.insert(&AI);
       return;
     }
 
@@ -1743,7 +1744,11 @@
   // Make sure non-instrumented allocas stay in the first basic block.
   // Otherwise, debug info is broken, because only first-basic-block allocas are
   // treated as regular stack slots.
-  for (auto *AI : NonInstrumentedStaticAllocaVec) AI->moveBefore(InsBefore);
+  if (InsBefore->getParent() == &F.getEntryBlock())
+    for (BasicBlock::iterator I = InsBefore; I != F.getEntryBlock().end(); ++I)
+      if (auto *AI = dyn_cast_or_null<AllocaInst>(I))
+        if (NonInstrumentedStaticAllocaVec.count(AI) > 0)
+          AI->moveBefore(InsBefore);
 
   SmallVector<ASanStackVariableDescription, 16> SVD;
   SVD.reserve(AllocaVec.size());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11339.30114.patch
Type: text/x-patch
Size: 3275 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150719/80fdc883/attachment.bin>