RFR 8079562: Crash in C2 compiled code with STATUS_FLOAT_MULTIPLE_TRAPS

Wed Aug 10 13:22:04 UTC 2016

This RFR is withdrawn.
Coleen

On 8/2/16 10:48 PM, Coleen Phillimore wrote:
>
>
> Hi, David, That would be a better test, I guess.  My goal here is to 
> fix this test from failing on windows 32 and salvage something out of 
> the existing test.   We could simply delete the test and file an RFE 
> to test fpcw better.
>
>
> On 8/2/16 10:34 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> So does this:
>>
>>      setCW( (FPU_DEFAULT | FPU_IEM) & ~EM_ZERODIVIDE );
>>
>> enable or disable hardware exceptions for FP-div-by-zero?
>
> I think it disables hardware exceptions.   I guess Christian's going 
> to have to answer this tomorrow.
>>
>> To be honest I'm not sure this test serves any real point in its 
>> current form any more. A general cross-platform test for the 
>> AlwaysRestoreFPU flag would be:
>>
>> static native int getFPUCW();
>> static native void setFPUCW(int cw);
>>
>> test() {
>>  int default_cw = getFPUCW();
>>  int new_cw = ~default_cw;
>>  setFPUCW(new_cw);
>>  assert(getFPU_CW() == default_cw, "AlwaysRestoreFPU didn't restore 
>> it!);
>> }
>>
>> but that kind of misses the point about triggering unhandled C++ 
>> exceptions. And isn't getting to the bottom of the internal error 
>> that was seen.
>>
>
> No, if that's required, I'll have to unassign myself from the bug, 
> quarantine the test, and save it for jdk10, because it might take a 
> long time to figure out, if we ever figure it out.  I don't think 
> there's a product issue here at least from my reading of the situation.
>
>> Sorry. Feel free to ignore me.
>>
>
> We never ignore you, David.
>
> Coleen
>
>> David
>>
>>
>> On 3/08/2016 12:14 PM, Coleen Phillimore wrote:
>>>
>>>
>>> On 8/2/16 9:32 PM, David Holmes wrote:
>>>> Hi Coleen,
>>>>
>>>> On 3/08/2016 11:16 AM, Coleen Phillimore wrote:
>>>>>
>>>>>
>>>>> On 8/2/16 8:35 PM, David Holmes wrote:
>>>>>> Hi Coleen,
>>>>>>
>>>>>> On 3/08/2016 8:34 AM, Coleen Phillimore wrote:
>>>>>>> Summary: Add AlwaysRestoreFPU and move the test to jtreg.
>>>>>>> Contributed-by: myself and christian.tornqvist at oracle.com
>>>>>>>
>>>>>>> Christian moved the test, I did some cleanup and added
>>>>>>> AlwaysRestoreFPU
>>>>>>> to it.
>>>>>>
>>>>>> You modified the original C code such that this comment is no longer
>>>>>> valid:
>>>>>>
>>>>>>   33 // Only valid for specific x86 based systems: linux-x86, or 
>>>>>> else
>>>>>> x86 with fpu_control.h
>>>>>
>>>>> Thanks, David.  I took out the rest of the comments but missed 
>>>>> this one.
>>>>>>
>>>>>> Not sure why you did that - we were running this test on more than
>>>>>> just Windows, despite the naming.
>>>>>
>>>>> Actually, the #ifdefs were intended to only run on 32 bit linux as 
>>>>> well
>>>>> as windows, but I don't think the ifdefs were correct, so the test
>>>>> always printed that it was skipped on platforms other than 
>>>>> windows.  The
>>>>> test is only useful on windows.   Modifying the fpcw didn't have an
>>>>> effect on linux x86.  Seems silly to run the test on platforms where
>>>>> it's not testing anything.
>>>>
>>>> It's been a while but I've been involved with this test in the past
>>>> and I thought it was doing similar testing on other x86 platforms. As
>>>> you say the main point is to verify that messing with the FPU control
>>>> word doesn't break anything so I'm not sure that is a reason not to
>>>> have such a test run on linux.
>>>
>>> The original point of the test is unknown to me.  The test was doing
>>> something that can cause the vm to crash or get the wrong behavior. The
>>> new point of the test is to test the AlwaysRestoreFPU flag. Since we 
>>> can
>>> provoke a crash on windows 32 bit only, this seems like a good
>>> restriction for the test.
>>>>
>>>>>>
>>>>>> I'm also unclear exactly what changed such that we need to add
>>>>>> AlwaysRestoreFPU ??
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8076284 allegedly changed 
>>>>> the
>>>>> behavior in 2015.
>>>>
>>>> I certainly can't make any connection between that bug and handling of
>>>> the FPU control word. :(
>>>>
>>>
>>> Me neither but the failure I got was from the divide by zero getting a
>>> windows internal error, not the C2 generated code.  Rereading the bug,
>>> Vladimir thought it could have come from the VC++ compiler change and
>>> not the fix.
>>>
>>>>> Setting floating point control word is something that we don't 
>>>>> promise
>>>>> to work with generated code or the jvm code, which is why this
>>>>> -XX:+AlwaysRestoreFPU flag was added.
>>>>
>>>> Understood, just puzzled about why this test suddenly requires it.
>>>> Given the FPU mode used by the VM has not changed, why does the test
>>>> now fail without this??
>>>>
>>>
>>> I don't know why it worked before.  There could have been a library
>>> change also that gets the internal error.  It wasn't an unhandled
>>> floating point exception.
>>>
>>>>>>
>>>>>> Can you add @Summary info to the Java test (and/or other commentary)
>>>>>> as it is far from clear exactly what this is testing.
>>>>>>
>>>>>
>>>>> Yes, I can do that.   How about:
>>>>>
>>>>>  * @summary Test that modifying the floating point control word 
>>>>> doesn't
>>>>> cause unhandled Windows floating point exceptions
>>>>
>>>> That's fine -thanks. Can you also document with a comment exactly what
>>>> our modification of the FPU control word is intended to do and how
>>>> that relates to the normal FPU mode used by the VM. This is something
>>>> I always have to go looking for :)
>>>
>>> Okay, how about:
>>>
>>>  * @summary Test that modifying the floating point control word doesn't
>>> cause an internal
>>>  *          error when turning off floating point exceptions for divide
>>> by zero.
>>>
>>> See new webrev for comments about the default.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8079562.02/webrev
>>>
>>>
>>> Again, adding the flag is the solution that we give customers. We also
>>> test for changing the fpcw with -Xcheck:jni.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>>
>>>>>
>>>>> Can summary lines be > 1 line or do they have to be one line?
>>>>
>>>> A tag runs until the next tag is encountered so the summary can span
>>>> multiple lines.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> open webrev at 
>>>>>>> http://cr.openjdk.java.net/~coleenp/8079562.01/webrev
>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8079562
>>>>>>>
>>>>>>> Tested with JPRT.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Coleen
>>>>>
>>>
>