[PATCH] [asan] rewrite asan's stack frame layout

Kostya Serebryany kcc at google.com
Thu Dec 5 23:49:41 PST 2013



================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:141
@@ -146,1 +140,3 @@
+       cl::desc("Realign stack to the value of this flag (power of two)"),
+       cl::Hidden, cl::init(1));
 static cl::opt<std::string> ClBlacklistFile("asan-blacklist",
----------------
Alexey Samsonov wrote:
> I think the default value should be 32.
I am not sure if it *should* be 32, but it's definitely what it was before.
I changed it to 32 now, may reconsider later.

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:512
@@ -518,3 +511,3 @@
             AI.isStaticAlloca() &&
             AI.getAlignment() <= RedzoneSize() &&
             AI.getAllocatedType()->isSized());
----------------
Alexey Samsonov wrote:
> The RedzoneSize() here may lie.
Good catch! Removed this check -- we now handle any alignment.
Update the new test and remove the old test that checked that we don't support large alignments.

================
Comment at: include/llvm/Transforms/Utils/ASanStackFrameLayout.h:18
@@ +17,3 @@
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/Instructions.h"
+
----------------
Alexey Samsonov wrote:
> You may use a forward declaration of AllocaInst instead of header here.
done

================
Comment at: include/llvm/Transforms/Utils/ASanStackFrameLayout.h:15
@@ +14,3 @@
+#define LLVM_TRANSFORMS_UTILS_ASANSTACKFRAMELAYOUT_H
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
----------------
Alexey Samsonov wrote:
> ArrayRef is unused
done

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1403
@@ -1481,1 +1402,3 @@
 
+  SmallVector<ASanStackVariableDescription, 16> SVD;
+  SVD.reserve(AllocaVec.size());
----------------
Alexey Samsonov wrote:
> Isn't 16 local variables too many?
I don't know. This is more like bike-shedding.

================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1470
@@ -1544,1 +1469,3 @@
+  for (size_t i = 0, n = SVD.size(); i < n; i++) {
+    AllocaInst *AI = static_cast<AllocaInst*>(SVD[i].AI);
     Value *NewAllocaPtr = IRB.CreateIntToPtr(
----------------
Alexey Samsonov wrote:
> No more need in static_cast
indeed! 


http://llvm-reviews.chandlerc.com/D2324



More information about the llvm-commits mailing list