RFR (S): 8245833: crash_with_sigfpe uses pthread_kill(SIGFPE) on macOS

[David Holmes]

On 30/05/2020 2:59 am, gerard ziemski wrote:
> On 5/29/20 11:52 AM, gerard ziemski wrote:
>> hi David,
>>
>> Thank you for the review.
>>
>> On 5/28/20 7:03 PM, David Holmes wrote:
>>> Hi Gerard,
>>>
>>> On 29/05/2020 3:34 am, gerard ziemski wrote:
>>>> hi all,
>>>>
>>>> Please review this small and simple fix, that implements 
>>>> crash_with_sigfpe() in a way that causes an actual crash on macOS, 
>>>> so it doesn't need to fallback that uses pthread_kill()
>>>>
>>>> bug link at https://bugs.openjdk.java.net/browse/JDK-8245833
>>>> webrev at http://cr.openjdk.java.net/~gziemski/8245833_rev1
>>>> passes Mach5 hs_tier1,2,3,4,5
>>>
>>> Fix looks fine.
>>>
>>> So presumably this old code:
>>>
>>>    volatile int x = 0;
>>>    volatile int y = 1/x;
>>>
>>> is actually elided by the compiler when we build for macOS?
>>
>> It's not exactly elided, since the compiler still generates assembly 
>> for that code, but I noticed that while normally the compiler would 
>> complain about the unused "y", in this case it does not, so it 
>> probably optimizes it without actually performing the division by 
>> zero, due to some compiler flag we are using (I don't know which one 
>> makes the difference here), i.e.:
>>
>>   volatile int x = 0;
>>   volatile int y = 1/x;
>>
>>   xorl  %eax, %eax
>>   .loc  37 1751 16              ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1751:16
>>   movl  %eax, -88(%rbp)
>>   .loc  37 1752 22              ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1752:22
>>   movl  -88(%rbp), %ecx
>>   .loc  37 1752 21 is_stmt 0    ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1752:21
>>   leal  1(%rcx), %edx
>>   cmpl  $3, %edx
>>   cmovael  %eax, %ecx
>>   .loc  37 1752 16              ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1752:16
>>   movl  %ecx, -152(%rbp)
>>
>> I don't see division instruction here, however for:
>>
>>  sigfpe_int = sigfpe_int/sigfpe_int;
>>
>>   .loc  37 1751 16              ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1751:16
>>   movl  _sigfpe_int(%rip), %eax
>>   .loc  37 1751 26 is_stmt 0    ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1751:26
>>   cltd
>>   idivl  _sigfpe_int(%rip)
>>   .loc  37 1751 14              ## 
>> open/src/hotspot/share/utilities/vmError.cpp:1751:14
>>   movl  %eax, _sigfpe_int(%rip)
>>
>> we see the "idivl" instruction in the assembly.
>>
>> For reference, a simple C test case with standard compiler flags 
>> produces:
>>
>>  volatile int x = 0;
>>  volatile int y = 1/x;
>>
>>  .loc  1 439 16                ## hello/main.cpp:439:16
>>  movl  $0, -20(%rbp)
>>  .loc  1 440 22                ## hello/main.cpp:440:22
>>  movl  -20(%rbp), %ecx
>>  .loc  1 440 21 is_stmt 0      ## hello/main.cpp:440:21
>>  movl  $1, %edx
>>  movl  %eax, -28(%rbp)         ## 4-byte Spill
>>  movl  %edx, %eax
>>  cltd
>>  idivl  %ecx
>>  .loc  1 440 16                ## hello/main.cpp:440:16
>>  movl  %eax, -24(%rbp)
>>  .loc  1 441 3 is_stmt 1       ## hello/main.cpp:441:3
>>
>> which also has the "idivl" instruction and also crashes, so it must be 
>> one of our compiler flags that optimizes the unused variable?
> 
> It must be more than optimizing an unused variable, because even when I 
> do use the "y" (print its vale out - it's 0) the code still will not 
> crash. Some other optimization is at play here...

Thanks for investigating. It is a puzzle. :) But as long as the new code 
successfully raises SIGFPE the change is good.

Cheers,
David

> 
> cheers