Skip to content

8360869: jcstress is able to crash jdk8 on aarch64 with jfr on#697

Closed
judovana wants to merge 4 commits intoopenjdk:masterfrom
judovana:gccVersion
Closed

8360869: jcstress is able to crash jdk8 on aarch64 with jfr on#697
judovana wants to merge 4 commits intoopenjdk:masterfrom
judovana:gccVersion

Conversation

@judovana
Copy link
Contributor

@judovana judovana commented Sep 15, 2025

We are seeing rare crashes in jcstress with jdk8 on aarch64 with jfron. Those crashes are real, however we were not able to reproduce with builds on gcc 5 and up. The real culprint was not found. but gcc version on aarch64 should be checked. Ideal version seems 7, but I'm not that bold.

I think this change must be accompanied by regenerating generated-configure but am not sure with process on it.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8360869 needs maintainer approval

Issue

  • JDK-8360869: jcstress is able to crash jdk8 on aarch64 with jfr on (Bug - P3 - Approved)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/697/head:pull/697
$ git checkout pull/697

Update a local copy of the PR:
$ git checkout pull/697
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/697/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 697

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/697.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 15, 2025

👋 Welcome back jvanek! 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
Copy link

openjdk bot commented Sep 15, 2025

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

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

8360869: jcstress is able to crash jdk8 on aarch64 with jfr on

Reviewed-by: aph

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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 (@gnu-andrew, @theRealAph) 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).

@openjdk openjdk bot added the rfr Pull request is ready for review label Sep 15, 2025
@mlbridge
Copy link

mlbridge bot commented Sep 15, 2025

Webrevs

@openjdk openjdk bot added approval Requires approval; will be removed when approval is received and removed approval Requires approval; will be removed when approval is received labels Sep 15, 2025
Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move the AC_MSG_RESULT earlier i.e.

      AC_MSG_RESULT([found ($COMPILER_VERSION_NUMBER])
      if test $COMPILER_VERSION_NUMBER_MAJOR -lt 5; then
        AC_MSG_ERROR([GCC < 5 is known to lead to incorrect compilation on aarch64. See JDK-8360869.])
      fi

And yes, configure needs to be regenerated at present.

I've removed the approval request. This should only be done after successful review and with a comment as to why it is suitable for inclusion. Easier way is with the /approval request command.

@judovana
Copy link
Contributor Author

I would move the AC_MSG_RESULT earlier i.e.

      AC_MSG_RESULT([found ($COMPILER_VERSION_NUMBER])
      if test $COMPILER_VERSION_NUMBER_MAJOR -lt 5; then
        AC_MSG_ERROR([GCC < 5 is known to lead to incorrect compilation on aarch64. See JDK-8360869.])
      fi

Sure, sounds good. Done.

And yes, configure needs to be regenerated at present.

Any recommended version of autoconf to use? When I use 2.72, the diff is 90% of the file.

I've removed the approval request. This should only be done after successful review and with a comment as to why it is suitable for inclusion. Easier way is with the /approval request command.

Sure thing. Whether to do it or so, is if course on you and @theRealAph . I will do my best on how to do that. Thanx!

@mlbridge
Copy link

mlbridge bot commented Sep 17, 2025

Mailing list message from Andrew Haley on jdk8u-dev:

On 17/09/2025 08:08, Ji?? Van?k wrote:

     AC\_MSG\_ERROR\(\[GCC \< 5 is known to lead to incorrect compilation on aarch64\. See JDK\-8360869\.\]\)

"Known" is a bit too strong. "GCC < 5 may incorrectly compile HotSpot"
is better.

--
Andrew Haley (he/him)
Java Platform Lead Engineer
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

@judovana
Copy link
Contributor Author

"Known" is a bit too strong. "GCC < 5 may incorrectly compile HotSpot"

sure. TY!

@gnu-andrew
Copy link
Member

I would move the AC_MSG_RESULT earlier i.e.

      AC_MSG_RESULT([found ($COMPILER_VERSION_NUMBER])
      if test $COMPILER_VERSION_NUMBER_MAJOR -lt 5; then
        AC_MSG_ERROR([GCC < 5 is known to lead to incorrect compilation on aarch64. See JDK-8360869.])
      fi

Sure, sounds good. Done.

Thanks. This looks good with aph's amendments as well.

It should mean the checking line is always completed with the version number and the error is on its own line.

And yes, configure needs to be regenerated at present.

Any recommended version of autoconf to use? When I use 2.72, the diff is 90% of the file.

I'll look at what I used. You may need to dust off an older version (2.72 sounds very new, not sure I've seen that one yet)

It can produce differing results even with the same version from different distro packages, so I would like to drop it as we have on later JDKs. If you would rather wait on that backport, that's an alternative option. I would like to finally get that into the January update.

I've removed the approval request. This should only be done after successful review and with a comment as to why it is suitable for inclusion. Easier way is with the /approval request command.

Sure thing. Whether to do it or so, is if course on you and @theRealAph . I will do my best on how to do that. Thanx!

This is just the general process across all update releases. If there are PRs in the approval queue that still need a review, it clogs it up and can mean that ones that are ready to go are missed. In theory, there are a lot more reviewers than maintainers :)

@judovana
Copy link
Contributor Author

judovana commented Sep 18, 2025

Thanx! Regenerated with self built 2.69. Output was sane this time, only I manually skipped about 50 hunks like this:

@@ -50158,7 +50167,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
 /* end confdefs.h.  */
 
 int
-main (void)
+main ()
 {
 return 0;
   ;

I guess thats done by too new gcc

@openjdk
Copy link

openjdk bot commented Sep 18, 2025

@judovana Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 16, 2025

@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@judovana
Copy link
Contributor Author

still waiting for resolution on inclusion

@judovana
Copy link
Contributor Author

Rebased to tip and reruning checks

@openjdk
Copy link

openjdk bot commented Nov 12, 2025

@judovana Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

@judovana
Copy link
Contributor Author

gc/concurrentMarkSweep/SystemGCOnForegroundCollector.java on linux and security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#globalsignrootcar6 on Mac do not seem related.

@judovana
Copy link
Contributor Author

@aph / @gnu-andrew ping? This seems to have all steps to get merged/closed finished, right?

@bridgekeeper
Copy link

bridgekeeper bot commented Dec 22, 2025

@judovana This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

@bridgekeeper
Copy link

bridgekeeper bot commented Jan 20, 2026

@judovana This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

@bridgekeeper bridgekeeper bot closed this Jan 20, 2026
@judovana
Copy link
Contributor Author

/open

@openjdk openjdk bot reopened this Jan 20, 2026
@openjdk
Copy link

openjdk bot commented Jan 20, 2026

@judovana This pull request is now open

@judovana
Copy link
Contributor Author

@gnu-andrew @theRealAph Can we please approve this or reject? TY!

@jerboaa
Copy link
Contributor

jerboaa commented Jan 27, 2026

This just needs an 8u Reviewer to approve. @judovana Please merge master to get an updated GHA run. Thanks!

Skipping about 50 hunks like:
@@ -50158,7 +50167,7 @@ ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
 /* end confdefs.h.  */

 int
-main (void)
+main ()
 {
 return 0;
   ;
@openjdk
Copy link

openjdk bot commented Jan 27, 2026

@judovana Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Jan 27, 2026
@jerboaa
Copy link
Contributor

jerboaa commented Jan 27, 2026

32 bit Windows build failure is handled by #752. MacOSX is handled by #738 and related follow-ups. Feel free to integrate.

@judovana
Copy link
Contributor Author

/integrate

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Jan 27, 2026
@openjdk
Copy link

openjdk bot commented Jan 27, 2026

@judovana
Your change (at version d9da88b) is now ready to be sponsored by a Committer.

@jerboaa
Copy link
Contributor

jerboaa commented Jan 27, 2026

/sponsor

@openjdk
Copy link

openjdk bot commented Jan 27, 2026

Going to push as commit ffdc8d8.

@openjdk openjdk bot added the integrated Pull request has been integrated label Jan 27, 2026
@openjdk openjdk bot closed this Jan 27, 2026
@openjdk openjdk bot 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 Jan 27, 2026
@openjdk
Copy link

openjdk bot commented Jan 27, 2026

@jerboaa @judovana Pushed as commit ffdc8d8.

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

@gnu-andrew
Copy link
Member

32 bit Windows build failure is handled by #752. MacOSX is handled by #738 and related follow-ups. Feel free to integrate.

And #738 will now be further delayed by merging the configure changes in this. I don't see what the sudden rush was for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integrated Pull request has been integrated

Development

Successfully merging this pull request may close these issues.

4 participants