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

[Stefan Karlsson]

On 2019-01-29 11:54, Aleksey Shipilev wrote:
> On 1/29/19 11:44 AM, Stefan Karlsson wrote:
>> ------------------------------------------------------------------------------
>> Did you intend to use & instead of && here ?:
>>
>> + while (delta > 0 & pc > low && !is_readable_pointer(low)) {
> Yes! Fixed.
>
>> ------------------------------------------------------------------------------Regarding this comment:
>>
>> + // Safe probing relies on the conjecture that the entire pages are either + // readable or
>> unreadable. Therefore, we cannot have delta larger than page + // size, since otherwise we can
>> discover "far" page as readable, while "near" + // page is not readable.
>>
>> The code doesn't seem to prevent this. If 'pc' is somewhere at the beginning of a page, then
>>
>> 'low = pc - delta' could cross over to the previous page.
> Which is exactly what we want. We want to probe that "near" page. If it ends ups being unreadable,
> delta adjustment should roll "low" up. If it is readable, we would dump the piece of it. Probably
> the comment is confusing: we "only" need to make sure there are no "un-probed" full pages within the
> probing range. Updated the comment.

(not sure if this is going to get garbled, like my previous mail)

For this to work as you intend it, max_delta must not be set to page 
size. If max_delta is set to page size, the following would end up with 
low in Page 0 and high in Page 2:

+--------+
| Page 0 |   readable
+--------+ < 'pc' beginning of Page 1
| Page 1|   unreadable
+--------+
| Page 2 |   readable
+--------+

Therefore it seems a bit off to cap with the page size in:
MIN2(256, min_page_size());

Given that you use 256 today, this isn't really a problem, but maybe 
future proof this a bit and use min_page_size / 2 to guarantee that 
either low or high ends up in Page 1?

Thanks,
StefanK
>
>> ------------------------------------------------------------------------------ The probing of safe
>> reads only happen at power-of-two offsets from 'pc'. Did you consider using binary search to find
>> the exact break point where the reading fails? It's probably overkill for this feature though.
> Overkill, IMO.
>
>> ------------------------------------------------------------------------------ I see that: bool
>> os::is_readable_range(const void* from, const void* to) { for (address p = align_down((address)from,
>> min_page_size()); p < to; p += min_page_size()) { if (!is_readable_pointer(p)) { return false; } }
>> return true; } is using min_page_size(), instead of os::vm_page_size(), which has this comment: //
>> Return a lower bound for page sizes. Also works before os::init completed. static size_t
>> min_page_size() { return 4 * K; } Is this something this code needs to consider as well?
>> os::vm_page_size() can assert / return -1 if called too early.
> Yes, os::min_page_size() is safer here, changed.
>
> New webrev:
>    http://cr.openjdk.java.net/~shade/8217879/webrev.04
>
>
> Thanks,
> -Aleksey
>