Fix Java 12 build failures #3980

theresa-m · 2018-12-10T20:28:16Z

Errors in java.lang.System caused by internal apis

JEP 334 stubs for Java 12 build needed for compilation

stubs for java.lang.Class
stubs for java.lang.invoke.VarHandle.VarHandleDesc

12 builds with extensions up to 0b941d8:

new unsafe methods xxReference and natives, invokeCleaner
MethodHandle.filterArgumentsWithCombiner()
MethodHandle.foldArgumentsWithCombiner()

Fixes: #3768
Fixes: #4158

Signed-off-by: Theresa Mammarella [email protected]

theresa-m · 2018-12-11T14:10:23Z

investigating reason for pr build failure

theresa-m · 2018-12-11T16:42:04Z

update jcl version 12 to vmconstantpool.xml additions. pr build should pass

theresa-m · 2018-12-12T21:24:32Z

updated java.lang.invoke.VarHandle.VarHandleDesc stubs to throw OpenJDKCompileStubThrowError instead of UnsupportedOperationException

theresa-m · 2019-01-08T15:13:49Z

I've added a commit to fix the compile errors described in #4158 to this pull request. This branch should now work again to compile Java 12.

theresa-m · 2019-01-08T15:25:03Z

update copyright dates to 2019

DanHeidinga · 2019-02-11T16:06:52Z

runtime/vm/MHInterpreter.cpp

+
+	/* [... filterHandle args descriptionBytes MethodTypeFrame filterHandle args] */
+	_currentThread->sp -= (filterArgSlots  + 1);
+	memcpy(_currentThread->sp, spPriorToFrameBuild, sizeof(UDATA) * (filterArgSlots+1));


I don't think this memcpy of the arguments is actually used. There's also nothing that describes them on the stack so if they were used, we'd have a potential issue post-gc.

Can you try removing this?

runtime/vm/MHInterpreter.cpp

DanHeidinga · 2019-02-11T16:15:14Z

runtime/vm/MHInterpreter.cpp

+			*spCombinerSlot = *(spFirstFilterArgSlot - argumentTypeSlots - 1);
+		}
+	}
+	_currentThread->sp = spCombinerSlot; /* after loop spCombinerSlot is at the top of the combiner arguments */


This shouldn't be necessary as UDATA *spCombinerSlot = _currentThread->sp + filterArgSlots;

Maybe add an assert that _currentThread->sp == spCombinerSlot?

It's necessary to do this here because the sp is previously set at the number of arguments for the "next" handle after the memcpy. This sets it to the correct number of arguments for the combiner handle which might be different. I will apply this comment to the version I'm working on without the memcpy though.

DanHeidinga · 2019-02-11T16:19:54Z

runtime/vm/MHInterpreter.cpp

+	UDATA *mhPtr = UNTAGGED_A0(frame);
+
+	/* Advance past the frame and single argument (now filterHandle is at the top of the stack)) */
+	bp = (UDATA *)(((J9SFStackFrame*)(bp+1)) + 1);


This assumes that the J9SFMethodTypeFrame is immediately after the PlaceHolderFrame:

J9SFStackFrame MethodHandle J9SFMethodTypeFrame

I don't see how this works with the extra copy of the args from the filterArgumentsWithCombiner method.

I think the extra args in the stack comment is a mistake it should be /* [... filterHandle args descriptionBytes MethodTypeFrame filterHandle PlaceHolderFrame combinerReturnValue] */ similar to what it was in the filterArgumentsWithCombinerHandle method

DanHeidinga · 2019-02-11T16:21:04Z

Are there test cases for the new MethodHandles? In a separate PR is fine.

theresa-m · 2019-02-13T14:56:03Z

Thanks for the review @DanHeidinga I've just pushed a commit that applies your changes. looking at this java.lang.System merge conflict that came up

- new unsafe methods xxReference and natives, invokeCleaner - ImplementMethodHandle.filterArgumentsWithCombiner() - Implement MethodHandle.foldArgumentsWithCombiner() Signed-off-by: Theresa Mammarella <[email protected]>

Signed-off-by: Theresa Mammarella <[email protected]>

Fix JCL compile errors caused by changes to internal APIs Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m · 2019-02-13T15:48:35Z

Merged my java.lang.System changes with those for #4660 to resolve merge conflict. Confirmed that this test is still passing.

DanHeidinga

With this change, the insertPlaceHolderFrame2 method isn't required and the change should be good to merge

DanHeidinga · 2019-02-13T20:00:32Z

runtime/vm/MHInterpreter.cpp

