[PATCH] D52242: [sanitizer] Destroy and close a range's vmar if all its memory was unmapped

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_fuchsia.cc:290
+    } else {
+      base_ = reinterpret_cast<void*>(addr + size);
+    }
----------------
cryptoad wrote:
> mcgrathr wrote:
> > This seems questionable and at least needs comments.
> > By moving base_ up you are telling the local bookkeeping that the reservation starts at the new base.
> > But the actual VMAR still exists and will always cover the full range set at its creation.
> > So the address space between the old base_ and the new base_ is still reserved, but you've lost track of it.
> > 
> I agree with you, the ReservedAddressRange in their current state do not allow distinction between committed & reserved.
> I think the original idea was to allow multiple `Unmap` to occur at the extremities of the mapping, but the use is currently confined to a full unmapping or a single pruning on either side in the event of Secondary aligned allocation.
> I can skip the update of `base_` & `size_` to keep track of the reserved area if it feels more accurate to you, it shouldn't impact the current usage scenario.
That does seem preferable, and with some comments about reserved regions that won't be ever be reused if that's the case.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52242