[PATCH] D56206: Introduce `RemoteAddressSpaceView` and `VMReadContext`.

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_remote_address_space_view.h:27
+ private:
+  static VMReadContext* vmrc;
+  static void* internal_read(uptr target_address, uptr size, bool writable) {
----------------
delcypher wrote:
> vitalybuka wrote:
> > Why all these stuff needs to be static?
> @vitalybuka Because the allocators don't receive an instance of `RemoteAddressSpaceView`, they receive the type as a template parameter (e.g. SizeClassAllocator64). This design choice means that everything in an implementation of the `AddressSpaceView` interface (i.e. `LocalAddressSpaceView` and `RemoteAddressSpaceView`) needs to be static.
> 
> To implement `RemoteAddressSpaceView` we **have to store some state**. All of this state and the read operations are encapsulated in the `VMReadContext` object. We cannot store any of this state inside the allocators because that could change the data-layout.
> 
> This design is not thread-safe of course but for allocator enumeration on Darwin will only ever be doing that in a single thread anyway.
Can you add some CHECKs validate this single thread assumption?


================
Comment at: lib/sanitizer_common/sanitizer_vm_read_context.h:27
+ private:
+  uptr local_address;
+  uptr target_address;
----------------
could you please init these inline?


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56206/new/

https://reviews.llvm.org/D56206