[foreign-abi] RFR: 8248420: Add a variant of VaList::make which takes a NativeScope [v7]

Jorn Vernee jvernee at openjdk.java.net
Thu Jul 16 18:59:22 UTC 2020


> Hi,
> 
> This patch adds overloads to several VaList related methods that might need to do allocations, namely, VaList::make,
> VaList::vargAsSegment, and VaList::copy.
> In order to share the existing code, I've added an `Allocator` helper interface that can wrap either a NativeScope, or
> be used as a functional interface to bind to MemorySegment::allocateNative.
> While testing this patch, I've also discovered a bug in the SysV implementation; The VaList::Builder was allocating a
> MemorySegment for the VaList, then creating the SysVVaList object wrapping that segment, and then filling in some of
> the contents of the segment. But, the SysVVaList constructor was also trying to read from the segment as well, and as a
> result was reading uninitialized fields. This is a problem when creating a VaList in Java, and then trying to read from
> it also in Java.  I've restructured the code a bit to fix this problem, moving the reading logic from the constructor
> to a static factory called readFromSegment, so that it's clear that only a fully initialized segment should be passed.
> The builder still calls the constructor directly, but it now requires all field values to be passed in explicitly.
> There was a similar problem in the AArch64 implementation, which I've tried to fix. Unfortunately I was not able to
> fully test this change since there was a failure elsewhere in the testing infrastructure, and I currently don't have
> direct access to an aarch64 machine to be able to debug that. I did end up fixing some build failures on aarch64 in the
> process though, which I will submit another PR for.  Thanks, Jorn

Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now
contains ten commits:

 - Test the Java VaList implementation on every platform
 - Fix whitespace error
 - Add extra test for liveness of copied VaList without using scope
 - Make close/isAlive behaviour more consistent across platforms
 - Update javadoc:
   - Add missing @throws tags
   - Change wording on VaList read methods
 - Update CSupport.java
   
   Typo
 - Rewrite the VaList javadoc to be more explicit about the statefulness of a va list instance, as well as adding various
   clarifications about the memory resource management behaviours of the make, copy, and close methods.
 - Add tests to verify that a va list copy is indeed unusable after closing the original, and fix the bug that this
   uncovered.
 - Add NativeScope overloads for VaList methods that need to allocate

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

Changes: https://git.openjdk.java.net/panama-foreign/pull/237/files
 Webrev: https://webrevs.openjdk.java.net/panama-foreign/237/webrev.06
  Stats: 913 lines in 10 files changed: 656 ins; 74 del; 183 mod
  Patch: https://git.openjdk.java.net/panama-foreign/pull/237.diff
  Fetch: git fetch https://git.openjdk.java.net/panama-foreign pull/237/head:pull/237

PR: https://git.openjdk.java.net/panama-foreign/pull/237


More information about the panama-dev mailing list