RFR: 8361433: [Big Endian] assert(verify_guards()) failed: Expected valid memory guards after 8357601 [v2]

[Martin Doerr]

On Sat, 5 Jul 2025 16:09:21 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use os::is_readable_range().
>
> src/hotspot/share/memory/guardedMemory.hpp line 122:
> 
>> 120:         // SafeFetch. It doesn't matter if the value read happens
>> 121:         // to be 0xFF as that is not what we expect anyway.
>> 122:         u_char val = (u_char) (SafeFetch32((int*)c, 0xFF) BIG_ENDIAN_ONLY(>> 24));
> 
> Your fix is fine. Pre-existing: I wonder about the SafeFetch. A byte pointer traverses byte-wise over the memory. We use it to load a 32-bit value via SafeFetch32. 3/4 of those loads will be unaligned and also redundant. (I am surprised the unaligned access works on all platforms).
> 
> The guards themselves can be located at unaligned addresses I think, so we cannot just use int loads. Therefore I would have just used the old code without safefetch but preceded it with a safefetch check:
> 
> 
> u_char* c = (u_char*) _guard;
> + if (!os::is_readable_pointer(align_up(c, BytesPerInt))) {
> +   return false;
> + }
>    u_char* end = c + GUARD_SIZE;
>    while (c < end) {
>      if (*c != badResourceValue) {
>        return false;
>      }
>      c++;
>    }
> 
> I think that would be pragmatic since the guard is only 16 byte and the chance of a guard straddling a page boundary is very small. But if one wanted to be more careful, one could use `os::is_readable_range()` instead.
> 
> @dholmes-ora  does this make sense?

Thanks, Thomas! I've changed to use `os::is_readable_range()`. Note that alignment is already handled inside of that function. Please take a look!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26140#discussion_r2187504943