[PATCH] Protection against stack-based memory corruption errors using SafeStack: compiler-rt runtime support library

[Kostya Serebryany]

================
Comment at: lib/safestack/safestack.cc:18
@@ +17,3 @@
+#include <stdio.h>
+#include <assert.h>
+#include <dlfcn.h>
----------------
You may find lots of utilities in sanitizer_common useful here. 
E.g. try CHECK, CHECK_EQ, etc instead of assert. 
In general, I'd like to see more code reuse between safe_stack and sanitizers. 

================
Comment at: lib/safestack/safestack.cc:42
@@ +41,3 @@
+// all symbols from pthread that we use dynamically
+#define __DECLARE_WRAPPER(fn) __typeof__(fn)* __d_ ## fn = NULL;
+
----------------
Why not simply force -lpthread in the driver? 

================
Comment at: lib/safestack/safestack.cc:51
@@ +50,3 @@
+
+// The unsafe stack pointer is stored in the TCB structure on these platforms
+#if defined(__i386__)
----------------
 I might be missing something, but can't we just use TLS to store the second stack? 

================
Comment at: lib/safestack/safestack.cc:145
@@ +144,3 @@
+#if defined(__linux__)
+    (void*) syscall(SYS_mmap,
+#else
----------------
theraven wrote:
> Why is this Linux only?  If the goal is to avoid LD_PRELOAD and similar interceptions, then it should be done everywhere (possibly with the exception of Darwin)
Don't you want to use internal_mmap from sanitizer_common/sanitizer_libc.h? 

================
Comment at: lib/safestack/safestack.cc:226
@@ +225,3 @@
+  if (attr != NULL) {
+    __d_pthread_attr_getstacksize(attr, &size);
+    __d_pthread_attr_getguardsize(attr, &guard);
----------------
Where possible, please reuse functions from sanitizer_common, e.g. here you may try GetThreadStackTopAndBottom

================
Comment at: lib/safestack/safestack.cc:266
@@ +265,3 @@
+
+int inited = 0;
+
----------------
please use static for anything that is is not supposed to be called/used from another module.  

================
Comment at: lib/safestack/safestack.cc:305
@@ +304,3 @@
+
+#ifndef NDEBUG
+extern "C"
----------------
why NDEBUG? 
I would prefer tnot to have a separate build for compiler-rt


================
Comment at: lib/safestack/safestack.cc:307
@@ +306,3 @@
+extern "C"
+__attribute__((visibility ("default")))
+void __llvm__safestack_dump() {
----------------
can you use SANITIZER_INTERFACE_ATTRIBUTE?

http://reviews.llvm.org/D6096