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

2020-11-13 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp:26-31
+#define ASSERT_REG(regname, offset, size)  
\
+  {
\
+const RegisterInfo *reg_info = ®_ctx.GetRegisterInfo()[lldb_##regname]; 
\
+EXPECT_EQ(reg_info->byte_offset, offset);  
\
+EXPECT_EQ(reg_info->byte_size, size);  
\
+  }

mgorny wrote:
> labath wrote:
> > ```
> > pair GetRegParams(const RegisterContext &ctx, unsigned 
> > reg) {
> >   const RegisterInfo &info = reg_ctx.GetRegisterInfo()[reg];
> >   return {info.byte_offset, info.byte_size};
> > }
> > ```
> Fun fact: `RegisterContextFreeBSD_*` is not a `RegisterContext&` but a 
> `RegisterInfoInterface&` ;-).
Yeah, tell me about it...



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:1-2
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_lldb_unittest(RegisterContextFreeBSDTests
+RegisterContextFreeBSDTests.cpp

Rename to `ProcessUtilityTests` -- all files in this folder should go into a 
single binary. We have more tests coming in D87442.



Comment at: lldb/unittests/Process/Utility/CMakeLists.txt:3
+  add_lldb_unittest(RegisterContextFreeBSDTests
+RegisterContextFreeBSDTests.cpp
+

It seems we have some files ending in `Tests`, but the prevalent convention is 
just `Test`



Comment at: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp:38
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd12.2"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};

I guess the version is not really relevant for this..


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

https://reviews.llvm.org/D91216

___
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-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp:32
+
+#define ASSERT_GPR_X86_64(regname) 
\
+  EXPECT_THAT( 
\

Also, please rename this to `EXPECT_GRP_X86_64`. `ASSERT_` is for macros that 
terminate test when they fail.


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

https://reviews.llvm.org/D91216

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


[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

2020-11-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:563-569
 ::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
  reg_value.GetByteSize());
 return WriteRegisterSet(set);
   case FPRegSet:
+::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
+ reg_value.GetBytes(), reg_value.GetByteSize());
+return WriteRegisterSet(set);

what if m_gpr and m_fpr were actually a single concatenated buffer? Could we 
then ditch GetFPROffset and just have a single memcpy here?


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

https://reviews.llvm.org/D91248

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-13 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:615
+  if (offset == LLDB_INVALID_XSAVE_OFFSET)
+return false;
+

mgorny wrote:
> labath wrote:
> > When does this return false? When the ymm regs are not available?
> Yes. Note that this change brings us closer to being able to disable whole 
> regsets — I just need to figure out how to do it ;-).
Ok, got it.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:35-38
+struct YMMSplitPtr {
+  void *xmm;
+  void *ymm_hi;
+};

Declare this inside the class (next to the function returning it), since it's 
really a private thing.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:83
   size_t GetDBROffset() const;
+  bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
 };

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > `void *&`
> > Are you sure about this? I feel like the whole `&` deal is quite confusing 
> > (read: shouldn't have happened). You don't know whether the method modifies 
> > the variables passed to it unless you look at the method prototype.
> well.. there certainly are code styles which prohibit non-const reference 
> arguments. However, llvm is not one of them. Also lldb has a issue with 
> compulsive null checks, so I think it's important to communicate the fact the 
> two arguments cannot be null, which reference arguments do.
> 
> Another possibility would be to just make this a real return value (pair *, void*> ?). The downside there is that it's not as self-documenting. Pick 
> your poison...
Ok, that feels a bit over-engineered, but works too...


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

2020-11-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:563-569
 ::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
  reg_value.GetByteSize());
 return WriteRegisterSet(set);
   case FPRegSet:
+::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
+ reg_value.GetBytes(), reg_value.GetByteSize());
+return WriteRegisterSet(set);

labath wrote:
> what if m_gpr and m_fpr were actually a single concatenated buffer? Could we 
> then ditch GetFPROffset and just have a single memcpy here?
We'd need `GetFPROffset()` for ptrace calls then ;-). I've got an another idea 
ready, just need to wait for some free RAM to test it. That said, we might 
reach the point where combining all these diffs is easier than reviewing them 
separately.


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

https://reviews.llvm.org/D91248

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


[Lldb-commits] [lldb] dc848a0 - [lldb][NFC] Fix flaky TestForwardDeclFromStdModule test

2020-11-13 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-13T11:40:51+01:00
New Revision: dc848a0888f0a14c03ff1e7dd7ab109db7a8c065

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

LOG: [lldb][NFC] Fix flaky TestForwardDeclFromStdModule test

"[lldb/DataFormatters] Display null C++ pointers as nullptr" added an assumption
that the member we check for is always a nullptr, but it is actually never
initialized. That causes the test to randomly fail due to the pointer having
some random value that isn't 0.

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector
 
b/lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector
index c2d77aab0711..b128a2b92353 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector
+++ 
b/lldb/test/API/commands/expression/import-std-module/forward_decl_from_module/root/usr/include/c++/v1/vector
@@ -7,7 +7,7 @@ namespace std {
 // Pretend to be a std::vector template we need to instantiate in LLDB
 // when import-std-module is enabled.
 template
-struct vector { class F; F *f; };
+struct vector { class F; F *f = nullptr; };
 // The definition of our forward declared nested class.
 template class vector::F { int x; };
   }



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


[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

2020-11-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:563-569
 ::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
  reg_value.GetByteSize());
 return WriteRegisterSet(set);
   case FPRegSet:
+::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
+ reg_value.GetBytes(), reg_value.GetByteSize());
+return WriteRegisterSet(set);

mgorny wrote:
> labath wrote:
> > what if m_gpr and m_fpr were actually a single concatenated buffer? Could 
> > we then ditch GetFPROffset and just have a single memcpy here?
> We'd need `GetFPROffset()` for ptrace calls then ;-). I've got an another 
> idea ready, just need to wait for some free RAM to test it. That said, we 
> might reach the point where combining all these diffs is easier than 
> reviewing them separately.
> We'd need GetFPROffset() for ptrace calls then ;-)
To get the address of the regset buffer, right?

I'd imagine that could be handled by some function like `GetFPRBuffer()` 
(similar one exists in linux), or even `GetBuffer(regset)`. I'd imagine that 
would be useful for the generic caching infrastructure as well.


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

https://reviews.llvm.org/D91248

___
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-13 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 305086.
mgorny added a comment.

Implemented requested changes.


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,102 @@
+//===-- 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
+//
+//===--===//
+
+// 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
+
+#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
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- /dev/null
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -0,0 +1,8 @@
+if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
+  add_lldb_unittest(ProcessUtilityTests
+RegisterContextFreeBSDTest.cpp
+
+LINK_LIBS
+  lldbPluginProcessUtility
+)
+endif()
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/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
@@ -18,6 +18,8 @@

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

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

Rebased.


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(dbreg32, dr[num]),   \
+ sizeof(dbreg32::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, i386) {
   ArchSpec arch{"i686-unknown-freebsd"};
@@ -196,5 +211,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;
   case lldb_bndstatus_i386:
 return lldb_bndstatus_x86_64;
-  case lldb_dr0_i386:
-  case lldb_dr1_i386:
-  case lldb_dr2_i386:
-  case lldb_dr3_i386:
-  case lldb_dr4_i386:
-  case lldb_dr5_i386:
-  case lldb_dr6_i386:
-  case lldb_dr7_i386:
-return lldb_dr0_x86_64 +

[Lldb-commits] [PATCH] D91248: [lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets

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

Rebased.


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

https://reviews.llvm.org/D91248

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.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
@@ -27,6 +27,10 @@
   return {info.byte_offset, info.byte_size};
 }
 
+#define EXPECT_OFF(regname, offset, size)  \
+  EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),   \
+  ::testing::Pair(offset + base_offset, size))
+
 #if defined(__x86_64__)
 
 #define EXPECT_GPR_X86_64(regname) \
