[PATCH] D11179: [asan] Invalid debug info for promotable allocas

[Kuba Brecka]

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

Hi everyone,

Since r230724 ("Skip promotable allocas to improve performance at -O0"), there is a regression in the generated debug info for those non-instrumented variables.  When inspecting such a variable's value in LLDB, you often get garbage instead of the actual value.  Fred was able to track this down:

> What happens is quite sad: The variable doesn’t get instrumented and thus should behave like a standard stack variable. However, ASAN’s instrumentation is inserted before the creation of the non-instrumented alloca. The only allocas that are considered standard stack variables are the ones declared in the first basic-block, but the initial instrumentation setup in the function breaks that invariant. Indeed, if moving the alloca to the first BB, things work. (This shows that the llvm.dbg.declare intrinsic is badly defined. Depending on whether the alloca it refers to is in the first BB or not, it means something quite different…)

> The simplest way that I can think of preventing this issue is to make sure that the uninstrumented allocas stay in the first BB. I think this is safe, because static promotable allocas shouldn’t depend on any other value, and moving them up they will still dominate their uses, thus the SSA form should be maintained. I might very well be missing some corner cases though. The following patch does that.


http://reviews.llvm.org/D11179

Files:
  lib/Transforms/Instrumentation/AddressSanitizer.cpp

Index: lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -525,6 +525,7 @@
   ShadowMapping Mapping;
 
   SmallVector<AllocaInst *, 16> AllocaVec;
+  SmallVector<AllocaInst *, 16> NonInstrumentedAllocaVec;
   SmallVector<Instruction *, 8> RetVec;
   unsigned StackAlignment;
 
@@ -625,7 +626,10 @@
 
   /// \brief Collect Alloca instructions we want (and can) handle.
   void visitAllocaInst(AllocaInst &AI) {
-    if (!ASan.isInterestingAlloca(AI)) return;
+    if (!ASan.isInterestingAlloca(AI)) {
+      if (AI.isStaticAlloca()) NonInstrumentedAllocaVec.push_back(&AI);
+      return;
+    }
 
     StackAlignment = std::max(StackAlignment, AI.getAlignment());
     if (ASan.isDynamicAlloca(AI))
@@ -1734,6 +1738,10 @@
   IRBuilder<> IRB(InsBefore);
   IRB.SetCurrentDebugLocation(EntryDebugLocation);
 
+  for (auto *AI : NonInstrumentedAllocaVec) {
+    AI->moveBefore(AllocaVec[0]);
+  }
+
   SmallVector<ASanStackVariableDescription, 16> SVD;
   SVD.reserve(AllocaVec.size());
   for (AllocaInst *AI : AllocaVec) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D11179.29649.patch
Type: text/x-patch
Size: 1206 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150714/4e87adb1/attachment.bin>