RFR: 8320275: assert(_chunk->bitmap().at(index)) failed: Bit not set at index [v2]

[Patricio Chilano Mateo]

On Thu, 30 Nov 2023 03:28:39 GMT, Dean Long <dlong at openjdk.org> wrote:

> OK, the use of `address` still seems misleading, but I can't think of anything much better, except perhaps `void*`.
> 
I think of it as just a range of memory we are passing. I don't immediately see void* as better since that could also point anywhere and not be aligned.

> Do we really need a version of num_stack_arg_slots() that rounds up?
> 
All the other callers in freeze/thaw calculate the size of the argument are in words based on this number (e.g. `frame::compiled_frame_stack_argsize()`). So if we have an odd number of stack slots we would end up calculating the wrong size.

> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 2175:
> 
>> 2173:   // we need to clear the bits that correspond to arguments as they reside in the caller frame
>> 2174:   // or they will keep objects that are otherwise unreachable alive
>> 2175:   address effective_end = UseCompressedOops ? end : align_down(end, wordSize);
> 
> Is the align_down for correctness, or just for the benefit of the new assert at line 2179?  Since it's not immediately obvious, I think it deserves a comment.

Because `end` is not necessarily word aligned anymore the pointer arithmetic we do in bit_index_for() would be UB, since `p` can point to the middle of an oop (in practice we would probably not see any issue because that's implemented as a substraction and then an arithmetic shift right which will round down the result). So we need to align `end` down if UseCompressedOops is not set. That last half word part should not contain an oop anyways so the assert is to verify that. I added a comment, please take a look.

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

PR Comment: https://git.openjdk.org/jdk/pull/16837#issuecomment-1833991601
PR Review Comment: https://git.openjdk.org/jdk/pull/16837#discussion_r1410833228