+	Assert_VM_true(spCombinerSlot == _currentThread->sp);
+
+	/* [... filterHandle args descriptionBytes MethodTypeFrame filterHandle PlaceHolderFrame combinerHandle combinerArgs] */
+	insertPlaceHolderFrame2(combinerArgSlots, methodHandle, J9VMJAVALANGINVOKEMETHODHANDLE_FILTERARGUMENTSWITHCOMBINERPLACEHOLDER_METHOD(_vm));


Rather than introducing the new helper, write the combiner MH into the stack after making the placeholder frame.

insertPlaceHolderFrame(combinerArgSlots, methodHandle, J9VMJAVALANGINVOKEMETHODHANDLE_FILTERARGUMENTSWITHCOMBINERPLACEHOLDER_METHOD(_vm)); ((j9object_t *)_currentThread->sp)[combinerArgSlots] = combinerHandle;

keithc-ca · 2019-02-13T20:39:08Z

To be certain I don't misunderstand your comments @DanHeidinga, I've reassigned this to you to merge when you're satisfied.

Signed-off-by: Theresa Mammarella <[email protected]>

DanHeidinga

lgtm

DanHeidinga · 2019-02-13T22:10:05Z

Jenkins test sanity zlinux jdk11

DanHeidinga · 2019-02-13T22:10:16Z

Jenkins test sanity xlinux jdk12

pshipton · 2019-02-13T23:31:59Z

Running a jdk12 sanity with these changes and #4531
https://ci.eclipse.org/openj9/view/Pipelines/job/Pipeline-Release-Build/40/

pshipton · 2019-02-13T23:34:31Z

Note the known issues with jdk12 sanity in https://github.com/eclipse/openj9/projects/12

DanHeidinga · 2019-02-14T00:51:06Z

Given the jdk11 build passed and the jdk12 compiles passed in Peter's build, I'm inclined to merge this. Any additional sanity test failures in 12 can be addressed in separate PRs

theresa-m · 2019-02-14T01:20:25Z

It looks to me like the failing pr build is not actually using these changes:

17:12:28 Using OPENJ9_REPO = https://github.com/eclipse/openj9.git OPENJ9_BRANCH = master OPENJ9_SHA = 89b39276bc647c6eafa5127db9dd3e3040e9a72b

pshipton · 2019-02-14T01:36:12Z

jdk12 testing in https://ci.eclipse.org/openj9/view/Pipelines/job/Pipeline-Release-Build/40/ shows only known issues.

cmdLineTester_CryptoTest_0 - openssl not ported to jdk12 yet

#4658
TestFlushReflectionCache_0

#4661
openj9_jsr292Test_0
openj9_jsr292Test_JitCount0_0

theresa-m mentioned this pull request Dec 10, 2018

Recent Java12 changes have moved some classes causing builds to now fail #3768

Closed

theresa-m force-pushed the working_12 branch 2 times, most recently from 5aeddd7 to 1d16a0b Compare December 10, 2018 20:49

pshipton added comp:vm jdk12 labels Dec 10, 2018

theresa-m changed the title ~~12 builds with extensions up to 0b941d8~~ Fix Java 12 build failures Dec 11, 2018

theresa-m force-pushed the working_12 branch 2 times, most recently from c1b218b to 2bd7ab1 Compare December 11, 2018 16:41

theresa-m force-pushed the working_12 branch 2 times, most recently from c6577b6 to 5a98969 Compare December 12, 2018 21:23

This was referenced Dec 13, 2018

Stubs for Constants API (jep334) #4030

Merged

Methods and tests for jep334 String #4105

Merged

theresa-m mentioned this pull request Dec 31, 2018

Methods and tests for jep334 Class #4157

Merged

theresa-m force-pushed the working_12 branch from 5a98969 to 0ecbadf Compare January 8, 2019 15:08

theresa-m force-pushed the working_12 branch 2 times, most recently from 7148e05 to 22f0e4f Compare January 8, 2019 15:24

This was referenced Jan 8, 2019

JEP 334 Constants API pr tracking #4195

Closed

Methods and tests for jep334 MethodType #4194

Merged

theresa-m force-pushed the working_12 branch from 22f0e4f to 1b7bab0 Compare January 9, 2019 16:32

This was referenced Jan 23, 2019

Method and tests for jep334 MethodHandle #4412

Merged

Jep334 VarHandle and VarHandleDesc #4449

Merged

theresa-m force-pushed the working_12 branch from 1b7bab0 to 07dcbd5 Compare January 28, 2019 15:11

pshipton added this to the Release 0.13.0 (Java 12) milestone Jan 28, 2019

theresa-m force-pushed the working_12 branch from 07dcbd5 to 61be6cf Compare January 28, 2019 19:35

DanHeidinga reviewed Feb 11, 2019

View reviewed changes

