-
Notifications
You must be signed in to change notification settings - Fork 739
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
jdk_nio_0_FAILED sun/nio/cs/TestStringCoding.java RuntimeException: getBytes(csn) failed -> x-IBM29626C #18974
Comments
cent8x86-svl-rt5-1, rhel8x86-svl-rt6-1 succeeded. Failures: rhel8x86-svl-rt7-1 sles15x86-svl-rt6-1 ubu22x86-svl-rt4-1 |
Occurred at JDK17 s390x_linux(
JDK21 aarch64_linux( |
@knn-k @Akira1Saitoh pls take a look. Note this is a blocker for the upcoming release. |
In #18819, In
The tree compares the input character with -94, -93, and -84, but those values should be 0xa2, 0xa3, and 0xac. |
Opened #19044. |
Running |
Opened #19056 for 0.44. |
JDK11 aarch64_mac RC1 build(
50x grinder - all failed JDK11 ppc64le_linux RC1 |
@hzongaro I can repeat the failure, on xlinux, using the following cut down testcase. It wasn't occurring for me with the default options, but occurred when using a
|
I suspect it's partly related to the compressedRefsShift, as with -Xmx values 512m - 3g I'm getting 0, and with -Xmx4g I'm getting 3. |
The problem didn't/doesn't occur with M2 builds. Besides the RC1 build it also occurs on the OpenJ9 open source build.
Changes:
|
My bad, on second look, OpenJ9 did change between the builds, with a change to create LATIN1 string constants correctly as compact Strings. |
It looks like Value Propagation is performing a transforming a
after:
The |
In the earlier instance of this failure described in the comment above, the trees looked like this:
If the string is encoded in a |
This new failure is not necessarily related to the previous ones. However this issue for the test got reopened and reused. This latest failure occurs on all platforms and is exposed due to #19348 |
Yes, I'm testing a fix. Sorry for the delay. |
Reopening until fix is merged onto 0.44 release branch. |
JDK11 ppc64le_linux(
50x grinder - the failure not reproduced. The JVM in question contains This appears to be the same failure but seems less frequent and not across platforms. |
I suspect the problem is specific to the Power 10 inline code generation for @zl-wang, could someone on your team take a look at this? |
@IBMJimmyk could you take a look at this new failure in priority? it is a blocker to 0.44 |
Yes, I can take a look at this. |
One problem I found is here: openj9/runtime/compiler/p/codegen/J9TreeEvaluator.cpp Lines 10865 to 10871 in fa4755a
|
@hzongaro has made it into an unsigned byte, such that it shouldn't fail there? |
I'm currently in the middle of writing a small test so I can better control what the inputs are and what they actually look like just before the call. I'll be able to answer that question after I finish that. But, this check is definitely wrong so it will need to be removed one way or another. |
This failure was NOT reproduced by JDK17 jdk_nio_0 p10sles021 - passed
|
for JDK11, the relevant Pre Instruction Selection Trees look like this:
Inlining is intentionall suppressed for
The fast path assembly for intrinsicIndexOfLatin1 starts like this:
An input of 0x80 into
That fails the check at I will check JDK17 next. |
For JDK17, the tree looks like this:
Inlining is intentionally suppressed for
Assembly still looks like this:
An input of 0x80 stays as 0x80:
This will pass the check at So that's why it failed under JDK11 but not JDK17. Next step is to look over the generated code to make sure it still works after this check is removed. |
jdk11 IL looks wrong? |
What's wrong with the JDK11 IL? I'm not seeing the problem. In JDK11, In JDK17, |
if ch hadn't gone through i2b-then-b2i transformation but kept an unsigned value, it would work even with ch value check in place, wouldn't it? i would take what you said: StringLatin1 ch argument is strictly a byte value, such that the checking code can be removed. |
Yes. But, I think the first
Then the b2i is inserted to widen the byte to an int to pass it into the call to intrinsicIndexOfLatin1 .
|
I've created a WIP PR for my proposed fix: While I expect this to work, I haven't fully tested the change yet so it is still marked WIP. The change remove the check to see if the upper bits of |
I ran a grinder with my fix. It was JDK11 on a Power 10 machine. All the tests passed: So my proposed fix does fix this issue. |
Failure link
From an internal build(
sles12x86-rtp-rt6-1
):Rerun in Grinder - Change TARGET to run only the failed test targets.
Optional info
Failure output (captured from console output)
50x internal grinder
Also seen on JDK22 x86-64_mac
The text was updated successfully, but these errors were encountered: