[lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE

[mgorny]

Summary

Provide a (conditional) support for the new PT_GETXSTATE
and PT_SETXSTATE ptrace() requests, and use them to implement getting
and setting YMM registers. The functions used for splitting
and recombining YMM register data are based on matching functions
in FreeBSD plugin, with some simplification and updates to match NetBSD
structures.

[mgorny]

Rebased for changes in NativeRegisterContextNetBSD_x86_64::GetSetForNativeRegNum(), and removed the conditions there.

[labath]

We have the same XSTATE<->YMM conversion functions in NativeProcessLinux. It would be nice to extract them to some common place (Plugins/Process/Utility, I guess :P). The rest seems pretty straight-forward, though you're repeating the patterns that I find really reduntant/annoying. Eg. I don't see the reason to check the null-ness of the XState buffer. It sounds like it should be a hard error for someone to call ReadXState without bothering to arrange for the storage buffer to exist.

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

239 ↗

(On Diff #205763)

Why would we ever want to do this?

[mgorny]

In D63545#1551627, @labath wrote:

We have the same XSTATE<->YMM conversion functions in NativeProcessLinux. It would be nice to extract them to some common place (Plugins/Process/Utility, I guess :P).

Hmm, I guess that's doable if we pass the relevant struct fields as pointers.

The rest seems pretty straight-forward, though you're repeating the patterns that I find really reduntant/annoying. Eg. I don't see the reason to check the null-ness of the XState buffer. It sounds like it should be a hard error for someone to call ReadXState without bothering to arrange for the storage buffer to exist.

Yeah, I guess that's something I can fix globally first.

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

239 ↗

(On Diff #205763)

I was asking myself the exact same thing! But the code is present both on Linux and FreeBSD, so I presumed there is some fancy use case I have no clue about.

[labath]

In D63545#1551675, @mgorny wrote:

In D63545#1551627, @labath wrote:

We have the same XSTATE<->YMM conversion functions in NativeProcessLinux. It would be nice to extract them to some common place (Plugins/Process/Utility, I guess :P).

Hmm, I guess that's doable if we pass the relevant struct fields as pointers.

Maybe have GetYMM(unsigned num, YMM& reg)/SetYMM(unsigned num, const YMM &reg) methods on the XSAVE struct ?

The rest seems pretty straight-forward, though you're repeating the patterns that I find really reduntant/annoying. Eg. I don't see the reason to check the null-ness of the XState buffer. It sounds like it should be a hard error for someone to call ReadXState without bothering to arrange for the storage buffer to exist.

Yeah, I guess that's something I can fix globally first.

Sounds great.

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

239 ↗

(On Diff #205763)

Yeah, I don't think that's a safe assumption around here. :)

[mgorny]

In D63545#1551681, @labath wrote:

In D63545#1551675, @mgorny wrote:

In D63545#1551627, @labath wrote:

We have the same XSTATE<->YMM conversion functions in NativeProcessLinux. It would be nice to extract them to some common place (Plugins/Process/Utility, I guess :P).

Hmm, I guess that's doable if we pass the relevant struct fields as pointers.

Maybe have GetYMM(unsigned num, YMM& reg)/SetYMM(unsigned num, const YMM &reg) methods on the XSAVE struct ?

We aren't using XSAVE struct. We are using NetBSD-specific struct xstate that has guaranteed fixed offsets.

[labath]

In D63545#1551703, @mgorny wrote:

In D63545#1551681, @labath wrote:

Maybe have GetYMM(unsigned num, YMM& reg)/SetYMM(unsigned num, const YMM &reg) methods on the XSAVE struct ?

We aren't using XSAVE struct. We are using NetBSD-specific struct xstate that has guaranteed fixed offsets.

Ah, right, I forgot about that part. In that case, it's probably not worth doing that...

[krytarowski]

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

239 ↗

(On Diff #205763)

Right now x86 supports only LE on NetBSD.

[mgorny]

Updated to use new XState conversion methods.

[mgorny]

…and removed stale declarations.

[mgorny]

This is ready to be reviewed now.

[mgorny]

Ping.

[labath]

I don't know whether the code is correct for NetBSD, but it looks ok from a general readability perspective.

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

 //===-- NativeRegisterContextNetBSD_x86_64.h --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #if defined(__x86_64__)
 
 #ifndef lldb_NativeRegisterContextNetBSD_x86_64_h
 #define lldb_NativeRegisterContextNetBSD_x86_64_h
 
 // clang-format off
 #include <sys/param.h>
 #include <sys/types.h>
+#include <sys/ptrace.h>
 #include <machine/reg.h>
 // clang-format on
 
 #include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 
+#if defined(PT_GETXSTATE) && defined(PT_SETXSTATE)
+#define HAVE_XSTATE
+#endif
+
 namespace lldb_private {
 namespace process_netbsd {
 
 class NativeProcessNetBSD;
 
 class NativeRegisterContextNetBSD_x86_64 : public NativeRegisterContextNetBSD {
 public:
   NativeRegisterContextNetBSD_x86_64(const ArchSpec &target_arch,
uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
                                  uint32_t watch_flags) override;
 
   lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
 private:
   // Private member types.
-  enum { GPRegSet, FPRegSet, DBRegSet };
+  enum { GPRegSet, FPRegSet, DBRegSet, XStateRegSet };
 
   // Private member variables.
   struct reg m_gpr_x86_64;
   struct fpreg m_fpr_x86_64;
   struct dbreg m_dbr_x86_64;
+#ifdef HAVE_XSTATE
+  struct xstate m_xstate_x86_64;
+#endif
 
   int GetSetForNativeRegNum(int reg_num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextNetBSD_x86_64_h
 
 #endif // defined(__x86_64__)

lldb/trunk/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

 #include "lldb/Utility/Status.h"
 
 #include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
 
 // clang-format off
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/uio.h>
 #include <x86/cpu.h>
+#include <x86/cpu_extended_state.h>
+#include <x86/specialreg.h>
 #include <elf.h>
 #include <err.h>
 #include <stdint.h>
 #include <stdlib.h>
 // clang-format on
 
 using namespace lldb_private;
 using namespace lldb_private::process_netbsd;

 
 int NativeRegisterContextNetBSD_x86_64::GetSetForNativeRegNum(
     int reg_num) const {
   if (reg_num <= k_last_gpr_x86_64)
     return GPRegSet;
   else if (reg_num <= k_last_fpr_x86_64)
     return FPRegSet;
   else if (reg_num <= k_last_avx_x86_64)
-    return -1; // AVX
+    return XStateRegSet; // AVX
   else if (reg_num <= k_last_mpxr_x86_64)
     return -1; // MPXR
   else if (reg_num <= k_last_mpxc_x86_64)
     return -1; // MPXC
   else if (reg_num <= lldb_dr7_x86_64)
     return DBRegSet; // DBR
   else
     return -1;
 }
 
 Status NativeRegisterContextNetBSD_x86_64::ReadRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_GETREGS, &m_gpr_x86_64);
   case FPRegSet:
     return DoRegisterSet(PT_GETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
     return DoRegisterSet(PT_GETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+    {
+      struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+      return DoRegisterSet(PT_GETXSTATE, &iov);
+    }
+#else
+    return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::ReadRegisterSet");
 }
 
 Status NativeRegisterContextNetBSD_x86_64::WriteRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return DoRegisterSet(PT_SETREGS, &m_gpr_x86_64);
   case FPRegSet:
     return DoRegisterSet(PT_SETFPREGS, &m_fpr_x86_64);
   case DBRegSet:
     return DoRegisterSet(PT_SETDBREGS, &m_dbr_x86_64);
+  case XStateRegSet:
+#ifdef HAVE_XSTATE
+    {
+      struct iovec iov = {&m_xstate_x86_64, sizeof(m_xstate_x86_64)};
+      return DoRegisterSet(PT_SETXSTATE, &iov);
+    }
+#else
+    return Status("XState is not supported by the kernel");
+#endif
   }
   llvm_unreachable("NativeRegisterContextNetBSD_x86_64::WriteRegisterSet");
 }
 
 Status
 NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
                                                  RegisterValue &reg_value) {
   Status error;
NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
     reg_value.SetBytes(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
                        reg_info->byte_size, endian::InlHostByteOrder());
     break;
+  case lldb_ymm0_x86_64:
+  case lldb_ymm1_x86_64:
+  case lldb_ymm2_x86_64:
+  case lldb_ymm3_x86_64:
+  case lldb_ymm4_x86_64:
+  case lldb_ymm5_x86_64:
+  case lldb_ymm6_x86_64:
+  case lldb_ymm7_x86_64:
+  case lldb_ymm8_x86_64:
+  case lldb_ymm9_x86_64:
+  case lldb_ymm10_x86_64:
+  case lldb_ymm11_x86_64:
+  case lldb_ymm12_x86_64:
+  case lldb_ymm13_x86_64:
+  case lldb_ymm14_x86_64:
+  case lldb_ymm15_x86_64:
+#ifdef HAVE_XSTATE
+    if (!(m_xstate_x86_64.xs_rfbm & XCR0_SSE) ||
+        !(m_xstate_x86_64.xs_rfbm & XCR0_YMM_Hi128)) {
+      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
+                                     reg_info->name);
+    } else {
+      uint32_t reg_index = reg - lldb_ymm0_x86_64;
+      YMMReg ymm = XStateToYMM(
+          m_xstate_x86_64.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+          m_xstate_x86_64.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
+      reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
+                         endian::InlHostByteOrder());
+    }
+#else
+    error.SetErrorString("XState queries not supported by the kernel");
+#endif
+    break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64:
   case lldb_dr7_x86_64:
Status NativeRegisterContextNetBSD_x86_64::WriteRegister(
   case lldb_xmm11_x86_64:
   case lldb_xmm12_x86_64:
   case lldb_xmm13_x86_64:
   case lldb_xmm14_x86_64:
   case lldb_xmm15_x86_64:
     ::memcpy(&m_fpr_x86_64.fxstate.fx_xmm[reg - lldb_xmm0_x86_64],
              reg_value.GetBytes(), reg_value.GetByteSize());
     break;
+  case lldb_ymm0_x86_64:
+  case lldb_ymm1_x86_64:
+  case lldb_ymm2_x86_64:
+  case lldb_ymm3_x86_64:
+  case lldb_ymm4_x86_64:
+  case lldb_ymm5_x86_64:
+  case lldb_ymm6_x86_64:
+  case lldb_ymm7_x86_64:
+  case lldb_ymm8_x86_64:
+  case lldb_ymm9_x86_64:
+  case lldb_ymm10_x86_64:
+  case lldb_ymm11_x86_64:
+  case lldb_ymm12_x86_64:
+  case lldb_ymm13_x86_64:
+  case lldb_ymm14_x86_64:
+  case lldb_ymm15_x86_64:
+#ifdef HAVE_XSTATE
+    if (!(m_xstate_x86_64.xs_rfbm & XCR0_SSE) ||
+        !(m_xstate_x86_64.xs_rfbm & XCR0_YMM_Hi128)) {
+      error.SetErrorStringWithFormat("register \"%s\" not supported by CPU/kernel",
+                                     reg_info->name);
+    } else {
+      uint32_t reg_index = reg - lldb_ymm0_x86_64;
+      YMMReg ymm;
+      ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize());
+      YMMToXState(ymm,
+          m_xstate_x86_64.xs_fxsave.fx_xmm[reg_index].xmm_bytes,
+          m_xstate_x86_64.xs_ymm_hi128.xs_ymm[reg_index].ymm_bytes);
+    }
+#else
+    error.SetErrorString("XState not supported by the kernel");
+#endif
+    break;
   case lldb_dr0_x86_64:
   case lldb_dr1_x86_64:
   case lldb_dr2_x86_64:
   case lldb_dr3_x86_64:
   case lldb_dr4_x86_64:
   case lldb_dr5_x86_64:
   case lldb_dr6_x86_64:
   case lldb_dr7_x86_64: