RFR: 8361712: Improve ShenandoahAsserts printing [v2]
Ashutosh Mehra
asmehra at openjdk.org
Thu Jul 24 20:30:58 UTC 2025
On Wed, 16 Jul 2025 14:01:30 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Small changes to ShenandoahAsserts and friends to improve fault-tolerance and usefulness:
>>
>> - In assert_correct, we now check the forwarding pointer first before extracting the narrowKlass from it. The forwarding pointer may be invalid. To do this, the forwarding-pointer-checking-block was moved up to before Klass* checks. We also use SafeFetch-based checks for some added safety.
>> - In assert_correct, we test the narrowKlass before decoding it. Otherwise, if it's 0, we'd assert, if garbage, the resulting Klass* would be garbage.
>> - In print_failure, we now check pointers for readability before handing them to print_obj
>> - In print_obj, we now print the extracted narrowKlass, too (useful on Lilliput with its terse ids)
>> - In print_obj, we print a small hex dump of the header (32bytes). With little cost, we get a glance at the raw header and the first members.
>> - In print_obj, we print to a single stringStream now and use auto indentation. Avoids having to deal with indentation and newlines manually and having to pay for four stringStreams.
>> - In assert_correct, I raised the print level for some of the klass-related assertions to safe_oop since the oop, since the hardened version of print_obj now avoids tripping over invalid Klass*
>>
>> Tests: I manually tested the patch with several manually broken oops of various flavours.
>>
>> Example output, invalid forwardee:
>>
>>
>> Referenced from:
>> no interior location recorded (probably a plain heap scan, or detached oop)
>>
>> Object:
>> 0x00000007cd800000 - nk 1798104 klass 0x00000000921b6fd8 [Ljava.lang.Object;
>>
>> allocated after mark start
>> after update watermark
>> marked strong
>> marked weak
>> not in collection set
>> mark: marked(0xdeaddeaddeaddeaf)
>> region: | 3812|R |Y|BTE 7cd800000, 7cd920bf8, 7cdc00000|TAMS 7cd800000|UWM 7cd800000|U 1154K|T 0B|G 0B|S 1154K|L 0B|CP 0
>>
>> 0x00000007cd800000: deaddeaddeaddeaf 00000a96001b6fd8 f9b005cff9b0054d f9b006d3f9b00651 .........o......M.......Q.......
>> 0x00000007cd800020: f9b00797f9b00755 f9b00992f9b0087d f9b009a0f9b00999 f9b009a7f9b0098e U.......}.......................
>>
>> Forwardee:
>> 0xdeaddeaddeaddeac - safe print, no details
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>
> - Merge branch 'openjdk:master' into harden-shenandoah-asserts
> - small revert
> - cosmetics
> - wip
> - wip
> - wip
> - wip
> - assert_correct
src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp line 63:
> 61: ResourceMark rm;
> 62: stringStream ss;
> 63: StreamIndentor si(&ss);
This seems redundant.
src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp line 140:
> 138:
> 139: if (!os::is_readable_pointer(obj)) {
> 140: level = _safe_unknown;
Correct me if I am wrong - shouldn't the caller be responsible for passing correct `level` to `print_failure`? If the caller passes a potentially invalid obj with level = _unsafe_oop then the caller should be updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26237#discussion_r2229509058
PR Review Comment: https://git.openjdk.org/jdk/pull/26237#discussion_r2229508942
More information about the shenandoah-dev
mailing list