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

[etienneb]

Summary

This patch is fixing unittests for sanitizer memory allocator.

There was two issues:

1. The VirtualAlloc can't reserve twice a memory range. The memory space used by the SizeClass allocator is reserved with NoAccess and pages are commited on demand (using MmapFixedOrDie).
2. The address space is allocated using two VirtualAlloc calls. The first one for the memory space, the second one for the AdditionnalSpace (after).
   
   On windows, they need to be freed separately.

[etienneb]

chrisha@ prosposed that we implement an other flavor of MMap to remap memory previously created with NoAccess.

Any toughs?

[rnk]

lib/sanitizer_common/sanitizer_win.cc

187

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.

204

Uh, yeah, committing memory with no access seems like the wrong behavior...

lib/sanitizer_common/tests/sanitizer_allocator_test.cc

39–41

Should these changes be under #if SANITIZER_WINDOWS so that we test the full address space range on Linux?

[rnk]

In D21900#471537, @etienneb wrote:

chrisha@ prosposed that we implement an other flavor of MMap to remap memory previously created with NoAccess.

I like this better. I think if we try to hard to pretend we have mmap we will eventually go crazy.

[etienneb]

I'll move forward with adding a new flavor of mmap.
I can't tell how hard that will be. :)

lib/sanitizer_common/tests/sanitizer_allocator_test.cc

39–41

I should take a deeper look to the range that Windows can accept.

[• chrisha]

In D21900#471571, @rnk wrote:

In D21900#471537, @etienneb wrote:

chrisha@ prosposed that we implement an other flavor of MMap to remap memory previously created with NoAccess.

I like this better. I think if we try to hard to pretend we have mmap we will eventually go crazy.

I think we should really build a table of the various memory concepts on Posix/Windows, and how they map to each other. And then build an abstract set of memory management functions that provides clean concepts across platforms.

(Without sitting down and trying it, I'm not sure that's possible or desirable in the long run.)

[etienneb]

fix

[etienneb]

nit

[etienneb]

nits

[rnk]

lgtm

lib/sanitizer_common/sanitizer_allocator.h

 class SizeClassAllocator64 {
  public:
   typedef typename SizeClassMap::TransferBatch Batch;
   typedef SizeClassAllocator64<kSpaceBeg, kSpaceSize, kMetadataSize,
       SizeClassMap, MapUnmapCallback> ThisT;
   typedef SizeClassAllocatorLocalCache<ThisT> AllocatorCache;
 
   void Init() {
+    uptr TotalSpaceSize = kSpaceSize + AdditionalSize();
     if (kUsingConstantSpaceBeg) {
       CHECK_EQ(kSpaceBeg, reinterpret_cast<uptr>(
-                              MmapFixedNoAccess(kSpaceBeg, kSpaceSize)));
+                              MmapFixedNoAccess(kSpaceBeg, TotalSpaceSize)));
     } else {
       NonConstSpaceBeg =
-          reinterpret_cast<uptr>(MmapNoAccess(kSpaceSize + AdditionalSize()));
+          reinterpret_cast<uptr>(MmapNoAccess(TotalSpaceSize));
       CHECK_NE(NonConstSpaceBeg, ~(uptr)0);
     }
     MapWithCallback(SpaceEnd(), AdditionalSize());
   }
 
   void MapWithCallback(uptr beg, uptr size) {
     CHECK_EQ(beg, reinterpret_cast<uptr>(MmapFixedOrDie(beg, size)));
     MapUnmapCallback().OnMap(beg, size);

lib/sanitizer_common/sanitizer_win.cc

void *p = VirtualAlloc((LPVOID)fixed_addr, size,
       MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
   if (p == 0)
     Report("ERROR: %s failed to "
            "allocate %p (%zd) bytes at %p (error code: %d)\n",
            SanitizerToolName, size, size, fixed_addr, GetLastError());
   return p;
 }
 
+// Memory space mapped by 'MmapFixedOrDie' must have been reserved by
+// 'MmapFixedNoAccess'.
 void *MmapFixedOrDie(uptr fixed_addr, uptr size) {
   void *p = VirtualAlloc((LPVOID)fixed_addr, size,
-      MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
+      MEM_COMMIT, PAGE_READWRITE);
   if (p == 0) {
     char mem_type[30];
     internal_snprintf(mem_type, sizeof(mem_type), "memory at address 0x%zx",
                       fixed_addr);
     ReportMmapFailureAndDie(size, mem_type, "allocate", GetLastError());
   }
   return p;
 }
 
 void *MmapNoReserveOrDie(uptr size, const char *mem_type) {
   // FIXME: make this really NoReserve?
   return MmapOrDie(size, mem_type);
 }
 
 void *MmapFixedNoAccess(uptr fixed_addr, uptr size, const char *name) {
   (void)name; // unsupported
   void *res = VirtualAlloc((LPVOID)fixed_addr, size,
-                           MEM_RESERVE | MEM_COMMIT, PAGE_NOACCESS);
+                           MEM_RESERVE, PAGE_NOACCESS);
   if (res == 0)
     Report("WARNING: %s failed to "
            "mprotect %p (%zd) bytes at %p (error code: %d)\n",
            SanitizerToolName, size, size, fixed_addr, GetLastError());
   return res;
 }
 
 void *MmapNoAccess(uptr size) {

lib/sanitizer_common/tests/sanitizer_allocator_test.cc

 #include <algorithm>
 #include <vector>
 #include <set>
 
 // Too slow for debug build
 #if !SANITIZER_DEBUG
 
 #if SANITIZER_CAN_USE_ALLOCATOR64
+#if SANITIZER_WINDOWS
+static const uptr kAllocatorSpace = 0x10000000000ULL;
+static const uptr kAllocatorSize  =  0x10000000000ULL;  // 1T.
+static const u64 kAddressSpaceSize = 1ULL << 40;
+#else
 static const uptr kAllocatorSpace = 0x700000000000ULL;
 static const uptr kAllocatorSize  = 0x010000000000ULL;  // 1T.
 static const u64 kAddressSpaceSize = 1ULL << 47;
+#endif
 
 typedef SizeClassAllocator64<
   kAllocatorSpace, kAllocatorSize, 16, DefaultSizeClassMap> Allocator64;
 
 typedef SizeClassAllocator64<
   kAllocatorSpace, kAllocatorSize, 16, CompactSizeClassMap> Allocator64Compact;
 #elif defined(__mips64)
 static const u64 kAddressSpaceSize = 1ULL << 40;
 #elif defined(__aarch64__)

 #if SANITIZER_CAN_USE_ALLOCATOR64
 TEST(SanitizerCommon, SizeClassAllocator64GetBlockBegin) {
   SizeClassAllocatorGetBlockBeginStress<Allocator64>();
 }
 TEST(SanitizerCommon, SizeClassAllocator64CompactGetBlockBegin) {
   SizeClassAllocatorGetBlockBeginStress<Allocator64Compact>();
 }
 TEST(SanitizerCommon, SizeClassAllocator32CompactGetBlockBegin) {
    SizeClassAllocatorGetBlockBeginStress<Allocator32Compact>();
 }
 #endif  // SANITIZER_CAN_USE_ALLOCATOR64
 
 struct TestMapUnmapCallback {
   static int map_count, unmap_count;
   void OnMap(uptr p, uptr size) const { map_count++; }
   void OnUnmap(uptr p, uptr size) const { unmap_count++; }
 };