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

Aleksey Shipilev shade at redhat.com
Tue Jan 29 10:54:00 UTC 2019


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.

> ------------------------------------------------------------------------------ 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



More information about the hotspot-dev mailing list