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

Can Linkage Checker detect the bad Java8-incompatible byte code #1605

Closed
suztomo opened this issue Aug 18, 2020 · 6 comments · Fixed by #1607
Closed

Can Linkage Checker detect the bad Java8-incompatible byte code #1605

suztomo opened this issue Aug 18, 2020 · 6 comments · Fixed by #1607
Labels
bug Something isn't working

Comments

@suztomo
Copy link
Contributor

suztomo commented Aug 18, 2020

Can Linkage Checker (Linkage Monitor) detect the wrong byte code that does not work with Java 8 (case protocolbuffers/protobuf#7827)

Wrong byte code for Java8 that has reference to CharBuffer CharBuffer.flip() (java.nio.CharBuffer.flip()Ljava/nio/CharBuffer). In JRE 8, it's Buffer Buffer.flip() (the return type is different).

As of now, references to all system classes (including java.nio.CharBuffer) are skipped.

@suztomo suztomo changed the title Can Linkage Checker detect the wrong byte code for Java 8? Can Linkage Checker detect the bad Java8-incompatible byte code Aug 18, 2020
@suztomo
Copy link
Contributor Author

suztomo commented Aug 19, 2020

For protocolbuffers/protobuf#7827, could Linkage Monitor detect this as part of the PR check, if Linkage Checker is capable to detect the invalid references?

It seems no. If the check builds protobuf-java JAR file with Java 11, then Linkage Monitor could detect it. If it was with Java 8, then the resulting JAR file is fine.

@elharo
Copy link
Contributor

elharo commented Aug 19, 2020

This is a really interesting one. Some take-aways:

  1. The compiler doesn't catch all linkage errors, even to classes in the JDK.
  2. We should report errors that the project (here Kristen) can't fix so they can understand them and report the errors to the team that can fix them.

@cpovirk
Copy link

cpovirk commented Aug 19, 2020

Nice. It sounds like this will check for compatibility against the JDK that you're using to build? So, if you want to check for compatibility with JDK 8, then you'll need to build with JDK 8? That makes it helpful for detecting problems in your dependencies but not necessarily helpful for detecting problems in your own code (since you'd already detect/avoid these by building with JDK 8 in the first place). That's fine; I had just been initially hoping for an improvement on Animal Sniffer, too.

@suztomo
Copy link
Contributor Author

suztomo commented Aug 19, 2020

compatibility against the JDK that you're using to build?

Yes, it detect the compatibility with the JDK Linkage Checker is running on.

If we enhance the check script to build protobuf-java in Java 11 and running Linkage Checker with Java 8, #1607 should detect the invalid method references.

Memo: script for current check https://github.com/protocolbuffers/protobuf/blob/master/tests.sh#L252 uses Java 8 for both.

@cpovirk
Copy link

cpovirk commented Aug 26, 2020

Thanks.

If you're interested, it is probably possible to detect problems even when building with Java 11: Java 9+ comes bundled with API signatures of not only the current version of Java but also of older versions. See $JAVA_HOME/lib/ct.sym, which contains files like 876/java/nio/ByteBuffer.sig, which I believe is just a classfile that contains the signatures of ByteBuffer under Java 6, 7, and 8.

That still requires that users tell the Linkage Checker which version of Java to check against. I don't suppose it possible to guess a sensible default based on the <source>, <target>, or <release> configuration for plugins like maven-compiler-plugin??

@suztomo
Copy link
Contributor Author

suztomo commented Aug 26, 2020

@cpovirk Thank you for information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants