8367553: [lworld] compiler/valhalla/inlinetypes/TestNullableArrays.java fails with segfault in C1 compiled code on aarch64

[marc-chevalier]

Seems that it was just JDK-8367151: [lworld] CorrectlyRestoreRfp.java triggers "bad oop found" during deoptimization/#1751. When I checked first if it still reproduces, #1751 wasn't integrated yet. And after investigation, it turned out to be the same issue. And after integration of the said PR, the problem didn't show again under much more extensive testing than what was enough to reproduce before.

In more details, the case I investigated was mostly in test76_verifier (occasionally in test78_verifier)

valhalla/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestNullableArrays.java

Lines 2100 to 2120 in acb511a

	public void test76_verifier() {
	MyValue1 vt = testValue1;
	Object[] out = new Object[1];
	MyValue1[] vva = (MyValue1[])ValueClass.newNullRestrictedNonAtomicArray(MyValue1.class, 42, MyValue1.DEFAULT);
	MyValue1[] vva_r = (MyValue1[])ValueClass.newNullRestrictedNonAtomicArray(MyValue1.class, 42, MyValue1.DEFAULT);
	vva_r[0] = vt;
	MyValue1[] vba = new MyValue1[42];
	MyValue1[] vba_r = new MyValue1[42];
	vba_r[0] = vt;
	Object[] result = test76(vva, vba, vt, out, 0);
	verify(result, vva_r);
	Asserts.assertEQ(out[0], vva_r[1]);
	result = test76(vva, vba, vt, out, 1);
	verify(result, vba_r);
	Asserts.assertEQ(out[0], vba_r[1]);
	result = test76(vva, vba, vt, out, 2);
	verify(result, vva_r);
	Asserts.assertEQ(out[0], vva_r[1]);
	result = test76(vva, vba, vt, out, 3);
	verify(result, vba_r);
	Asserts.assertEQ(out[0], vba_r[1]);

that is C1-compiled. During a call to C2-compiled verify:

valhalla/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestNullableArrays.java

Lines 1145 to 1153 in acb511a

	static void verify(Object[] src, Object[] dst) {
	for (int i = 0; i < src.length; ++i) {
	if (src[i] != null) {
	Asserts.assertEQ(((MyInterface)src[i]).hash(), ((MyInterface)dst[i]).hash());
	} else {
	Asserts.assertEQ(dst[i], null);
	}
	}
	}

an oop is passed in rfp. Sometimes, the GC moves the object during the execution of verify, it updated the wrong copy of RFP on the stack. Then verify hits an uncommon trap, and in the process picks the wrong rfp by exactly the same mechanism as in JDK-8367151. Later in test76_verify, the content of rfp, that now contains a wrong oop, is used, garbage is read, segfault.

The fact that is goes through an uncommon trap makes it a slightly different flavor than what I've seen before. Also, I should have taken my own advice, and use SerialGC, it would have saved me some complications!

I suggest we just un-problem list now.

Thanks,
Marc

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

• JDK-8367553: [lworld] compiler/valhalla/inlinetypes/TestNullableArrays.java fails with segfault in C1 compiled code on aarch64 (Bug - P3)

Reviewers

• Tobias Hartmann (@TobiHartmann - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1766/head:pull/1766
$ git checkout pull/1766

Update a local copy of the PR:
$ git checkout pull/1766
$ git pull https://git.openjdk.org/valhalla.git pull/1766/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1766

View PR using the GUI difftool:
$ git pr show -t 1766

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1766.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back mchevalier! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@marc-chevalier This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8367553: [lworld] compiler/valhalla/inlinetypes/TestNullableArrays.java fails with segfault in C1 compiled code on aarch64

Reviewed-by: thartmann

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1 new commit pushed to the lworld branch:

• acb511a: 8371993: [lworld] Aarch64: save bad values instead of rfp and lr above the extension space

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

[mlbridge]

Webrevs

• 00: Full (0e69cb34)

[TobiHartmann]

Thanks for the thorough investigation. Looks good to me!

[marc-chevalier]

/integrate

Thanks!

[openjdk]

Going to push as commit 36847ef.
Since your change was applied there has been 1 commit pushed to the lworld branch:

• acb511a: 8371993: [lworld] Aarch64: save bad values instead of rfp and lr above the extension space

Your commit was automatically rebased without conflicts.

[openjdk]

@marc-chevalier Pushed as commit 36847ef.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.