[Lldb-commits] [lldb] 193a9b3 - Revert "[lldb] Use translated full ftag values"

Mikhail Goncharov via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 19 06:25:16 PST 2020


Author: Mikhail Goncharov
Date: 2020-11-19T15:24:59+01:00
New Revision: 193a9b374e24d31b30095f2789f1994bc0c7b663

URL: https://github.com/llvm/llvm-project/commit/193a9b374e24d31b30095f2789f1994bc0c7b663
DIFF: https://github.com/llvm/llvm-project/commit/193a9b374e24d31b30095f2789f1994bc0c7b663.diff

LOG: Revert "[lldb] Use translated full ftag values"

This reverts commit c43abf043692babf9ad4f8bded2fdf6ab9c354b0.

Test commands/register/register/register_command/TestRegisters.py fails.
Buildbot http://lab.llvm.org:8011/#/changes/4149

Added: 
    

Modified: 
    lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
    lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
    lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
    lldb/source/Plugins/Process/Utility/CMakeLists.txt
    lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
    lldb/test/Shell/Register/x86-64-fp-read.test
    lldb/test/Shell/Register/x86-64-fp-write.test
    lldb/test/Shell/Register/x86-fp-read.test
    lldb/test/Shell/Register/x86-fp-write.test
    lldb/unittests/Process/Utility/CMakeLists.txt

Removed: 
    lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
    lldb/unittests/Process/Utility/RegisterContextTest.cpp


