8250810: Push missing parts of JDK-8248817 #274

rnkovacs · 2021-08-23T07:49:47Z

This is a refactoring patch with no functional change. It didn't apply cleanly. Differences to the original commit:

In thread_windows_x86.cpp:

Some removed lines were simpler on tip because of JDK-8248240.

In os_windows.cpp:

The block around L2461 is no longer present on tip, that and a bit of the next block can be simplified.
The conditions on L2539 (JDK-8234372) and L2549 (JDK-8213199) (original line numbers) are different on tip. We actually didn't touch those, they only show up colored in the diff because of the whitespace changes.
The surrounding code is different around L2586 because of JDK-8191278, but the changes are the same.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8250810: Push missing parts of JDK-8248817

Reviewers

Andrew Haley (@theRealAph - Reviewer) ⚠️ Review applies to a06cf59
Martin Doerr (@TheRealMDoerr - Reviewer)

Contributors

Ludovic Henry <[email protected]>
Bernhard Urban-Forster <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk11u-dev pull/274/head:pull/274
$ git checkout pull/274

Update a local copy of the PR:
$ git checkout pull/274
$ git pull https://git.openjdk.java.net/jdk11u-dev pull/274/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 274

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk11u-dev/pull/274.diff

bridgekeeper · 2021-08-23T07:51:17Z

👋 Welcome back rnkovacs! 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 · 2021-08-23T07:52:37Z

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

rnkovacs · 2021-08-23T07:54:33Z

/contributor add Ludovic Henry [email protected]
/contributor add Bernhard Urban-Forster [email protected]

openjdk · 2021-08-23T07:54:43Z

@rnkovacs
Contributor Ludovic Henry <[email protected]> successfully added.

openjdk · 2021-08-23T07:56:50Z

@rnkovacs
Contributor Bernhard Urban-Forster <[email protected]> successfully added.

mlbridge · 2021-08-23T08:02:43Z

Webrevs

phohensee · 2021-08-23T16:41:17Z

Why not backport 8248817 first?

rnkovacs · 2021-08-23T17:01:31Z

Hm, I remember visiting that bug, but I just assumed we've done that already without checking more carefully. Thanks for noticing! I'll add it to the list.

lewurm · 2021-10-01T10:05:46Z

Just as a reference, this PR should go in first (backport of 8248817 as @phohensee pointed out): #300

theRealAph · 2021-10-03T10:09:12Z

src/hotspot/os/windows/os_windows.cpp

-                              !ExecMem);
-            return EXCEPTION_CONTINUE_EXECUTION;
-          } else
+        // If it's a legal stack address map the entire region in


Suggested change

// If it's a legal stack address map the entire region in

// If it's a legal stack address, map the entire region in

Desperately needs a comma.

theRealAph

Looks OK, although I'm no Windows expert.

rnkovacs · 2021-10-04T02:40:48Z

Thank you. Comma added.

openjdk · 2021-10-04T06:38:45Z

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

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

8250810: Push missing parts of JDK-8248817

Push changes from JDK-8248817 that were accidentally excluded from the commit.

Co-authored-by: Ludovic Henry <[email protected]>
Co-authored-by: Bernhard Urban-Forster <[email protected]>
Reviewed-by: aph, mdoerr

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 14 new commits pushed to the master branch:

5de08c5: 8230067: Add optional automatic retry when running jtreg tests
efef574: 8180573: Refactor sun/security/tools shell tests to plain java tests
2b9261a: 8273894: ConcurrentModificationException raised every time ReferralsCache drops referral
a2bd0a9: 8272836: Limit run time for java/lang/invoke/LFCaching tests
6f51eb4: 8273373: Zero: Cannot invoke JVM in primordial threads on Zero
d86cf10: 8223138: Small clean-up in loop-tree support.
3b88f74: 8187649: ArrayIndexOutOfBoundsException in java.util.JapaneseImperialCalendar
6285123: 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1
89ee611: 8223923: C2: Missing interference with mismatched unsafe accesses
f12af7e: 8266182: Automate manual steps listed in the test jdk/sun/security/pkcs12/ParamsTest.java
... and 4 more: https://git.openjdk.java.net/jdk11u-dev/compare/2990ff065935cc2a2b487fb88335e58b7edaa6cf...master

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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@theRealAph, @TheRealMDoerr) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

RealCLanger · 2021-10-04T07:09:08Z

The patch ran fine through SAP's nightly tests. Please flag for fix-request approval in JBS.

TheRealMDoerr

LGTM.

rnkovacs · 2021-10-04T16:26:29Z

Thanks! Tag added. (Fix request comment coming soon - I haven't received a response to my authorship request, so I'm still asking colleagues to do this for me.)

rnkovacs · 2021-10-04T20:24:02Z

Fix request comment added as well (thanks @lewurm).

rnkovacs · 2021-10-05T16:06:41Z