@@ -62,6 +66,57 @@
   EXPECT_GPR_X86_64(rflags);
   EXPECT_GPR_X86_64(rsp);
   EXPECT_GPR_X86_64(ss);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_x86_64].byte_offset;
+
+  // assert against FXSAVE struct
+  EXPECT_OFF(fctrl_x86_64, 0x00, 2);
+  EXPECT_OFF(fstat_x86_64, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  EXPECT_OFF(ftag_x86_64, 0x04, 2);
+  EXPECT_OFF(fop_x86_64, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // 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(fooff_x86_64, 0x10, 4);
+  EXPECT_OFF(foseg_x86_64, 0x14, 4);
+  EXPECT_OFF(mxcsr_x86_64, 0x18, 4);
+  EXPECT_OFF(mxcsrmask_x86_64, 0x1C, 4);
+  EXPECT_OFF(st0_x86_64, 0x20, 10);
+  EXPECT_OFF(st1_x86_64, 0x30, 10);
+  EXPECT_OFF(st2_x86_64, 0x40, 10);
+  EXPECT_OFF(st3_x86_64, 0x50, 10);
+  EXPECT_OFF(st4_x86_64, 0x60, 10);
+  EXPECT_OFF(st5_x86_64, 0x70, 10);
+  EXPECT_OFF(st6_x86_64, 0x80, 10);
+  EXPECT_OFF(st7_x86_64, 0x90, 10);
+  EXPECT_OFF(mm0_x86_64, 0x20, 8);
+  EXPECT_OFF(mm1_x86_64, 0x30, 8);
+  EXPECT_OFF(mm2_x86_64, 0x40, 8);
+  EXPECT_OFF(mm3_x86_64, 0x50, 8);
+  EXPECT_OFF(mm4_x86_64, 0x60, 8);
+  EXPECT_OFF(mm5_x86_64, 0x70, 8);
+  EXPECT_OFF(mm6_x86_64, 0x80, 8);
+  EXPECT_OFF(mm7_x86_64, 0x90, 8);
+  EXPECT_OFF(xmm0_x86_64, 0xA0, 16);
+  EXPECT_OFF(xmm1_x86_64, 0xB0, 16);
+  EXPECT_OFF(xmm2_x86_64, 0xC0, 16);
+  EXPECT_OFF(xmm3_x86_64, 0xD0, 16);
+  EXPECT_OFF(xmm4_x86_64, 0xE0, 16);
+  EXPECT_OFF(xmm5_x86_64, 0xF0, 16);
+  EXPECT_OFF(xmm6_x86_64, 0x100, 16);
+  EXPECT_OFF(xmm7_x86_64, 0x110, 16);
+  EXPECT_OFF(xmm8_x86_64, 0x120, 16);
+  EXPECT_OFF(xmm9_x86_64, 0x130, 16);
+  EXPECT_OFF(xmm10_x86_64, 0x140, 16);
+  EXPECT_OFF(xmm11_x86_64, 0x150, 16);
+  EXPECT_OFF(xmm12_x86_64, 0x160, 16);
+  EXPECT_OFF(xmm13_x86_64, 0x170, 16);
+  EXPECT_OFF(xmm14_x86_64, 0x180, 16);
+  EXPECT_OFF(xmm15_x86_64, 0x190, 16);
 }
 #endif
 
@@ -98,5 +153,48 @@
   EXPECT_GPR_I386(esp);
   EXPECT_GPR_I386(ss);
   EXPECT_GPR_I386(gs);
+
+  // fctrl is the first FPR field, it is used to determine offset of the whole
+  // FPR struct
+  size_t base_offset = reg_ctx.GetRegisterInfo()[lldb_fctrl_i386].byte_offset;
+
+  // assert against FXSAVE struct
+  EXPECT_OFF(fctrl_i386, 0x00, 2);
+  EXPECT_OFF(fstat_i386, 0x02, 2);
+  // TODO: This is a known bug, abridged ftag should is 8 bits in length.
+  EXPECT_OFF(ftag_i386, 0x04, 2);
+  EXPECT_OFF(fop_i386, 0x06, 2);
+  // NB: Technically fiseg/foseg are 16-bit long and the higher 16 bits
+  // are reserved.  However, we use them to access/recombine 64-bit FIP/FDP.
+  EXPECT_OFF(fioff_i386, 0x08, 4);
+  EXPECT_OFF(fiseg_i386, 0x0C, 4);
+  EXPECT_OFF(fooff_i386, 0x10, 4);
+  EXPECT_OFF(foseg_i386, 0x14, 4);
+  EXPECT_OFF(mxcsr_i386, 0x18, 4);
+  EXPECT_OFF(mxcsrmask_i386, 0x1C, 4);
+  EXPECT_OFF(st0_i386, 0x20, 10);
+  EXPECT_OFF(st1_i386, 0x30, 10);
+  EXPECT_OFF(st2_i386, 0x40, 10);
+  EXPECT_OFF(st3_i386, 0x50, 10);
+  EXPECT_OFF(st4_i386, 0x60, 10);
+  EXPECT_OFF(st5_i386, 0x70, 10);
+  EXPECT_OFF(st6_i386, 0x80, 10);
+  EXPECT_OFF(st7_i386, 0x90, 10);
+  EXPECT_OFF(mm0_i386, 0x20, 8);
+  EXPECT_OFF(mm1_i386, 0x30, 8);
+  EXPECT_OFF(mm2_i386, 0x40, 8);
+  EXPECT_OFF(mm3_i386, 0x50, 8);
+  EXPECT_OFF(mm4_i386, 0x60, 8);
+  EXPECT_OFF(mm5_i386, 0x70, 8);
+  EXPECT_OFF(mm6_i386, 0x80, 8);
+  EXPECT_OFF(mm7_i386, 0x90, 8);
+  EXPECT_OFF(xmm0_i386, 0xA0, 16);
+  EXPECT_OFF(xmm1_i386, 0xB0, 16);
+  EXPECT_OFF(xmm2_i386, 0xC0, 16);
+  EXPECT_OFF(xmm3_i386, 0xD0, 16);
+  EXPECT_OFF(xmm4_i386, 0xE0, 16);
+  EXPECT_OFF(xmm5_i386, 0xF0, 16);
+  EXPECT_OFF(xmm6_i386, 0x100, 16);
+  EXPECT_OFF(xmm7_i386, 0x110, 16);
 }
 #endif
Index: l

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Split LLDB-visible regsets into separate AVX and MPX.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -32,6 +32,11 @@
 
 class NativeProcessFreeBSD;
 
+struct YMMSplitPtr {
+  void *xmm;
+  void *ymm_hi;
+};
+
 class NativeRegisterContextFreeBSD_x86_64
 : public NativeRegisterContextFreeBSD,
   public NativeRegisterContextWatchpoint_x86 {
@@ -58,15 +63,13 @@
 private:
   // Private member types.
   enum RegSetKind {
+YMMRegSet,
+MaxXSaveSet = YMMRegSet,
+
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
   };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
-  };
 
   // Private member variables.
   std::array m_gpr;
@@ -82,6 +85,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // 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
@@ -101,23 +101,29 @@
   k_num_fpr_registers_x86_64,
   "g_fpu_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
-lldb_ymm0_x86_64, lldb_ymm1_x86_64, lldb_ymm2_x86_64, lldb_ymm3_x86_64,
-lldb_ymm4_x86_64, lldb_ymm5_x86_64, lldb_ymm6_x86_64, lldb_ymm7_x86_64,
-lldb_ymm8_x86_64, lldb_ymm9_x86_64, lldb_ymm10_x86_64, lldb_ymm11_x86_64,
-lldb_ymm12_x86_64, lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+static const uint32_t g_avx_regnums_x86_64[] = {
+lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
+lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
+lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
+lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_x86_64) / sizeof(g_avx_regnums_x86_64[0])) -
+  1 ==
+  k_num_avx_registers_x86_64,
+  "g_avx_regnums_x86_64 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_x86_64[] = {
 // Note: we currently do not provide them but this is needed to avoid
 // unnamed groups in SBFrame::GetRegisterContext().
