Fix bug where we request a shadow memory one page larger than necessary.

[delcypher]

Summary

The passed in space_size had left_padding added to it.
This is wrong because in the implementation FindAvailableMemoryRange
the computed size of the found region has the left_padding subtracted
(possibly more due to alignment) already.

Here's the relevant snippet from FindAvailableMemoryRange.

// We found a free region [free_begin..address-1].
uptr gap_start = RoundUpTo((uptr)free_begin + left_padding, alignment);
uptr gap_end = RoundDownTo((uptr)address, alignment);
uptr gap_size = gap_end > gap_start ? gap_end - gap_start : 0;
if (size < gap_size) {
  return gap_start;
}

In the size comparison (size < gap_size) gap_size doesn't
include padding so size shouldn't either.

rdar://problem/66603866

[yln]

Ahh, this is directly related to D85387 and already addresses one of my questions there!
Happy with this change, let's get @tejohnson's feedback.

Thanks for investigating this!

[tejohnson]

It looks reasonable, but adding @vitalybuka since this predates my recent refactoring, and he is probably a better person to review. It looks like the win code has the exact same issue, so should get a similar fix. The linux code doesn't seem to (it doesn't call FindAvailableMemoryRange, and doesn't have a similar comparison).

[delcypher]

@yln Note there are other bugs here but I plan to fix this in future commits (e.g. if (size < gap_size) should be if (size <= gap_size)).

[kubamracek]

This looks reasonable, but just for verification, could you post the "before" and "after" of the little shadow map printout from ASAN_OPTIONS=verbosity=1?

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

 uptr MapDynamicShadow(uptr shadow_size_bytes, uptr shadow_scale,
                       uptr min_shadow_base_alignment, uptr &high_mem_end) {
   const uptr granularity = GetMmapGranularity();
   const uptr alignment =
       Max<uptr>(granularity << shadow_scale, 1ULL << min_shadow_base_alignment);
   const uptr left_padding =
       Max<uptr>(granularity, 1ULL << min_shadow_base_alignment);
 
-  uptr space_size = shadow_size_bytes + left_padding;
+  uptr space_size = shadow_size_bytes;
 
   uptr largest_gap_found = 0;
   uptr max_occupied_addr = 0;
   VReport(2, "FindDynamicShadowStart, space_size = %p\n", space_size);
   uptr shadow_start =
       FindAvailableMemoryRange(space_size, alignment, left_padding,
                                &largest_gap_found, &max_occupied_addr);
   // If the shadow doesn't fit, restrict the address space to make it fit.
   if (shadow_start == 0) {
     VReport(
         2,
         "Shadow doesn't fit, largest_gap_found = %p, max_occupied_addr = %p\n",
         largest_gap_found, max_occupied_addr);
     uptr new_max_vm = RoundDownTo(largest_gap_found << shadow_scale, alignment);
     if (new_max_vm < max_occupied_addr) {
       Report("Unable to find a memory range for dynamic shadow.\n");
       Report(
           "space_size = %p, largest_gap_found = %p, max_occupied_addr = %p, "
           "new_max_vm = %p\n",
           space_size, largest_gap_found, max_occupied_addr, new_max_vm);
       CHECK(0 && "cannot place shadow");
     }
     RestrictMemoryToMaxAddress(new_max_vm);
     high_mem_end = new_max_vm - 1;
-    space_size = (high_mem_end >> shadow_scale) + left_padding;
+    space_size = (high_mem_end >> shadow_scale);
     VReport(2, "FindDynamicShadowStart, space_size = %p\n", space_size);
     shadow_start = FindAvailableMemoryRange(space_size, alignment, left_padding,
                                             nullptr, nullptr);
     if (shadow_start == 0) {
       Report("Unable to find a memory range after restricting VM.\n");
       CHECK(0 && "cannot place shadow after restricting vm");
     }
   }