[PATCH] Implement variable-sized alloca instrumentation.

Kostya Serebryany kcc at google.com
Thu Nov 6 13:24:23 PST 2014


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:169
@@ -164,1 +168,3 @@
        cl::init("__asan_"));
+static cl::opt<bool> ClInstrumentAllocas("asan-instrument-allocas",
+       cl::desc("instrument dynamic allocas"), cl::Hidden, cl::init(false));
----------------
Please add a test for this flag in test/Instrumentation/AddressSanitizer/
to check asan-instrument-alloca=0,  asan-instrument-alloca=1, and default setting
Similar to test/Instrumentation/AddressSanitizer/instrumentation-with-call-threshold.ll

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:547
@@ +546,3 @@
+
+  void handleDynamicAllocaCall(AllocaInst *AI) {
+    IRBuilder<> IRBefore(AI);
----------------
This will not create a left red zone, right? 
And even if it will (due to alignment) it will not poison it. 

I would prefer to create both left and right redzones and [un]poison them inline with one 4-byte store for the left rz and one or two 4-byte stores for the right one. 
Make sure to make the new size 0 mod 32



================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:557
@@ +556,3 @@
+    unsigned AllocaAlign = AI->getAlignment();
+    unsigned Align = std::max(kAllocaRedzoneSize, AllocaAlign);
+    const unsigned kPartialRedzoneSizeMax = kAllocaRedzoneSize - 1;
----------------
do you really need to always align this by 32?
Maybe use max(ClRealignStack, AllocaAlign)?

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:586
@@ +585,3 @@
+    assert(isa<Instruction>(NewAddress));
+    cast<Instruction>(NewAddress)->setOperand(0, AI);
+  }
----------------
A cleaner way is to create a new AllocaInst, just like we do in another place in this file. 
Then do eraseFromParent on the old one. 

================
Comment at: lib/asan/asan_fake_stack.cc:240
@@ -239,1 +239,3 @@
 
+static ALWAYS_INLINE void AllocaPoison(const uptr addr, const uptr size,
+                                       bool doPoison) {
----------------
We probably don't need these at all if we inline the poisoning and unpoisoning.

http://reviews.llvm.org/D6055






More information about the llvm-commits mailing list