[Lldb-commits] [PATCH] D91268: [lldb] [Process/FreeBSDRemote] Access debug registers via offsets

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 305364.
mgorny added a comment.

Fix `native_i386_dbregs` on i386.


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

https://reviews.llvm.org/D91268

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -37,6 +37,8 @@
   EXPECT_THAT( \
   GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
   ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+#define EXPECT_DBR_X86_64(num) \
+  EXPECT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, x86_64) {
   ArchSpec arch{"x86_64-unknown-freebsd"};
@@ -117,6 +119,16 @@
   EXPECT_OFF(xmm13_x86_64, 0x170, 16);
   EXPECT_OFF(xmm14_x86_64, 0x180, 16);
   EXPECT_OFF(xmm15_x86_64, 0x190, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_x86_64].byte_offset;
+  EXPECT_DBR_X86_64(0);
+  EXPECT_DBR_X86_64(1);
+  EXPECT_DBR_X86_64(2);
+  EXPECT_DBR_X86_64(3);
+  EXPECT_DBR_X86_64(4);
+  EXPECT_DBR_X86_64(5);
+  EXPECT_DBR_X86_64(6);
+  EXPECT_DBR_X86_64(7);
 }
 #endif
 
@@ -126,6 +138,9 @@
   EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),\
   ::testing::Pair(offsetof(native_i386_regs, r_##regname), \
   sizeof(native_i386_regs::r_##regname)))
+#define EXPECT_DBR_I386(num)   \
+  EXPECT_OFF(dr##num##_i386, offsetof(native_i386_dbregs, dr[num]),\
+ sizeof(native_i386_dbregs::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, i386) {
   ArchSpec arch{"i686-unknown-freebsd"};
@@ -133,8 +148,10 @@
 
 #if defined(__i386__)
   using native_i386_regs = ::reg;
+  using native_i386_dbregs = ::dbreg;
 #else
   using native_i386_regs = ::reg32;
+  using native_i386_dbregs = ::dbreg32;
 #endif
 
   EXPECT_GPR_I386(fs);
@@ -196,5 +213,15 @@
   EXPECT_OFF(xmm5_i386, 0xF0, 16);
   EXPECT_OFF(xmm6_i386, 0x100, 16);
   EXPECT_OFF(xmm7_i386, 0x110, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_i386].byte_offset;
+  EXPECT_DBR_I386(0);
+  EXPECT_DBR_I386(1);
+  EXPECT_DBR_I386(2);
+  EXPECT_DBR_I386(3);
+  EXPECT_DBR_I386(4);
+  EXPECT_DBR_I386(5);
+  EXPECT_DBR_I386(6);
+  EXPECT_DBR_I386(7);
 }
 #endif
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
===
--- lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h
@@ -40,7 +40,6 @@
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
-private:
   const RegisterInfo *GetDR(int num) const;
 };
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -71,17 +71,17 @@
   // Private member variables.
   std::array m_gpr;
   std::array m_fpr; // FXSAVE
-  struct dbreg m_dbr;
+  std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
-  int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
   size_t GetFPROffset() const;
+  size_t GetDBROffset() const;
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
@@ -247,8 +247,7 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)),
-  m_dbr() {
+  native_thread, CreateRegisterInfoInterface(target_arch)) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
 }
 
@@ -296,15 +295,6 @@
 return lldb_bndcfgu_x86_64;

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: pengfei.
mgorny requested review of this revision.

The FXSAVE/XSAVE data can have two different layouts on x86_64.  When
called as FXSAVE/XSAVE..., the Instruction Pointer and Address Pointer
registers are reported using a 16-bit segment identifier and a 32-bit
offset.  When called as FXSAVE64/XSAVE64..., they are reported using
a complete 64-bit offsets instead.

LLDB has historically followed GDB and unconditionally used to assume
the 32-bit layout, with the slight modification of possibly
using a 32-bit segment register (i.e. extending the register into
the reserved 16 upper bits).  When the underlying operating system used
FXSAVE64/XSAVE64..., the pointer was split into two halves,
with the upper half repored as the segment registers.  While
reconstructing the full address was possible on the user end (and e.g.
the FPU register tests did that), it certainly was not the most
convenient option.

