RFR (S) 8217879: hs_err should print more instructions in hex dump

[Thomas Stüfe]

On Mon, Jan 28, 2019 at 9:55 PM Aleksey Shipilev <shade at redhat.com> wrote:

> On 1/28/19 8:51 PM, Thomas Stüfe wrote:
> > I agree that a larger dump would be helpful, I had the same thought in
> the past. I know others
> > prefer slim hs-err files, but they would have to chime in.
>
> There is a bespoke rule in Hotspot that choosing between more asserts and
> fastdebug performance one
> should choose more asserts. I am mentally extending this to hs_err:
> choosing between more debugging
> information and slimmer hs_err one should choose more debugging
> information :)
>
> > os::print_hex_dump_surrounding(address pivot, size_t len, size_t
> unitsize)
> >
> > and let that function print the pivot address up front (obviously
> avoiding to name it "pc"), then
> > the dump (+- len around pivot) and maybe a little ">" marker at the
> start of the pivot line.
>
> ...except current function outputs "Instructions" header, so it is dumping
> raw instruction stream. I
> used to call it os::print_hex_dump_near, but renamed it because of that
> "Instructions" header.
>

It is okay I guess. There are a number of possible code reshufflings there,
but that can be left for later.


>
>
> > In addition, I never liked that "os::print_context" even calls this. The
> function name sounds
> > harmless, like a simple "print the ucontext structure" but is
> potentially dangerous since it just
> > dereferences whatever it finds in pc. So even though it looks that way
> it is by no means a general
> > purpose function, but only usable in error reporting. Your patch makes
> it more dangerous since the
> > printed area now is larger and so we run a larger risk in segfaulting.
> >
> > I have a version of os::print_hex_dump locally somewhere which uses
> SafeFetch32 to print out the hex
> > dump, printing little "?" for unmapped memory. I think that would be the
> correct way.
>
> Yes, my early version had something like this:
>
>  address bottom = pc - delta;
>  address top = pc + delta;
>  while (pc > bottom && !is_readable_pointer(bottom)) bottom++;
>  while (pc < top && !is_readable_pointer(top)) top--;
>  os::print_hex_dump(bottom, top, ...);
>
> ...which makes it safe by reducing the dump window to only readable
> memory. But then I realized that
> vmError machinery would not trip hard on this failure, and would just
> print this message:
>
> Instructions: (pc=0x0000000000000000)
> 0x0000000000000000:
> [error occurred during error reporting (printing registers, top of stack,
> instructions near pc), id
> 0xb, SIGSEGV (0xb) at pc=0x00007f9e0a6e59a5]
>
> ...so I backed off for simplicity. I can reinstate this safety net.
>
>
Sounds reasonable. Unless we happen to run on architectures with very small
page sizes ;)

..thomas

-Aleksey
>
>