/integrate

openjdk · 2021-10-05T16:08:08Z

@rnkovacs
Your change (at version 5a2e329) is now ready to be sponsored by a Committer.

RealCLanger · 2021-10-05T16:10:54Z

/sponsor

openjdk · 2021-10-05T16:12:55Z

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

5de08c5: 8230067: Add optional automatic retry when running jtreg tests
efef574: 8180573: Refactor sun/security/tools shell tests to plain java tests
2b9261a: 8273894: ConcurrentModificationException raised every time ReferralsCache drops referral
a2bd0a9: 8272836: Limit run time for java/lang/invoke/LFCaching tests
6f51eb4: 8273373: Zero: Cannot invoke JVM in primordial threads on Zero
d86cf10: 8223138: Small clean-up in loop-tree support.
3b88f74: 8187649: ArrayIndexOutOfBoundsException in java.util.JapaneseImperialCalendar
6285123: 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1
89ee611: 8223923: C2: Missing interference with mismatched unsafe accesses
f12af7e: 8266182: Automate manual steps listed in the test jdk/sun/security/pkcs12/ParamsTest.java
... and 4 more: https://git.openjdk.java.net/jdk11u-dev/compare/2990ff065935cc2a2b487fb88335e58b7edaa6cf...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-10-05T16:13:13Z

@RealCLanger @rnkovacs Pushed as commit ddc3288.

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

openjdk bot changed the title ~~Backport a4eaf9536c272862b5ec856bf263679be09bddc9~~ 8250810: Push missing parts of JDK-8248817 Aug 23, 2021

openjdk bot added the backport Port of a pull request already in a different code base label Aug 23, 2021

rnkovacs marked this pull request as ready for review August 23, 2021 07:56

openjdk bot added the rfr Pull request is ready for review label Aug 23, 2021

rnkovacs mentioned this pull request Aug 23, 2021

JDK-8253947: Implementation: JEP 388: Windows AArch64 Support #222

Closed

3 tasks

rnkovacs force-pushed the 8250810-push-missing branch from 87bec48 to a06cf59 Compare August 29, 2021 20:35

rnkovacs mentioned this pull request Aug 30, 2021

8248238: Implementation: JEP 388: Windows AArch64 Support #301

Closed

3 tasks

openjdk bot mentioned this pull request Aug 30, 2021

8253015: Aarch64: Move linux code out from generic CPU feature detection #299

Closed

3 tasks

theRealAph reviewed Oct 3, 2021

View reviewed changes

theRealAph approved these changes Oct 3, 2021

View reviewed changes

Backport a4eaf95

5a2e329

rnkovacs force-pushed the 8250810-push-missing branch from a06cf59 to 5a2e329 Compare October 4, 2021 02:38

openjdk bot added the ready Pull request is ready to be integrated label Oct 4, 2021

TheRealMDoerr approved these changes Oct 4, 2021

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Oct 5, 2021

openjdk bot closed this Oct 5, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Oct 5, 2021

lewurm mentioned this pull request Oct 14, 2021

8248817: Windows: Improving common cross-platform code #300

Closed

3 tasks

	// If it's a legal stack address map the entire region in
	// If it's a legal stack address, map the entire region in

8250810: Push missing parts of JDK-8248817 #274

8250810: Push missing parts of JDK-8248817 #274

Uh oh!

Conversation

rnkovacs commented Aug 23, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Contributors

Reviewing

Uh oh!

bridgekeeper bot commented Aug 23, 2021

Uh oh!

openjdk bot commented Aug 23, 2021

Uh oh!

rnkovacs commented Aug 23, 2021

Uh oh!

openjdk bot commented Aug 23, 2021

Uh oh!

openjdk bot commented Aug 23, 2021

Uh oh!

mlbridge bot commented Aug 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

phohensee commented Aug 23, 2021

Uh oh!

rnkovacs commented Aug 23, 2021

Uh oh!

lewurm commented Oct 1, 2021

Uh oh!

theRealAph Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

theRealAph left a comment

Choose a reason for hiding this comment

Uh oh!

rnkovacs commented Oct 4, 2021

Uh oh!

openjdk bot commented Oct 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RealCLanger commented Oct 4, 2021

Uh oh!

TheRealMDoerr left a comment

Choose a reason for hiding this comment

Uh oh!

rnkovacs commented Oct 4, 2021

Uh oh!

rnkovacs commented Oct 4, 2021

Uh oh!

rnkovacs commented Oct 5, 2021

Uh oh!

openjdk bot commented Oct 5, 2021

Uh oh!

RealCLanger commented Oct 5, 2021

Uh oh!

openjdk bot commented Oct 5, 2021

Uh oh!

openjdk bot commented Oct 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

6 participants

rnkovacs commented Aug 23, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 23, 2021 •

edited

Loading

openjdk bot commented Oct 4, 2021 •

edited

Loading