[PATCH] Protection against stack-based memory corruption errors using SafeStack

[Matthias Braun]

Very interesting approach. I am not the right person to approve this patch though, so just some comments:


================
Comment at: include/llvm/Support/Compiler.h:32-35
@@ -31,2 +31,6 @@
 
+#ifndef __has_cpp_attribute
+# define __has_cpp_attribute(x) 0
+#endif
+
 #ifndef __has_builtin
----------------
This should probably go into a separate patch.

================
Comment at: include/llvm/Transforms/Instrumentation.h:135-137
@@ -134,1 +134,5 @@
 
+/// createSafeStackPass - This pass splits the stack into a safe stack and
+/// an unsafe stack to protect against stack-based overflow vulnerabilities.
+FunctionPass *createSafeStackPass();
+
----------------
Don't repeat method names in documentation comments, this practice is discouraged.

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:172
@@ +171,3 @@
+
+  enum { StackAlignment = 16 };
+
----------------
Why is it necessary to have some minimum alignment and why is it 16?

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:310
@@ +309,3 @@
+
+  AllocaInst *DynamicTop = nullptr;
+
----------------
Maybe move this declaration down in front of the if that first defines it.

================
Comment at: test/Transforms/SafeStack/cast.ll:6-7
@@ +5,4 @@
+
+; PtrToInt/IntToPtr Cast
+; Requires no protector.
+
----------------
I would have expected this to require a protector.

http://reviews.llvm.org/D6094

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/