theresa-m added 3 commits February 13, 2019 09:57

12 builds with extensions up to 0b941d8

9dab9d3

- new unsafe methods xxReference and natives, invokeCleaner - ImplementMethodHandle.filterArgumentsWithCombiner() - Implement MethodHandle.foldArgumentsWithCombiner() Signed-off-by: Theresa Mammarella <[email protected]>

JEP 334 stubs for Java 12 build

b8cefda

Signed-off-by: Theresa Mammarella <[email protected]>

Java 12 compile errors: java.lang.Sytem

7d5cb6e

Fix JCL compile errors caused by changes to internal APIs Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m force-pushed the working_12 branch from 726f776 to d8ddd09 Compare February 13, 2019 15:47

theresa-m force-pushed the working_12 branch from d8ddd09 to 254822d Compare February 13, 2019 16:04

DanHeidinga reviewed Feb 13, 2019

View reviewed changes

keithc-ca assigned DanHeidinga and unassigned keithc-ca Feb 13, 2019

theresa-m force-pushed the working_12 branch from 254822d to df6ea47 Compare February 13, 2019 21:15

Updates from reviews

402142b

Signed-off-by: Theresa Mammarella <[email protected]>

theresa-m force-pushed the working_12 branch from df6ea47 to 402142b Compare February 13, 2019 21:15

DanHeidinga approved these changes Feb 13, 2019

View reviewed changes

pshipton merged commit 7ce88fd into eclipse-openj9:master Feb 14, 2019

theresa-m deleted the working_12 branch February 25, 2019 14:15

This was referenced Feb 26, 2019

Refactor fold native MHInterpreter #4895

Merged

master: Fix memory semantics for Unsafe.getAndSet* #4909

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Java 12 build failures #3980

Fix Java 12 build failures #3980

theresa-m commented Dec 10, 2018 •

edited

Loading

theresa-m commented Dec 11, 2018

theresa-m commented Dec 11, 2018

theresa-m commented Dec 12, 2018

theresa-m commented Jan 8, 2019

theresa-m commented Jan 8, 2019

DanHeidinga Feb 11, 2019

DanHeidinga Feb 11, 2019

theresa-m Feb 13, 2019

DanHeidinga Feb 11, 2019

theresa-m Feb 13, 2019

DanHeidinga commented Feb 11, 2019

theresa-m commented Feb 13, 2019

theresa-m commented Feb 13, 2019 •

edited

Loading

DanHeidinga left a comment

DanHeidinga Feb 13, 2019

theresa-m Feb 13, 2019

keithc-ca commented Feb 13, 2019

DanHeidinga left a comment

DanHeidinga commented Feb 13, 2019

DanHeidinga commented Feb 13, 2019

pshipton commented Feb 13, 2019

pshipton commented Feb 13, 2019

DanHeidinga commented Feb 14, 2019

theresa-m commented Feb 14, 2019

pshipton commented Feb 14, 2019 •

edited

Loading

Fix Java 12 build failures #3980

Fix Java 12 build failures #3980

Conversation

theresa-m commented Dec 10, 2018 • edited Loading

theresa-m commented Dec 11, 2018

theresa-m commented Dec 11, 2018

theresa-m commented Dec 12, 2018

theresa-m commented Jan 8, 2019

theresa-m commented Jan 8, 2019

DanHeidinga Feb 11, 2019

Choose a reason for hiding this comment

DanHeidinga Feb 11, 2019

Choose a reason for hiding this comment

theresa-m Feb 13, 2019

Choose a reason for hiding this comment

DanHeidinga Feb 11, 2019

Choose a reason for hiding this comment

theresa-m Feb 13, 2019

Choose a reason for hiding this comment

DanHeidinga commented Feb 11, 2019

theresa-m commented Feb 13, 2019

theresa-m commented Feb 13, 2019 • edited Loading

DanHeidinga left a comment

Choose a reason for hiding this comment

DanHeidinga Feb 13, 2019

Choose a reason for hiding this comment

theresa-m Feb 13, 2019

Choose a reason for hiding this comment

keithc-ca commented Feb 13, 2019

DanHeidinga left a comment

Choose a reason for hiding this comment

DanHeidinga commented Feb 13, 2019

DanHeidinga commented Feb 13, 2019

pshipton commented Feb 13, 2019

pshipton commented Feb 13, 2019

DanHeidinga commented Feb 14, 2019

theresa-m commented Feb 14, 2019

pshipton commented Feb 14, 2019 • edited Loading

theresa-m commented Dec 10, 2018 •

edited

Loading

theresa-m commented Feb 13, 2019 •

edited

Loading

pshipton commented Feb 14, 2019 •

edited

Loading