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

Find and consume all -X / -XX options relevant to the JIT #21126

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Feb 13, 2025

This PR addresses #21122.

The proposed change iterates over all J9::ExternalOptions and calls FIND_AND_CONSUME_VMARG on the ones that are consumed by the JIT, caching the arg index. This ensures that -XX:-IgnoreUnrecognizedXXColonOptions does not cause the JVM to terminate incorrectly.

Aside from the root cause of #21122, there were several options that were only processed using FIND_ARG_IN_VMARGS, which would also run into the -XX:-IgnoreUnrecognizedXXColonOptions issue; this means that simply moving checkArgsConsumed as I suggested in #21122 isn't a complete solution.

Closes #21122

@dsouzai
Copy link
Contributor Author

dsouzai commented Feb 13, 2025

@mpirvu what do you think of this proposal? If it seems ok to you, I can clean it up a bit (add doxygen comments, better commit messages) and remove the WIP.

Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM. I think we should add, either here or in the docs, some brief instructions on how to add a new external option.

@mpirvu mpirvu self-assigned this Feb 14, 2025
@dsouzai dsouzai marked this pull request as ready for review February 14, 2025 16:22
@dsouzai
Copy link
Contributor Author

dsouzai commented Feb 14, 2025

@mpirvu good for review now.

This commit adds the ExternalOptionsMetadata struct as well as a table
of this struct to contain information about External Options that the
JIT processes. This commit also adds a method to find and possibly
consume the options, caching the arg index. Finally, this commit adds
some documentation to describe how to add a new external option.

Signed-off-by: Irwin D'Souza <[email protected]>
This commit updates the JIT to use the External Options Metadata table.
This ensures that all external options that are used to control JIT
functionality are consumed, which prevents the JVM from terminating
early with an error when -XX:-IgnoreUnrecognizedXXColonOptions is
specified.

Signed-off-by: Irwin D'Souza <[email protected]>
@mpirvu
Copy link
Contributor

mpirvu commented Feb 14, 2025

jenkins test sanity all jdk17

@mpirvu
Copy link
Contributor

mpirvu commented Feb 14, 2025

There are many failures in the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-XX:-IgnoreUnrecognizedXXColonOptions does not recognize all XX options
2 participants