[jdk20] RFR: 8294744: AArch64: applications/kitchensink/Kitchensink.java crashed: assert(oopDesc::is_oop(obj)) failed: not an oop [v2]

[Patricio Chilano Mateo]

On Fri, 6 Jan 2023 15:25:30 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> Please review the following patch. The value we set initially for extended_sp on natives frames doesn't account for the oop that could be pushed to the stack in case the method throws an exception. This can create a situation in Interpreter::_throw_exception_entry where we push an exception oop to the Java expression stack below the actual physical stack pointer. When JFR is present though a JavaThread could receive a suspend signal right after that push. On Linux aarch64, because there is no red zone defined (nor implemented it seems), the pushed oop gets overwritten during the setup and execution of the signal handler. This later leads to a crash when popping the oop back and rethrowing in the caller of the native method. There are more details in the bug comments.
>> 
>> To fix it I used the same technique we use for normal Java frames, i.e. add extra space to extended_sp when creating the frame to account for the max space needed.
>> 
>> 
>> 
>> I tested the patch by running Kitchensink.java around 150 times on mach5 with no failures (without the patch 50 runs would already show ~10 failures on average). I also run tiers1-6 for sanity check.
>> 
>> Thanks,
>> Patricio
>
> Patricio Chilano Mateo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8294744
>  - v1

> Looks good to me. JVM on linux-riscv64 bears the same problem as it has the same code flow as aarch64 on this part [1][2]. And linux-riscv64 doesn't have a redzone defined/implemented either, which I have verified with a small C program with inline assembly like you do for x86 and aarch64. So I have prepared a fix for linux-riscv64. This has passed sanity tests on HiFive Unmatched. Could you please add this change while you are on it?
> 
> ```
> diff --git a/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp b/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
> index 49a2b87c232..213e360c20e 100644
> --- a/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
> +++ b/src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
> @@ -776,8 +776,12 @@ void TemplateInterpreterGenerator::generate_fixed_frame(bool native_call) {
>      // Move SP out of the way
>      __ mv(sp, t0);
>    } else {
> -    __ sd(sp, Address(sp, 5 * wordSize));
> +    // Make sure there is room for the exception oop pushed in case method throws
> +    // an exception (see TemplateInterpreterGenerator::generate_throw_exception())
> +    __ sub(t0, sp, 2 * wordSize);
> +    __ sd(t0, Address(sp, 5 * wordSize));
>      __ sd(zr, Address(sp, 4 * wordSize));
> +    __ mv(sp, t0);
>    }
>  }
> ```
> 
> [1] https://bugs.openjdk.org/browse/JDK-8290280 [2] [openjdk/jdk at 4dd236b](https://github.com/openjdk/jdk/commit/4dd236b40abfeb1200e884021b90226046bc4b85)
>
Sure, added.

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

PR: https://git.openjdk.org/jdk20/pull/85