RFR: JDK-8299790: os::print_hex_dump is racy

[Aleksey Shipilev]

On Sun, 16 Jul 2023 09:55:43 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.

Some comments...

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

> 944: static bool print_hex_location(outputStream* st, address p, int unitsize) {
> 945:   intptr_t i = 0;
> 946:   const intptr_t errval = 0x1717;

Unused?

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

> 958:     }
> 959:     uint64_t value = (((uint64_t)i2) << 32) | i;
> 960:     st->print("%016" FORMAT64_MODIFIER "x", value);

This implies endianness, it is not? We handle endianness for 64-bit version below.

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

> 997:   st->print(PTR_FORMAT ":   ", p2i(logical_p));
> 998:   while (p < end) {
> 999:     if (!print_hex_location(st, p, unitsize)) {

Pull the `??????` printout straight into `print_hex_location` and remove the need for the return value?

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

PR Review: https://git.openjdk.org/jdk/pull/14895#pullrequestreview-1532670478
PR Review Comment: https://git.openjdk.org/jdk/pull/14895#discussion_r1265299924
PR Review Comment: https://git.openjdk.org/jdk/pull/14895#discussion_r1265305779
PR Review Comment: https://git.openjdk.org/jdk/pull/14895#discussion_r1265288633