-lldb_bnd0_x86_64, lldb_bnd1_x86_64, lldb_bnd2_x86_64, lldb_bnd3_x86_64,
-lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
+lldb_bnd0_x86_64,   lldb_bnd1_x86_64,lldb_bnd2_x86_64,
+lldb_bnd3_x86_64,   lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) /
-   sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_mpx_regnums_x86_64) / sizeof(g_mpx_regnums_x86_64[0])) -
   1 ==
-  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
-  "g_xstate_regnums_x86_64 has wrong number of register infos");
+  k_num_mpx_registers_x86_64,
+  "g_mpx_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
@@ -165,20 +171,27 @@
   k_num_fpr_registers_i386,
   "g_fpu_regnums_i386 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
-lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
-lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+static const uint32_t g_avx_regnums_i386[] = {
+lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
+lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_i386) / sizeof(g_avx_regnums_i386[0])) -
+  1 ==
+  k_num_avx_registers_i386,
+  "g_avx_regnums_i386 has wrong number of register infos");
+
+static const 

[Lldb-commits] [PATCH] D91411: [lldb] [Process/FreeBSDRemote] Optimize regset pointer logic

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

Create a helper GetOffsetRegSetData() method to get pointer
to the regset data accounting for the necessary offset.  Establish
the offsets in the constructor and store them in the structure.  This
avoids having to add new Get*Offset() methods and combines some common
code.


https://reviews.llvm.org/D91411

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -69,6 +69,7 @@
 GPRegSet,
 FPRegSet,
 DBRegSet,
+MaxRegSet = DBRegSet,
   };
 
   // Private member variables.
@@ -77,14 +78,14 @@
   std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
+  std::array m_regset_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
-  size_t GetFPROffset() const;
-  size_t GetDBROffset() const;
+  uint8_t *GetOffsetRegSetData(uint32_t set);
   llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
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
@@ -262,8 +262,28 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)) {
+  native_thread, CreateRegisterInfoInterface(target_arch)),
+  m_regset_offsets({0}) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
+  std::array first_regnos;
+
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::x86:
+first_regnos[FPRegSet] = lldb_fctrl_i386;
+first_regnos[DBRegSet] = lldb_dr0_i386;
+break;
+  case llvm::Triple::x86_64:
+first_regnos[FPRegSet] = lldb_fctrl_x86_64;
+first_regnos[DBRegSet] = lldb_dr0_x86_64;
+break;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }
+
+  for (int i = FPRegSet; i < MaxRegSet + 1; ++i)
+m_regset_offsets[i] = GetRegisterInfoInterface()
+  .GetRegisterInfo()[first_regnos[i]]
+  .byte_offset;
 }
 
 // CONSIDER after local and llgs debugging are merged, register set support can
@@ -429,15 +449,9 @@
 
   switch (set) {
   case GPRegSet:
-reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case FPRegSet:
-reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case DBRegSet:
-reg_value.SetBytes(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+reg_value.SetBytes(GetOffsetRegSetData(set) + reg_info->byte_offset,
reg_info->byte_size, endian::InlHostByteOrder());
 break;
   case YMMRegSet: {
@@ -493,15 +507,9 @@
 
   switch (set) {
   case GPRegSet:
-::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
- reg_value.GetByteSize());
-break;
   case FPRegSet:
-::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
- reg_value.GetBytes(), reg_value.GetByteSize());
-break;
   case DBRegSet:
-::memcpy(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+::memcpy(GetOffsetRegSetData(set) + reg_info->byte_offset,
  reg_value.GetBytes(), reg_value.GetByteSize());
 break;
   case YMMRegSet: {
@@ -591,36 +599,23 @@
   return res.ToError();
 }
 
-size_t NativeRegisterContextFreeBSD_x86_64::GetFPROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_fctrl_i386;
-break;
-  case llvm::Triple::x86_64:
-regno = lldb_fctrl_x86_64;
+uint8_t *
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set) {
+  uint8_t *base;
+  switch (set) {
+  case GPRegSet:
+base = m_gpr.data();
 break;
-  default:
-llvm_unreachable("Unhandled target architecture.");
-  }
-
-  return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
-}
-
-size_t NativeRegiste

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Perform offsetting in `GetOffsetRegSetData()` to avoid UB.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -69,6 +69,7 @@
 GPRegSet,
 FPRegSet,
 DBRegSet,
+MaxRegSet = DBRegSet,
   };
 
   // Private member variables.
@@ -77,14 +78,14 @@
   std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
+  std::array m_regset_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
-  size_t GetFPROffset() const;
-  size_t GetDBROffset() const;
+  uint8_t *GetOffsetRegSetData(uint32_t set, size_t reg_offset);
   llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
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
@@ -262,8 +262,28 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)) {
+  native_thread, CreateRegisterInfoInterface(target_arch)),
+  m_regset_offsets({0}) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
+  std::array first_regnos;
+
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::x86:
+first_regnos[FPRegSet] = lldb_fctrl_i386;
+first_regnos[DBRegSet] = lldb_dr0_i386;
+break;
+  case llvm::Triple::x86_64:
+first_regnos[FPRegSet] = lldb_fctrl_x86_64;
+first_regnos[DBRegSet] = lldb_dr0_x86_64;
+break;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }
+
+  for (int i = FPRegSet; i < MaxRegSet + 1; ++i)
+m_regset_offsets[i] = GetRegisterInfoInterface()
+  .GetRegisterInfo()[first_regnos[i]]
+  .byte_offset;
 }
 
 // CONSIDER after local and llgs debugging are merged, register set support can
@@ -429,15 +449,9 @@
 
   switch (set) {
   case GPRegSet:
-reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case FPRegSet:
-reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case DBRegSet:
-reg_value.SetBytes(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+reg_value.SetBytes(GetOffsetRegSetData(set, reg_info->byte_offset),
reg_info->byte_size, endian::InlHostByteOrder());
 break;
   case YMMRegSet: {
@@ -493,15 +507,9 @@
 
   switch (set) {
   case GPRegSet:
-::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
- reg_value.GetByteSize());
-break;
   case FPRegSet:
-::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
- reg_value.GetBytes(), reg_value.GetByteSize());
-break;
   case DBRegSet:
-::memcpy(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
  reg_value.GetBytes(), reg_value.GetByteSize());
 break;
   case YMMRegSet: {
@@ -591,36 +599,25 @@
   return res.ToError();
 }
 
-size_t NativeRegisterContextFreeBSD_x86_64::GetFPROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_fctrl_i386;
-break;
-  case llvm::Triple::x86_64:
-regno = lldb_fctrl_x86_64;
+uint8_t *
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
+ size_t reg_offset) {
+  uint8_t *base;
+  switch (set) {
+  case GPRegSet:
+base = m_gpr.data();
 break;
-  default:
-llvm_unreachable("Unhandled target architecture.");
-  }
-
-  return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
-}
-
-size_t NativeRegisterContextFreeBSD_x86_64::GetDBROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTarg

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

2020-11-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D91293#2393764 , @mgorny wrote:

> Perform offsetting in `GetOffsetRegSetData()` to avoid UB.

Shouldn't have the last update gone to  D91411 
 instead ?


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Upload the correct diff.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -32,6 +32,11 @@
 
 class NativeProcessFreeBSD;
 
+struct YMMSplitPtr {
+  void *xmm;
+  void *ymm_hi;
+};
+
 class NativeRegisterContextFreeBSD_x86_64
 : public NativeRegisterContextFreeBSD,
   public NativeRegisterContextWatchpoint_x86 {
@@ -58,15 +63,13 @@
 private:
   // Private member types.
   enum RegSetKind {
+YMMRegSet,
+MaxXSaveSet = YMMRegSet,
+
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
   };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
-  };
 
   // Private member variables.
   std::array m_gpr;
@@ -82,6 +85,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // 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
@@ -101,23 +101,29 @@
   k_num_fpr_registers_x86_64,
   "g_fpu_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
-lldb_ymm0_x86_64, lldb_ymm1_x86_64, lldb_ymm2_x86_64, lldb_ymm3_x86_64,
-lldb_ymm4_x86_64, lldb_ymm5_x86_64, lldb_ymm6_x86_64, lldb_ymm7_x86_64,
-lldb_ymm8_x86_64, lldb_ymm9_x86_64, lldb_ymm10_x86_64, lldb_ymm11_x86_64,
-lldb_ymm12_x86_64, lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+static const uint32_t g_avx_regnums_x86_64[] = {
+lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
+lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
+lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
+lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_x86_64) / sizeof(g_avx_regnums_x86_64[0])) -
+  1 ==
+  k_num_avx_registers_x86_64,
+  "g_avx_regnums_x86_64 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_x86_64[] = {
 // Note: we currently do not provide them but this is needed to avoid
 // unnamed groups in SBFrame::GetRegisterContext().
-lldb_bnd0_x86_64, lldb_bnd1_x86_64, lldb_bnd2_x86_64, lldb_bnd3_x86_64,
-lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
+lldb_bnd0_x86_64,   lldb_bnd1_x86_64,lldb_bnd2_x86_64,
+lldb_bnd3_x86_64,   lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) /
-   sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_mpx_regnums_x86_64) / sizeof(g_mpx_regnums_x86_64[0])) -
   1 ==
-  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
-  "g_xstate_regnums_x86_64 has wrong number of register infos");
+  k_num_mpx_registers_x86_64,
+  "g_mpx_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
@@ -165,20 +171,27 @@
   k_num_fpr_registers_i386,
   "g_fpu_regnums_i386 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
-lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
-lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+static const uint32_t g_avx_regnums_i386[] = {
+lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
+lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_i386) / sizeof(g_avx_regnums_i386[0])) -
+  1 ==
+  k_num_avx_registers_i386,
+  "g_avx_regnums_i386 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_i386[]

[Lldb-commits] [PATCH] D91411: [lldb] [Process/FreeBSDRemote] Optimize regset pointer logic

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

Perform offsetting in GetOffsetRegSetData() to avoid UB.


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

https://reviews.llvm.org/D91411

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -69,6 +69,7 @@
 GPRegSet,
 FPRegSet,
 DBRegSet,
+MaxRegSet = DBRegSet,
   };
 
   // Private member variables.
@@ -77,14 +78,14 @@
   std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
+  std::array m_regset_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
-  size_t GetFPROffset() const;
-  size_t GetDBROffset() const;
+  uint8_t *GetOffsetRegSetData(uint32_t set);
   llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
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
@@ -262,8 +262,28 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)) {
+  native_thread, CreateRegisterInfoInterface(target_arch)),
+  m_regset_offsets({0}) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
+  std::array first_regnos;
+
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::x86:
+first_regnos[FPRegSet] = lldb_fctrl_i386;
+first_regnos[DBRegSet] = lldb_dr0_i386;
+break;
+  case llvm::Triple::x86_64:
+first_regnos[FPRegSet] = lldb_fctrl_x86_64;
+first_regnos[DBRegSet] = lldb_dr0_x86_64;
+break;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }
+
+  for (int i = FPRegSet; i < MaxRegSet + 1; ++i)
+m_regset_offsets[i] = GetRegisterInfoInterface()
+  .GetRegisterInfo()[first_regnos[i]]
+  .byte_offset;
 }
 
 // CONSIDER after local and llgs debugging are merged, register set support can
@@ -429,15 +449,9 @@
 
   switch (set) {
   case GPRegSet:
-reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case FPRegSet:
-reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case DBRegSet:
-reg_value.SetBytes(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+reg_value.SetBytes(GetOffsetRegSetData(set) + reg_info->byte_offset,
reg_info->byte_size, endian::InlHostByteOrder());
 break;
   case YMMRegSet: {
@@ -493,15 +507,9 @@
 
   switch (set) {
   case GPRegSet:
-::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
- reg_value.GetByteSize());
-break;
   case FPRegSet:
-::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
- reg_value.GetBytes(), reg_value.GetByteSize());
-break;
   case DBRegSet:
-::memcpy(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+::memcpy(GetOffsetRegSetData(set) + reg_info->byte_offset,
  reg_value.GetBytes(), reg_value.GetByteSize());
 break;
   case YMMRegSet: {
@@ -591,36 +599,23 @@
   return res.ToError();
 }
 
-size_t NativeRegisterContextFreeBSD_x86_64::GetFPROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_fctrl_i386;
-break;
-  case llvm::Triple::x86_64:
-regno = lldb_fctrl_x86_64;
+uint8_t *
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set) {
+  uint8_t *base;
+  switch (set) {
+  case GPRegSet:
+base = m_gpr.data();
 break;
-  default:
-llvm_unreachable("Unhandled target architecture.");
-  }
-
-  return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
-}
-
-size_t NativeRegisterContextFreeBSD_x86_64::GetDBROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_dr0_i386;
+  case 

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

In D91293#2393803 , @labath wrote:

> In D91293#2393764 , @mgorny wrote:
>
>> Perform offsetting in `GetOffsetRegSetData()` to avoid UB.
>
> Shouldn't have the last update gone to  D91411 
>  instead ?

Indeed.


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

https://reviews.llvm.org/D91293

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Restore old register set indices, to unbreak output.

(Note: I'm going to make that struct move soonish)


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -32,6 +32,11 @@
 
 class NativeProcessFreeBSD;
 
+struct YMMSplitPtr {
+  void *xmm;
+  void *ymm_hi;
+};
+
 class NativeRegisterContextFreeBSD_x86_64
 : public NativeRegisterContextFreeBSD,
   public NativeRegisterContextWatchpoint_x86 {
@@ -60,12 +65,10 @@
   enum RegSetKind {
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
-  };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
+YMMRegSet,
+MPXRegSet,
+MaxRegSet = MPXRegSet,
   };
 
   // Private member variables.
@@ -73,7 +76,7 @@
   std::array m_fpr; // FXSAVE
   std::array m_dbr;
   std::vector m_xsave;
-  std::array m_xsave_offsets;
+  std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
@@ -82,6 +85,7 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // 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
@@ -101,23 +101,29 @@
   k_num_fpr_registers_x86_64,
   "g_fpu_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
-lldb_ymm0_x86_64, lldb_ymm1_x86_64, lldb_ymm2_x86_64, lldb_ymm3_x86_64,
-lldb_ymm4_x86_64, lldb_ymm5_x86_64, lldb_ymm6_x86_64, lldb_ymm7_x86_64,
-lldb_ymm8_x86_64, lldb_ymm9_x86_64, lldb_ymm10_x86_64, lldb_ymm11_x86_64,
-lldb_ymm12_x86_64, lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+static const uint32_t g_avx_regnums_x86_64[] = {
+lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
+lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
+lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
+lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_x86_64) / sizeof(g_avx_regnums_x86_64[0])) -
+  1 ==
+  k_num_avx_registers_x86_64,
+  "g_avx_regnums_x86_64 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_x86_64[] = {
 // Note: we currently do not provide them but this is needed to avoid
 // unnamed groups in SBFrame::GetRegisterContext().
-lldb_bnd0_x86_64, lldb_bnd1_x86_64, lldb_bnd2_x86_64, lldb_bnd3_x86_64,
-lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
+lldb_bnd0_x86_64,   lldb_bnd1_x86_64,lldb_bnd2_x86_64,
+lldb_bnd3_x86_64,   lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) /
-   sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_mpx_regnums_x86_64) / sizeof(g_mpx_regnums_x86_64[0])) -
   1 ==
