On Sat, 17 Dec 2022 06:48:13 GMT, Justin King <jck...@openjdk.org> wrote:

>> This change instruments Metaspace for ASan. Metaspace allocates memory using 
>> `mmap`/`munmap` which ASan is not aware of. Fortunately ASan supports 
>> applications [manually poisoning/unpoisoning 
>> memory](https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning).
>>  ASan is able to detect poisoned memory, similar to `use-after-free`, and 
>> will raise an error similarly called `use-after-poison`. This provides and 
>> extra layer of defense and confidence.
>> 
>> The header `sanitizers/address.h` defines macros for poisoning/unpoisoning 
>> memory regions. These macros can be used regardless of build mode. When ASan 
>> is not available, they are implemented using a NOOP approach which still 
>> compiles the arguments but does so such that they will be stripped out by 
>> the compiler due to being unreachable. This helps with maintenance.
>> 
>> This also has the added benefit of making 
>> [LSan](https://bugs.openjdk.org/browse/JDK-8298445) more accurate and 
>> deterministic, as LSan will not look for pointers to malloc memory in 
>> poisoned memory regions.
>> 
>> IMO the benefit of doing this greatly outweighs the cost.
>
> Justin King has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Exclude more zapping when ASan is in use
>   
>   Signed-off-by: Justin King <jck...@google.com>

This doesn't look "too terrible", but I can't comment on the actual poisoning 
strategies.

Cheers.

src/hotspot/share/runtime/os.cpp line 949:

> 947: // parent stack frames, read outside of initialized memory, and etc. So 
> we tell ASan to not
> 948: // instrument this function.
> 949: NO_SANITIE_ADDRESS void os::print_hex_dump(outputStream* st, address 
> start, address end,

Typo: NO_SANITIE_ADDRESS ->NO_SANITIZE_ADDRESS ?

src/hotspot/share/sanitizers/address.h line 2:

> 1: /*
> 2:  * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

Oracle didn't write this, it should have your/your-company's copyright.

src/hotspot/share/sanitizers/address.h line 44:

> 42: #endif
> 43: #else
> 44: #define NO_SANITIE_ADDRESS

Typos

src/hotspot/share/sanitizers/address.h line 56:

> 54: #else
> 55: // NOOP implementation which preserves the arguments, ensuring they still 
> compile, but ensures they
> 56: // are stripped due to being unreachable.

Why is this necessary?

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

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

Reply via email to