RFR(S): 8211852: inspect stack during error reporting

[Doerr, Martin]

Hi David and Götz,

thanks again for reviewing. I've pushed it and added a some more description and an example to the bug.

Best regards,
Martin


-----Original Message-----
From: David Holmes <david.holmes at oracle.com> 
Sent: Freitag, 12. Oktober 2018 09:42
To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR(S): 8211852: inspect stack during error reporting

On 12/10/2018 5:07 PM, Doerr, Martin wrote:
> Hi David,
> 
>> Assume addr is aligned and points to CAFEBABE then we would print
>> CAFEBABE. But if addr points to FE then we'd print FEBABE? And if BA
>> then BABE, and if BE then just BE. Is that correct?
> Exactly. We only performed a readability check for addr. The aligned address + sizeof(intptr_t) can already point to unreadable memory. (I didn't want to use page sizes here to determine an upper bound.)
> Usually, the memory elements are naturally aligned, so I think it's ok to dump fewer bytes for unaligned addresses.
> Do you agree or do you have a different proposal?

It's fine, I just wanted to be sure I understood.

Thanks,
David

> Thanks and best regards,
> Martin
> 
> 
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Freitag, 12. Oktober 2018 03:29
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S): 8211852: inspect stack during error reporting
> 
> Hi Martin,
> 
> On 11/10/2018 7:53 PM, Doerr, Martin wrote:
>> Hi David,
>>
>> I've removed the "(byte x of)" output. This new version just prints fewer bytes for unaligned addresses:
>> http://cr.openjdk.java.net/~mdoerr/8211852_stack_info/webrev.01/
> 
> I'm struggling to understand what this loop prints:
> 
> 1157     for (address p = addr; p < align_up(addr + 1,
> sizeof(intptr_t)); ++p) {
> 1158       st->print(" %02x", *(u1*)p);
> 
> Assume addr is aligned and points to CAFEBABE then we would print
> CAFEBABE. But if addr points to FE then we'd print FEBABE? And if BA
> then BABE, and if BE then just BE. Is that correct?
> 
> Thanks,
> David
> 
>> Your other requests are also reflected in this webrev.
>>
>> Maybe other reviewers have additional wishes.
>>
>> Thanks,
>> Martin
>>
>>
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Donnerstag, 11. Oktober 2018 10:01
>> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(S): 8211852: inspect stack during error reporting
>>
>> Hi Martin,
>>
>> On 9/10/2018 9:30 PM, Doerr, Martin wrote:
>>> Hi David,
>>>
>>> thank you for reviewing my change.
>>>
>>> An example of the output is:
>>> stack at sp + 5 slots: 0x00007f905f4fa618 points into unknown readable memory: 0x00007f905f4fa618:   20 a5 3f 5f 90 7f 00 00
>>> The "(byte x of)" only shows up for unaligned addresses.
>>
>> Sure - but when it does it seems a little odd.
>>
>>> "print_hex_dump" prints the address again so I could replace it by a loop. I think this makes sense.
>>
>> Okay.
>>
>>> I have chosen bytewise output because it's the most conservative assumption. So larger types need to get interpreted in reverse order on little endian.
>>
>> I was wondering about endianness and how to interpret the "byte x of".
>>
>>>> src/hotspot/share/utilities/vmError.cpp
>>>> Seems odd to have:
>>>> 751   STEP("printing top of stack info")
>>>> then
>>>> 769   STEP("printing registers, top of stack, instructions near pc")
>>>>
>>>> as they both refer to "top of stack". Should they be merged (I know you
>>>> touch on this above)? Should they be the other way round? Should they
>>>> use different descriptions?
>>>
>>> I had also asked myself such kind of questions.
>>> An argument against combining the steps is that if one of them fails (which can happen in rare cases) we still have the other one.
>>> I don't have a specific opinion about the order. I'm willing to change if you prefer them the other way round.
>>
>> I think the new stack details should come after the existing register
>> ToS etc.
>>
>>> Would "inspecting top of stack" be a better description?
>>
>> Sure.
>>
>>>> Any particular reason to show 8 slots? Or is it just a "Goldilocks number"?
>>> This is the number which is used in step "printing registers, top of stack, instructions near pc" on x86 which is the main platform I'd like to address with this change because I think it has the most complex stack usage.
>>
>> Ok.
>>
>> Thanks,
>> David
>>
>>> Further improvement requests are appreciated.
>>>
>>> Best regards,
>>> Martin
>>>