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

[Volodymyr Kuznetsov]

================
Comment at: cmake/config-ix.cmake:259
@@ -257,2 +258,3 @@
   mipsel mips64 mips64el powerpc64 powerpc64le)
+filter_available_targets(SAFESTACK_SUPPORTED_ARCH x86_64 i386 i686)
 
----------------
jfb wrote:
> I haven't been through the LLVM-side code yet, but what prevents other architectures from being supported? IIUC nothing is x86 specific since it's in ``lib/Transforms``, right?
Good point, in principle the code is fairly cross platform (although we only tested it on x86).

================
Comment at: lib/safestack/safestack.cc:174
@@ +173,3 @@
+  if (initialized)
+    return;
+
----------------
jfb wrote:
> Is this initialization check useful if `__attribute__((constructor(0)))` was used? Can this be done concurrently, or is it just called multiple times from the same thread of execution? If concurrent then initialization is racy.
The primary reason for this check is to handle cases when libsafestack is linked into multiple DSOs and __safestack_init ends up being on multiple constructor lists. It can only be called concurrently if multiple DSOs are being dlopen'ed in parallel. I think that dlopen itself isn't thread-safe and shouldn't be used that way but, perhaps, for extra safety it's useful to make this code non-racy using atomics.

================
Comment at: lib/safestack/safestack.cc:225
@@ +224,3 @@
+void *__get_safe_stack_ptr() {
+  return (char*) __builtin_frame_address(0) + 2*sizeof(void*);
+}
----------------
jfb wrote:
> Please add a comment on why `2*sizeof(void*)` is correct.
In fact, it might depend on the architecture and even on the calling convention. Perhaps we should just remove this function altogether, as it is not really related to SafeStack, and we ended up not using it anyway neither in Chrome nor FreeBSD.

http://reviews.llvm.org/D6096

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