[foreign-memaccess+abi] RFR: 8296973: saving errno on a value-returning function crashes the JVM

[Maurizio Cimadamore]

On Thu, 17 Nov 2022 18:40:18 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> There are 2 bugs:
> 
> - Frame allocation code is not in the right order. There's an optimization where the area used for spilling return values around slow path calls is shared with the shadow space and stack args area. This is possible since the latter are only used before the native call, and the former only after. But, the order of the code that allocates this space is incorrect, which means that the spill area for return values is shared with _all_ the data in the frame. This also recently includes the address of the captured state segment (for `errno`). In case there are no stack arguments and no shadow space (as on linux in most cases), this means in practice that a spill of the return value will overwrite the capture state address. (previously we've also had the return buffer address, but use of a return buffer and spilling of return values are mutually exclusive, so it was never a problem).
> - The `CaptureCallState` linker option adds an additional leading parameter (the captured state segment), which can mess up `SharedUtils::adaptDowncallForIMR`.
> 
> I've extended the existing `CaptureCallState` test to include the most interesting cases: simple scalar returns, and various struct returns that are: returned in a single register (no return buffer), returned in multiple registers (yes return buffer), or returned through ABI-level in-memory-return pointers (using `SharedUtils::adaptDowncallForIMR`).
> 
> I've also done a big sanity test where I adapted `TestDowncallScope` to also set and capture `errno` with each call. That seemed overkill to include in the repo though (?)
> 
> Thanks

Looks good - testing coverage is also reasonable. Let's see how it goes and decide later if we need a more comprehensive test.

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

Marked as reviewed by mcimadamore (Committer).

PR: https://git.openjdk.org/panama-foreign/pull/753