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

J9 (8u275, 11.0.9) passed the JUnit test while OpenJDK (8u275, 9.0.4, 11.0.9) and J9 (9.0.4) threw a VerifyError. #11684

Closed
fuzzy000 opened this issue Jan 19, 2021 · 5 comments · Fixed by #11753

Comments

@fuzzy000
Copy link

JVM Versions

We used the following versions of J9:

openjdk version "1.8.0_275"
OpenJDK Runtime Environment (build 1.8.0_275-b01)
Eclipse OpenJ9 VM (build openj9-0.23.0, JRE 1.8.0 Linux amd64-64-Bit Compressed References 20201110_845 (JIT enabled, AOT enabled)
OpenJ9   - 0394ef754
OMR      - 582366ae5
JCL      - b52d2ff7ee based on jdk8u275-b01)
openjdk version "9.0.4-adoptopenjdk"
OpenJDK Runtime Environment (build 9.0.4-adoptopenjdk+12)
Eclipse OpenJ9 VM (build openj9-0.9.0, JRE 9 Linux amd64-64-Bit Compressed References 20180814_248 (JIT enabled, AOT enabled)
OpenJ9   - 24e53631
OMR      - fad6bf6e
JCL      - feec4d2ae based on jdk-9.0.4+12)
openjdk version "11.0.9" 2020-10-20
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.9+11)
Eclipse OpenJ9 VM AdoptOpenJDK (build openj9-0.23.0, JRE 11 Linux amd64-64-Bit Compressed References 20201022_810 (JIT enabled, AOT enabled)
OpenJ9   - 0394ef754
OMR      - 582366ae5
JCL      - 3b09cfd7e9 based on jdk-11.0.9+11)

The used OpenJDK versions:

openjdk version "1.8.0_275"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_275-b01)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.275-b01, mixed mode)
openjdk version "9.0.4"
OpenJDK Runtime Environment (build 9.0.4+11)
OpenJDK 64-Bit Server VM (build 9.0.4+11, mixed mode)
openjdk version "11.0.9" 2020-10-20
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.9+11)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.9+11, mixed mode)

Summary of the Problem

We made some changes to a class file in the JUnit project, and let several JVMs execute the corresponding JUnit test. We found that in a test case, J9 (8u275, 11.0.9) just passed while OpenJDK (8u275, 9.0.4, 11.0.9) and J9 (9.0.4) threw a VerifyError. The output is shown as follows:

J9 (8u275, 11.0.9):

Time: 0.007
OK (5 tests)

J9 (9.0.4):

There was 1 failure:
1) initializationError(org.junit.internal.ChecksTest)
java.lang.VerifyError: JVMVRFY012 stack shape inconsistent; class=org/junit/internal/Checks, method=notNull(Ljava/lang/Object;)Ljava/lang/Object;, pc=57
Exception Details:
  Location:
    org/junit/internal/Checks.notNull(Ljava/lang/Object;)Ljava/lang/Object; @57: JBaload1
  Reason:
    Type top (current frame, locals[1]) is not assignable to 'reference' type
  Current Frame:
    bci: @57
    flags: { }
    locals: { 'java/lang/Object', top, integer, integer, integer, integer }
    stack: { }
        at org.junit.runners.ParentRunner.<init>(ParentRunner.java:101)
        at org.junit.runners.BlockJUnit4ClassRunner.<init>(BlockJUnit4ClassRunner.java:84)
        at org.junit.runners.JUnit4.<init>(JUnit4.java:23)
        ...
FAILURES!!!
Tests run: 1,  Failures: 1

OpenJDK (8u275, 9.0.4, 11.0.9)

There was 1 failure:
1) initializationError(org.junit.internal.ChecksTest)
java.lang.VerifyError: (class: org/junit/internal/Checks, method: notNull signature: (Ljava/lang/Object;)Ljava/lang/Object;) Register 1 contains wrong type
        at org.junit.runners.ParentRunner.<init>(ParentRunner.java:101)
        at org.junit.runners.BlockJUnit4ClassRunner.<init>(BlockJUnit4ClassRunner.java:84)
        at org.junit.runners.JUnit4.<init>(JUnit4.java:23)
        ...
