[PATCH] D20745: [esan] Add handling of large stack size rlimits

[Evgeniy Stepanov via llvm-commits]

eugenis added inline comments.

================
Comment at: lib/esan/CMakeLists.txt:21
@@ +20,3 @@
+  # Assume Linux
+  list(APPEND ESAN_SOURCES
+    esan_linux.cpp)
----------------
bruening wrote:
> bruening wrote:
> > eugenis wrote:
> > > You append esan_linux.cpp to the list, but it's already in it.
> > > Also, we usually do it in the source like this:
> > > #include "sanitizer_common/sanitizer_platform.h"
> > > #if SANITIZER_FREEBSD || SANITIZER_LINUX
> > > 
> > > 
> > > 
> > This is cmake, though.  This is following how tsan adds tsan_platform_linux.cc (well tsan doesn't leave the file in the main list too ;)).  Is there a cmake variable set in a parent file distinguishing Linux from BSD?
> Or are you suggesting to not say "assume linux" here, and have the source file say "#if SANITIZER_LINUX" if it really won't work on BSD (should prob be _posix if it works on BSD)?
Hm, I don't see why this cmake change would be necessary if the C++ source had the proper guards. The TSan thing could be legacy.


================
Comment at: lib/esan/esan_linux.cpp:47
@@ +46,3 @@
+
+bool fixMmapAddr(void **Addr, SIZE_T Size, int Flags) {
+  if (*Addr) {
----------------
bruening wrote:
> eugenis wrote:
> > MSan handles this by mprotect-ing all regions that don't have a shadow mapping, and then the kernel takes care of the rest. Is it hard to do here because of the variable shadow scale?
> I don't see how that stops a MAP_FIXED mmap?
Right, it does not. Actually, MSan does not check this condition well enough - it used to have a simple mapping where if was sufficient to only check the start address, but now it can let through an invalid mmap...


http://reviews.llvm.org/D20745