From 193a9b374e24d31b30095f2789f1994bc0c7b663 Mon Sep 17 00:00:00 2001 From: Mikhail Goncharov Date: Thu, 19 Nov 2020 15:23:25 +0100 Subject: [PATCH] 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 --- .../NativeRegisterContextFreeBSD_x86_64.cpp | 23 ++----- .../Linux/NativeRegisterContextLinux_x86_64.cpp | 46 +++++--------- .../NetBSD/NativeRegisterContextNetBSD_x86_64.cpp | 10 +-- lldb/source/Plugins/Process/Utility/CMakeLists.txt | 1 - .../Process/Utility/RegisterContext_x86.cpp | 58 ----------------- .../Plugins/Process/Utility/RegisterContext_x86.h | 20 +----- lldb/test/Shell/Register/x86-64-fp-read.test | 4 +- lldb/test/Shell/Register/x86-64-fp-write.test | 3 +- lldb/test/Shell/Register/x86-fp-read.test | 4 +- lldb/test/Shell/Register/x86-fp-write.test | 3 +- lldb/unittests/Process/Utility/CMakeLists.txt | 1 - .../Process/Utility/RegisterContextTest.cpp | 73 ---------------------- 12 files changed, 37 insertions(+), 209 deletions(-) delete mode 100644 lldb/source/Plugins/Process/Utility/RegisterContext_x86.cpp delete mode 100644 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 ea5400c..ea2494d 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(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 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(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 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 6462441..20cd5e3 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(&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(&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 038dc12..9176938 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 st_regs{ - reinterpret_cast(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 9965d89..2fcbe97 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 b21c72b..0000000 --- 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 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 76e004c..27a1bad 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h +++ b/lldb/source/Plugins/Process/Utility/RegisterContext_x86.h @@ -12,7 +12,6 @@ #include #include -#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 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 8c1ffc5..2be85a6 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 f6021b2..ad7cfe7 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 aad00ac..3c676c1 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 93c5dfb..38ffe16 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 ae79478..0041a94 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 f0aa607..0000000 --- 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 - -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 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 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 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); - } -} -- 2.7.4