[PATCH] D18003: [tsan] Disable randomized address space on linux when necessary.

[Yabin Cui via llvm-commits]

yabinc added inline comments.

================
Comment at: lib/tsan/rtl/tsan_platform_linux.cc:267
@@ -265,1 +266,3 @@
 
+static uptr GetHeapEnd() {
+  MemoryMappingLayout proc_maps(true);
----------------
dvyukov wrote:
> Please call it somehow differently from "Heap". Heap commonly means "malloc heap" throughout the runtime, as is HeapMemEnd below.
> I think we are mainly looking for mmaped modules here, so "ModulesEnd" would be fine. "MmapEnd" would be fine as well.
> 
I am confused what is kHeapMem is tsan_platform.h for? In my current understanding, if the malloc map is allocated using brk(), the space is just above bss; if the malloc map is allocated using mmap(), the space should have no difference than other mmaped areas. like the one in my x86_64 linux pc:
006f0000-006f9000 rw-p 000f0000 fc:00 1838842                            /bin/bash
006f9000-006ff000 rw-p 00000000 00:00 0 
00f8e000-01401000 rw-p 00000000 00:00 0                                  [heap]

To make me more confusing, there is a gap between kHeapMemEnd and kHiAppMemBeg in x86_64, how is it decided?

For the code here, I am looking for mmaped modules, but I think it is probably represented by kHeapMem in tsan_platform.h, that's why I compare GetHeapEnd() < HeapMemEnd() below.

================
Comment at: lib/tsan/rtl/tsan_platform_linux.cc:311
@@ +310,3 @@
+    if (old_personality != -1 && (old_personality & ADDR_NO_RANDOMIZE) == 0) {
+      if (GetHeapEnd() < HeapMemEnd()) {
+        Report("WARNING: Program is run with randomized virtual address space,"
----------------
dvyukov wrote:
> I am trying to convince myself that it won't break any of linux setups. We can have pie/non-pie binary. COMPAT mapping (setarch -L) is not supported now on x86 (modules mapped at 0x2a). Also disabled randomization is _not_ supported on x86 (modules mapped at 0x55).
> So you have a good explanation as to why it all will continue to work?
> I guess that we always have at least 1 dynamic library (static libc linking is not supported), so on x86 glibc should be mapped at 0x7f in supported configuration (randomization + no compat mapping). So GetHeapEnd should return 0x7f, and that's larger than HeapMemEnd.
> 
> But I think it will make failure mode for at least x86+COMPAT mapping much worse (infinite exec recursion instead of a readable message). Potentially there are other cases on other platforms (e.g. power/mips) when that will happen as well. Or maybe they legally have modules below heap?
> 
> I think it makes sense to tread more gradually and enable it just for aarch64 with an explanation as to why we are doing this. Other platforms will be enable to enable this by just altering ifdef condition later if necessary.
> 
> Or am I missing something?
> 
> 
Frankly I don't understand what is COMPAT mapping and try `setarch -L` fails on my x86_64 pc. I'd like to learn if you can give me more contexts or instructions about how to test.
As I can see, on x86_64, mmap base is always at HiAppMem range, thus larger than HeapMemEnd.
x86+COMPAT mapping, how to experiment that? setarch -L seems don't work.
I don't think there will a infinite loop of reexec, because if CHECK(personality()) fails, it will exit. In consideration of not breaking existing behavior, I remove the CHECK() and don't reexec if failed to set personality. I am not sure if it is proper.

I just realize that the check of GetHeapEnd() < HeapMemEnd() doesn't work on aarch64 39-bit.  Because there is a hole between kHeapMemEnd and kHiAppMemBeg, whose shadow mem space is taken by kMidAppMem. It means even if GetHeapEnd() is bigger than HeapMemEnd(), it can be like 0x7d20000000, and takes the same shadow mem as kMidAppMem. Currently I think we have two choices, one is making it android specific and reexec without checking real mmap space, another is keep the checking and change Mapping39 of aarch64 (but I am not confident as I don't know what kHeapMem stands for).


http://reviews.llvm.org/D18003