Graal: [20.1] SEGV in StubRoutines::counterMode_AESCrypt

[christhalinger]

This one is failing for a long time in our nightly testing but I wasn't able to reproduce it locally. Finally I realized that we need to run the while com/sun/crypto/provider/Cipher suite with concurrency set to 1. Very likely because that uses the same agent process and triggers the compilation.

It fails with release and fastdebug builds on both Linux and macOS:

make run-test-only JTREG="VM_OPTIONS=-XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:+UseG1GC; JOBS=1" TEST="com/sun/crypto/provider/Cipher/" CONF=release

[2020-05-18 10:47:34,200] Agent[1]: stdout: #
[2020-05-18 10:47:34,201] Agent[1]: stdout: # A fatal error has been detected by the Java Runtime Environment:
[2020-05-18 10:47:34,201] Agent[1]: stdout: #
[2020-05-18 10:47:34,201] Agent[1]: stdout: #  SIGSEGV (0xb) at pc=0x00000001134f4180, pid=88478, tid=37923
[2020-05-18 10:47:34,201] Agent[1]: stdout: #
[2020-05-18 10:47:34,201] Agent[1]: stdout: # JRE version: OpenJDK Runtime Environment (11.0.6) (build 11.0.6.0.4-internal+0-TwitterJDK)
[2020-05-18 10:47:34,201] Agent[1]: stdout: # Java VM: OpenJDK 64-Bit Server VM (11.0.6.0.4-internal+0-TwitterJDK, mixed mode, tiered, jvmci, jvmci compiler, compressed oops, g1 gc, bsd-amd64)
[2020-05-18 10:47:34,201] Agent[1]: stdout: # Problematic frame:
[2020-05-18 10:47:34,201] Agent[1]: stdout: # v  ~StubRoutines::counterMode_AESCrypt
[2020-05-18 10:47:34,201] Agent[1]: stdout: #
[2020-05-18 10:47:34,201] Agent[1]: stdout: # No core dump will be written. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
[2020-05-18 10:47:34,201] Agent[1]: stdout: #
[2020-05-18 10:47:34,201] Agent[1]: stdout: # An error report file with more information is saved as:
[2020-05-18 10:47:34,201] Agent[1]: stdout: # /Users/cthalinger/twitter-11-upstream-graal/build/macosx-x86_64-normal-server-release/test-support/jtreg_test_jdk_com_sun_crypto_provider_Cipher/scratch/hs_err_pid88478.log
[2020-05-18 10:47:34,218] Agent[1]: stdout: Could not load hsdis-amd64.dylib; library not loadable; PrintAssembly is disabled
[2020-05-18 10:47:34,240] Agent[1]: stdout: #
[2020-05-18 10:47:34,240] Agent[1]: stdout: # If you would like to submit a bug report, please visit:
[2020-05-18 10:47:34,240] Agent[1]: stdout: #   https://jira.twitter.biz/browse/VM
[2020-05-18 10:47:34,240] Agent[1]: stdout: #

[christhalinger]

@gilles-duboscq I heard you are the release manager. This bug seems like a serious issue so I think we should fix this before 20.1 goes out.

[gilles-duboscq]

It's too late for 20.1.0 unfortunately.

[tkrodriguez]

Isn't this most likely missing null checks on the arguments to the intrinsic? I don't see any in our substitutions or in the callers. I thought they changed the intrinsics in JDK to perform sanity checks before the intrinsification point? Wasn't that part of the point of the HotSpotIntrinsicCandidate stuff?

[christhalinger]

Yes, that seems to be the case:

https://github.com/openjdk/jdk11u/blob/d46b56eb81b07b452141c7dcd078d33fc8e49d63/src/java.base/share/classes/com/sun/crypto/provider/CounterMode.java#L174

[tkrodriguez]

Ah, I was looking at the wrong cipher. I was able to reproduce this locally and it looks like embeddedCipher.K is null https://github.com/oracle/graal/blob/master/compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/CounterModeSubstitutions.java#L68 so kPtr is 0x18 instead of being a real pointer.

[tkrodriguez]

This cipher requires a predicate check on the embedded cipher and it's just not being done. https://github.com/graalvm/labs-openjdk-11/blob/master/src/hotspot/share/opto/library_call.cpp#L6089 We crash because the embedded cipher is actually a DESCrypt. I'll create an issue for this.

[tkrodriguez]

This has been broken for over a year so thanks for catching this. We're missing a unit test for this but even if we had one it probably would have checked that it triggered properly, not that it didn't trigger for the wrong cipher...

[mur47x111]

Should be fixed by https://github.com/oracle/graal/commit/0aa8509fcd1659ad51dcb41023ffcaa56df1eb3e

[christhalinger]

I have no idea what these duplicate changesets are but this is the one that actually has the changes in the repo: a76fbee7df663ca6189ae10818a8c7cb1485d5ee

[tkrodriguez]

They aren't duplicate. github presents merge changesets as the diff between itself and the merge parent. So instead of presenting git show 0aa8509 it's presenting git diff 0aa8509^...0aa8509. A handy git alias is this

        prdiff = "!sh -c 'r=`git rev-parse $0` && git diff $r^...$r'"

So you can say git prdiff 0aa8509 or even git prdiff :/GR-23633

[christhalinger]

LOL thanks! ;)

[christhalinger]

Anyway, I can report the bug is fixed. I'll give it a day or two of nightly testing and then I will close the issue.

[christhalinger]

Looks good. Thanks for fixing this.