8372806: [lworld] x64: save bad values instead of rfp and lr above the extension space

[marc-chevalier]

That's the x86 counterpart of JDK-8371993.

I've added some comments, using the offsets given in frame_x86.hpp to make sure to put the frame start at the right place. In particular, the frame start is 2 pointer sizes under the sender's sp:

valhalla/src/hotspot/cpu/x86/frame_x86.hpp

Lines 61 to 62 in 3c41c2a

	// non-interpreter frames
	sender_sp_offset = 2,

Unlike aarch64, with x64 we have only one copy of rbp. As with aarch64, I had to get rid of an assert that can't be checked anymore. A small price to pay.

Now, in debug, instead of

pop    r13
sub    rsp,0x20
push   r13

we have

pop    r13
sub    rsp,0x20
mov    DWORD PTR [rsp-0x4],0xdeadda7a
mov    DWORD PTR [rsp-0x8],0xdeadda7a
sub    rsp,0x8

I've kept the pop r13 to limit the difference of behavior between debug and product builds: both will overwrite r13 with the return address, whether it's a good idea or not.

And at runtime, on my favorite compiler/valhalla/inlinetypes/CorrectlyRestoreRfp.java example, instead of the stack:

0x7fd7345fe660:	0x000000042724f5d0	0x00007fd7345fe750  <-- rsp
0x7fd7345fe670:	0xffffffffffffffff	0x00007fd740acc0f7
0x7fd7345fe680:	0x00007fd700000000	0x000000042724f5d0
0x7fd7345fe690:	0x000000056f517f28	0x00007fd7390002a6
0x7fd7345fe6a0:	0x00007fd72c90c8b3	0x0000000000000078  <--             #                | sp_inc
0x7fd7345fe6b0:	0x00007fd7345fe750	0x00007fd740541306  <-- rsp - 0x50  #            rbp | return address
0x7fd7345fe6c0:	0x000000056f49f4e0	0x000000042724f5d0  <--             #  String (arg2) | Object (arg3)
0x7fd7345fe6d0:	0x0000000000000000	0x00007fd740541306  <-- rsp - 0x70  # boolean (arg4) | return address
0x7fd7345fe6e0:	0x00007fd740541306	0x0000000000000000
0x7fd7345fe6f0:	0x000000042724f5d0	0x000000056f49f4e0

we have

0x7f93cfdfe660:	0x000000042724f5d0	0x00007f93cfdfe750  <-- rsp
0x7f93cfdfe670:	0xffffffffffffffff	0x00007f93e4acc107
0x7f93cfdfe680:	0x00007f9300000000	0x000000042724f5d0
0x7f93cfdfe690:	0x000000056f517fb0	0x00007f93dd0002a6
0x7f93cfdfe6a0:	0x00007f93d090c8b3	0x0000000000000078  <--             #                | sp_inc
0x7f93cfdfe6b0:	0x00007f93cfdfe750	0xdeadda7adeadda7a  <-- rsp - 0x50  #            rbp | >>>> bad word <<<<
0x7f93cfdfe6c0:	0x000000056f49f540	0x000000042724f5d0  <--             #  String (arg2) | Object (arg3)
0x7f93cfdfe6d0:	0x0000000000000000	0x00007f93e4541306  <-- rsp - 0x70  # boolean (arg4) | return address
0x7f93cfdfe6e0:	0x00007f93e4541306	0x0000000000000000
0x7f93cfdfe6f0:	0x000000042724f5d0	0x000000056f49f540

I had to problem list some virtual thread tests, in the same fashion as for aarch64, as it used the wrong return address. The assert I had to remove was a sign that only one of the return address is updated in case of deopt, so only one of them is reliable. To help with that, I've added frame::compiled_frame_details like for aarch64, that makes sure to do all the fixing internally.

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-8372806: [lworld] x64: save bad values instead of rfp and lr above the extension space (Enhancement - P4)

Reviewers

• Tobias Hartmann (@TobiHartmann - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1839

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1839.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:

8372806: [lworld] x64: save bad values instead of rfp and lr above the extension space

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 84 new commits pushed to the lworld branch:

• 69399ce: 8373875: [lworld] runtime/cds/DumpSymbolAndStringTable.java fails with --enable-preview
• e98a149: 8374028: [lworld] LoadableDescriptors order should be deterministic
• 1e6e72e: 8372686: [lworld] runtime/cds/serviceability/ReplaceCriticalClasses.java fails with --enable-preview
• ... and 81 more: https://git.openjdk.org/valhalla/compare/49bff14fef20d3edb0e8ae5d3d2c40449d248926...lworld

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 (eaa25540)

[TobiHartmann]

Nice comments! The changes look good to me.

[marc-chevalier]

/integrate

Thanks for review!

[openjdk]

Going to push as commit 328cfbe.
Since your change was applied there have been 200 commits pushed to the lworld branch:

• fe500a0: 8367408: [lworld] runtime/cds/appcds/methodHandles/MethodHandlesGeneralTest.java#aot fails
• f682e44: 8368799: [lworld] Heapwalking functions may report array classes several times
• 423e693: 8373987: [lworld] exploded-image/test broken since disable patching (JDK-8373806)
• ... and 197 more: https://git.openjdk.org/valhalla/compare/49bff14fef20d3edb0e8ae5d3d2c40449d248926...lworld

Your commit was automatically rebased without conflicts.

[openjdk]

@marc-chevalier Pushed as commit 328cfbe.

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