RFR: 8296469: Instrument VMError::report with reentrant iteration step for register and stack printing [v9]

[Axel Boldt-Christmas]

On Fri, 5 May 2023 14:08:45 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>> 
>>  - Merge remote-tracking branch 'upstream_jdk/master' into vmerror_report_register_stack_reentrant
>>  - Add test
>>  - Fix and strengthen print_stack_location
>>  - Missed variable rename
>>  - Copyright
>>  - Rework logic and use continuation state for reattempts
>>  - Merge remote-tracking branch 'upstream_jdk/master' into vmerror_report_register_stack_reentrant
>>  - Restructure os::print_register_info interface
>>  - Code syle and line length
>>  - Merge Fix
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/2009dc2b...2e12b4a5
>
> src/hotspot/share/utilities/vmError.cpp line 173:
> 
>> 171: }
>> 172: 
>> 173: static bool check_stack_headroom(Thread* thread,
> 
> Could you please write a short comment what the return means? From the code, I assume true means "not enough headroom"? Maybe rename function to "stack_has_headroom"?

Done.

> src/hotspot/share/utilities/vmError.cpp line 187:
> 
>> 185:   const ptrdiff_t stack_headroom = stack_pointer - stack_bottom;
>> 186:   return (stack_pointer < stack_bottom || stack_headroom < 0 ||
>> 187:           static_cast<size_t>(stack_headroom) < headroom);
> 
> Could be shortened. E.g. `return stack_pointer - headroom < stack_bottom` ?

I reworked the logic. It should be clearer now. And deal properly with over-/underflows. 
I noticed when testing on different OS that not including the stack guard size was a bad idea. Some platforms had more than 64K as a guard size.

> src/hotspot/share/utilities/vmError.cpp line 476:
> 
>> 474:   continuation = i + 1;
>> 475:   const frame fr = os::fetch_frame_from_context(context);
>> 476:   while (i < 8) {
> 
> Can we name this constant (function scope const is fine, something like "number_of_stack_slots" or so).

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/11017#discussion_r1193408494
PR Review Comment: https://git.openjdk.org/jdk/pull/11017#discussion_r1193410544
PR Review Comment: https://git.openjdk.org/jdk/pull/11017#discussion_r1193411031