[lldb] [Process/NetBSD] Fix wrongly mapping mm* registers

[mgorny]

Summary

Fix mistake that mapped mm* registers into the space for xmm* registers,
rather than the one shared with st* registers. In other words,
'register read mmN' now correctly shows the mmN register rather than
part of xmmN.

Includes a minimal lit regression test.

[labath]

I really like how you've approached testing this. There's just one thing that bothers me. Using a .s file means it's going to be hard to make this test work on other OSs (and we should be able to test something like this in a cross-OS manner). All elf targets should probably be fine, but I am pretty sure darwin and windows will choke on this.

I think it would be better to write this as a C(++) file and move the register setting code to inline asm. That should be more-or-less supported on all platforms (when compiling with clang). You should be able to test that by trying a cross-compilation with clang (clang -target x86_64-pc-windows -c foo.c). I've tried it with a simplified snipped from your test and it seemed to work fine.

(Note that I still expect this test to fail on windows, because AFAIK we don't support reading these registers on windows yet, but it would be good if at least the source code compiled there.)

lldb/lit/Register/x86-mm-xmm-read.test

1 ↗

(On Diff #193905)

Does this mean "the host is an x86 system", or "x86 is a configured llvm target"? My impression is that in means the latter https://github.com/llvm-mirror/lldb/blob/master/lit/lit.cfg.py#L49, but we obviously want the former here.

7–22 ↗

(On Diff #193905)

Maybe you could drop the -NEXTs here (or even replace them by -DAGs), because (I presume) you are not interested in the order the registers get printed, just their values.

[mgorny]

Thanks. I'll try converting it to C now.

lldb/lit/Register/x86-mm-xmm-read.test

1 ↗

(On Diff #193905)

Do you have any idea how to achieve the former? I can't find anything looking like it. Though I suppose it's entirely possible that there's no such thing atm.

[mgorny]

Rewritten the test to C++ with inline assembly, and switched to CHECK-DAG. Still need to figure out how to filter it for correct platform.

[labath]

lldb/lit/Register/x86-mm-xmm-read.test

1 ↗

(On Diff #193905)

Yeah, it looks like there isn't anything like that at the moment. Maybe you could add something like system-x86 and use that in this test ?

+ @zturner, if he has any ideas here.

[mgorny]

Updated 'REQUIRES'. I've looked through other LLVM tests, and noticed there's already 'native' concept (when host==target), and some 'target*' features for systems. I've chosen to combine the former with new 'target-x86'. I'll link the other patch in a minute.

[labath]

The new version looks really great. Thank you.

As I mentioned previously, I think this will probably fail on windows, so I suggest you look out for the bot and be prepared to add an XFAIL here.

[mgorny]

Will do, thanks.

lit/Register/Inputs/x86-mm-xmm-read.cpp

+#include <cstdint>
+
+struct alignas(16) xmm_t {
+  uint64_t a, b;
+};
+
+int main() {
+  uint64_t mm0 = 0x0102030405060708;
+  uint64_t mm1 = 0x1112131415161718;
+  uint64_t mm2 = 0x2122232425262728;
+  uint64_t mm3 = 0x3132333435363738;
+  uint64_t mm4 = 0x4142434445464748;
+  uint64_t mm5 = 0x5152535455565758;
+  uint64_t mm6 = 0x6162636465666768;
+  uint64_t mm7 = 0x7172737475767778;
+
+  xmm_t xmm0[2] = { 0x020406080A0C0E01, 0x030507090B0D0F00 };
+  xmm_t xmm1[2] = { 0x121416181A1C1E11, 0x131517191B1D1F10 };
+  xmm_t xmm2[2] = { 0x222426282A2C2E21, 0x232527292B2D2F20 };
+  xmm_t xmm3[2] = { 0x323436383A3C3E31, 0x333537393B3D3F30 };
+  xmm_t xmm4[2] = { 0x424446484A4C4E41, 0x434547494B4D4F40 };
+  xmm_t xmm5[2] = { 0x525456585A5C5E51, 0x535557595B5D5F50 };
+  xmm_t xmm6[2] = { 0x626466686A6C6E61, 0x636567696B6D6F60 };
+  xmm_t xmm7[2] = { 0x727476787A7C7E71, 0x737577797B7D7F70 };
+
+  asm volatile(
+    "movq    %0, %%mm0\n\t"
+    "movq    %1, %%mm1\n\t"
+    "movq    %2, %%mm2\n\t"
+    "movq    %3, %%mm3\n\t"
+    "movq    %4, %%mm4\n\t"
+    "movq    %5, %%mm5\n\t"
+    "movq    %6, %%mm6\n\t"
+    "movq    %7, %%mm7\n\t"
+    "\n\t"
+    "movaps  %8, %%xmm0\n\t"
+    "movaps  %9, %%xmm1\n\t"
+    "movaps  %10, %%xmm2\n\t"
+    "movaps  %11, %%xmm3\n\t"
+    "movaps  %12, %%xmm4\n\t"
+    "movaps  %13, %%xmm5\n\t"
+    "movaps  %14, %%xmm6\n\t"
+    "movaps  %15, %%xmm7\n\t"
+    "\n\t"
+    "int3"
+    :
+    : "g"(mm0), "g"(mm1), "g"(mm2), "g"(mm3), "g"(mm4), "g"(mm5), "g"(mm6),
+      "g"(mm7), "m"(xmm0), "m"(xmm1), "m"(xmm2), "m"(xmm3), "m"(xmm4),
+      "m"(xmm5), "m"(xmm6), "m"(xmm7)
+    : "%mm0", "%mm1", "%mm2", "%mm3", "%mm4", "%mm5", "%mm6", "%mm7",
+      "%xmm0", "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7"
+  );
+
+  return 0;
+}

lit/Register/x86-mm-xmm-read.test

+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clang++ %p/Inputs/x86-mm-xmm-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+
+register read --all
+# CHECK-DAG: mm0 = 0x0102030405060708
+# CHECK-DAG: mm1 = 0x1112131415161718
+# CHECK-DAG: mm2 = 0x2122232425262728
+# CHECK-DAG: mm3 = 0x3132333435363738
+# CHECK-DAG: mm4 = 0x4142434445464748
+# CHECK-DAG: mm5 = 0x5152535455565758
+# CHECK-DAG: mm6 = 0x6162636465666768
+# CHECK-DAG: mm7 = 0x7172737475767778
+# CHECK-DAG: xmm0 = {0x01 0x0e 0x0c 0x0a 0x08 0x06 0x04 0x02 0x00 0x0f 0x0d 0x0b 0x09 0x07 0x05 0x03}
+# CHECK-DAG: xmm1 = {0x11 0x1e 0x1c 0x1a 0x18 0x16 0x14 0x12 0x10 0x1f 0x1d 0x1b 0x19 0x17 0x15 0x13}
+# CHECK-DAG: xmm2 = {0x21 0x2e 0x2c 0x2a 0x28 0x26 0x24 0x22 0x20 0x2f 0x2d 0x2b 0x29 0x27 0x25 0x23}
+# CHECK-DAG: xmm3 = {0x31 0x3e 0x3c 0x3a 0x38 0x36 0x34 0x32 0x30 0x3f 0x3d 0x3b 0x39 0x37 0x35 0x33}
+# CHECK-DAG: xmm4 = {0x41 0x4e 0x4c 0x4a 0x48 0x46 0x44 0x42 0x40 0x4f 0x4d 0x4b 0x49 0x47 0x45 0x43}
+# CHECK-DAG: xmm5 = {0x51 0x5e 0x5c 0x5a 0x58 0x56 0x54 0x52 0x50 0x5f 0x5d 0x5b 0x59 0x57 0x55 0x53}
+# CHECK-DAG: xmm6 = {0x61 0x6e 0x6c 0x6a 0x68 0x66 0x64 0x62 0x60 0x6f 0x6d 0x6b 0x69 0x67 0x65 0x63}
+# CHECK-DAG: xmm7 = {0x71 0x7e 0x7c 0x7a 0x78 0x76 0x74 0x72 0x70 0x7f 0x7d 0x7b 0x79 0x77 0x75 0x73}

source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
   case lldb_mm0_x86_64:
   case lldb_mm1_x86_64:
   case lldb_mm2_x86_64:
   case lldb_mm3_x86_64:
   case lldb_mm4_x86_64:
   case lldb_mm5_x86_64:
   case lldb_mm6_x86_64:
   case lldb_mm7_x86_64:
-    reg_value.SetBytes(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_mm0_x86_64],
+    reg_value.SetBytes(&m_fpr_x86_64.fxstate.fx_87_ac[reg - lldb_mm0_x86_64],
                        reg_info->byte_size, endian::InlHostByteOrder());
     break;
   case lldb_xmm0_x86_64:
   case lldb_xmm1_x86_64:
   case lldb_xmm2_x86_64:
   case lldb_xmm3_x86_64:
   case lldb_xmm4_x86_64:
   case lldb_xmm5_x86_64:
Status NativeRegisterContextNetBSD_x86_64::WriteRegister(
   case lldb_mm0_x86_64:
   case lldb_mm1_x86_64:
   case lldb_mm2_x86_64:
   case lldb_mm3_x86_64:
   case lldb_mm4_x86_64:
   case lldb_mm5_x86_64:
   case lldb_mm6_x86_64:
   case lldb_mm7_x86_64:
-    ::memcpy(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_mm0_x86_64],
+    ::memcpy(&m_fpr_x86_64.fxstate.fx_87_ac[reg - lldb_mm0_x86_64],
              reg_value.GetBytes(), reg_value.GetByteSize());
     break;
   case lldb_xmm0_x86_64:
   case lldb_xmm1_x86_64:
   case lldb_xmm2_x86_64:
   case lldb_xmm3_x86_64:
   case lldb_xmm4_x86_64:
   case lldb_xmm5_x86_64: