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

[23] generate workaround for new JIT bug into eclipse.ini #2954

Closed
stephan-herrmann opened this issue Sep 13, 2024 · 5 comments
Closed

[23] generate workaround for new JIT bug into eclipse.ini #2954

stephan-herrmann opened this issue Sep 13, 2024 · 5 comments
Milestone

Comments

@stephan-herrmann
Copy link
Contributor

As per eclipse-jdt/eclipse.jdt.ui#1639 many test failures could be traced back to a bug in the JIT of JVM 23, whereby an NPE was raised in a code location that provably never saw a null.

The bug can be worked around by passing the following JVM arg:

-XX:CompileCommand=exclude,org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer::getExtendedRange

I'll add a p2.inf file to jdt.core to automatically add this option to eclipse.ini.

Now, the following would cause p2 to wrongly detect , as the end of the argument:

addJvmArg(jvmArg:-XX:CompileCommand=exclude,org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer::getExtendedRange);

Documentation of the java launcher mentions that , can be replaced with a space, and recommends to put the command in double quotes.

The following is understood by p2, would, however, cause the eclipse launcher to garble the entry such that the JVM cannot grok it:

addJvmArg(jvmArg:-XX:CompileCommand="exclude org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer::getExtendedRange");

In the end it seems that space is accepted even without quotes, both in p2.inf and in eclipse.ini.

@stephan-herrmann
Copy link
Contributor Author

Resolved by #2955

@stephan-herrmann
Copy link
Contributor Author

Verified: updating Eclipse SDK 4.33 with https://download.eclipse.org/eclipse/updates/4.33-Y-builds/Y20240914-1000 adds the JIT-exclusion to eclipse.ini. After that, launching eclipse prints:

CompileCommand: exclude org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.getExtendedRange bool exclude = true

This means the syntax is accepted by java.

@merks
Copy link
Contributor

merks commented Sep 20, 2024

@stephan-herrmann

I'm in the process of producing JustJ 23 JREs. Once I complete that, Java 23 will be available in the installers and users could use it to create an 2024-09 installation with it. That makes me wonder about this vm argument that I noticed. I.e., I could add it here:

image

But I don't really understand its purpose exactly...

@stephan-herrmann
Copy link
Contributor Author

I'm in the process of producing JustJ 23 JREs. Once I complete that, Java 23 will be available in the installers and users could use it to create an 2024-09 installation with it. That makes me wonder about this vm argument that I noticed. I.e., I could add it here:
[...]

But I don't really understand its purpose exactly...

I'm glad someone notices :)

Java 23 VM has the interesting capability to throw NPE from provably null-free code. More specifically, for one particular method in JDT when the JIT compiles it twice (assumeably because it was found to be relevant for optimization), the resulting code is broken. I guess the JIT re-orders statements so that the code dereferences a local variable before it is assigned. Of course such re-ordering is illegal.

The only way we can defend against this particular case is by telling the JIT not to touch that specific method. That's what this directive does:

-XX:CompileCommand=exclude,org.eclipse.jdt.internal.core.dom.rewrite.ASTRewriteAnalyzer::getExtendedRange

Obviously that is a point fix only for that one observed instance of the bug (which caused hundreds of failures in our test suite), it does not fix the JIT bug, which could still affect other methods as well...

Perhaps you saw me fighting with separator chars (, or ) and quoting. So the way to test if the result is effective is launching eclipse and looking for a line on console saying

CompileCommand: exclude org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.getExtendedRange bool exclude = true

@merks
Copy link
Contributor

merks commented Sep 20, 2024

Thank you 🙏

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

No branches or pull requests

2 participants