RFR (S) 8144518: ClassVerboseTest crashes on Windows

[David Holmes]

Hi Coleen,

On 18/01/2017 12:13 PM, Coleen Phillimore wrote:
>
>
> On 1/17/17 7:02 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> The bug report does not explain the problem so it is unclear whether
>> this workaround is minimal. Also some commentary somewhere would be
>> useful else the bug might return inadvertently - more generally I'd
>> like to understand what other code might be impacted by this.
>
> Christian and I spent a solid week looking for the windows Visual Studio
> bug that caused this but weren't able to find it.  He verified that it's
> fixed in VS2015.  I think that's in the bug report.  The code generated
> is in the bug report with annotations of what lines the code it was
> executing and which instructions caused the truncation and sign
> extension leading to a negative offset from the start of _bcp.

I'm not seeing that listing in the bug report. What code was generated 
versus what code should have been generated is precisely what I was 
looking for. And some idea of what problematic source code looks like.

> There's a small reproducer in the bug report for 8144518.
>
> I did a grep of all the "address.*-" and looked through the lines of
> code but I didn't generate windows assembly code to examine for all the
> pointer subtractions.  I don't plan to do this.   From the reproducer,
> there needed to be the swap_u4, even though the load that crashed was
> before the swap_u4 code.  The scenario that caused this bug seems very
> specific and if it exists in any other places in the jvm, we can keep an
> eye out for it.  This crash, while intermittent, was fairly consistent.

Very intermittent - so I guess we rarely hit the incorrectly compiled 
code path.

> We think it's best to get this workaround into jdk 9 before ZBB since
> this bug has been seen several times, and we finally narrowed down the
> problem and don't have to close it as not reproduceable again.
>
> There is a line above the dest_offset_at code which I was going to
> remove, since it's describing code I changed, which doesn't make for a
> good comment.  Any further commentary or explanation of this bug will be
> vague, since we don't have the VS compiler to debug to find the real
> problem, and won't make sense in the source code.
>
> Did you look at the code? It's a simplification of the expression that
> was in the original, that would have been better from the start.

Yes I looked at the code. AFAICS you now pass in an unaligned value and 
align it internally, while previously the caller had to do the alignment 
first - right? That inversion somehow bypasses the code that gets 
incorrectly compiled.

Thanks,
David
-----

> Thanks,
> Coleen
>
>>
>> Thanks,
>> David
>>
>> On 18/01/2017 8:49 AM, Coleen Phillimore wrote:
>>> I should have also sent this to hotspot-dev, since Bytecode_tableswitch
>>> is used by the compiler ci code.
>>> thanks,
>>> Coleen
>>>
>>>
>>> On 1/17/17 1:49 PM, Coleen Phillimore wrote:
>>>> Summary: simplify Bytecode_tableswitch code so windows doesn't
>>>> generate bad code for it.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8144518.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8144518
>>>>
>>>> Verified generated code does not have sign extended value that is
>>>> subtracted, giving the wrong offset.  Ran all rbt nightly tests. Ran
>>>> some -Xcomp tests.
>>>>
>>>> See more info in bug https://bugs.openjdk.java.net/browse/JDK-8171968
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>>
>