8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario #42

minborg · 2024-01-09T08:10:16Z

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8321923 to be approved
Commit message must refer to an issue

Issues

JDK-8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario (Bug - P3)
JDK-8321923: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario (CSR)

Reviewers

Maurizio Cimadamore (@mcimadamore - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk22.git pull/42/head:pull/42
$ git checkout pull/42

Update a local copy of the PR:
$ git checkout pull/42
$ git pull https://git.openjdk.org/jdk22.git pull/42/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 42

View PR using the GUI difftool:
$ git pr show -t 42

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk22/pull/42.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-01-09T08:10:54Z

👋 Welcome back pminborg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-01-09T08:12:31Z

This backport pull request has now been updated with issue from the original commit.

openjdk · 2024-01-09T08:13:25Z

@minborg The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-01-09T08:16:33Z

Webrevs

00: Full (62837c72)

mcimadamore · 2024-01-09T17:11:42Z

src/java.base/share/classes/java/lang/foreign/MemorySegment.java

     * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
-     * @throws UnsupportedOperationException if this segment is
-     *         {@linkplain #isReadOnly() read-only}
+     *         or {@code offset < 0}


Why is UOE being removed here (and elsewhere) ?

This one is redundant and there is an IAE covering the same case a bit down.

mcimadamore · 2024-01-10T15:22:06Z

test/jdk/java/foreign/TestSegmentAllocators.java

+
+            // WrongThreadException if this method is called from a thread {@code T},
+            // such that {@code source.isAccessibleBy(T) == false}
+            CompletableFuture<Arena> future = CompletableFuture.supplyAsync(Arena::ofConfined);


I believe TestScopedOperations should check for WTE as well?

That test has two methods: testOpAfterClose and testOpOutsideConfinement, which are executed on all the test cases.

Good idea. We could do that in a separate PR.

openjdk · 2024-01-10T15:28:16Z

@minborg This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario

Reviewed-by: mcimadamore

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 7 new commits pushed to the master branch:

da24559: 8316241: Test jdk/jdk/jfr/jvm/TestChunkIntegrity.java failed
33f07b5: 8310995: missing @SInCE tags in 36 jdk.dynalink classes
28db238: 8322163: runtime/Unsafe/InternalErrorTest.java fails on Alpine after JDK-8320886
6b79e79: 8323241: jcmd manpage should use lists for argument lists
a8df559: 8321480: ISO 4217 Amendment 176 Update
acc4829: 8322725: (tz) Update Timezone Data to 2023d
e5f3366: 8322418: Problem list gc/TestAllocHumongousFragment.java subtests for 8298781

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

minborg · 2024-01-12T14:40:03Z

/integrate

openjdk · 2024-01-12T14:41:46Z

Going to push as commit 3909d74.
Since your change was applied there have been 25 commits pushed to the master branch:

d115295: 8323190: Segfault during deoptimization of C2-compiled code
71a05bf: 8322538: remove fatal from JVM_VirtualThread functions for !INCLUDE_JVMTI
d3f18d0: 8321685: Missing ResourceMark in code called from JvmtiEnvBase::get_vthread_jvf
07a8911: 8322003: JShell - Incorrect type inference in lists of records implementing interfaces
749f749: 8323066: gc/g1/TestSkipRebuildRemsetPhase.java fails with 'Skipping Remembered Set Rebuild.' missing
6022e73: 8322513: Build failure with minimal
46f1df3: 8323571: Regression in source resolution process
3984a00: 8322237: Heap dump contains duplicate thread records for mounted virtual threads
4ea14b2: 8322971: KEM.getInstance() should check if a 3rd-party security provider is signed
3daa936: 8318971: Better Error Handling for Jar Tool When Processing Non-existent Files
... and 15 more: https://git.openjdk.org/jdk22/compare/29ed3878cc51fadccc0023795e056195d108a9a2...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-01-12T14:41:57Z

@minborg Pushed as commit 3909d74.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Backport 7edd10e5fa71dafbbad23455553b7f5ff0a75ac9

62837c7

openjdk bot changed the title ~~Backport 7edd10e5fa71dafbbad23455553b7f5ff0a75ac9~~ 8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario Jan 9, 2024

openjdk bot added backport clean rfr Pull request is ready for review labels Jan 9, 2024

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jan 9, 2024

mcimadamore reviewed Jan 9, 2024

View reviewed changes

mcimadamore reviewed Jan 10, 2024

View reviewed changes

mcimadamore approved these changes Jan 10, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 10, 2024

openjdk bot added the integrated Pull request has been integrated label Jan 12, 2024

openjdk bot closed this Jan 12, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 12, 2024

8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario #42

8321786: SegmentAllocator:allocateFrom(ValueLayout, MemorySegment,ValueLayout,long,long) spec mismatch in exception scenario #42

Uh oh!

Conversation

minborg commented Jan 9, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Jan 9, 2024

Uh oh!

openjdk bot commented Jan 9, 2024

Uh oh!

openjdk bot commented Jan 9, 2024

Uh oh!

mlbridge bot commented Jan 9, 2024

Webrevs

Uh oh!

mcimadamore Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

minborg Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

mcimadamore Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

mcimadamore Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

minborg Jan 12, 2024

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Jan 10, 2024

Uh oh!

minborg commented Jan 12, 2024

Uh oh!

openjdk bot commented Jan 12, 2024

Uh oh!

openjdk bot commented Jan 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

minborg commented Jan 9, 2024 •

edited by openjdk bot

Loading