################################################################################
diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
index ea5400c55713..ea2494dabf27 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -451,16 +451,10 @@ NativeRegisterContextFreeBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet: {
-    void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
-    FXSAVE *fpr = reinterpret_cast<FXSAVE *>(m_fpr.data());
-    if (data == &fpr->ftag) // ftag
-      reg_value.SetUInt16(
-          AbridgedToFullTagWord(fpr->ftag, fpr->fstat, fpr->stmm));
-    else
-      reg_value.SetBytes(data, reg_info->byte_size, endian::InlHostByteOrder());
+  case DBRegSet:
+    reg_value.SetBytes(GetOffsetRegSetData(set, reg_info->byte_offset),
+                       reg_info->byte_size, endian::InlHostByteOrder());
     break;
-  }
   case YMMRegSet: {
     llvm::Optional<YMMSplitPtr> ymm_reg = GetYMMSplitReg(reg);
     if (!ymm_reg) {
@@ -517,15 +511,10 @@ Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
   switch (set) {
   case GPRegSet:
   case FPRegSet:
-  case DBRegSet: {
-    void *data = GetOffsetRegSetData(set, reg_info->byte_offset);
-    FXSAVE *fpr = reinterpret_cast<FXSAVE *>(m_fpr.data());
-    if (data == &fpr->ftag) // ftag
-      fpr->ftag = FullToAbridgedTagWord(reg_value.GetAsUInt16());
-    else
-      ::memcpy(data, reg_value.GetBytes(), reg_value.GetByteSize());
+  case DBRegSet:
+    ::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
+             reg_value.GetBytes(), reg_value.GetByteSize());
     break;
-  }
   case YMMRegSet: {
     llvm::Optional<YMMSplitPtr> ymm_reg = GetYMMSplitReg(reg);
     if (!ymm_reg) {

diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
index 6462441249c0..20cd5e3f62ff 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -530,13 +530,6 @@ NativeRegisterContextLinux_x86_64::ReadRegister(const RegisterInfo *reg_info,
   assert((reg_info->byte_offset - m_fctrl_offset_in_userarea) < sizeof(FPR));
   uint8_t *src = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
                  m_fctrl_offset_in_userarea;
-
-  if (src == reinterpret_cast<uint8_t *>(&m_xstate->fxsave.ftag)) {
-    reg_value.SetUInt16(AbridgedToFullTagWord(
-        m_xstate->fxsave.ftag, m_xstate->fxsave.fstat, m_xstate->fxsave.stmm));
-    return error;
-  }
-
   switch (reg_info->byte_size) {
   case 1:
     reg_value.SetUInt8(*(uint8_t *)src);
@@ -646,28 +639,23 @@ Status NativeRegisterContextLinux_x86_64::WriteRegister(
              sizeof(FPR));
       uint8_t *dst = (uint8_t *)m_xstate.get() + reg_info->byte_offset -
                      m_fctrl_offset_in_userarea;
-
-      if (dst == reinterpret_cast<uint8_t *>(&m_xstate->fxsave.ftag))
-        m_xstate->fxsave.ftag = FullToAbridgedTagWord(reg_value.GetAsUInt16());
-      else {
-        switch (reg_info->byte_size) {
-        case 1:
-          *(uint8_t *)dst = reg_value.GetAsUInt8();
-          break;
-        case 2:
-          *(uint16_t *)dst = reg_value.GetAsUInt16();
-          break;
-        case 4:
-          *(uint32_t *)dst = reg_value.GetAsUInt32();
-          break;
-        case 8:
-          *(uint64_t *)dst = reg_value.GetAsUInt64();
-          break;
-        default:
-          assert(false && "Unhandled data size.");
-          return Status("unhandled register data size %" PRIu32,
-                        reg_info->byte_size);
-        }
+      switch (reg_info->byte_size) {
+      case 1:
+        *(uint8_t *)dst = reg_value.GetAsUInt8();
+        break;
+      case 2:
+        *(uint16_t *)dst = reg_value.GetAsUInt16();
+        break;
+      case 4:
+        *(uint32_t *)dst = reg_value.GetAsUInt32();
+        break;
+      case 8:
+        *(uint64_t *)dst = reg_value.GetAsUInt64();
+        break;
+      default:
+        assert(false && "Unhandled data size.");
+        return Status("unhandled register data size %" PRIu32,
+                      reg_info->byte_size);
       }
     }
 

diff  --git a/lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp b/lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
index 038dc125c853..917693884681 100644
--- a/lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ b/lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -607,13 +607,9 @@ NativeRegisterContextNetBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
   case lldb_fstat_x86_64:
     reg_value = (uint16_t)m_xstate.xs_fxsave.fx_sw;
     break;
-  case lldb_ftag_x86_64: {
-    llvm::ArrayRef<MMSReg> st_regs{
-        reinterpret_cast<MMSReg *>(m_xstate.xs_fxsave.fx_87_ac), 8};
-    reg_value = (uint16_t)AbridgedToFullTagWord(
-        m_xstate.xs_fxsave.fx_tw, m_xstate.xs_fxsave.fx_sw, st_regs);
+  case lldb_ftag_x86_64:
+    reg_value = (uint16_t)m_xstate.xs_fxsave.fx_tw;
     break;
-  }
   case lldb_fop_x86_64:
     reg_value = (uint64_t)m_xstate.xs_fxsave.fx_opcode;
     break;
@@ -909,7 +905,7 @@ Status NativeRegisterContextNetBSD_x86_64::WriteRegister(
     m_xstate.xs_fxsave.fx_sw = reg_value.GetAsUInt16();
     break;
   case lldb_ftag_x86_64:
-    m_xstate.xs_fxsave.fx_tw = FullToAbridgedTagWord(reg_value.GetAsUInt16());
+    m_xstate.xs_fxsave.fx_tw = reg_value.GetAsUInt16();
     break;
   case lldb_fop_x86_64:
     m_xstate.xs_fxsave.fx_opcode = reg_value.GetAsUInt16();

diff  --git a/lldb/source/Plugins/Process/Utility/CMakeLists.txt b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
index 9965d89f4c4b..2fcbe977dcc1 100644
--- a/lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -12,7 +12,6 @@ add_lldb_library(lldbPluginProcessUtility
   NativeRegisterContextRegisterInfo.cpp
   NativeRegisterContextWatchpoint_x86.cpp
   NetBSDSignals.cpp
-  RegisterContext_x86.cpp
   RegisterContextDarwin_arm.cpp
   RegisterContextDarwin_arm64.cpp
   RegisterContextDarwin_i386.cpp

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp b/lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
deleted file mode 100644
index b21c72bd9621..000000000000
--- a/lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-//===-- RegisterContext_x86.cpp ---------------------------------*- 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "RegisterContext_x86.h"
-
-using namespace lldb_private;
-
-// Convert the 8-bit abridged FPU Tag Word (as found in FXSAVE) to the full
-// 16-bit FPU Tag Word (as found in FSAVE, and used by gdb protocol).  This
-// requires knowing the values of the ST(i) registers and the FPU Status Word.
-uint16_t lldb_private::AbridgedToFullTagWord(uint8_t abridged_tw, uint16_t sw,
-                                             llvm::ArrayRef<MMSReg> st_regs) {
-  // Tag word is using internal FPU register numbering rather than ST(i).
-  // Mapping to ST(i): i = FPU regno - TOP (Status Word, bits 11:13).
-  // Here we start with FPU reg 7 and go down.
-  int st = 7 - ((sw >> 11) & 7);
-  uint16_t tw = 0;
-  for (uint8_t mask = 0x80; mask != 0; mask >>= 1) {
-    tw <<= 2;
-    if (abridged_tw & mask) {
-      // The register is non-empty, so we need to check the value of ST(i).
-      uint16_t exp =
-          st_regs[st].comp.sign_exp & 0x7fff; // Discard the sign bit.
-      if (exp == 0) {
-        if (st_regs[st].comp.mantissa == 0)
-          tw |= 1; // Zero
-        else
-          tw |= 2; // Denormal
-      } else if (exp == 0x7fff)
-        tw |= 2; // Infinity or NaN
-      // 0 if normal number
-    } else
-      tw |= 3; // Empty register
-
-    // Rotate ST down.
-    st = (st - 1) & 7;
-  }
-
-  return tw;
-}
-
-// Convert the 16-bit FPU Tag Word to the abridged 8-bit value, to be written
-// into FXSAVE.
-uint8_t lldb_private::FullToAbridgedTagWord(uint16_t tw) {
-  uint8_t abridged_tw = 0;
-  for (uint16_t mask = 0xc000; mask != 0; mask >>= 2) {
-    abridged_tw <<= 1;
-    // full TW uses 11 for empty registers, aTW uses 0
-    if ((tw & mask) != mask)
-      abridged_tw |= 1;
-  }
-  return abridged_tw;
-}

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h b/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
index 76e004ce0ceb..27a1bad4d53f 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
@@ -12,7 +12,6 @@
 #include <cstddef>
 #include <cstdint>
 
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/Support/Compiler.h"
 
@@ -240,23 +239,10 @@ enum {
 
 // Generic floating-point registers
 
-LLVM_PACKED_START
-struct MMSRegComp {
-  uint64_t mantissa;
-  uint16_t sign_exp;
-};
-
 struct MMSReg {
-  union {
-    uint8_t bytes[10];
-    MMSRegComp comp;
-  };
+  uint8_t bytes[10];
   uint8_t pad[6];
 };
-LLVM_PACKED_END
-
-static_assert(sizeof(MMSRegComp) == 10, "MMSRegComp is not 10 bytes of size");
-static_assert(sizeof(MMSReg) == 16, "MMSReg is not 16 bytes of size");
 
 struct XMMReg {
   uint8_t bytes[16]; // 128-bits for each XMM register
@@ -383,10 +369,6 @@ inline void YMMToXState(const YMMReg& input, void* xmm_bytes, void* ymmh_bytes)
   ::memcpy(ymmh_bytes, input.bytes + sizeof(XMMReg), sizeof(YMMHReg));
 }
 
-uint16_t AbridgedToFullTagWord(uint8_t abridged_tw, uint16_t sw,
-                               llvm::ArrayRef<MMSReg> st_regs);
-uint8_t FullToAbridgedTagWord(uint16_t tw);
-
 } // namespace lldb_private
 
 #endif

diff  --git a/lldb/test/Shell/Register/x86-64-fp-read.test b/lldb/test/Shell/Register/x86-64-fp-read.test
index 8c1ffc586e4a..2be85a6c3b35 100644
--- a/lldb/test/Shell/Register/x86-64-fp-read.test
+++ b/lldb/test/Shell/Register/x86-64-fp-read.test
@@ -15,7 +15,9 @@ print &zero
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# CHECK-DAG: ftag = 0xea58
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted
+# CHECK-DAG: ftag = 0x007f
 # CHECK-DAG: fop = 0x0033
 # CHECK-DAG: fip = [[FDIV]]
 # CHECK-DAG: fdp = [[ZERO]]

diff  --git a/lldb/test/Shell/Register/x86-64-fp-write.test b/lldb/test/Shell/Register/x86-64-fp-write.test
index f6021b247037..ad7cfe7a7ee4 100644
--- a/lldb/test/Shell/Register/x86-64-fp-write.test
+++ b/lldb/test/Shell/Register/x86-64-fp-write.test
@@ -8,7 +8,8 @@ process launch
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-register write ftag 0x2a58
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: fxrstor64 apparently truncates this to 48 bits, and sign extends

diff  --git a/lldb/test/Shell/Register/x86-fp-read.test b/lldb/test/Shell/Register/x86-fp-read.test
index aad00ac4e68c..3c676c1d2118 100644
--- a/lldb/test/Shell/Register/x86-fp-read.test
+++ b/lldb/test/Shell/Register/x86-fp-read.test
@@ -14,7 +14,9 @@ print &zero
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# CHECK-DAG: ftag = 0xea58
+# TODO: the following value is incorrect, it's a bug in the way
+# FXSAVE/XSAVE is interpreted
+# CHECK-DAG: ftag = 0x007f
 # CHECK-DAG: fop = 0x0033
 # CHECK-DAG: fioff = [[FDIV]]
 # CHECK-DAG: fooff = [[ZERO]]

diff  --git a/lldb/test/Shell/Register/x86-fp-write.test b/lldb/test/Shell/Register/x86-fp-write.test
index 93c5dfbae77d..38ffe16bab02 100644
--- a/lldb/test/Shell/Register/x86-fp-write.test
+++ b/lldb/test/Shell/Register/x86-fp-write.test
@@ -7,7 +7,8 @@ process launch
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-register write ftag 0x2a58
+# TODO: fix it to use proper ftag values instead of 'abridged'
+register write ftag 0x00ff
 register write fop 0x0033
 # the exact addresses do not matter, we want just to verify FXSAVE
 # note: segment registers are not supported on all CPUs

diff  --git a/lldb/unittests/Process/Utility/CMakeLists.txt b/lldb/unittests/Process/Utility/CMakeLists.txt
index ae79478d746f..0041a94a79a3 100644
--- a/lldb/unittests/Process/Utility/CMakeLists.txt
+++ b/lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,5 +1,4 @@
 add_lldb_unittest(ProcessUtilityTests
-  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS

diff  --git a/lldb/unittests/Process/Utility/RegisterContextTest.cpp b/lldb/unittests/Process/Utility/RegisterContextTest.cpp
deleted file mode 100644
index f0aa6076d8f2..000000000000
--- a/lldb/unittests/Process/Utility/RegisterContextTest.cpp
+++ /dev/null
@@ -1,73 +0,0 @@
-//===-- RegisterContextTest.cpp -------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "gtest/gtest.h"
-
-#include "Plugins/Process/Utility/RegisterContext_x86.h"
-
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/Support/FormatVariadic.h"
-
-#include <array>
-
-using namespace lldb_private;
-
-struct TagWordTestVector {
-  uint16_t sw;
-  uint16_t tw;
-  uint8_t tw_abridged;
-  int st_reg_num;
-};
-
-constexpr MMSReg st_from_comp(uint64_t mantissa, uint16_t sign_exp) {
-  MMSReg ret = {};
-  ret.comp.mantissa = mantissa;
-  ret.comp.sign_exp = sign_exp;
-  return ret;
-}
-
-const std::array<MMSReg, 8> st_regs = {
-    st_from_comp(0x8000000000000000, 0x4000), // +2.0
-    st_from_comp(0x3f00000000000000, 0x0000), // 1.654785e-4932
-    st_from_comp(0x0000000000000000, 0x0000), // +0
-    st_from_comp(0x0000000000000000, 0x8000), // -0
-    st_from_comp(0x8000000000000000, 0x7fff), // +inf
-    st_from_comp(0x8000000000000000, 0xffff), // -inf
-    st_from_comp(0xc000000000000000, 0xffff), // nan
-    st_from_comp(0x8000000000000000, 0xc000), // -2.0
-};
-
-const std::array<TagWordTestVector, 8> tag_word_test_vectors{
-    TagWordTestVector{0x3800, 0x3fff, 0x80, 1},
-    TagWordTestVector{0x3000, 0x2fff, 0xc0, 2},
-    TagWordTestVector{0x2800, 0x27ff, 0xe0, 3},
-    TagWordTestVector{0x2000, 0x25ff, 0xf0, 4},
-    TagWordTestVector{0x1800, 0x25bf, 0xf8, 5},
-    TagWordTestVector{0x1000, 0x25af, 0xfc, 6},
-    TagWordTestVector{0x0800, 0x25ab, 0xfe, 7},
-    TagWordTestVector{0x0000, 0x25a8, 0xff, 8},
-};
-
-TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
-  for (const auto &x : llvm::enumerate(tag_word_test_vectors)) {
-    SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
-    std::array<MMSReg, 8> test_regs;
-    for (int i = 0; i < x.value().st_reg_num; ++i)
-      test_regs[i] = st_regs[x.value().st_reg_num - i - 1];
-    EXPECT_EQ(
-        AbridgedToFullTagWord(x.value().tw_abridged, x.value().sw, test_regs),
-        x.value().tw);
-  }
-}
-
-TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
-  for (const auto &x : llvm::enumerate(tag_word_test_vectors)) {
-    SCOPED_TRACE(llvm::formatv("tag_word_test_vectors[{0}]", x.index()));
-    EXPECT_EQ(FullToAbridgedTagWord(x.value().tw), x.value().tw_abridged);
-  }
-}


        


More information about the lldb-commits mailing list