RFR: 8314508: Improve how relativized pointers are printed by frame::describe [v2]

[Fredrik Bredberg]

On Thu, 7 Mar 2024 20:45:17 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Fredrik Bredberg 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 three additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8314508_relativized_ptr_frame_describe
>>  - Small white space fix
>>  - 8314508: Improve how relativized pointers are printed by frame::describe
>
> src/hotspot/share/runtime/frame.cpp line 1634:
> 
>> 1632:       st->print_cr(" %s  %s %s", spacer, spacer, fv.description);
>> 1633:     } else {
>> 1634:       if (*fv.description == '#' && isdigit(fv.description[1])) {
> 
> Is it possible for fv.description to be null?  its sort of unclear from the context. There are several pointer dereferences here.
> 
> Can you add a comment about what the format of this is?  What is fv.description supposed to be? #<what number?>
> 
> And fp.location is the relativized offset, if fp.description is NOT #<number>?
> 
> This really does need some comment.  And maybe a local variable to say what *fv.location is.
> 
> And after testing the #<some number> output, the code still has to see if fv.location is between -100 and 100?  Should it not just check fp != nullptr ?

@coleenp 

I'll try to answer as good as I can.

**Q1:** Is it possible for `fv.description` to be null? its sort of unclear from the context. There are several pointer dereferences here.
**A1:** If `fv.description` ever had been null (before any of my changes) it would have lead to a crash, so in my world it is never null.

**Q2:** Can you add a comment about what the format of this is? What is `fv.description` supposed to be? #<what number?>
**A2:** If you look at the stack frame in the description of this PR you'll see the contents of the `fv.description` after the `<address>: <content>` columns.
    The only `fv.description` string starting with a '#' is the line for the saved frame pointer. So, `#10 method java.lang.invoke.LambdaForm...` basicaly means frame 10.
    This is also how I find the frame pointer.

**Q3:** This really does need some comment. And maybe a local variable to say what `*fv.location` is.
**A3:** `fv.location` contains the `<address>:` columns value and `*fv.location` contains the `<content>` columns value.

However if the value at an address is relativized like 0x000000409e14cc20 in the example, the real content of address 0x000000409e14cc20 is 0x0000000000000003. So instead of showing the contents as 3 (which we used to do before my patch) we show the real derelativized pointer value which is 0x000000409e14cc88 and also add the text `(relativized: fp+3)` to try to explain to anyone that's confused by the fact that the real content of the stack address is 3.

**Q4:** And after testing the # output, the code still has to see if fv.location is between -100 and 100? Should it not just check `fp != nullptr` ?
**A4:** No. In order for us to believe that `*fv.location` is a fp-relative value it must be a reasonable small positive/negative number. Otherwise we would print all large pointer values as fp-relative.

The oddball situation is that I've found frame pointer addresses pointing to a content which is within -100 and 100, with a `fv.description` string starting with a '#'. And those a not relativized values, and therefore should not be printed as such. Hence the `*fv.description != '#'` condition.

Hope this shed some light on this code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18102#discussion_r1517771365