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

8244720: Check MethodType and FunctionDescritpor used when linking #158

Closed
wants to merge 2 commits into from

Conversation

@JornVernee
Copy link
Member

@JornVernee JornVernee commented May 11, 2020

Hi,

This patch adds exhaustive checking to the MethodType and FunctionDescriptor used to link down calls and upcalls.

These checks define and enforce a set of acceptable carrier types, as well as which carrier type & memory layout combinations are acceptable. The set of accepted carrier types is:

  1. The primitives: byte, short, char, int, long, float and double (excluding void and boolean)
  2. MemoryAddress
  3. MemorySegment

For (1), it is also checked that the used MemoryLayout is a ValueLayout, and that the size of the carrier matches the size of the layout. For (2) the expected layout must also be a ValueLayout, and again the size is checked. For (3) it is only checked that the layout is a GroupLayout, (since we don't have access to the size of the segment when linking).

This makes it easier to reason about the set of MethodType and FunctionDescriptor combinations that can be used during linking, as well as helping to catch any errors made with mismatching carrier types and memory layouts.

The additional checks turned up 2 cases of carrier type to memory layout mismatch in StdLibTest, which I've fixed.

Thanks,
Jorn


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Issue

  • JDK-8244720: Check MethodType and FunctionDescritpor used when linking

Reviewers

Download

$ git fetch https://git.openjdk.java.net/panama-foreign pull/158/head:pull/158
$ git checkout pull/158

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented May 11, 2020

👋 Welcome back jvernee! A progress list of the required criteria for merging this PR into foreign-abi will be added to the body of your pull request.

@openjdk openjdk bot added the rfr label May 11, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented May 11, 2020

Webrevs

Copy link
Collaborator

@mcimadamore mcimadamore left a comment

Good fix overall - it seems like it also pointed out subtle issues in some of our tests. I've added some (optional) suggestions about code reuse.

@openjdk
Copy link

@openjdk openjdk bot commented May 11, 2020

@JornVernee This change now passes all automated pre-integration checks, type /integrate in a new comment to proceed. After integration, the commit message will be:

8244720: Check MethodType and FunctionDescritpor used when linking

Reviewed-by: mcimadamore
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /solves command.

There are currently no new commits on the foreign-abi branch since the last update of the source branch of this PR. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you would like to avoid potential automatic rebasing, specify the current head hash when integrating, like this: /integrate 39a8e0103919fd59d6d7b87ece60f17d8d6b6da6.

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

@openjdk openjdk bot added the ready label May 11, 2020
@JornVernee JornVernee force-pushed the JornVernee:TypeCheck branch from 6e55e6c to cc8402a May 11, 2020
@JornVernee
Copy link
Member Author

@JornVernee JornVernee commented May 11, 2020

/integrate

@openjdk openjdk bot closed this May 11, 2020
@openjdk openjdk bot added the integrated label May 11, 2020
@openjdk
Copy link

@openjdk openjdk bot commented May 11, 2020

@JornVernee
Pushed as commit 5deb924.

@openjdk openjdk bot removed ready rfr labels May 11, 2020
@JornVernee JornVernee deleted the JornVernee:TypeCheck branch May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.