FAILURES!!!
Tests run: 1,  Failures: 1

Diagnostic files

VerifyIssue.zip

Steps to reproduce the behavior:

  1. extract the VerifyIssue.zip
  2. In directory VerifyIssue, run command java -cp sootOutput/junit-junit/:hamcrest-all-1.3.jar:junit-4.12.jar org.junit.runner.JUnitCore org.junit.internal.ChecksTest

Execution environment

  • OS and version: Ubuntu 16.04.6 LTS
  • CPU model: Intel(R) Xeon(R) CPU E5-4610 v4 @ 1.80GHz
  • Number of CPU cores: 4CPUs, each has 10 cores
  • Size of physical memory: 16384 MB * 20
@pshipton
Copy link
Member

@tajila @ChengJin01 fyi

@ChengJin01
Copy link

Will investigate to see what happened to the verifier.

@ChengJin01
Copy link

It looks like the issue was related to the fix on the generated stackmap of class files (version <=49) at #9419 (intended for the issue detected at #9385). If so, the work in there needs to be double-checked to see whether the initial solution was incomplete or any corner case was encountered in the test here.

@ChengJin01
Copy link

ChengJin01 commented Jan 21, 2021

Looking at the bytecode the failing method org/junit/internal/Checks, method: notNull signature: (Ljava/lang/Object;)Ljava/lang/Object;)

!j9method 0x00000000001D0610 --> org/junit/internal/Checks.notNull(Ljava/lang/Object;)Ljava/lang/Object;
> !j9method 0x00000000001D0610 
J9Method at 0x1d0610 {
  Fields for J9Method:
	0x0: U8* bytecodes = !j9x 0x00007FAF262470C4 // <6*?Y"
	0x8: struct J9ConstantPool* constantPool = !j9constantpool 0x00000000001D08C0 (flags = 0x0)
	0x10: void* methodRunAddress = !j9x 0x0000000000000006
	0x18: volatile void* extra = !j9x 0x000000000000176B
}
Signature: org/junit/internal/Checks.notNull(Ljava/lang/Object;)Ljava/lang/Object; !bytecodes 0x00000000001D0610
ROM Method: !j9rommethod 0x00007FAF262470B0
Next Method: !j9method 0x00000000001D0630
> !bytecodes 0x00000000001D0610
  Name: notNull
  Signature: (Ljava/lang/Object;)Ljava/lang/Object;
  Access Flags (3040009): public static 
  Max Stack: 2
  Argument Count: 1
  Temp Count: 5

    0 iconst5 
    1 istore1 
    2 iconst1 
    3 istore2 
    4 iconst5 
    5 istore3 
    6 iconst5 
    7 istore 5
    9 aload0 
   10 ifnonnull 99
   13 ldc 7 (java.lang.String) <org.junit.internal.Checks: java.lang.Object notNull(java.lang.Object)> **** Executed Line: **** 1 **** if r0 != null goto return r0
   15 invokestatic 11 Print.logPrint(Ljava/lang/String;)V
   18 iload1 
   19 iconstm1 
   20 iadd 
   21 istore1 
   22 iload1 
   23 ifge 27
   26 nop 
   27 new 12 java/lang/NullPointerException
   30 astore1   <------------------
   31 ldc 2 (java.lang.String) <org.junit.internal.Checks: java.lang.Object notNull(java.lang.Object)> **** Executed Line: **** 2 **** $r1 = new java.lang.NullPointerException
   33 invokestatic 11 Print.logPrint(Ljava/lang/String;)V
   36 iload3 
   37 iconstm1 
   38 iadd 
   39 istore 4
   41 iload 4
   43 ifge 57   <------------
   46 iload2 
   47 iconstm1 
   48 iadd 
   49 istore 4
   51 iload 4
   53 ifle 99
   56 nop 
   57 aload1  <-------
   58 invokespecial 8 java/lang/NullPointerException.<init>()V
   61 ldc 4 (java.lang.String) <org.junit.internal.Checks: java.lang.Object notNull(java.lang.Object)> **** Executed Line: **** 3 **** specialinvoke $r1.<java.lang.NullPointerException: void <init>()>()
   63 invokestatic 11 Print.logPrint(Ljava/lang/String;)V
   66 iinc 5 -1
   69 iload 5
   71 ifle 96
   74 iload 5
   76 lookupswitch pairs 1
        default         56
              4         26
   96 nop 
   97 aload1 
   98 athrow 
   99 aload0 
  100 return1 

