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

[Peter Collingbourne]

================
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
----------------
MatzeB wrote:
> This should probably go into a separate patch.
r237766

================
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();
+
----------------
MatzeB wrote:
> Don't repeat method names in documentation comments, this practice is discouraged.
Done

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:172
@@ +171,3 @@
+
+  enum { StackAlignment = 16 };
+
----------------
MatzeB wrote:
> Why is it necessary to have some minimum alignment and why is it 16?
For the same reason that the regular stack has an alignment, e.g. see http://stackoverflow.com/questions/4175281/what-does-it-mean-to-align-the-stack

16 seems like a reasonable upper bound on the alignment of objects that we might expect to appear on the stack on most common targets. We can always change this number later if there is a compelling need to, but at least to start with it's simplest to pick a constant and use it everywhere.

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

================
Comment at: test/Transforms/SafeStack/cast.ll:6-7
@@ +5,4 @@
+
+; PtrToInt/IntToPtr Cast
+; Requires no protector.
+
----------------
MatzeB wrote:
> I would have expected this to require a protector.
No. Simply converting a pointer to an integer (or vice versa) cannot leak the address of the safe stack, as the conversion by itself does not cause the address to escape the function. The situation may be different depending on what we do with the converted pointer (e.g. if we store it somewhere), but in this case we are just doing a conversion.

http://reviews.llvm.org/D6094

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