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

Experimental support for loading JIMAGE (JDK 9+) files #1046

Merged
merged 1 commit into from
Feb 11, 2021

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Jan 27, 2021

This bumps the crucible submodule to include GaloisInc/crucible#638, which adds basic support for handling JDK 9 or later. JDK 9+ packages its standard library not in a JAR file, but in a JIMAGE file. For more details on how crucible-jvm handles JIMAGE files, refer to Note [Loading classes from JIMAGE files] in Lang.JVM.Codebase.

This fixes #861, although there are still unsolved issues that arise when using modern JDKs with certain classes, such as String. As a result, I have decided to label support for JDK 9+ as experimental:

Other things:

  • crucible-jvm: Add --java-bin-dirs crucible#636 and Lang.JVM.Codebase: experimental support for loading JIMAGE files crucible#638 upstreamed the code from SAWScript.JavaTools and SAWScript.ProcessUtils into crucible-jvm, so we can remove these modules in favor of importing Lang.JVM.JavaTools and Lang.JVM.ProcessUtils from crucible-jvm.

  • I removed the dependency on the xdg-basedir, as it was unused. This dependency was likely added quite some time ago, and it appears that saw-script switched over to using XDG-related functionality from the directory library since then. I opted to use directory to find the .cache directory as well, so I have made that clear in the .cabal file.

  • The biJavaCodebase :: Codebase field of BuiltinContext is completely unused, which I noticed when making changes to the Codebase type. Let's just remove it.

@RyanGlScott RyanGlScott force-pushed the wip/T861-take-two branch 2 times, most recently from d4189fd to 86f648b Compare February 2, 2021 14:52
@RyanGlScott
Copy link
Contributor Author

I've rebased this on top of #1005, so the diff is slightly more amenable to reviewing than it was before. I'm still relying on a hack (copy-pasting the entire definition of Codebase and related functions in SAWScript.JavaCodebase) to avoid having to change any submodules, but you may find it easier to review what the changes look like if they were upstreamed in crucible-jvm.

@RyanGlScott RyanGlScott force-pushed the wip/T861-take-two branch 5 times, most recently from decacf0 to 119b4b6 Compare February 8, 2021 14:37
@RyanGlScott RyanGlScott changed the title Support JDK 9 or later Experimental support for loading JIMAGE (JDK 9+) files Feb 8, 2021
@RyanGlScott RyanGlScott marked this pull request as ready for review February 8, 2021 14:39
@RyanGlScott
Copy link
Contributor Author

I've rebased this on top of GaloisInc/crucible@e1531a0, and I have updated the documentation to make it clear that support for JDK 9+ is experimental. This is now ready for review.

I briefly considered if saw should emit a warning every time a user tries running saw with JDK 9+ or later (much like GaloisInc/crucible@e1531a0 does), but in the end, I decided against it. It's possible that someone could be running saw solely to verify LLVM code, but if they have JDK 9+ on their PATH, then they'd get annoying warnings about a SAW backend that they're not using. We could envision a more complex way to track if Java code is being verified—for example, we could have an IORef Bool that tracks if a JVM-related SAW function has been invoked before, and if such a function is invoked, emit a warning and change the IORef Bool such that we don't emit the warning more than once per SAW invocation. The amount of elbow grease required to get that working didn't seem to justify the payoff, so I simply updated the documentation and called it a day.

.github/workflows/build.yml Outdated Show resolved Hide resolved
src/SAWScript/Crucible/JVM/BuiltinsJVM.hs Outdated Show resolved Hide resolved
src/SAWScript/JavaExpr.hs Outdated Show resolved Hide resolved
This bumps the `crucible` submodule to include GaloisInc/crucible#638, which
adds basic support for handling JDK 9 or later. JDK 9+ packages its standard
library not in a JAR file, but in a JIMAGE file. For more details on how
`crucible-jvm` handles JIMAGE files, refer to
`Note [Loading classes from JIMAGE files]` in `Lang.JVM.Codebase`.

This fixes #861, although there are still unsolved issues that arise when
using modern JDKs with certain classes, such as `String`. As a result, I have
decided to label support for JDK 9+ as experimental:

* I have updated the SAW documentation to mention these shortcomings.
* I have opened GaloisInc/crucible#641 to track the remaining issues.

Other things:

* GaloisInc/crucible#636 and GaloisInc/crucible#638 upstreamed the code from
  `SAWScript.JavaTools` and `SAWScript.ProcessUtils` into `crucible-jvm`, so
  we can remove these modules in favor of importing `Lang.JVM.JavaTools` and
  `Lang.JVM.ProcessUtils` from `crucible-jvm`.

* I removed the dependency on the `xdg-basedir`, as it was unused. This
  dependency was likely added quite some time ago, and it appears that
  `saw-script` switched over to using XDG-related functionality from the
  `directory` library since then. I opted to use `directory` to find the
  `.cache` directory as well, so I have made that clear in the `.cabal` file.

* The `biJavaCodebase :: Codebase` field of `BuiltinContext` is completely
  unused, which I noticed when making changes to the `Codebase` type. Let's
  just remove it.
@RyanGlScott RyanGlScott added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Feb 11, 2021
@mergify mergify bot merged commit c4ab82b into master Feb 11, 2021
@mergify mergify bot deleted the wip/T861-take-two branch February 11, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modern Java support
2 participants