RFR: 8340392: Handle OopStorage in location decoder [v3]

[Aleksey Shipilev]

On Wed, 18 Sep 2024 20:22:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix 32-bit builds
>
> src/hotspot/share/gc/shared/oopStorage.cpp line 1151:
> 
>> 1149: bool OopStorage::Block::print_containing(oop* addr, outputStream* st) {
>> 1150:   if (contains(addr)) {
>> 1151:     st->print(INTPTR_FORMAT " is a pointer %u/" SIZE_FORMAT " into block %zu",
> 
> s/SIZE_FORMAT/%zu/
> s/INTPTR_FORMAT/PTR_FORMAT/ - because it's semantically a pointer.

Should be fixed in new commit.

> src/hotspot/share/runtime/os.cpp line 1322:
> 
>> 1320: 
>> 1321:   // Ask if any OopStorage knows about this address.
>> 1322:   if (OopStorageSet::print_containing((oop*)addr, st)) {
> 
> `addr` might not be oop-aligned, in which case this cast and use might lead to UB.  I think this should
> be gated on `is_aligned(addr, alignof(oop))`.

I don't want to lose the match if the pointer is unaligned. The unaligned pointer might is still be technically _in range_ that is covered by the OopStorage. See new commit, I think we can handle it more accurately without losing this match?

> test/hotspot/gtest/gc/shared/test_oopStorageSet.cpp line 109:
> 
>> 107: class OopStorageSetTest::VM_PrintAtSafepoint : public VM_GTestExecuteAtSafepoint {
>> 108: private:
>> 109:  class PrintContainingClosure : public Closure {
> 
> Need another leading space here for proper indentation.

Right, should be fixed in new commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21072#discussion_r1766185213
PR Review Comment: https://git.openjdk.org/jdk/pull/21072#discussion_r1766187319
PR Review Comment: https://git.openjdk.org/jdk/pull/21072#discussion_r1766185355