[PATCH] D50330: [RFC] Implement out-of-process allocator enumeration for macOS

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In https://reviews.llvm.org/D50330#1214896, @kcc wrote:

> > ForEachChunkOutOfProcess needs access to internal structures of the allocator.
>
> That's exactly why I proposed to consider implementing a public API instead. 
>  It might be hard, but I'd like to know why before we proceed with the current change.


Sorry for taking so long to revisit this. I hope you'll find this version of the patch more palatable.

I tried looking at the idea of a "public API" and I see no reasonable way to do this. The out-of-process code has to know lots of internal details about the implementations of the allocators that I see no practical way to abstract. In particular the fact we are performing out-of-process reads couples us even very tightly to an allocator implementation because  we have to be very careful to only call APIs on an allocator that do not dereference pointers. In several cases I have had to implement out-of-process versions of existing APIs because the existing ones aren't designed to work with the allocator being in a different process. To me these are very strong indicators that it does not make sense to pull the out-of-process enumeration code out of allocators.

Previously you raised concerns about having extra complexity inside the ASan allocators that no other platforms are using. The new version of the patch aims at a compromise that partially addresses this.

In this patch all of the out-of-process functions are declared in the allocator classes but they are defined in other header files. The idea here is that only platforms (i.e. Darwin) that actually need the out-of-process functionality should include the `*_oop.h` header files. All other platforms simply don't include those header files which means the out-of-process code never ends up inside the sanitizer libraries. In this patch **only** the unit tests consume the `*_oop.h` header files so we can ensure that compilation doesn't get broken. So in this patch the new out-of-process code doesn't ever appear in the sanitizer libraries. The intention in later patches is that the Darwin ASan library will also consume the `*_oop.h` header files but no other platform will.

This partially fixes your concerns because all non-Darwin sanitizer libraries will not contain out-of-process enumerator code. What this does not fix is the maintenance burden of maintaining two separate allocator enumerator implementations. This is unavoidable unless we combine them into an abstracted version of `ForEachChunk(...)` (and friends) that describes both in and out-of-process implementations.

Please let me know what you think.


https://reviews.llvm.org/D50330