Introduce a two additional 'fip' and 'fdp' registers that overlap
with 'fiseg'/'fioff' and 'foseg'/'foff' respectively, and report
the complete 64-bit address.


https://reviews.llvm.org/D91497

Files:
  
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/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
  lldb/test/Shell/Register/x86-64-fp-read.test
  lldb/test/Shell/Register/x86-fp-read.test
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -83,8 +83,10 @@
   // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
   EXPECT_OFF(fioff_x86_64, 0x08, 4);
   EXPECT_OFF(fiseg_x86_64, 0x0C, 4);
+  EXPECT_OFF(fip_x86_64, 0x08, 8);
   EXPECT_OFF(fooff_x86_64, 0x10, 4);
   EXPECT_OFF(foseg_x86_64, 0x14, 4);
+  EXPECT_OFF(fdp_x86_64, 0x10, 8);
   EXPECT_OFF(mxcsr_x86_64, 0x18, 4);
   EXPECT_OFF(mxcsrmask_x86_64, 0x1C, 4);
   EXPECT_OFF(st0_x86_64, 0x20, 10);
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -1,5 +1,5 @@
 # XFAIL: system-windows
-# REQUIRES: native && (target-x86 || target-x86_64)
+# REQUIRES: native && target-x86
 # RUN: %clangxx_host -g %p/Inputs/x86-fp-read.cpp -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch
Index: lldb/test/Shell/Register/x86-64-fp-read.test
===
--- /dev/null
+++ lldb/test/Shell/Register/x86-64-fp-read.test
@@ -0,0 +1,38 @@
+# XFAIL: system-windows
+# REQUIRES: native && target-x86_64
+# RUN: %clangxx_host -g %p/Inputs/x86-fp-read.cpp -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch
+# CHECK: Process {{.*}} stopped
+
+# fdiv (%rbx) gets encoded into 2 bytes, int3 into 1 byte
+print (void*)($pc-3)
+# CHECK: (void *) $0 = [[FDIV:0x[0-9a-f]*]]
+print (void*)($fiseg*0x1 + $fioff)
+# CHECK: (void *) $1 = [[FDIV]]
+print &zero
+# CHECK: (uint32_t *) $2 = [[ZERO:0x[0-9a-f]*]]
+print (uint32_t*)($foseg * 0x1 + $fooff)
+# CHECK: (uint32_t *) $3 = [[ZERO]]
+
+register read --all
+# CHECK-DAG: fctrl = 0x037b
+# CHECK-DAG: fstat = 0x8084
+# 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]]
+
+# CHECK-DAG: st{{(mm)?}}0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
+# CHECK-DAG: st{{(mm)?}}1 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x3f 0x00 0x00}
+# CHECK-DAG: st{{(mm)?}}2 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+# CHECK-DAG: st{{(mm)?}}3 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80}
+# CHECK-DAG: st{{(mm)?}}4 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0x7f}
+# CHECK-DAG: st{{(mm)?}}5 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}
+# CHECK-DAG: st{{(mm)?}}6 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}
+# CHECK-DAG: st{{(mm)?}}7 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
+
+process continue
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-x86-registe

[Lldb-commits] [PATCH] D91497: [lldb] Add explicit 64-bit fip/fdp registers on x86_64

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

Tested on Linux, FreeBSD and NetBSD. I've presumed we want fip/fdp only on 
amd64.




Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:84
 lldb_fctrl_x86_64, lldb_fstat_x86_64, lldb_ftag_x86_64,
-lldb_fop_x86_64,   lldb_fiseg_x86_64, lldb_fioff_x86_64,
-lldb_foseg_x86_64, lldb_fooff_x86_64, lldb_mxcsr_x86_64,
+lldb_fop_x86_64,   lldb_fiseg_x86_64, lldb_fioff_x86_64, 
lldb_fip_x86_64,
+lldb_foseg_x86_64, lldb_fooff_x86_64, lldb_fdp_x86_64, 
lldb_mxcsr_x86_64,

Note to self: remember to `clang-format` before committing.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h:99
lldb_mm##i##_x86_64 },  
\
nullptr, nullptr, nullptr, 0
\
   }

