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

[Axel Boldt-Christmas]

On Fri, 20 Jan 2023 10:11:54 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Is this about the variable size of the object? In its simplest form, the approach could be done with a word-sized datum that fits into a void*, no additional memory needed. You can put the void* directly on the stack.
> 
> In fact, we could even just pass an int, or a long, and declare this as continuation point which the implementation should interprete as it sees fit. On most platforms, that would correspond to the register number. Similar to what you do now.
> 
> ```
> os::print_register_info(const ucontext_t* c, outputStream* st, int& continuation_point) {
>   // .. start printing at register number conti_point + 1
> }
> ```
> 
> and
> 
> ```
> os::print_register_info(const ucontext_t* c, outputStream* st) {
>   int conti_point = 0;
>   print_register_info(c, st, conti_point);
> }
> ```
> 
> You could hide those behind a typedef or a class for nicer aesthetics:
> 
> ```
> typedef int continuation_point_t;
> static const continuation_point_t INIT_POINT = 0;
> ```
> 
> or
> 
> ```
> class ContinuationPoint {
>   int _v;
>   ContinuationPoint() : _v(0) {}
>   int raw_value() ... 
>   void increment() ...
> }
> ```
> 
> This would be a pragmatic approach that also translates well to printing callstacks, the other thing we want with more robustness.
> 

I like the pragmatic approach with `os::print_register_info(const ucontext_t* c, outputStream* st, int& continuation_point)` more than `void*`. I will look into how this approach might look with callstacks as well.


> I disagree with that. I find the `REENTRANT_STEP_IF` logic difficult to parse compared to the old solution. To me, it is also not clear how STEP timeouts are handled. Backporting fixes across such an intrusive patch would be painful.

Fair enough. I generally dislike code duplication. Maybe just adding a multiple STEPs with some continuation logic will more easily please all parties (including future backporters). However it does lose the feature that it currently disables retrying based on stack depth, and number of global reentry failures. Can always add it explicitly to the repeated STEPs, a little verbose and in my own opinion a little harder to understand.

> Side note, if others disagree with me and think this is the way to go, I won't hold up the patch.

If the repeated STEPs and continuation logic produces less friction I am all for it. In the end the result that is most valuable is improved hs_err files. And that would be a nice step in that direction.

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

PR: https://git.openjdk.org/jdk/pull/11017