RFR 8203466: intermittent crash at jdk.internal.misc.Unsafe::getObjectVolatile (native)

[coleen.phillimore at oracle.com]

Hi, I just pushed this because I wasn't expecting more comments.

On 9/18/18 9:55 AM, Doerr, Martin wrote:
> Hi Coleen,
>
> is it really safe to keep a value in R9 across the barrier's runtime call?

I was going to save r15 also to thread local, but since r9 wasn't the 
problem we observed and is not aliased to something like rscratch1, 
which invites reuse, I think it's safe for now.  I don't think this code 
is future-proof and hand coded assembly for performance is something I 
wish we could avoid.

> It's specified as volatile in [1].
>
> I think it'd be nice if we had a dedicated stub calling convention. Calling (almost) only cpu specific stubs via os+cpu specific C calling conventions is a mess. But thanks for fixing the issue.

I would be nice if this was less of a mess.  Let me know if you find 
that this doesn't fix the issue.  This wasn't reproduceable.
thanks,
Coleen
>
> Best regards,
> Martin
>
> [1] https://docs.microsoft.com/en-us/cpp/build/register-usage?view=vs-2017
>
>
> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of coleen.phillimore at oracle.com
> Sent: Dienstag, 18. September 2018 14:55
> To: Erik Österlund <erik.osterlund at oracle.com>; hotspot-dev at openjdk.java.net
> Subject: Re: RFR 8203466: intermittent crash at jdk.internal.misc.Unsafe::getObjectVolatile (native)
>
> Thanks Erik!
> Coleen
>
> On 9/18/18 5:45 AM, Erik Österlund wrote:
>> Hi Coleen,
>>
>> Looks good. I guess the whole thing looks horrible in a way, but the
>> patch is good.
>> Thanks for fixing this.
>>
>> /Erik
>>
>> On 2018-09-18 01:47, coleen.phillimore at oracle.com wrote:
>>>
>>> On 9/17/18 5:17 PM, dean.long at oracle.com wrote:
>>>> Can you move the JavaThread _WIN64 changes to thread_windows_x86.hpp?
>>> Thanks for looking at this.  I moved it to the better place:
>>>
>>> http://cr.openjdk.java.net/~coleenp/8203466.02/webrev/index.html
>>>
>>> Retested with build and ran one test 100 times that failed once.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>> dl
>>>>
>>>> On 9/17/18 5:05 AM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>> On 9/14/18 4:01 PM, dean.long at oracle.com wrote:
>>>>>> Hi Coleen.  In the bug you asked:
>>>>>>
>>>>>>> Why can't we just push rdi and rsi to the stack and restore
>>>>>> them? This code appears to be leaf code so shouldn't care about
>>>>>> stack format
>>>>>>
>>>>>> Did that ever get answered?  If there is concern about the pushes
>>>>>> and pops canceling out at the end, you could always reserve the
>>>>>> stack space at the beginning, after
>>>>>>
>>>>>> __ enter();
>>>>>>
>>>>>> and then setup_arg_regs/restore_arg_regs would not need to adjust
>>>>>> the stack pointer.
>>>>> The reason I didn't do this is that I couldn't get it to work for
>>>>> all the various call sites.  Also, the barrier code is going to
>>>>> need the thread register sometimes which wasn't available. So I
>>>>> went with this.  I'm somewhat unclear with how this is called from
>>>>> the c2 code.  If you want to try this solution, I should probably
>>>>> give up and reassign it to compiler.
>>>>>
>>>>> thanks,
>>>>> coleen
>>>>>
>>>>>> dl
>>>>>>
>>>>>> On 9/14/18 11:32 AM, coleen.phillimore at oracle.com wrote:
>>>>>>> Summary: Save windows 64 callee saved registers rsi, rdi to
>>>>>>> Thread, save r15, also callee saved to r9
>>>>>>>
>>>>>>> This is done for generated stubs that do arraycopy that need GC
>>>>>>> barrier code because GC assumes that r10 is scratch, since it's
>>>>>>> defined as rscratch1.  See bug for more details. Thanks Erik for
>>>>>>> your help and the diagnosis.
>>>>>>>
>>>>>>> Tested with mach5 hs-tier1-7.  We don't have a reproduceable test
>>>>>>> case.
>>>>>>>
>>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8203466.01/webrev
>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8203466
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>