[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like where this is going.



================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h:262
     DEFINE_FPR(fioff,     ptr.i386_.fioff, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),
+    DEFINE_FPR(fip,       ptr.x86_64.fip,  LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),
     DEFINE_FPR(foseg,     ptr.i386_.foseg, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),
----------------
mgorny wrote:
> @labath, do you want me to set overlaps here like rax/eax... does? If yes, any suggestion on the style?
What do you mean, exactly? I'm guessing these will have overlapping offsets in the `g` packet to the the offsetof computation, will they not?

Do you mean the sub-reg/value-reg lists?


================
Comment at: lldb/test/Shell/Register/x86-64-fp-read.test:1
+# XFAIL: system-windows
+# REQUIRES: native && target-x86_64
----------------
I expect this will fail on darwin as well.


================
Comment at: lldb/test/Shell/Register/x86-64-fp-read.test:11-12
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+print (void*)($fiseg*0x100000000 + $fioff)
+# CHECK: (void *) $1 = [[FDIV]]
+print &zero
----------------
Move this part after the `fip` check and add a comment to indicate its legacy/compat status?


================
Comment at: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp:82-83
   EXPECT_OFF(fop_x86_64, 0x06, 2);
   // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
   // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
   EXPECT_OFF(fioff_x86_64, 0x08, 4);
----------------
I guess this isn't really true anymore?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91497/new/

https://reviews.llvm.org/D91497