[PATCH] D43318: [asan] Be more careful and verbose when allocating dynamic shadow memory

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: lib/asan/asan_mac.cc:75
     uptr new_max_vm = RoundDownTo(largest_gap_found << SHADOW_SCALE, alignment);
+    if (new_max_vm < max_occupied_vm) {
+      Report("Unable to find a memory range for dynamic shadow.\n");
----------------
george.karpenkov wrote:
> I don't quite understand semantics here.
> `max_occupied_vm` to me means "maximal size of occupied virtual memory".
> Then why it's a problem if a new maximal size is smaller? Shouldn't it be a problem if it's larger?
`max_occupied_vm` is the largest address that it already occupied by something. We're trying to restrict the VM space with `new_max_vm` being the new VM limit. This check is to make sure we don't restrict too much (which would overwrite memory that is already used).

Suggestions for better name of `max_occupied_vm`?


================
Comment at: lib/sanitizer_common/sanitizer_mac.cc:901
   if (largest_gap_found) *largest_gap_found = 0;
+  if (max_vm_address) *max_vm_address = 0;
   while (kr == KERN_SUCCESS) {
----------------
george.karpenkov wrote:
> Is this assignment necessary? Maximum size is zero unless set otherwise?
I'd rather not leave garbage in `max_vm_address` if for some reason the loop below doesn't find any regions at all.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D43318