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

[Thomas Stuefe]

On Mon, 7 Aug 2023 08:29:32 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> 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.

I thought about this too but then refrained from implementing a "read intptr_t then print bytes" to keep the patch complexity low. 

I think another, simpler, patch would be worthwhile where we split print_hex_dump into two variants, one using SafeFetch and one just reading, and use the former one only if we are unsure about the memory content. E.g. at hs-err file dumping. There is no need to pay for SafeFetch when hexdumping e.g. for logging.

Maybe for a future RFE.

> 
> 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.

Oh, I missed that. Nice :)

Thanks for the review!

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

PR Comment: https://git.openjdk.org/jdk/pull/14895#issuecomment-1667539639