[PATCH] D21900: [compiler-rt] Fix sanitizer memory allocator on win64.

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:185
@@ -184,2 +184,3 @@
 void *MmapFixedOrDie(uptr fixed_addr, uptr size) {
   void *p = VirtualAlloc((LPVOID)fixed_addr, size,
+      MEM_COMMIT, PAGE_READWRITE);
----------------
Which is more common, committing previously reserved memory or reserve+committing a large chunk of memory? We should do the common one first.

Also, this needs a comment that VirtualAlloc with MEM_RESERVE will fail if the memory was previously reserved, and in that case we should only pass MEM_COMMIT.

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:204
@@ -203,3 +206,3 @@
   void *res = VirtualAlloc((LPVOID)fixed_addr, size,
-                           MEM_RESERVE | MEM_COMMIT, PAGE_NOACCESS);
   if (res == 0)
----------------
Uh, yeah, committing memory with no access seems like the wrong behavior...

================
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:32-34
@@ -31,5 +31,5 @@
 #if SANITIZER_CAN_USE_ALLOCATOR64
-static const uptr kAllocatorSpace = 0x700000000000ULL;
-static const uptr kAllocatorSize  = 0x010000000000ULL;  // 1T.
-static const u64 kAddressSpaceSize = 1ULL << 47;
+static const uptr kAllocatorSpace = 0x10000000000ULL;
+static const uptr kAllocatorSize  =  0x10000000000ULL;  // 1T.
+static const u64 kAddressSpaceSize = 1ULL << 40;
 
----------------
Should these changes be under #if SANITIZER_WINDOWS so that we test the full address space range on Linux?


http://reviews.llvm.org/D21900