In normal case, there should be a invokespecial bytecode for java/lang/NullPointerException before 30 astore1 to initialize the java/lang/NullPointerException object. Given this is a modified method intended to trigger a VerifyError at this point, the error should be captured when merging stack frame 43 ifge 57 during the runtime verification, which means there was something wrong in the generated stackmaps.

I scrutinized the code fix at #9419 and figured the original intention was to only exclude BCV_SPECIAL_INIT from being set with top (placeholder) on the stackmap frame to ensure the uninitialized_this flag is set in setInitializedThisStatus() to resolve the issue at #9385 but BCV_SPECIAL_NEW was incorrectly excluded by specifying BCV_SPECIAL in the code:

#define	BCV_SPECIAL							(BCV_SPECIAL_INIT | BCV_SPECIAL_NEW)

/runtime/bcverify/bcverify.c
						if ((targetItem != (UDATA) (BCV_BASE_TYPE_TOP))
						&& ((targetItem & BCV_SPECIAL) == 0) <----------
						) {
							*targetStackPtr = (UDATA) (BCV_BASE_TYPE_TOP);
							rewalk = TRUE;
						}

and subsequently the error with BCV_SPECIAL_NEW was incorrectly skipped at /runtime/bcverify/rtverify.c

				if ((*targetPtr & BCV_SPECIAL) && verifyData->createdStackMap) { <--------
					/* Generated stackmaps can skip the check on the target slot with BCV_SPECIAL
					 * as this slot is set up based on the bytecode itself rather than decompressed stackmaps.
					 */
				} else {
					Trc_RTV_matchStack_PrimitiveOrSpecialMismatchException(...);
					rc = BCV_FAIL; /* fail - primitive or special mismatch */
					goto _incompatibleType;
				}

I already created a fix at ChengJin01@dbed590 to only exclude BCV_SPECIAL_INIT which works good as expected to capture the verification error as follows

$ ../jdk11_openj9_fix_v0/bin/java  -cp sootOutput/junit-junit/:hamcrest-all-1.3.jar:junit-4.12.jar org.junit.runner.JUnitCore org.junit.internal.ChecksTest
JUnit version 4.13

verifyBytecodes: Class.Method.Sig = org/junit/internal/Checks.notNull.(Ljava/lang/Object;)Ljava/lang/Object;, ***** verifyData->errorDetailCode: = 0
.E
Time: 0.004
There was 1 failure:
1) initializationError(org.junit.internal.ChecksTest)
java.lang.VerifyError: JVMVRFY012 stack shape inconsistent; class=org/junit/internal/Checks, method=notNull(Ljava/lang/Object;)Ljava/lang/Object;, pc=57
Exception Details:
  Location:
    org/junit/internal/Checks.notNull(Ljava/lang/Object;)Ljava/lang/Object; @57: JBaload1
  Reason:
    Type top (current frame, locals[1]) is not assignable to 'reference' type
  Current Frame:
    bci: @57
    flags: { }
    locals: { 'java/lang/Object', top, integer, integer, integer, integer }
    stack: { }
	at org.junit.runners.ParentRunner.<init>(ParentRunner.java:101)
	at org.junit.runners.BlockJUnit4ClassRunner.<init>(BlockJUnit4ClassRunner.java:84)
	at org.junit.runners.JUnit4.<init>(JUnit4.java:23)
	at org.junit.internal.builders.JUnit4Builder.runnerForClass(JUnit4Builder.java:10)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:70)
	at org.junit.internal.builders.AllDefaultPossibilitiesBuilder.runnerForClass(AllDefaultPossibilitiesBuilder.java:37)
	at org.junit.runner.Computer.getRunner(Computer.java:50)
	at org.junit.runner.Computer$1.runnerForClass(Computer.java:31)
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:70)
	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:125)
	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:111)
	at org.junit.runners.Suite.<init>(Suite.java:81)
	at org.junit.runner.Computer$2.<init>(Computer.java:33)
	at org.junit.runner.Computer.getSuite(Computer.java:28)
	at org.junit.runner.Request.classes(Request.java:77)
	at org.junit.runner.JUnitCommandLineParseResult.createRequest(JUnitCommandLineParseResult.java:116)
	at org.junit.runner.JUnitCore.runMain(JUnitCore.java:77)
	at org.junit.runner.JUnitCore.main(JUnitCore.java:36)