@labath, also, shouldn't we have overlaps set between ST and MM registers?



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h:262
 DEFINE_FPR(fioff, ptr.i386_.fioff, LLDB_INVALID_REGNUM, 
LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),
+DEFINE_FPR(fip,   ptr.x86_64.fip,  LLDB_INVALID_REGNUM, 
LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),
 DEFINE_FPR(foseg, ptr.i386_.foseg, LLDB_INVALID_REGNUM, 
LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM),

@labath, do you want me to set overlaps here like rax/eax... does? If yes, any 
suggestion on the style?


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

https://reviews.llvm.org/D91497

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91216: [lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 305371.
mgorny added a comment.

Create the unit test unconditionally, guard the code with `__FreeBSD__` define.


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

https://reviews.llvm.org/D91216

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/unittests/Process/CMakeLists.txt
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -0,0 +1,108 @@
+//===-- RegisterContextFreeBSDTests.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
+//
+//===--===//
+
+#if defined(__FreeBSD__)
+
+// clang-format off
+#include 
+#include 
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::pair GetRegParams(RegisterInfoInterface &ctx,
+   uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#if defined(__x86_64__)
+
+#define EXPECT_GPR_X86_64(regname) \
+  EXPECT_THAT( \
+  GetRegParams(reg_ctx, lldb_##regname##_x86_64),  \
+  ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  EXPECT_GPR_X86_64(r15);
+  EXPECT_GPR_X86_64(r14);
+  EXPECT_GPR_X86_64(r13);
+  EXPECT_GPR_X86_64(r12);
+  EXPECT_GPR_X86_64(r11);
+  EXPECT_GPR_X86_64(r10);
+  EXPECT_GPR_X86_64(r9);
+  EXPECT_GPR_X86_64(r8);
+  EXPECT_GPR_X86_64(rdi);
+  EXPECT_GPR_X86_64(rsi);
+  EXPECT_GPR_X86_64(rbp);
+  EXPECT_GPR_X86_64(rbx);
+  EXPECT_GPR_X86_64(rdx);
+  EXPECT_GPR_X86_64(rcx);
+  EXPECT_GPR_X86_64(rax);
+  EXPECT_GPR_X86_64(fs);
+  EXPECT_GPR_X86_64(gs);
+  EXPECT_GPR_X86_64(es);
+  EXPECT_GPR_X86_64(ds);
+  EXPECT_GPR_X86_64(rip);
+  EXPECT_GPR_X86_64(cs);
+  EXPECT_GPR_X86_64(rflags);
+  EXPECT_GPR_X86_64(rsp);
+  EXPECT_GPR_X86_64(ss);
+}
+
+#endif // defined(__x86_64__)
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#define EXPECT_GPR_I386(regname)   \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),\
+  ::testing::Pair(offsetof(native_i386_regs, r_##regname), \
+  sizeof(native_i386_regs::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+#if defined(__i386__)
+  using native_i386_regs = ::reg;
+#else
+  using native_i386_regs = ::reg32;
+#endif
+
+  EXPECT_GPR_I386(fs);
+  EXPECT_GPR_I386(es);
+  EXPECT_GPR_I386(ds);
+  EXPECT_GPR_I386(edi);
+  EXPECT_GPR_I386(esi);
+  EXPECT_GPR_I386(ebp);
+  EXPECT_GPR_I386(ebx);
+  EXPECT_GPR_I386(edx);
+  EXPECT_GPR_I386(ecx);
+  EXPECT_GPR_I386(eax);
+  EXPECT_GPR_I386(eip);
+  EXPECT_GPR_I386(cs);
+  EXPECT_GPR_I386(eflags);
+  EXPECT_GPR_I386(esp);
+  EXPECT_GPR_I386(ss);
+  EXPECT_GPR_I386(gs);
+}
+
+#endif // defined(__i386__) || defined(__x86_64__)
+
+#endif // defined(__FreeBSD__)
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -0,0 +1,5 @@
+add_lldb_unittest(ProcessUtilityTests
+  RegisterContextFreeBSDTest.cpp
+
+  LINK_LIBS
+lldbPluginProcessUtility)
Index: lldb/unittests/Process/CMakeLists.txt
===
--- lldb/unittests/Process/CMakeLists.txt
+++ lldb/unittests/Process/CMakeLists.txt
@@ -3,6 +3,7 @@
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 endif()
+add_subdirectory(Utility)
 add_subdirectory(minidump)
 
 add_lldb_unittest(ProcessEventDataTests
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFr

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

Translate between abridged and full ftag values in order to expose
the latter in the gdb-remote protocol while the former are used by
FXSAVE/XSAVE...  This matches the gdb behavior.


https://reviews.llvm.org/D91504

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  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
  lldb/unittests/Process/Utility/RegisterContextTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/RegisterContextTest.cpp
@@ -0,0 +1,65 @@
+//===-- 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 
+
+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;
+}
+
+std::array st_regs = {
+	st_from_comp(0x8000, 0x4000), // +2.0
+	st_from_comp(0x3f00, 0x), // 1.654785e-4932
+	st_from_comp(0x, 0x), // +0
+	st_from_comp(0x, 0x8000), // -0
+	st_from_comp(0x8000, 0x7fff), // +inf
+	st_from_comp(0x8000, 0x), // -inf
+	st_from_comp(0xc000, 0x), // nan
+	st_from_comp(0x8000, 0xc000), // -2.0
+};
+
+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{0x, 0x25a8, 0xff, 8},
+};
+
+TEST(RegisterContext_x86Test, AbridgedToFullTagWord) {
+  for (TagWordTestVector &x : tag_word_test_vectors) {
+std::array test_regs;
+for (int i = 0; i < x.st_reg_num; ++i)
+  test_regs[i] = st_regs[x.st_reg_num - i - 1];
+EXPECT_EQ(AbridgedToFullTagWord(x.tw_abridged, x.sw, test_regs.data()), x.tw);
+  }
+}
+
+TEST(RegisterContext_x86Test, FullToAbridgedTagWord) {
+  for (TagWordTestVector &x : tag_word_test_vectors)
+EXPECT_EQ(FullToAbridgedTagWord(x.tw), x.tw_abridged);
+}
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(ProcessUtilityTests
+  RegisterContextTest.cpp
   RegisterContextFreeBSDTest.cpp
 
   LINK_LIBS
Index: lldb/test/Shell/Register/x86-fp-write.test
===
--- lldb/test/Shell/Register/x86-fp-write.test
+++ lldb/test/Shell/Register/x86-fp-write.test
@@ -7,8 +7,7 @@
 register write fctrl 0x037b
 register write fstat 0x8884
 # note: this needs to enable all registers for writes to be effective
-# TODO: fix it to use proper ftag values instead of 'abridged'
-register write ftag 0x00ff
+register write ftag 0x2a58
 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
Index: lldb/test/Shell/Register/x86-fp-read.test
===
--- lldb/test/Shell/Register/x86-fp-read.test
+++ lldb/test/Shell/Register/x86-fp-read.test
@@ -8,9 +8,7 @@
 register read --all
 # CHECK-DAG: fctrl = 0x037b
 # CHECK-DAG: fstat = 0x8084
-# TODO: the following value is incorrect, it's a bug in the way
-# FXSAVE/XSAVE is interpreted
-# CHECK-DAG: ftag = 0x007f
+# CHECK-DAG: ftag = 0xea58
 # CHECK-DAG: fop = 0x0033
 
 # CHECK-DAG: st{{(mm)?}}0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0x00 0x40}
Index: lldb/test/Shell/Register/x86-64-fp-write.test
===
--- lldb/test/Shell/Register/x86-64-fp-write.test
+++ lldb/test/Shell/Register/x86-64-fp-write.test
@@ -8,8 +8,7 @@
 register write fctrl 0x037b
 register write fstat 0

[Lldb-commits] [PATCH] D91504: [lldb] Use translated full ftag values [WIP]

2020-11-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

I'll port to other platforms once we agree on the approach.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:534
+  // TODO
+  if (reg_info->kinds[lldb::eRegisterKindLLDB] == lldb_ftag_x86_64) {
+uint8_t abridged_tw = *(uint8_t *)src;

@labath, any suggestion what kind of test to use here? Maybe against 
`&m_xstate->fxsave.ftag` address?


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

https://reviews.llvm.org/D91504

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-15 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
tammela requested review of this revision.
Herald added a subscriber: JDevlieghere.

These callbacks are set using the following:

  breakpoint command add -s lua -o "print('hello world!')"

The user supplied script is executed as:

  function (frame, bp_loc, ...)
 
  end

So the local variables 'frame', 'bp_loc' and vararg are all accessible.
Any global variables declared will persist in the Lua interpreter.
A user should never hold 'frame' and 'bp_loc' in a global variable as
these userdatas are context dependent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91508

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/bindings/lua/lua.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
  lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -43,6 +43,13 @@
 };
 } // namespace
 
+struct lua_State;
+extern "C" bool LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, void *baton, const lldb::StackFrameSP &sb_frame,
+const lldb::BreakpointLocationSP &sb_bp_loc) {
+  return false;
+}
+
 TEST_F(ScriptInterpreterTest, Plugin) {
   EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
   EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -26,3 +26,27 @@
   EXPECT_EQ(llvm::toString(std::move(error)),
 "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
 }
+
+TEST(LuaTest, RunCallbackValid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "foo = 1") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunCallbackInvalid) {
+  Lua lua;
+  LuaCallback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "nil = foo") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"nil = foo\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,5 @@
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o 'print(1)'
+run
+# CHECK: 1
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_h_
 
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-enumerations.h"
 
 namespace lldb_private {
 class Lua;
 class ScriptInterpreterLua : public ScriptInterpreter {
 public:
+  class CommandDataLua : public BreakpointOptions::CommandData {
+  public:
+CommandDataLua() : BreakpointOptions::CommandData() {
+  interpreter = lldb::eScriptLanguageLua;
+}
+  };
+
   ScriptInterpreterLua(Debugger &debugger);
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_sessio

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-15 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela added a comment.

@JDevlieghere

When writing this patch I noticed that there is no mechanism in-place to remove 
the Python/Lua function when the breakpoint is removed or when the callback 
function is replaced.
The class that sets the callback provides `ClearCallback()` but it never calls 
into the `ScriptInterpreter` to clean up.
Although the real world impact is not that big, it's crucial for a small memory 
footprint. Any thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

In D91241#2391245 , @labath wrote:

> In D91241#2391156 , @omjavaid wrote:
>
>> I guess GDB standard does enforce ascending order, here is what it says 
>> about regnum:
>>
>> "The register’s number. If omitted, a register’s number is one greater than 
>> that of the previous register (either in the current feature or in a 
>> preceding feature); the first register in the target description defaults to 
>> zero. This register number is used to read or write the register; e.g. it is 
>> used in the remote p and P packets, and registers appear in the g and G 
>> packets in order of increasing register number."
>>
>> https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html
>
> I've just read that paragraph before writing that, but that's not how I would 
> interpret it. What I think that says is:
>
> - in the `g` packet, registers appear in the increasing register number 
> order. (I think we agree on that part)
> - **if the register number is omitted**, the registers are assigned 
> increasing register numbers. Here, I think the bold part is very important, 
> as it means the rest of the sentence describes default/fallback behavior. If 
> the stub specifies the register number explicitly, then I'd say it's free to 
> send order the register descriptions any way it likes. It can even combine 
> things and set explicit numbers for some  register, but not others...

I gave thought and looked at GDB handling of g/G packet offsets. 
Important points to note:

1. GDB assigns register numbers if they are not provided, with scheme last 
regnum + 1.
2. GDB maintains registers list in order of their placement in XML regardless 
of any specific register numbers.
3. GDB sends g/G packet based on register numbers in order of increasing 
register numbers. For this gdb creates a sorted list based of register numbers 
and then assigns offsets accordingly.

In LLDB we are trying to support legacy offset field which means offset 
calculation has to be done in location where we parse an XML register entry 
(ParseRegisters) or qRegisterInfo packet 
(ProcessGDBRemote::BuildDynamicRegisterInfo).

Also both of above functions are just inserting into register's information 
list maintained by DynamicRegisterInfo class so there is not enough leverage 
available to make offset calculation in one single location without giving up 
offset field altogether. We may write a helper function to do offset post 
processing for pseudo registers but that will still be a part of 
DynamicRegisterInfo class and may be called from ParseRegisters and 
ProcessGDBRemote::BuildDynamicRegisterInfo.

IMO, lets keep this current patch as it is unless we decide on giving up offset 
field for all architectures.


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

https://reviews.llvm.org/D91241

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits