Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8280901: MethodHandle::linkToNative stub is missing w/ -Xint #7459

Closed
wants to merge 2 commits into from

Conversation

iwanowww
Copy link

@iwanowww iwanowww commented Feb 14, 2022

MethodHandle::linkToNative linker doesn't have a dedicated stub for interpreter. A stub for compiled code is shared and it is invoked through i2c stub when accessed from interpreter. In interpreter-only mode, stubs for compiled code are not generated and linkToNative ends up in a broken state where Method::_from_interpreted_entry points to i2c stub while Method::_from_compiled_entry points to c2i stub.

Proposed fix unconditionally generates a stub for MethodHandle::linkToNative case irrespective whether it is a interpreter-only mode or not.

Testing: test/jdk/java/foreign/ w/ -Xint


Progress

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

Issue

  • JDK-8280901: MethodHandle::linkToNative stub is missing w/ -Xint

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7459/head:pull/7459
$ git checkout pull/7459

Update a local copy of the PR:
$ git checkout pull/7459
$ git pull https://git.openjdk.java.net/jdk pull/7459/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7459

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7459.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Feb 14, 2022

👋 Welcome back vlivanov! 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 openjdk bot added the hotspot hotspot-dev@openjdk.org label Feb 14, 2022
@openjdk
Copy link

openjdk bot commented Feb 14, 2022

@iwanowww
The hotspot label was successfully added.

@iwanowww iwanowww marked this pull request as ready for review February 14, 2022 13:53
@openjdk openjdk bot added the rfr Pull request is ready for review label Feb 14, 2022
@mlbridge
Copy link

mlbridge bot commented Feb 14, 2022

Webrevs

@mcimadamore
Copy link
Contributor

Thanks for the fix - maybe consider adding some extra test combinations in TestMatrix (this test is not automated, so it's not run by build and test infra).

Copy link
Member

@shipilev shipilev left a comment

Choose a reason for hiding this comment

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

Looks fine to me, thanks for fixing!

@openjdk
Copy link

openjdk bot commented Feb 14, 2022

@iwanowww 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:

8280901: MethodHandle::linkToNative stub is missing w/ -Xint

Reviewed-by: shade, kvn

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

  • b95310b: 8282220: contentType should not be a PKCS7's member
  • bc43320: 8281543: Remove unused code/headerfile dtraceAttacher.hpp
  • f953952: 8281745: Create a regression test for JDK-4514331
  • e0b4962: 8282190: Typo in javadoc of java.time.format.DateTimeFormatter#getDecimalStyle
  • e1c98bd: 8281523: Accessibility: Conversion from string literal loses const qualifier
  • cc7cf81: 8280861: Robot color picker broken on Linux with scaling above 100%
  • d7a706a: 8253757: Add LLVM-based backend for hsdis
  • bdae1d8: 8282147: [TESTBUG] waitForIdle after creating frame in JSpinnerMouseAndKeyPressTest.java
  • 51f4420: 8282130: (bf) Remove unused ARRAY_BASE_OFFSET, ARRAY_INDEX_SCALE from read-only Heap Buffers
  • 34aae32: 8282166: JDK-8282158 changed ECParameters' package by accident
  • ... and 115 more: https://git.openjdk.java.net/jdk/compare/3ce1c5b6ce02749ef8f9d35409b7bcbf27f47203...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.

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

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Feb 14, 2022
@iwanowww
Copy link
Author

Thanks for the reviews, Maurizio, Aleksey, and Vladimir.

maybe consider adding some extra test combinations in TestMatrix

I decided to extend test/jdk/java/foreign/TestDowncall.java to run a single test in -Xint mode:

----------messages:(5/559)----------
command: testng -Xint ... -Dgenerator.sample.factor=100000 TestDowncall
...
elapsed time (seconds): 1.031
...
----------System.out:(7/249)----------
test TestDowncall.testDowncall(0, "f0_V__", VOID, [], []): success

===============================================
java/foreign/TestDowncall.java
Total tests run: 1, Passes: 1, Failures: 0, Skips: 0
===============================================

@iwanowww
Copy link
Author

/integrate

@openjdk
Copy link

openjdk bot commented Feb 23, 2022

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

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Feb 23, 2022
@openjdk openjdk bot closed this Feb 23, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 23, 2022
@openjdk
Copy link

openjdk bot commented Feb 23, 2022

@iwanowww Pushed as commit f86f38a.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotspot hotspot-dev@openjdk.org integrated Pull request has been integrated
4 participants