RFR: JDK-8298908: Instrument Metaspace for ASan [v2]

[Justin King]

On Tue, 10 Jan 2023 06:57:01 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> This doesn't look "too terrible", but I can't comment on the actual poisoning strategies.
>> 
>> Cheers.
>
>> Forcing 2 reviewers to ensure @dholmes-ora can chime in before moving forward.
> 
> Well I won't be able to Review as not familiar enough with the code, so you'll need a second reviewer anyway. I don't hate this to the point of outright rejecting it but I do have general concerns about whether we should be directly supporting such tools in our codebase, and if we should whether these are the right tools. So I've asked other hotspot folk to chime in.

> > > Forcing 2 reviewers to ensure @dholmes-ora can chime in before moving forward.
> > 
> > 
> > Well I won't be able to Review as not familiar enough with the code, so you'll need a second reviewer anyway. I don't hate this to the point of outright rejecting it but I do have general concerns about whether we should be directly supporting such tools in our codebase, and if we should whether these are the right tools. So I've asked other hotspot folk to chime in.
> 
> I dislike this too. I wondered whether we could hide it behind an "interface for asan-like tools", where we have a `os::poison_memory(range)` and `os::unpoison_memory(range)` function. Those functions could in turn call whatever tool is configured. At least then we don't pollute the code base with tool specifics.
> 
> Granted, it sounds a bit fig leafy as long as there is only Asan. But if we wanted, we could implement a primitive poisoner tool in hotspot by mprotecting if the range spans whole pages.

ASan is really one of a kind in this respect. I am not aware of any other tools like it, due to it requiring compiler support. GCC, Clang, and now [MSVC](https://learn.microsoft.com/en-us/cpp/sanitizers/asan-building?view=msvc-170) all support ASan. Given its wide adoption and touching all major platforms, it's a bit of a de facto standard at this point. It's also noted on MSVC [docs](https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-170) that additional sanitizers may be supported in the future as well.

Also since we are using macros anyway (i.e. `ASAN_POISON_MEMORY_REGION`), it would be fairly easy to swap out the implementation to be something else if wanted such as the suggested primitive poisoner.

-------------

PR: https://git.openjdk.org/jdk/pull/11702