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

[Dan Liew via Phabricator via llvm-commits]

delcypher marked 2 inline comments as done.
delcypher added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_vm_read_context.h:24
+ public:
+  typedef __sanitizer::process_vm_read_handle_t ProcessHandle;
+
----------------
vitalybuka wrote:
> delcypher wrote:
> > vitalybuka wrote:
> > > Just define "typedef int ProcessHandle;" and remove process_vm_read_handle_t
> > > On other platforms it does not matter, and we will add more specific type if needed later.
> > Apart from that fact that it needs to be `unsigned` on Darwin. I'm not super comfortable doing that because the declaration of `__sanitizer::process_vm_read_handle_t` in `sanitizer_internal_defs.h` allows to detect if the system declaration of `task_t` disagrees with the Sanitizer runtimes type. This is because on Darwin we include system headers that do define the `task_t` type.
> then just define process_vm_read_handle_t as task_t in sanitizer_internal_defs for Darwin?
I tried doing this in my internal implementation. It breaks compilation because `sanitizer_internal_defs` gets included very early and `task_t` at that point is not defined. Let's just keep it.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56206