RFR (S): 8245509: Crash handler itself crashes when reporting Unsafe.putInt(0) crash

David Holmes david.holmes at oracle.com
Mon Jun 1 06:44:13 UTC 2020



On 30/05/2020 1:55 am, gerard ziemski wrote:
> hi Coleen, David,
> 
> Thank you for the reviews.
> 
> 
> On 5/28/20 6:59 PM, David Holmes wrote:
>> Hi Gerard,
>>
>> On 29/05/2020 3:36 am, gerard ziemski wrote:
>>> hi all,
>>>
>>> Please review this small and simple fix that adds ResourceMark to 
>>> crash handler when printing register info, without which can trigger 
>>> fatal error if the contents of a register happens to be an oop, which 
>>> uses as_C_string(), that in turn 
>>> calls ResourceArea::allocate_bytes(), which requires a ResourceMark.
>>>
>>> If thread is not available, we do not attempt to print anything at 
>>> this step (i.e. print_register_info()), but the very next step (i.e. 
>>> print_context()) will always print the registers as HEX values.
>>>
>>> bug link at https://bugs.openjdk.java.net/browse/JDK-8245509
>>> webrev at http://cr.openjdk.java.net/~gziemski/8245509_rev1
>>> passes Mach5 hs_tier1,2,3,4,5
>>
>> This fixes the current problem and can be pushed.
>>
>> But as Coleen notes in the bug report there are other callers of 
>> os::print_location that might also need a ResourceMark, and based on 
>> your stack trace it seems it is Universe::heap()->print_location that 
>> is the true source of the problem and where the ResourceMark perhaps 
>> should be? So I think we need a follow up bug to fix the broader problem.
> 
> Thank you David for pointing this out, I misread Coleen's comment - I 
> thought the "(and one with one)" referred to the "thread" argument of 
> ResourceMark, not ResourceMark itself.
> 
> So I think BlockLocationPrinter::print_location() would be the actual 
> place that needs the ResourceMark (without the thread argument), is that 
> correct?

For G1 that is what I believe Universe::heap()->print_location will end 
up calling:

bool G1CollectedHeap::print_location(outputStream* st, void* addr) const {
   return BlockLocationPrinter<G1CollectedHeap>::print_location(st, addr);
}

but it in turn is calling:

  o->print_on(st);

where o is an Oop, and eventually the problem surfaces in 
Klass::oop_print_on. So that seems to be where the ResourceMark should 
be unless we have designated it something where the caller is expected 
to have the RM ... which it is, and the caller in this case is 
InstanceKlass::oop_print_on which neither has a ResourceMark nor states 
that a RM is needed in the caller. So arguably that is where the RM 
should be placed.

I'd vote for putting the ResourceMark as far down the call chain as 
possible, but Coleen deals with this code more often than I and there 
may be a reason to move it higher to e.g. 
BlockLocationPrinter::print_location as you suggested.

Cheers,
David
-----

> 
> 
> cheers
> 


More information about the hotspot-runtime-dev mailing list