FAILURES!!!
Tests run: 1,  Failures: 1

and also works for the original issue at #9385

../jdk11_openj9_fix_v0/bin/java -cp sootOutput/junit-junit/:hamcrest-all-1.3.jar org.junit.runner.JUnitCore org.junit.internal.runners.ErrorReportingRunnerTest
JUnit version 4.13

verifyBytecodes: Class.Method.Sig = org/junit/internal/Checks.notNull.(Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;, ***** verifyData->errorDetailCode: = 0
.E.E
Time: 0.011
There were 2 failures:
1) givenInvalidTestClassErrorAsCause(org.junit.internal.runners.ErrorReportingRunnerTest)
java.lang.VerifyError: JVMVRFY017 <init> does not call this <init> or super <init>; class=org/junit/internal/runners/ErrorReportingRunner, method=<init>(Ljava/lang/Class;Ljava/lang/Throwable;)V, pc=57
Exception Details:
  Location:
    org/junit/internal/runners/ErrorReportingRunner.<init>(Ljava/lang/Class;Ljava/lang/Throwable;)V @57: JBreturn
  Reason:
    Error exists in the bytecode.
	at org.junit.internal.runners.ErrorReportingRunnerTest.givenInvalidTestClassErrorAsCause(ErrorReportingRunnerTest.java:50)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
...
2) givenInvalidTestClass_integrationTest(org.junit.internal.runners.ErrorReportingRunnerTest)
java.lang.VerifyError: JVMVRFY017 <init> does not call this <init> or super <init>; class=org/junit/internal/runners/ErrorReportingRunner, method=<init>(Ljava/lang/Class;Ljava/lang/Throwable;)V, pc=57
Exception Details:
  Location:
    org/junit/internal/runners/ErrorReportingRunner.<init>(Ljava/lang/Class;Ljava/lang/Throwable;)V @57: JBreturn
  Reason:
    Error exists in the bytecode.
	at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:76)
	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:125)
	at org.junit.runners.model.RunnerBuilder.runners(RunnerBuilder.java:111)
	at org.junit.runners.Suite.<init>(Suite.java:81)
	at org.junit.runner.Computer$2.<init>(Computer.java:33)
...

FAILURES!!!
Tests run: 2,  Failures: 2

Will launch personal builds and verify the internal tests to ensure there is no break in the existing test suites.

ChengJin01 pushed a commit to ChengJin01/openj9 that referenced this issue Jan 22, 2021
The change is to reinforce the initial intention that only
a target slot with BCV_SPECIAL_INIT is exempted from being
set with top given setInitializedThisStatus() needs to check
BCV_SPECIAL_INIT to flag the uninitialized_this object.

Fixes: eclipse-openj9#11683,eclipse-openj9#11684,eclipse-openj9#11685

Signed-off-by: Cheng Jin <[email protected]>
@ChengJin01
Copy link

The issue should be closed as the fix at #11753 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants