NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

Support JDK 11

Open msridhar opened this issue 6 years ago • 11 comments

WIP at https://github.com/uber/NullAway/tree/ms/jdk-11. We should get the core NullAway functionality working first. Some notes:

  • I don't think coveralls works yet
  • Android-related sample app building will be a pain
  • This JarInfer-related code won't work on JDK 11; see here. We should probably disable this handler by default and enable via a flag anyway (along with a couple other rarely-used handlers).

msridhar avatar Dec 02 '18 20:12 msridhar

/cc @lazaroclapp @subarnob

msridhar avatar Dec 02 '18 20:12 msridhar

Another issue here: WALA seems to fail on JDK 11:

> Task :jar-infer:test-java-lib-jarinfer:jar FAILED
Exception in thread "main" java.lang.IllegalStateException: could not find boot classpath
        at com.ibm.wala.util.PlatformUtil.getBootClassPathJars(PlatformUtil.java:62)
        at com.ibm.wala.properties.WalaProperties.getJ2SEJarFiles(WalaProperties.java:64)
        at com.ibm.wala.util.config.AnalysisScopeReader.processScopeDefLine(AnalysisScopeReader.java:177)
        at com.ibm.wala.util.config.AnalysisScopeReader.read(AnalysisScopeReader.java:80)
        at com.ibm.wala.util.config.AnalysisScopeReader.readJavaScope(AnalysisScopeReader.java:53)
        at com.ibm.wala.util.config.AnalysisScopeReader.makePrimordialScope(AnalysisScopeReader.java:192)
        at com.uber.nullaway.jarinfer.DefinitelyDerefedParamsDriver.run(DefinitelyDerefedParamsDriver.java:160)
        at com.uber.nullaway.jarinfer.JarInfer.main(JarInfer.java:91)

msridhar avatar Dec 03 '18 17:12 msridhar

Did some digging here around what it would take to support running JarInfer on JDK 11. There are two issues:

  1. The WALA issue from this comment is due to the fact that since JDK 9 the standard libraries are no longer distributed as jar files. We can work around this by providing our own standard library jar (basically a stub jar for analysis); shouldn't be too hard.

  2. The code using URLClassLoader is trickier to work around. Here I think the best solution is to change JarInfer to rewrite library jars with nullability annotations, rather than generating stub files. A shorter term solution would be to pass in the stubs as a separate argument to NullAway, rather than just including in the class path. In either case, this will be more work to fix than 1.

FYI @lazaroclapp @kageiit

msridhar avatar Dec 31 '18 16:12 msridhar

I think code using UrlClassLoader is fine as we can just ignore classloaders which do not typecast safely. That said, the eventual plan is to rewrite jarinfer to embed stubs in jars along with bytecode instead of producing stub jars. We can probably have a switch to jarinfer to support both output jar/aar modes.

kageiit avatar Jan 03 '19 23:01 kageiit

@lazaroclapp thoughts here?

msridhar avatar Jan 03 '19 23:01 msridhar

Basically, I agree that the long term solution is bytecode rewriting to add the annotations, which further buys us Kotlin support automatically.

As for the short term solutions, I see two options: add a check around that cast that will make that handler basically a no-op for Java 9 and above, or make code specific to Java 9+ to get the url out of a BuiltinClassLoader. The later seems doable by using reflection to access private fields (e.g. LoadedModule.codeSourceURL), but might need to be specialized per Java version supported 🤷‍♂️ . I'd be fine with either as a stopgap solution and can make the either change.

lazaroclapp avatar Jan 04 '19 00:01 lazaroclapp

Ok that makes sense. It might be better to crash with a relevant message rather than no-op. The JarInfer handler is off by default now, so users would only see the crash if they opted into JarInfer. Even better would be the workaround via reflection. I’m fine with only supporting JDK 11 but not intermediate releases since it’s a temporary fix.

msridhar avatar Jan 04 '19 01:01 msridhar

current status?

ctcpip avatar Jun 18 '19 21:06 ctcpip

I think core NullAway should work fine on JDK 11. It's lesser-used features like JarInfer that are still being worked on. @lazaroclapp any other details?

msridhar avatar Jun 18 '19 21:06 msridhar

Any news related with Java 11 support? Recently, I tried to integrate NullAway with a small Java 11 project built with Gradle and it didn't work. I received errors during the build, but I don't have logs now. I can try to reproduce this situation later.

pwittchen avatar Sep 09 '19 18:09 pwittchen

Basic NullAway on Java 11 should work, we'd be happy to look at an issue here with a repo case.

lazaroclapp avatar Sep 09 '19 18:09 lazaroclapp