[foreign-memaccess+abi] RFR: 8265751: MemoryAddress should have a scope accessor [v7]

Maurizio Cimadamore mcimadamore at openjdk.java.net
Fri Apr 23 18:25:03 UTC 2021


> Following the recent API changes, I think `MemoryAddress` has been left in an inconsistent state. On the one hand, we clearly want `MemoryAddress` to have a link to a `ResourceScope`; we clearly state so in the javadoc for:
> 
> * `CLinker::upcallStub`
> * `MemorySegment::address`
> 
> (and we don't say, but we probably should in `VaList::address`).
> 
> This link is crucial, as it prevents scope closure, if an address is passed to a native function and the scope is implicit.
> 
> However, this dependency is, currently, ad-hoc, and not cleanly stated in the API.
> 
> This patch rectifies that; now all memory address have a scope (which can be the "global scope" for addresses obtained from native, or from `long` values).
> 
> There are not many changes in terms of implementation, but by putting the association between addresses and scopes front and center we can make the javadoc clearer.
> 
> Also, this allows us to move the scope check from `MemorySegment::address` (that has always been a bit odd) to `MemoryAddress::toRawLongAddress` - that is, closer to where the address value is actually used.
> 
> This allows us to, effectively, make calls to downcall method handles *safer*, since now a scope check will take place before the raw long value is extracted.
> 
> Note that this doesn't negatively affect performance: the check was occurring even before, albeit at a different level (closer to the user code). I've run all the downcall benchmarks we have and found no regressions.
> 
> I've added a test which makes sure that passing "closed" segments/addresses to native function is no longer possible.

Maurizio Cimadamore has updated the pull request incrementally with two additional commits since the last revision:

 - Tweak text for MemoryAddress::NULL
 - Polish javadoc some more

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

Changes:
  - all: https://git.openjdk.java.net/panama-foreign/pull/513/files
  - new: https://git.openjdk.java.net/panama-foreign/pull/513/files/801eccdb..9b594382

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=panama-foreign&pr=513&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=panama-foreign&pr=513&range=05-06

  Stats: 5 lines in 1 file changed: 2 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/panama-foreign/pull/513.diff
  Fetch: git fetch https://git.openjdk.java.net/panama-foreign pull/513/head:pull/513

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


More information about the panama-dev mailing list