[11u] RFR(S): 8223266: PPC64: Check for branch to illegal address before checking for mem serialization

[Gustavo Romero]

Hi Goetz,

Thanks a lot for reviewing it!

I've just added the tag and the comment requesting for the approval to push.

I'll ping when the approval request is updated.

Best regards,
Gustavo  

On 05/06/2019 04:09 AM, Lindenmaier, Goetz wrote:
> Hi Gustavo,
> 
>>> Looking at the issue:
>>> actually the current VM would think the signal is a
>>> memory serialization and return true, while it is
>>> a real crash? (Or a simulated one as in the test?)
>>
>> is_memory_serialization() never returns, so the VM never knows if it's
>> indeed a memory serialization case or not.
>>
>> Because the issue on some old kernels forbids us to rely on 'si_addr' we
>> are inspecting the instruction at 'pc' to extract the registers used in
>> the instruction to determine the actual faulty address before calling
>> os::is_memory_serialize_page(), similarly to what we do to dertermine the
>> faulty address in get_stack_bang_address().
>>
>> In doing it's assumed that SIGSEGV can only be caused due to a load/store
>> (Data Storage Interrupt), because it assumes it's  always possible to read
>> the instruction at 'pc'. This is not true, particularly for a SIGSEGV due
>> to a branch to an invalid address (e.g. not mapped / no permission to
>> read/exec address), which is generated due to Instruction Storage Interrupt
>> (ISI). Hence we can use the interruption type (passed in 'trap' member to
>> the signal handler) to discern when it's possible to inspect the 'pc' (DSI)
>> and when it's not possible (ISI).
>>
>> The test case 13 precisely stresses that case of branching to an invalid
>> address by forcing a function bad pointer = 0xf and calling it (on PPC64
>> that crash shows pc=0xc in the hs_err log due to the code alignment
>> requirements for execution).
>>
>> So the additional code block checking for SIGSEGV related to UseMemBar
>> feature on 11u needs to be adapted like the  previous block using
>> get_stack_bang_address(), which is already fixed on jdk/jdk tip.
> 
> Thanks for the detailed explanation!  Our tests are green, too.
> 
> Reviewed.
> You can push to jdk11u-dev once the bug gets the jdk11u-fix-yes tag.
> 
> Best regards,
>   Goetz.
> 
>