-  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
-  "g_xstate_regnums_x86_64 has wrong number of register infos");
+  k_num_mpx_registers_x86_64,
+  "g_mpx_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
@@ -165,20 +171,27 @@
   k_num_fpr_registers_i386,
   "g_fpu_regnums_i386 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
-lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
-lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+static const uint32_t g_avx_regnums_i386[] = {
+lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
+lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((

[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Move `YMMSplitPtr` inside the class.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -60,12 +60,10 @@
   enum RegSetKind {
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
-  };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
+YMMRegSet,
+MPXRegSet,
+MaxRegSet = MPXRegSet,
   };
 
   // Private member variables.
@@ -73,7 +71,7 @@
   std::array m_fpr; // FXSAVE
   std::array m_dbr;
   std::vector m_xsave;
-  std::array m_xsave_offsets;
+  std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
@@ -82,6 +80,12 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+
+  struct YMMSplitPtr {
+void *xmm;
+void *ymm_hi;
+  };
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // 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
@@ -101,23 +101,29 @@
   k_num_fpr_registers_x86_64,
   "g_fpu_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
-lldb_ymm0_x86_64, lldb_ymm1_x86_64, lldb_ymm2_x86_64, lldb_ymm3_x86_64,
-lldb_ymm4_x86_64, lldb_ymm5_x86_64, lldb_ymm6_x86_64, lldb_ymm7_x86_64,
-lldb_ymm8_x86_64, lldb_ymm9_x86_64, lldb_ymm10_x86_64, lldb_ymm11_x86_64,
-lldb_ymm12_x86_64, lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+static const uint32_t g_avx_regnums_x86_64[] = {
+lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
+lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
+lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
+lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_x86_64) / sizeof(g_avx_regnums_x86_64[0])) -
+  1 ==
+  k_num_avx_registers_x86_64,
+  "g_avx_regnums_x86_64 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_x86_64[] = {
 // Note: we currently do not provide them but this is needed to avoid
 // unnamed groups in SBFrame::GetRegisterContext().
-lldb_bnd0_x86_64, lldb_bnd1_x86_64, lldb_bnd2_x86_64, lldb_bnd3_x86_64,
-lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
+lldb_bnd0_x86_64,   lldb_bnd1_x86_64,lldb_bnd2_x86_64,
+lldb_bnd3_x86_64,   lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) /
-   sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_mpx_regnums_x86_64) / sizeof(g_mpx_regnums_x86_64[0])) -
   1 ==
-  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
-  "g_xstate_regnums_x86_64 has wrong number of register infos");
+  k_num_mpx_registers_x86_64,
+  "g_mpx_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
@@ -165,20 +171,27 @@
   k_num_fpr_registers_i386,
   "g_fpu_regnums_i386 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
-lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
-lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+static const uint32_t g_avx_regnums_i386[] = {
+lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
+lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_i386) / sizeof(g_avx_regnums_i386[0])) -
+  1 ==
+  k_num_avx_registers_i386,
+  "g_avx_regnums_i386 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_i38

[Lldb-commits] [PATCH] D91411: [lldb] [Process/FreeBSDRemote] Optimize regset pointer logic

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

Rebase


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

https://reviews.llvm.org/D91411

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -72,14 +72,14 @@
   std::array m_dbr;
   std::vector m_xsave;
   std::array m_xsave_offsets;
+  std::array m_regset_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
-  size_t GetFPROffset() const;
-  size_t GetDBROffset() const;
+  uint8_t *GetOffsetRegSetData(uint32_t set, size_t reg_offset);
 
   struct YMMSplitPtr {
 void *xmm;
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
@@ -262,8 +262,28 @@
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
 : NativeRegisterContextRegisterInfo(
-  native_thread, CreateRegisterInfoInterface(target_arch)) {
+  native_thread, CreateRegisterInfoInterface(target_arch)),
+  m_regset_offsets({0}) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
+  std::array first_regnos;
+
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::x86:
+first_regnos[FPRegSet] = lldb_fctrl_i386;
+first_regnos[DBRegSet] = lldb_dr0_i386;
+break;
+  case llvm::Triple::x86_64:
+first_regnos[FPRegSet] = lldb_fctrl_x86_64;
+first_regnos[DBRegSet] = lldb_dr0_x86_64;
+break;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }
+
+  for (int i: {FPRegSet, DBRegSet})
+m_regset_offsets[i] = GetRegisterInfoInterface()
+  .GetRegisterInfo()[first_regnos[i]]
+  .byte_offset;
 }
 
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
@@ -429,15 +449,9 @@
 
   switch (set) {
   case GPRegSet:
-reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case FPRegSet:
-reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
-   reg_info->byte_size, endian::InlHostByteOrder());
-break;
   case DBRegSet:
-reg_value.SetBytes(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+reg_value.SetBytes(GetOffsetRegSetData(set, reg_info->byte_offset),
reg_info->byte_size, endian::InlHostByteOrder());
 break;
   case YMMRegSet: {
@@ -495,15 +509,9 @@
 
   switch (set) {
   case GPRegSet:
-::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
- reg_value.GetByteSize());
-break;
   case FPRegSet:
-::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
- reg_value.GetBytes(), reg_value.GetByteSize());
-break;
   case DBRegSet:
-::memcpy(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+::memcpy(GetOffsetRegSetData(set, reg_info->byte_offset),
  reg_value.GetBytes(), reg_value.GetByteSize());
 break;
   case YMMRegSet: {
@@ -595,36 +603,25 @@
   return res.ToError();
 }
 
-size_t NativeRegisterContextFreeBSD_x86_64::GetFPROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_fctrl_i386;
-break;
-  case llvm::Triple::x86_64:
-regno = lldb_fctrl_x86_64;
+uint8_t *
+NativeRegisterContextFreeBSD_x86_64::GetOffsetRegSetData(uint32_t set,
+ size_t reg_offset) {
+  uint8_t *base;
+  switch (set) {
+  case GPRegSet:
+base = m_gpr.data();
 break;
-  default:
-llvm_unreachable("Unhandled target architecture.");
-  }
-
-  return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
-}
-
-size_t NativeRegisterContextFreeBSD_x86_64::GetDBROffset() const {
-  uint32_t regno;
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-regno = lldb_dr0_i386;
+  case FPRegSet:
+base = m_fpr.data();
 break;
-  case llvm::Triple::x86_64:
-regno = lldb_dr0_x86_64;
+  case

[Lldb-commits] [PATCH] D91422: [lldb] [Process/FreeBSDRemote] Check for regset support early [WIP]

2020-11-13 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.

@labath, I'm trying to copy the logic from Linux here but for some reason 
unsupported regsets are now reported as 'unknown' instead of not all. Any clue 
what I'm doing wrong?


https://reviews.llvm.org/D91422

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h


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
@@ -86,6 +86,7 @@
 void *ymm_hi;
   };
   llvm::Optional GetYMMSplitReg(uint32_t reg);
+  bool IsRegisterSetAvailable(uint32_t set_index) 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
@@ -289,7 +289,7 @@
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
   uint32_t sets = 0;
   for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
-if (GetSetForNativeRegNum(set_index))
+if (IsRegisterSetAvailable(set_index))
   ++sets;
   }
 
@@ -298,6 +298,9 @@
 
 const RegisterSet *
 NativeRegisterContextFreeBSD_x86_64::GetRegisterSet(uint32_t set_index) const {
+  if (!IsRegisterSetAvailable(set_index))
+return nullptr;
+
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
 return &g_reg_sets_i386[set_index];
@@ -648,4 +651,28 @@
   return YMMSplitPtr{&fpreg->sv_xmm[reg_index], &ymmreg[reg_index]};
 }
 
+bool NativeRegisterContextFreeBSD_x86_64::IsRegisterSetAvailable(
+uint32_t set_index) const {
+  switch (static_cast(set_index)) {
+  case GPRegSet:
+  case FPRegSet:
+  case DBRegSet:
+return true;
+  case YMMRegSet: {
+struct ptrace_xstate_info info;
+Status ret = NativeProcessFreeBSD::PtraceWrapper(
+PT_GETXSTATE_INFO, GetProcessPid(), &info, sizeof(info));
+if (!ret.Success())
+  return false;
+
+assert(info.xsave_mask & XFEATURE_ENABLED_X87);
+assert(info.xsave_mask & XFEATURE_ENABLED_SSE);
+return !!(info.xsave_mask & XFEATURE_ENABLED_YMM_HI128);
+  }
+  case MPXRegSet:
+return false;
+  }
+  llvm_unreachable("Unknown register set");
+}
+
 #endif // defined(__x86_64__)
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
@@ -33,8 +33,8 @@
   CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) = 0;
 
 protected:
