[15] RFR(S): 8244433: Remove saving of RSP in Assembler::pusha_uncached()

[Christian Hagedorn]

Hi Vladimir

On 14.05.20 20:52, Vladimir Kozlov wrote:
> Changes looks good.
> 
> I looked and this code existed from day one. And I don't remember we 
> ever had an issue with possible RSP corrupted values. I don't think you 
> need to worry about this after you fixed print code.

That's good, thank you for checking and your review!

Best regards,
Christian

> On 5/14/20 5:35 AM, Christian Hagedorn wrote:
>> Thank you Tobias for your review!
>>
>> Best regards,
>> Christian
>>
>> On 14.05.20 14:31, Tobias Hartmann wrote:
>>> Hi Christian,
>>>
>>> this looks good to me.
>>>
>>> Best regards,
>>> Tobias
>>>
>>> On 14.05.20 11:38, Christian Hagedorn wrote:
>>>> Hi
>>>>
>>>> Please review the following enhancement for x86:
>>>> https://bugs.openjdk.java.net/browse/JDK-8244433
>>>> http://cr.openjdk.java.net/~chagedorn/8244433/webrev.00/
>>>>
>>>> This removes the move instruction for saving the actual value of RSP in
>>>> Assembler::pusha_uncached()/pusha(). The original value of RSP is 
>>>> normally not used on the stack as
>>>> the value of RSP will automatically be restored after popa to the 
>>>> same value before doing pusha.
>>>> There are two locations, however, where we need to know the original 
>>>> value of RSP in order to print
>>>> it. But these places can also compute the correct value of RSP by 
>>>> using the new value of RSP after
>>>> pusha and adding 16 * wordSize to it. I fixed those.
>>>>
>>>> We still keep the same alignment by subtracting 16 * wordSize from 
>>>> RSP in pusha. Does anybody see
>>>> any potential problems by not saving the value of RSP on the stack 
>>>> with pusha?
>>>>
>>>> Either way, as Erik Ö. has pointed out, the Windows x64 ABI does not 
>>>> specify a red zone of 128
>>>> bytes. If we are unlucky and get an interrupt between saving RSP and 
>>>> decrementing RSP in the current
>>>> code, we could end up with a corrupt value for RSP on Windows. 
>>>> Therefore, we do need to fix
>>>> pusha_uncached() if we still want to save the old value of RSP. For 
>>>> example, we could first subtract
>>>> 16 * wordSize and then calculate the correct value:
>>>>
>>>> subq(rsp, 16 * wordSize);
>>>> movq(Address(rsp, 11 * wordSize), rsp);
>>>> addq(Address(rsp, 11 * wordSize), 16 * wordSize);
>>>>
>>>> Thank you!
>>>>
>>>> Best regards,
>>>> Christian