That's the x86 counterpart of 
[JDK-8371993](https://bugs.openjdk.org/browse/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:

https://github.com/openjdk/valhalla/blob/3c41c2aa442076827cb4480373a1d481e481cdf1/src/hotspot/cpu/x86/frame_x86.hpp#L61-L62

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

-------------

Commit messages:
 - Add frame::compiled_frame_details + problem list
 - Comments and zap

Changes: https://git.openjdk.org/valhalla/pull/1839/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1839&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8372806
  Stats: 158 lines in 7 files changed: 113 ins; 29 del; 16 mod
  Patch: https://git.openjdk.org/valhalla/pull/1839.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1839/head:pull/1839

PR: https://git.openjdk.org/valhalla/pull/1839

Reply via email to