-  virtual NativeProcessFreeBSD &GetProcess();
-  virtual ::pid_t GetProcessPid();
+  virtual NativeProcessFreeBSD &GetProcess() const;
+  virtual ::pid_t GetProcessPid() const;
 };
 
 } // namespace process_freebsd
Index: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
@@ -20,10 +20,10 @@
 #include 
 // clang-format on
 
-NativeProcessFreeBSD &NativeRegisterContextFreeBSD::GetProcess() {
+NativeProcessFreeBSD &NativeRegisterContextFreeBSD::GetProcess() const {
   return static_cast(m_thread.GetProcess());
 }
 
-::pid_t NativeRegisterContextFreeBSD::GetProcessPid() {
+::pid_t NativeRegisterContextFreeBSD::GetProcessPid() const {
   return GetProcess().GetID();
 }


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
@@ -86,6 +86,7 @@
 void *ymm_hi;
   };
   llvm::Optional GetYMMSplitReg(uint32_t reg);
+  bool IsRegisterSetAvailable(uint32_t set_index) const;
 };
 
 } // namespace process_freebsd
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
===
--- lldb/s

[Lldb-commits] [PATCH] D91422: [lldb] [Process/FreeBSDRemote] Check for regset support early [WIP]

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

Hmm, if I add `GetUserRegisterCount()` override, then registers start 
disappearing. Except that wrong registers disappear.


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

https://reviews.llvm.org/D91422

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


[Lldb-commits] [PATCH] D91293: [lldb] [Process/FreeBSDRemote] Modernize and simplify YMM logic

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

Rename AVX/MPX regsets to avoid matching strings in 
`commands/register/register/register_command/TestRegisters.py` that cause the 
test to wrongly assume that I have MPX on this machine.


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

https://reviews.llvm.org/D91293

Files:
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

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
@@ -60,12 +60,10 @@
   enum RegSetKind {
 GPRegSet,
 FPRegSet,
-XSaveRegSet,
 DBRegSet,
-  };
-  enum {
-YMMXSaveSet,
-MaxXSaveSet = YMMXSaveSet,
+YMMRegSet,
+MPXRegSet,
+MaxRegSet = MPXRegSet,
   };
 
   // Private member variables.
@@ -73,7 +71,7 @@
   std::array m_fpr; // FXSAVE
   std::array m_dbr;
   std::vector m_xsave;
-  std::array m_xsave_offsets;
+  std::array m_xsave_offsets;
 
   llvm::Optional GetSetForNativeRegNum(int reg_num) const;
 
@@ -82,6 +80,12 @@
 
   size_t GetFPROffset() const;
   size_t GetDBROffset() const;
+
+  struct YMMSplitPtr {
+void *xmm;
+void *ymm_hi;
+  };
+  llvm::Optional GetYMMSplitReg(uint32_t reg);
 };
 
 } // 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
@@ -101,23 +101,29 @@
   k_num_fpr_registers_x86_64,
   "g_fpu_regnums_x86_64 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_x86_64[] = {
-lldb_ymm0_x86_64, lldb_ymm1_x86_64, lldb_ymm2_x86_64, lldb_ymm3_x86_64,
-lldb_ymm4_x86_64, lldb_ymm5_x86_64, lldb_ymm6_x86_64, lldb_ymm7_x86_64,
-lldb_ymm8_x86_64, lldb_ymm9_x86_64, lldb_ymm10_x86_64, lldb_ymm11_x86_64,
-lldb_ymm12_x86_64, lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+static const uint32_t g_avx_regnums_x86_64[] = {
+lldb_ymm0_x86_64,   lldb_ymm1_x86_64,  lldb_ymm2_x86_64,  lldb_ymm3_x86_64,
+lldb_ymm4_x86_64,   lldb_ymm5_x86_64,  lldb_ymm6_x86_64,  lldb_ymm7_x86_64,
+lldb_ymm8_x86_64,   lldb_ymm9_x86_64,  lldb_ymm10_x86_64, lldb_ymm11_x86_64,
+lldb_ymm12_x86_64,  lldb_ymm13_x86_64, lldb_ymm14_x86_64, lldb_ymm15_x86_64,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_x86_64) / sizeof(g_avx_regnums_x86_64[0])) -
+  1 ==
+  k_num_avx_registers_x86_64,
+  "g_avx_regnums_x86_64 has wrong number of register infos");
+
+static const uint32_t g_mpx_regnums_x86_64[] = {
 // Note: we currently do not provide them but this is needed to avoid
 // unnamed groups in SBFrame::GetRegisterContext().
-lldb_bnd0_x86_64, lldb_bnd1_x86_64, lldb_bnd2_x86_64, lldb_bnd3_x86_64,
-lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
+lldb_bnd0_x86_64,   lldb_bnd1_x86_64,lldb_bnd2_x86_64,
+lldb_bnd3_x86_64,   lldb_bndcfgu_x86_64, lldb_bndstatus_x86_64,
 LLDB_INVALID_REGNUM // register sets need to end with this flag
 };
-static_assert((sizeof(g_xstate_regnums_x86_64) /
-   sizeof(g_xstate_regnums_x86_64[0])) -
+static_assert((sizeof(g_mpx_regnums_x86_64) / sizeof(g_mpx_regnums_x86_64[0])) -
   1 ==
-  k_num_avx_registers_x86_64 + k_num_mpx_registers_x86_64,
-  "g_xstate_regnums_x86_64 has wrong number of register infos");
+  k_num_mpx_registers_x86_64,
+  "g_mpx_regnums_x86_64 has wrong number of register infos");
 
 // x86 debug registers.
 static const uint32_t g_dbr_regnums_x86_64[] = {
@@ -165,20 +171,27 @@
   k_num_fpr_registers_i386,
   "g_fpu_regnums_i386 has wrong number of register infos");
 
