RFR: 8254252: Generic arraycopy stub overwrites callee-save rdi register on 64-bit Windows

[Tobias Hartmann]

On Mon, 12 Oct 2020 19:27:59 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Since [JDK-8241825](https://bugs.openjdk.java.net/browse/JDK-8241825), MacroAssembler::load_klass requires a temporary
>> register to decode the klass pointer. In the generic arraycopy stub, rdi is used for that on 64-bit Windows because r9
>> is already used as an argument register: https://hg.openjdk.java.net/jdk/jdk/rev/0bb101fbeb10#l17.32
>> The problem is that rdi is callee-save [1] but not restored when returning from the stub. This leads to register
>> corruption and more or less random crashes in the caller.
>> Although JDK-8241825 is part of JDK 15, this was never a problem because we did not set the _WIN64 macro in adlc and as
>> a result accidentally treated rdi (and rsi) as caller-save:
>> https://github.com/openjdk/jdk/blob/b9873e18330b7e43ca47bc1c0655e7ab20828f7a/src/hotspot/cpu/x86/x86_64.ad#L89
>> Now that this got fixed as part of [JDK-8248238](https://bugs.openjdk.java.net/browse/JDK-8248238) [2], we hit the bug.
>> 
>> Unfortunately, we are running out of caller-save registers on Windows. Since rcx, rdx, r8 and r9 are used for
>> arguments, only rax, r10 and r11 remain which are already used as temporary registers and live throughout the stub
>> code. I've decided to free up r11 by postponing the load of the array length which is only needed on Windows anyway
>> (because only Windows passes the 5th argument on stack).  Thanks, Tobias
>> 
>> [1] https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019
>> [2] https://openjdk.github.io/cr/?repo=jdk&pr=212&range=11#sdiff-8
>
> @TobiHartmann It is okay to use push/pop to preserve registers in stubs if code become cleaner:
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp#L3647
> Did you consider it?

@vnkozlov Yes, I did consider pushing/poping rdi on Windows but the `generate_generic_copy` method jumps into other
stubs like `entry_checkcast_arraycopy` that then exit the frame. We would therefore need to make sure to pop before
jumping to another stub and be careful to not pop twice when exiting through `L_failed` or falling through.

I've updated the PR with a push/pop version of the fix. Please let me know which version you prefer.

-------------

PR: https://git.openjdk.java.net/jdk/pull/603