RFR: JDK-8299790: os::print_hex_dump is racy [v2]

[Aleksey Shipilev]

On Mon, 17 Jul 2023 18:39:03 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Tiny fix for a tiny problem. 
>> 
>> `os::print_hex_dump` uses `os::is_readable_pointer` to check the to-be-printed memory for readability; `os::is_readable_pointer` uses `SafeFetch` to probe the memory for access, which is good, but then, by the time we actually print that information, we reread the memory location again. It may be unreadable now (either because the region had been unmapped or protected by a concurrent thread), and we would crash the VM.
>> 
>> The patch rewrites the function to not use `os::is_readable_pointer`, but to use `SafeFetch` to read from memory directly and then use the result of that read for printing. That requires a bit of bit fiddling, since we only can read word-wise, but the hex-dump could be in units between bytes and qwords.
>> 
>> Tests: manual and GHA-driven gtests on all platforms. The gtests test this function exhaustively.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   feedback Aleksey; small code reshuffle

Looks good.

It's a pity that we would read full `intptr_t` for every unit, which would mean that reading byte-unit block of N bytes would issue 8*N SafeFetch reads. But I guess it is fine, given the old code did `SafeFetch32` anyway.

That looks like fixing another implicit bug, if we are reading the unaligned 64-bit location at readable boundary: `is_readable_pointer` would read 32-bits and return `true` for first 4 bytes of the location, and then we go in and read 64 bits with `print_hex_readable_pointer` with `unitsize = 8`. Which might break if the last 4 bytes of that read are actually outside the readable range.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14895#pullrequestreview-1564821225