[Lldb-commits] [PATCH] D57745: [x64] Process the B field of the REX prefix correctly for the PUSH and POP instructions

Tue Feb 5 16:11:43 PST 2019

jasonmolenda accepted this revision.
jasonmolenda added a comment.

The change looks good to me, thanks for fixing this.  I'm not sure you're testing what you meant to test in TestPopRBPWithREX because you're pushing/popping r13.  You could get the unwind state at byte offset 2 (after the push r13 has executed) and see that you can GetRegisterInfo() r13, and the unwind state at byte offset 4 and verify that you can't GetRegisterInfo() r13.  That's a good test to make sure we handle the B bit correctly.

================
Comment at: unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp:1953
+      0x41, 0x55, // pushq %rbp
+      0x41, 0x5d, // popq %rbp
+      0x90        // nop
----------------
These are pushing/popping r13 aren't they?  0x40 0x55 gives us register 5 (rbp), but 0x41 0x55 gives us register 13 (r13) if I'm reading the intel manuals right (volume 2a, section 2.2.1.2 "More on REX Prefix Fields").

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57745