-// x86 64-bit registers available via XState.
-static const uint32_t g_xstate_regnums_i386[] = {
-lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
-lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+static const uint32_t g_avx_regnums_i386[] = {
+lldb_ymm0_i386, lldb_ymm1_i386, lldb_ymm2_i386, lldb_ymm3_i386,
+lldb_ymm4_i386, lldb_ymm5_i386, lldb_ymm6_i386, lldb_ymm7_i386,
+LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
+static_assert((sizeof(g_avx_regnums_i386) / sizeof(g_avx_regnums_i386[0])) -
+  1 ==
+

[Lldb-commits] [lldb] e5a82b4 - [lldb] Fix SymbolFile/PDB/udt-layout.test

2020-11-13 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-13T09:30:40-08:00
New Revision: e5a82b4594d700fe5afd2eaa7e03a213ac91d895

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

LOG: [lldb] Fix SymbolFile/PDB/udt-layout.test

Update the test for 406ad187486b4277fc82a2c0714ae53396e47928

Added: 


Modified: 
lldb/test/Shell/SymbolFile/PDB/udt-layout.test

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/PDB/udt-layout.test 
b/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
index 726f633efe5b..0ee9dcf6771b 100644
--- a/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
+++ b/lldb/test/Shell/SymbolFile/PDB/udt-layout.test
@@ -5,8 +5,8 @@ RUN: %lldb -b -s %S/Inputs/UdtLayoutTest.script -- %t.exe | 
FileCheck %s
 CHECK:(int) int C::abc = 123
 CHECK:(List [16]) ls = {
 CHECK:  [15] = {
-CHECK:Prev = 0x
-CHECK:Next = 0x
+CHECK:Prev = nullptr
+CHECK:Next = nullptr
 CHECK:Value = {
 CHECK:  B<0> = {
 CHECK:A = {



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


[Lldb-commits] [lldb] 5a4b2e1 - The AssertRecognizer used the module from a frames SC without checking it was non-null.

2020-11-13 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2020-11-13T11:41:32-08:00
New Revision: 5a4b2e1541f399c146a4ef5cf8f6aaf8b258a77b

URL: 
https://github.com/llvm/llvm-project/commit/5a4b2e1541f399c146a4ef5cf8f6aaf8b258a77b
DIFF: 
https://github.com/llvm/llvm-project/commit/5a4b2e1541f399c146a4ef5cf8f6aaf8b258a77b.diff

LOG: The AssertRecognizer used the module from a frames SC without checking it 
was non-null.

I only have a crash report for this.  I could reproduce it with a slightly older
lldb by running an expression that called pthread_kill, but we started making 
modules
for our expression JIT code, so that no longer triggers the bug.  I can't think 
of another
good way to test it but the fix is obvious.

Added: 


Modified: 
lldb/source/Target/AssertFrameRecognizer.cpp

Removed: 




diff  --git a/lldb/source/Target/AssertFrameRecognizer.cpp 
b/lldb/source/Target/AssertFrameRecognizer.cpp
index fe5fa3a362f8..cb671040d14f 100644
--- a/lldb/source/Target/AssertFrameRecognizer.cpp
+++ b/lldb/source/Target/AssertFrameRecognizer.cpp
@@ -130,7 +130,8 @@ AssertFrameRecognizer::RecognizeFrame(lldb::StackFrameSP 
frame_sp) {
 SymbolContext sym_ctx =
 prev_frame_sp->GetSymbolContext(eSymbolContextEverything);
 
-if (!sym_ctx.module_sp->GetFileSpec().FileEquals(location.module_spec))
+if (!sym_ctx.module_sp ||
+!sym_ctx.module_sp->GetFileSpec().FileEquals(location.module_spec))
   continue;
 
 ConstString func_name = sym_ctx.GetFunctionName();



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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I'm late to the party, but I actually don't think this is a good change as it 
disables the normal LLDB backtraces. The current test errors we see on Green 
Dragon look now like this:

  Assertion failed: (size() >= N && "Dropping more elements than exist"), 
function drop_front, file 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/ADT/StringRef.h,
 line 655.
  Fatal Python error: Aborted
  
  Current thread 0x00010e0f9dc0 (most recent call first):
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lib/python3.7/site-packages/lldb/__init__.py",
 line 2717 in HandleCommand
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 2129 in runCmd
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py",
 line 43 in test_apropos_with_process
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 413 in runMethod
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 383 in run
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 458 in __call__
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 117 in _wrapped_run
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 115 in _wrapped_run
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 85 in run
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 66 in __call__
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/runner.py",
 line 165 in run
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py",
 line 1017 in run_suite
File 
"/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/dotest.py",
 line 7 in 

It's practically impossible to say from the backtrace which commit is to blame 
for the crash as I only know we run the command `apropos env` and then we crash 
in StringRef.

Do we have a way of getting both the LLVM and the Python backtrace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

In D87637#2395246 , @teemperor wrote:

> I'm late to the party, but I actually don't think this is a good change as it 
> disables the normal LLDB backtraces. The current test errors we see on Green 
> Dragon look now like this:
>
>   Assertion failed: (size() >= N && "Dropping more elements than exist"), 
> function drop_front, file 
> /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/ADT/StringRef.h,
>  line 655.
>   Fatal Python error: Aborted
>   
>   Current thread 0x00010e0f9dc0 (most recent call first):
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lib/python3.7/site-packages/lldb/__init__.py",
>  line 2717 in HandleCommand
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
>  line 2129 in runCmd
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py",
>  line 43 in test_apropos_with_process
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 413 in runMethod
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 383 in run
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 458 in __call__
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 117 in _wrapped_run
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 115 in _wrapped_run
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 85 in run
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 66 in __call__
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/runner.py",
>  line 165 in run
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py",
>  line 1017 in run_suite
> File 
> "/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/dotest.py",
>  line 7 in 
>
> It's practically impossible to say from the backtrace which commit is to 
> blame for the crash as I only know we run the command `apropos env` and then 
> we crash in StringRef.
>
> Do we have a way of getting both the LLVM and the Python backtrace?

I'm not really sure what you mean by the LLDB backtrace or how this is 
inhibiting it.

IIUC, the faulthandler here is triggered when the test *receives* a signal 
killing it (e.g. if it hangs and lit or whatever kills it after some timeout). 
If the LLDB process that this calls crashes, it shouldn't affect that. SWIG 
maybe is complicating things here.

Do you have a way to reproduce the issue you're talking about? e.g. "git 
checkout  && ninja check-lldb-api"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I think Raphael is referring to the LLVM's `PrettyStackTrace` which prints a 
backtrace when you crash. Other than the driver I don't think we enable that in 
LLDB, so maybe LLVM was registering them by default if you had no signal 
handler installed?

By default the LLVM signal handlers  propagate the signal once they're done 
with it, so I see no reason why we couldn't have both. I think it should work 
if we ensure libLLDB calls `llvm::EnablePrettyStackTrace()`, but if we go that 
route the behavior should be opt in, libraries shouldn't be setting signal 
handlers by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

+1 to what Jonas said.

You can reproduce this by just adding an `abort();` call at the start of 
`CommandInterpreter::HandleCommand` and then for example like the 
`TestApropos.py` test.

> If the LLDB process that this calls crashes, it shouldn't affect that.

LLDB is loaded as a Python module *inside* the `Python` process, so Python and 
LLDB actually do share a process/signals (that's why when LLDB crashes in a 
Python test, you actually see the 'Python' process crashing. It's just a loaded 
shared library).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

Thanks, I'll take a look at that.

For SIGTERM (issued by test runners to handle timeouts), the stack trace is 
printed via `faulthandler.register(signal.SIGTERM, chain=True)`. The 
`chain=True` //should// cause previous signal handlers registered for SIGTERM 
to execute.

For all other signal types, this is enabled with `faulthandler.enable()`, and 
there doesn't appear to be any equivalent option AFAICT, so it's plausible that 
it's overwriting the signal handler. Maybe we could replace that with calls to 
`faulthandler.register()` for each signal, but hopefully there's a cleaner way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

At head:

  $ ninja check-lldb-api-commands-apropos-basic
  [2/3] Running lit suite 
/home/rupprecht/src/llvm-project/lldb/test/API/commands/apropos/basic
  FAIL: lldb-api :: commands/apropos/basic/TestApropos.py (1 of 1)
   TEST 'lldb-api :: commands/apropos/basic/TestApropos.py' 
FAILED 
  Script:
  --
  /usr/bin/python3.8 /home/rupprecht/src/llvm-project/lldb/test/API/dotest.py 
<...> -p TestApropos.py
  --
  Exit Code: -6
  
  Command Output (stdout):
  --
  lldb version 12.0.0 (https://github.com/llvm/llvm-project.git revision 
703ef17e7a0a0f51e1d000bb1f71ad437a9933e4)
clang revision 703ef17e7a0a0f51e1d000bb1f71ad437a9933e4
llvm revision 703ef17e7a0a0f51e1d000bb1f71ad437a9933e4
  
  --
  Command Output (stderr):
  --
  Fatal Python error: Aborted
  
  Current thread 0x7f1541aea740 (most recent call first):
File 
"/home/rupprecht/src/llvm-build/dev/lib/python2.7/dist-packages/lldb/__init__.py",
 line 3352 in HandleCommand
File 
"/home/rupprecht/src/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 2129 in runCmd
File 
"/home/rupprecht/src/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py",
 line 1949 in setUp
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 377 in run
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 458 in __call__
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 117 in _wrapped_run
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 115 in _wrapped_run
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 85 in run
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
 line 66 in __call__
File 
"/home/rupprecht/src/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/runner.py",
 line 165 in run
File 
"/home/rupprecht/src/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py",
 line 1013 in run_suite
File "/home/rupprecht/src/llvm-project/lldb/test/API/dotest.py", line 7 in 

  
  --
  
  
  
  Failed Tests (1):
lldb-api :: commands/apropos/basic/TestApropos.py

With `registerFaulthandler()` commented out:

  $ ninja check-lldb-api-commands-apropos-basic
  [2/3] Running lit suite 
/home/rupprecht/src/llvm-project/lldb/test/API/commands/apropos/basic
  FAIL: lldb-api :: commands/apropos/basic/TestApropos.py (1 of 1)
   TEST 'lldb-api :: commands/apropos/basic/TestApropos.py' 
FAILED 
  Script:
  --
  /usr/bin/python3.8 /home/rupprecht/src/llvm-project/lldb/test/API/dotest.py 
<...> -p TestApropos.py
  --
  Exit Code: -6
  
  Command Output (stdout):
  --
  lldb version 12.0.0 (https://github.com/llvm/llvm-project.git revision 
703ef17e7a0a0f51e1d000bb1f71ad437a9933e4)
clang revision 703ef17e7a0a0f51e1d000bb1f71ad437a9933e4
llvm revision 703ef17e7a0a0f51e1d000bb1f71ad437a9933e4
  
  --
  
  
  
  Failed Tests (1):
lldb-api :: commands/apropos/basic/TestApropos.py

So, it doesn't seem like the LLVM signal handler is being enabled, at least not 
by default -- maybe it has something to do with my cmake config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [PATCH] D87637: [lldb/test] Enable faulthandler in dotest

2020-11-13 Thread Jordan Rupprecht via Phabricator via lldb-commits
rupprecht added a comment.

My cmake config appears to be correct. I tried adding this to 
`CommandInterpreter::HandleCommand`:

  lldbassert(ENABLE_BACKTRACES && "back traces are not enabled!");
  lldbassert(false && "crash!");

Running LLDB directly does what you describe:

  $ bin/lldb
  (lldb) help
  lldb: 
/home/rupprecht/src/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:1655:
 bool lldb_private::CommandInterpreter::HandleCommand(const char *, 
lldb_private::LazyBool, lldb_private::CommandReturnObject &, 
lldb_private::ExecutionContext *, bool, bool): Assertion `false && "crash!"' 
failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
  Stack dump:
  0.  Program arguments: bin/lldb 
   #0 0x003a283a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:563:11
   #1 0x003a2a0b PrintStackTraceSignalHandler(void*) 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Unix/Signals.inc:630:1
   #2 0x003a102b llvm::sys::RunSignalHandlers() 
/home/rupprecht/src/llvm-project/llvm/lib/Support/Signals.cpp:70:5

However, when I disable the faulthandler added here and run the lldb tests, I 
only see the "Assertion `false && "crash!"' failed" part. So: yes, something in 
the lldb test suite is messing with the signal handler, but it's not this (or 
it's not *only* this).
I recall running across other signal handling code in the lldb test framework 
(in the unittest2 bits), I'll see if I can find that again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87637

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


[Lldb-commits] [lldb] 6c0cd56 - [lldb] Make `process connect` behave the same in sync and async mode.

2020-11-13 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-13T17:39:30-08:00
New Revision: 6c0cd5676e0a0feaf836e0399023a6e21224467b

URL: 
https://github.com/llvm/llvm-project/commit/6c0cd5676e0a0feaf836e0399023a6e21224467b
DIFF: 
https://github.com/llvm/llvm-project/commit/6c0cd5676e0a0feaf836e0399023a6e21224467b.diff

LOG: [lldb] Make `process connect` behave the same in sync and async mode.

I think the check for whether the process is connected is totally bogus
in the first place, but on the off-chance that's it's not, we should
behave the same in synchronous and asynchronous mode.

Added: 
lldb/test/Shell/Commands/command-process-connect.test

Modified: 
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 0e0b61f1534f..e8bfbd889299 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -837,18 +837,6 @@ std::string PlatformRemoteGDBServer::MakeUrl(const char 
*scheme,
   return std::string(result.GetString());
 }
 
-lldb::ProcessSP PlatformRemoteGDBServer::ConnectProcess(
-llvm::StringRef connect_url, llvm::StringRef plugin_name,
-lldb_private::Debugger &debugger, lldb_private::Target *target,
-lldb_private::Status &error) {
-  if (!IsRemote() || !IsConnected()) {
-error.SetErrorString("Not connected to remote gdb server");
-return nullptr;
-  }
-  return Platform::ConnectProcess(connect_url, plugin_name, debugger, target,
-  error);
-}
-
 size_t PlatformRemoteGDBServer::ConnectToWaitingProcesses(Debugger &debugger,
   Status &error) {
   std::vector connection_urls;

diff  --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
index 297b482eb87a..e43cd0e55c6d 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
@@ -154,12 +154,6 @@ class PlatformRemoteGDBServer : public Platform, private 
UserIDResolver {
 
   const lldb::UnixSignalsSP &GetRemoteUnixSignals() override;
 
-  lldb::ProcessSP ConnectProcess(llvm::StringRef connect_url,
- llvm::StringRef plugin_name,
- lldb_private::Debugger &debugger,
- lldb_private::Target *target,
- lldb_private::Status &error) override;
-
   size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger,
lldb_private::Status &error) override;
 

diff  --git a/lldb/test/Shell/Commands/command-process-connect.test 
b/lldb/test/Shell/Commands/command-process-connect.test
new file mode 100644
index ..c4761360d541
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-process-connect.test
@@ -0,0 +1,9 @@
+# Synchronous
+# RUN: %lldb -o 'platform select remote-gdb-server' -o 'process connect 
connect://localhost:4321' 2>&1 | FileCheck %s
+
+# Asynchronous
+# RUN: echo -e 'platform select remote-gdb-server\nprocess connect 
connect://localhost:4321' | %lldb 2>&1 | FileCheck %s
+
+# CHECK: Platform: remote-gdb-server
+# CHECK: Connected: no
+# CHECK: error: Failed to connect port



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


[Lldb-commits] [lldb] 875be9f - [lldb] Mark command-process-connect as unsupported on Windows

2020-11-13 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-13T20:02:05-08:00
New Revision: 875be9f454c31c94701bdf4e28f8bea07a8c9c79

URL: 
https://github.com/llvm/llvm-project/commit/875be9f454c31c94701bdf4e28f8bea07a8c9c79
DIFF: 
https://github.com/llvm/llvm-project/commit/875be9f454c31c94701bdf4e28f8bea07a8c9c79.diff

LOG: [lldb] Mark command-process-connect as unsupported on Windows

Windows doesn't support remote connections.

Added: 


Modified: 
lldb/test/Shell/Commands/command-process-connect.test

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-process-connect.test 
b/lldb/test/Shell/Commands/command-process-connect.test
index c4761360d541..30782243d4ed 100644
--- a/lldb/test/Shell/Commands/command-process-connect.test
+++ b/lldb/test/Shell/Commands/command-process-connect.test
@@ -1,3 +1,5 @@
+# UNSUPPORTED: system-windows
+
 # Synchronous
 # RUN: %lldb -o 'platform select remote-gdb-server' -o 'process connect 
connect://localhost:4321' 2>&1 | FileCheck %s
 



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