[PATCH] asan: do not instrument direct inbounds accesses to stack variables

[Dmitry Vyukov]

Hi kcc, chandlerc,

This is most likely wrong.

But it eliminates 33% of instrumentation on webrtc/modules_unittests (number of memory accesses goes down from 290152 to 193998) and reduces binary size by 15% (from 74M to 64M) _and_ all existing asan tests pass.
This means that either our tests are bad or we are missing a good optimization opportunity.

Currently we instrument even the following code, which looks wrong:

define void @foo() uwtable sanitize_address {
entry:
  %a = alloca i32, align 4
  store i32 42, i32* %a, align 4
  ret void
}

I've found one case which fails with this change. We do not instrument the store in this case:

define void @bar() sanitize_address {
entry:
  %a = alloca [10 x i32], align 4
  %e = getelementptr inbounds [10 x i32]* %a, i32 0, i64 12
  store i32 42, i32* %e, align 4
  ret void
; CHECK-LABEL: define void @bar
; CHECK: __asan_report
; CHECK: ret void
}

However, compiler should be able to recognize such cases (and it does because it issues a warning).

I am a bit lost.
There is Value::isGEPWithNoNotionalOverIndexing which looks like what we need (should eliminate array accesses with constant inbounds indices and struct field accesses). However, it does not give the desired effect.
Value::stripInBoundsConstantOffsets also looks like what we need and it gives the desired effect. However, there is a note that "Value::isGEPWithNoNotionalOverIndexing is not equivalant to, a subset of, or a superset of the "inbounds" property". Which suggests that stripInBoundsConstantOffsets is not what we want.

Also, is it possible to refer to an AllocaInst outside of its lifetime? It is not possible in C/C++ due to scoping rules, you simply can't mention a name that is out of scope. However, I am not sure about llvm and its transformations.

I am looking for advice here.

http://reviews.llvm.org/D7583

Files:
  lib/Transforms/Instrumentation/AddressSanitizer.cpp
  test/Instrumentation/AddressSanitizer/instrument-stack.ll

Index: lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -211,6 +211,8 @@
           "Number of optimized accesses to global arrays");
 STATISTIC(NumOptimizedAccessesToGlobalVar,
           "Number of optimized accesses to global vars");
+STATISTIC(NumOptimizedAccessesToStackVar,
+          "Number of optimized accesses to stack vars");
 
 namespace {
 /// Frontend-provided metadata for source location.
@@ -876,6 +878,22 @@
     }
   }
 
+  // A direct inbounds access to a stack variable is always valid.
+  if (isa<AllocaInst>(Addr->stripInBoundsConstantOffsets())) {
+    NumOptimizedAccessesToStackVar++;
+    return;
+  }
+/*
+  This does not give the desired result.
+  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Addr)) {
+    if (CE->isGEPWithNoNotionalOverIndexing() &&
+        isa<AllocaInst>(CE->getOperand(0))) {
+      NumOptimizedAccessesToStackVar++;
+      return;
+    }
+  }
+*/
+
   Type *OrigPtrTy = Addr->getType();
   Type *OrigTy = cast<PointerType>(OrigPtrTy)->getElementType();
 
Index: test/Instrumentation/AddressSanitizer/instrument-stack.ll
===================================================================
--- test/Instrumentation/AddressSanitizer/instrument-stack.ll
+++ test/Instrumentation/AddressSanitizer/instrument-stack.ll
@@ -0,0 +1,46 @@
+; This test checks that we are not instrumenting direct inbound stack accesses.
+; RUN: opt < %s -asan -asan-module -S | FileCheck %s
+
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+;@sink = global i32* null, align 4
+
+define void @foo() uwtable sanitize_address {
+entry:
+  %a = alloca i32, align 4
+  store i32 42, i32* %a, align 4
+  ret void
+; CHECK-LABEL: define void @foo
+; CHECK-NOT: __asan_report
+; CHECK: ret void
+}
+
+define void @baz(i64 %i) sanitize_address {
+entry:
+  %a = alloca [10 x i32], align 4
+  %e = getelementptr inbounds [10 x i32]* %a, i32 0, i64 %i
+  store i32 42, i32* %e, align 4
+  ret void
+; CHECK-LABEL: define void @baz
+; CHECK: __asan_report
+; CHECK: ret void
+}
+
+define void @bar() sanitize_address {
+entry:
+  %a = alloca [10 x i32], align 4
+  %e = getelementptr inbounds [10 x i32]* %a, i32 0, i64 12
+  store i32 42, i32* %e, align 4
+  ret void
+; CHECK-LABEL: define void @bar
+; CHECK: __asan_report
+; CHECK: ret void
+}
+
+define void @endoftests() sanitize_address {
+entry:
+  ret void
+; CHECK-LABEL: define void @endoftests
+}
+

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7583.19817.patch
Type: text/x-patch
Size: 2732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150212/7a5aa3c6/attachment.bin>