date:20210923

[Lldb-commits] [lldb] 5685eb9 - [lldb] Fix DomainSocket::GetSocketName for unnamed sockets

2021-09-23 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-09-23T12:30:18+02:00
New Revision: 5685eb950da7c6901c8b264a3c93e8ea63b34d3d

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

LOG: [lldb] Fix DomainSocket::GetSocketName for unnamed sockets

getpeername will return addrlen = 2 (sizeof sa_family_t) for unnamed
sockets (those not assigned a name with bind(2)). This is typically true
for client sockets as well as those created by socketpair(2).

This GetSocketName used to crash for sockets which were connected to
these kinds of sockets. Now it returns an empty string.

Added: 


Modified: 
lldb/source/Host/posix/DomainSocket.cpp
lldb/unittests/Host/SocketTest.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/DomainSocket.cpp 
b/lldb/source/Host/posix/DomainSocket.cpp
index 7322b15200b4d..790458ee13d00 100644
--- a/lldb/source/Host/posix/DomainSocket.cpp
+++ b/lldb/source/Host/posix/DomainSocket.cpp
@@ -127,29 +127,34 @@ void DomainSocket::DeleteSocketFile(llvm::StringRef name) 
{
 }
 
 std::string DomainSocket::GetSocketName() const {
-  if (m_socket != kInvalidSocketValue) {
-struct sockaddr_un saddr_un;
-saddr_un.sun_family = AF_UNIX;
-socklen_t sock_addr_len = sizeof(struct sockaddr_un);
-if (::getpeername(m_socket, (struct sockaddr *)&saddr_un, &sock_addr_len) 
==
-0) {
-  std::string name(saddr_un.sun_path + GetNameOffset(),
-   sock_addr_len -
-   offsetof(struct sockaddr_un, sun_path) -
+  if (m_socket == kInvalidSocketValue)
+return "";
+
+  struct sockaddr_un saddr_un;
+  saddr_un.sun_family = AF_UNIX;
+  socklen_t sock_addr_len = sizeof(struct sockaddr_un);
+  if (::getpeername(m_socket, (struct sockaddr *)&saddr_un, &sock_addr_len) !=
+  0)
+return "";
+
+  if (sock_addr_len <= offsetof(struct sockaddr_un, sun_path))
+return ""; // Unnamed domain socket
+
+  llvm::StringRef name(saddr_un.sun_path + GetNameOffset(),
+   sock_addr_len - offsetof(struct sockaddr_un, sun_path) -
GetNameOffset());
-  if (name.back() == '\0') name.pop_back();
-  return name;
-}
-  }
-  return "";
+  if (name.back() == '\0')
+name = name.drop_back();
+
+  return name.str();
 }
 
 std::string DomainSocket::GetRemoteConnectionURI() const {
-  if (m_socket != kInvalidSocketValue) {
-return std::string(llvm::formatv(
-"{0}://{1}",
-GetNameOffset() == 0 ? "unix-connect" : "unix-abstract-connect",
-GetSocketName()));
-  }
-  return "";
+  std::string name = GetSocketName();
+  if (name.empty())
+return name;
+
+  return llvm::formatv(
+  "{0}://{1}",
+  GetNameOffset() == 0 ? "unix-connect" : "unix-abstract-connect", name);
 }

diff  --git a/lldb/unittests/Host/SocketTest.cpp 
b/lldb/unittests/Host/SocketTest.cpp
index 27d42f835718b..5593b7726919b 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -225,6 +225,8 @@ TEST_P(SocketTest, DomainGetConnectURI) {
   EXPECT_TRUE(UriParser::Parse(uri, scheme, hostname, port, path));
   EXPECT_EQ(scheme, "unix-connect");
   EXPECT_EQ(path, domain_path);
+
+  EXPECT_EQ(socket_b_up->GetRemoteConnectionURI(), "");
 }
 #endif
 



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

[Lldb-commits] [PATCH] D110269: Fix LLDB build on old Linux kernels

2021-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I'm pretty sure Caleb does not have commit access. Walter, would you do the 
honors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110269

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

2021-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

lgtm, modulo comments.




Comment at: lldb/test/API/functionalities/dlopen/TestDlopen.py:1
+import lldb
+from lldbsuite.test.decorators import *

I'd put this under `functionalities/load_after_attach/TestLoadAfterAttach.py` 
to better fit into the existing naming scheme, and to emphasize the attach 
aspect of the test (as that is what really makes this test special).



Comment at: lldb/test/API/functionalities/dlopen/b.cpp:1-4
+
+
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
+

What's up with the whitespace?



Comment at: lldb/test/API/functionalities/dlopen/main.cpp:11-17
+void sleepFor(int milliseconds) {
+ #if _WIN32
+Sleep(milliseconds);
+#else
+usleep(milliseconds*1000);
+#endif
+}

`std::this_thread::sleep_for`



Comment at: lldb/test/API/functionalities/dlopen/main.cpp:26
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");

emrekultursay wrote:
> labath wrote:
> > see dylib.h and the functions within (the inferior of TestLoadUnload uses 
> > them) for a windows-compatible way to load shared libraries.
> Since we are attaching to an already running process, which cannot find the 
> dynamic library unless I pass the full path to `dlopen()`. That's why I 
> couldn't use dylib.h (which doesn't add full path), but created my own 
> version here.
Ok, what I think you're saying is that when we run a process for attaching, we 
skip the code paths which set ((DY)LD_LIBRARY_)PATH, which is what makes the 
relative imports work. It shouldn't be too hard to extend the launch 
infrastructure to do that. Let's commit this in this form, and I'll do that as 
a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

2021-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

cool


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

https://reviews.llvm.org/D110027

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

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

2021-09-23 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good, just be careful about sentinels.




Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:571-578
-if (!value_regs.empty()) {
-  value_regs.push_back(LLDB_INVALID_REGNUM);
-  reg_info.value_regs = value_regs.data();
-}
-if (!invalidate_regs.empty()) {
-  invalidate_regs.push_back(LLDB_INVALID_REGNUM);
-  reg_info.invalidate_regs = invalidate_regs.data();

It looks like you're not doing it here.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4461-4464
+if (!reg_info.value_regs.empty())
+  reg_info.value_regs.push_back(LLDB_INVALID_REGNUM);
+if (!reg_info.invalidate_regs.empty())
+  reg_info.invalidate_regs.push_back(LLDB_INVALID_REGNUM);

Can we avoid pushing the sentinel here? I'd hope this can be done during the 
conversion to the "RegisterInfo" format...



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4602
+uint32_t local_regnum = it.index();
+RemoteRegisterInfo &remote_reg_info = it.value();
+// Use remote regnum if available, previous remote regnum + 1 when not.

i.e., add a `push(sentinel)` here.


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

https://reviews.llvm.org/D110025

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

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

2021-09-23 Thread Michał Górny via Phabricator via lldb-commits

mgorny marked 3 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4461-4464
+if (!reg_info.value_regs.empty())
+  reg_info.value_regs.push_back(LLDB_INVALID_REGNUM);
+if (!reg_info.invalidate_regs.empty())
+  reg_info.invalidate_regs.push_back(LLDB_INVALID_REGNUM);

labath wrote:
> Can we avoid pushing the sentinel here? I'd hope this can be done during the 
> conversion to the "RegisterInfo" format...
Yes, that makes sense and reduces code duplication even more. Thanks!



Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4602
+uint32_t local_regnum = it.index();
+RemoteRegisterInfo &remote_reg_info = it.value();
+// Use remote regnum if available, previous remote regnum + 1 when not.

labath wrote:
> i.e., add a `push(sentinel)` here.
Hmm, actually you made me realize I'm pushing empty arrays instead of `nullptr` 
sometimes. Let's fix that as well.


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

https://reviews.llvm.org/D110025

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

2021-09-23 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay updated this revision to Diff 374536.
emrekultursay marked 4 inline comments as done.
emrekultursay added a comment.

Renamed test to load_after_attach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/load_after_attach/Makefile
  lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
  lldb/test/API/functionalities/load_after_attach/b.cpp
  lldb/test/API/functionalities/load_after_attach/main.cpp

Index: lldb/test/API/functionalities/load_after_attach/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/main.cpp
@@ -0,0 +1,45 @@
+#ifdef _WIN32
+#include 
+#else
+#include 
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+// We do not use the dylib.h implementation, because
+// we need to pass full path to the dylib.
+void* dylib_open(const char* full_path) {
+#ifdef _WIN32
+  return LoadLibraryA(full_path);
+#else
+  return dlopen(full_path, RTLD_LAZY);
+#endif
+}
+
+int main(int argc, char* argv[]) {
+  assert(argc == 2 && "argv[1] must be the full path to lib_b library");
+  const char* dylib_full_path= argv[1];
+  printf("Using dylib at: %s\n", dylib_full_path);
+
+  // Wait until debugger is attached.
+  int main_thread_continue = 0;
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+std::this_thread::sleep_for(std::chrono::seconds(1));  // break here
+if (main_thread_continue) {
+  break;
+}
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* dylib_handle = dylib_open(dylib_full_path);
+  assert(dylib_handle && "dlopen failed");
+
+  return i; // break after dlopen
+}
Index: lldb/test/API/functionalities/load_after_attach/b.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/b.cpp
@@ -0,0 +1 @@
+int LLDB_DYLIB_EXPORT b_function() { return 500; }
Index: lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
===
--- /dev/null
+++ lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -0,0 +1,63 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfRemote
+def test_load_after_attach(self):
+self.build()
+
+ctx = self.platformContext
+lib_name = ctx.shlib_prefix + 'lib_b.' + ctx.shlib_extension
+
+exe = self.getBuildArtifact("a.out")
+lib = self.getBuildArtifact(lib_name)
+
+# Spawn a new process.
+# use realpath to workaround llvm.org/pr48376
+# Pass path to solib for dlopen to properly locate the library.
+popen = self.spawnSubprocess(os.path.realpath(exe), args = [os.path.realpath(lib)])
+pid = popen.pid
+
+# Attach to the spawned process.
+self.runCmd("process attach -p " + str(pid))
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# Continue until first breakpoint.
+breakpoint1 = self.target().BreakpointCreateBySourceRegex(
+"// break here", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint1.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint1)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list does not contain liblib_b before dlopen.
+self.match(
+"image list",
+patterns = [lib_name],
+matching = False,
+msg = lib_name + " should not have been in image list")
+
+# Change a variable to escape the loop
+self.runCmd("expression main_thread_continue = 1")
+
+# Continue so that dlopen is called.
+breakpoint2 = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.cpp"))
+self.assertEqual(breakpoint2.GetNumResolvedLocations(), 1)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint2)
+self.assertEqual(len(stopped_threads), 1)
+
+# Check that image list contains liblib_b after dlopen.
+self.match(
+"image list",
+patterns = [lib_name],
+matching = True,
+msg = lib_name + " missing in image list")
+
Index: lldb/test/API/functionalities/load_after_attach/Makefile

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

2021-09-23 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay added a comment.

Done. Can you also submit it please?




Comment at: lldb/test/API/functionalities/dlopen/main.cpp:26
+  // dlopen the 'liblib_b.so' shared library.
+  void* h = dlopen(solib, RTLD_LAZY);
+  assert(h && "dlopen failed?");

labath wrote:
> emrekultursay wrote:
> > labath wrote:
> > > see dylib.h and the functions within (the inferior of TestLoadUnload uses 
> > > them) for a windows-compatible way to load shared libraries.
> > Since we are attaching to an already running process, which cannot find the 
> > dynamic library unless I pass the full path to `dlopen()`. That's why I 
> > couldn't use dylib.h (which doesn't add full path), but created my own 
> > version here.
> Ok, what I think you're saying is that when we run a process for attaching, 
> we skip the code paths which set ((DY)LD_LIBRARY_)PATH, which is what makes 
> the relative imports work. It shouldn't be too hard to extend the launch 
> infrastructure to do that. Let's commit this in this form, and I'll do that 
> as a follow-up.
Yes. That SGTM. Thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [lldb] b03e701 - [lldb] [gdb-remote] Refactor getting remote regs to use local vector

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T17:21:55+02:00
New Revision: b03e701c145365ba339657ead54a2e0cc5c02776

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

LOG: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

Refactor remote register getters to collect them into a local
std::vector rather than adding them straight into DynamicRegisterInfo.
The purpose of this change is to lay groundwork for switching value_regs
and invalidate_regs to use local LLDB register numbers rather than
remote numbers.

Differential Revision: https://reviews.llvm.org/D110025

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 7dd1f340cb331..5b49c0c2c652a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -454,7 +454,7 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) 
{
 return;
 
   char packet[128];
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
+  std::vector registers;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
StringExtractorGDBRemote::eResponse;
@@ -470,53 +470,26 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   if (response_type == StringExtractorGDBRemote::eResponse) {
 llvm::StringRef name;
 llvm::StringRef value;
-ConstString reg_name;
-ConstString alt_name;
-ConstString set_name;
-std::vector value_regs;
-std::vector invalidate_regs;
-std::vector dwarf_opcode_bytes;
-RegisterInfo reg_info = {
-nullptr,   // Name
-nullptr,   // Alt name
-0, // byte size
-reg_offset,// offset
-eEncodingUint, // encoding
-eFormatHex,// format
-{
-LLDB_INVALID_REGNUM, // eh_frame reg num
-LLDB_INVALID_REGNUM, // DWARF reg num
-LLDB_INVALID_REGNUM, // generic reg num
-reg_num, // process plugin reg num
-reg_num  // native register number
-},
-nullptr,
-nullptr,
-nullptr, // Dwarf expression opcode bytes pointer
-0// Dwarf expression opcode bytes length
-};
+RemoteRegisterInfo reg_info;
 
 while (response.GetNameColonValue(name, value)) {
   if (name.equals("name")) {
-reg_name.SetString(value);
+reg_info.name.SetString(value);
   } else if (name.equals("alt-name")) {
-alt_name.SetString(value);
+reg_info.alt_name.SetString(value);
   } else if (name.equals("bitsize")) {
-value.getAsInteger(0, reg_info.byte_size);
-reg_info.byte_size /= CHAR_BIT;
+reg_info.byte_size =
+StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
   } else if (name.equals("offset")) {
-if (value.getAsInteger(0, reg_offset))
-  reg_offset = UINT32_MAX;
+reg_info.byte_offset =
+StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
   } else if (name.equals("encoding")) {
 const Encoding encoding = Args::StringToEncoding(value);
 if (encoding != eEncodingInvalid)
   reg_info.encoding = encoding;
   } else if (name.equals("format")) {
-Format format = eFormatInvalid;
-if (OptionArgParser::ToFormat(value.str().c_str(), format, nullptr)
+if (!OptionArgParser::ToFormat(value.str().c_str(), 
reg_info.format, nullptr)
 .Success())
-  reg_info.format = format;
-else {
   reg_info.format =
   llvm::StringSwitch(value)
   .Case("binary", eFormatBinary)
@@ -533,59 +506,36 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
-}
   } else if (name.equals("set")) {
-set_name.SetString(value);
+reg_info.set_name.SetString(value);
   } else if (name.equals("gcc") || name.equals("ehframe")) {
-if (value.getAsInteger(0, reg_info.kinds[eRegisterKindEHFrame]))
-  reg_info.kinds[eRegisterKindEHF

[Lldb-commits] [lldb] 6fbed33 - [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T17:21:56+02:00
New Revision: 6fbed33d4a7de2229c40e6318f223092d3a23848

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

LOG: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

Switch the gdb-remote client logic to use local (LLDB) register numbers
in value_regs/invalidate_regs rather than remote regnos. This involves
translating regnos received from lldb-server.

Differential Revision: https://reviews.llvm.org/D110027

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index 2755e5f93d5ad..721a03745c3ef 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -665,9 +665,7 @@ void DynamicRegisterInfo::ConfigureOffsets() {
   if (reg.byte_offset == LLDB_INVALID_INDEX32) {
 uint32_t value_regnum = reg.value_regs[0];
 if (value_regnum != LLDB_INVALID_INDEX32)
-  reg.byte_offset =
-  GetRegisterInfoAtIndex(remote_to_local_regnum_map[value_regnum])
-  ->byte_offset;
+  reg.byte_offset = GetRegisterInfoAtIndex(value_regnum)->byte_offset;
   }
 }
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index df5d052d2e33b..92a5227a9a3a8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -253,7 +253,7 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const 
RegisterInfo *reg_info) {
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *prim_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
+GetRegisterInfo(eRegisterKindLLDB, prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
 else {
@@ -384,7 +384,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *value_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, reg);
+GetRegisterInfo(eRegisterKindLLDB, reg);
 if (value_reg_info == nullptr)
   success = false;
 else
@@ -405,7 +405,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx])
 SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-   eRegisterKindProcessPlugin, reg),
+   eRegisterKindLLDB, reg),
false);
 }
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5b49c0c2c652a..eec0c55e75bed 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4591,13 +4591,35 @@ void ProcessGDBRemote::AddRemoteRegisters(
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
+  std::map remote_to_local_map;
   uint32_t remote_regnum = 0;
+  for (auto it : llvm::enumerate(registers)) {
+RemoteRegisterInfo &remote_reg_info = it.value();
+
+// Assign successive remote regnums if missing.
+if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
+  remote_reg_info.regnum_remote = remote_regnum;
+
+// Create a mapping from remote to local regnos.
+remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
+
+remote_regnum = remote_reg_info.regnum_remote + 1;
+  }
+
   for (auto it : llvm::enumerate(registers)) {
 uint32_t local_regnum = it.index();
 RemoteRegisterInfo &remote_reg_info = it.value();
-// Use remote regnum if available, previous remote regnum + 1 when not.
-if (remote_reg_info.regnum_remote != LLDB_INVALID_REGNUM)
-  remote_regnum = remote_reg_info.regnum_remote;
+
+auto proc_to_lldb = [&remote_to_local_map](uint32_t process_regnum) {
+  auto lldb_regit = remote_to_local_map.find(process_regnum);
+

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

2021-09-23 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked 2 inline comments as done.
Closed by commit rGb03e701c1453: [lldb] [gdb-remote] Refactor getting remote 
regs to use local vector (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110025?vs=373530&id=374566#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110025

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -44,6 +44,23 @@
 }
 namespace process_gdb_remote {
 
+struct RemoteRegisterInfo {
+  ConstString name;
+  ConstString alt_name;
+  ConstString set_name;
+  uint32_t byte_size = LLDB_INVALID_INDEX32;
+  uint32_t byte_offset = LLDB_INVALID_INDEX32;
+  lldb::Encoding encoding = lldb::eEncodingUint;
+  lldb::Format format = lldb::eFormatHex;
+  uint32_t regnum_dwarf = LLDB_INVALID_REGNUM;
+  uint32_t regnum_ehframe = LLDB_INVALID_REGNUM;
+  uint32_t regnum_generic = LLDB_INVALID_REGNUM;
+  uint32_t regnum_remote = LLDB_INVALID_REGNUM;
+  std::vector value_regs;
+  std::vector invalidate_regs;
+  std::vector dwarf_opcode_bytes;
+};
+
 class ThreadGDBRemote;
 
 class ProcessGDBRemote : public Process,
@@ -394,11 +411,14 @@
 
   DynamicLoader *GetDynamicLoader() override;
 
-  bool GetGDBServerRegisterInfoXMLAndProcess(ArchSpec &arch_to_use,
- std::string xml_filename,
- uint32_t &cur_reg_remote,
- uint32_t &cur_reg_local);
+  bool GetGDBServerRegisterInfoXMLAndProcess(
+ArchSpec &arch_to_use, std::string xml_filename,
+std::vector ®isters);
 
+  // Convert RemoteRegisterInfos into RegisterInfos and add to the dynamic
+  // register list.
+  void AddRemoteRegisters(std::vector ®isters,
+  const ArchSpec &arch_to_use);
   // Query remote GDBServer for register information
   bool GetGDBServerRegisterInfo(ArchSpec &arch);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -454,7 +454,7 @@
 return;
 
   char packet[128];
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
+  std::vector registers;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
StringExtractorGDBRemote::eResponse;
@@ -470,53 +470,26 @@
   if (response_type == StringExtractorGDBRemote::eResponse) {
 llvm::StringRef name;
 llvm::StringRef value;
-ConstString reg_name;
-ConstString alt_name;
-ConstString set_name;
-std::vector value_regs;
-std::vector invalidate_regs;
-std::vector dwarf_opcode_bytes;
-RegisterInfo reg_info = {
-nullptr,   // Name
-nullptr,   // Alt name
-0, // byte size
-reg_offset,// offset
-eEncodingUint, // encoding
-eFormatHex,// format
-{
-LLDB_INVALID_REGNUM, // eh_frame reg num
-LLDB_INVALID_REGNUM, // DWARF reg num
-LLDB_INVALID_REGNUM, // generic reg num
-reg_num, // process plugin reg num
-reg_num  // native register number
-},
-nullptr,
-nullptr,
-nullptr, // Dwarf expression opcode bytes pointer
-0// Dwarf expression opcode bytes length
-};
+RemoteRegisterInfo reg_info;
 
 while (response.GetNameColonValue(name, value)) {
   if (name.equals("name")) {
-reg_name.SetString(value);
+reg_info.name.SetString(value);
   } else if (name.equals("alt-name")) {
-alt_name.SetString(value);
+reg_info.alt_name.SetString(value);
   } else if (name.equals("bitsize")) {
-value.getAsInteger(0, reg_info.byte_size);
-reg_info.byte_size /= CHAR_BIT;
+reg_info.byte_size =
+StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
   } else if (name.equals("offset")) {
-if (value.getAsInteger(0, reg_offset))
-  reg_offset = UINT32_MAX;
+reg_info.byte_offset =
+StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
   } else if

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

2021-09-23 Thread Michał Górny via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6fbed33d4a7d: [lldb] [gdb-remote] Use local regnos for 
value_regs/invalidate_regs (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110027?vs=373814&id=374567#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110027

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4591,13 +4591,35 @@
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
+  std::map remote_to_local_map;
   uint32_t remote_regnum = 0;
+  for (auto it : llvm::enumerate(registers)) {
+RemoteRegisterInfo &remote_reg_info = it.value();
+
+// Assign successive remote regnums if missing.
+if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
+  remote_reg_info.regnum_remote = remote_regnum;
+
+// Create a mapping from remote to local regnos.
+remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
+
+remote_regnum = remote_reg_info.regnum_remote + 1;
+  }
+
   for (auto it : llvm::enumerate(registers)) {
 uint32_t local_regnum = it.index();
 RemoteRegisterInfo &remote_reg_info = it.value();
-// Use remote regnum if available, previous remote regnum + 1 when not.
-if (remote_reg_info.regnum_remote != LLDB_INVALID_REGNUM)
-  remote_regnum = remote_reg_info.regnum_remote;
+
+auto proc_to_lldb = [&remote_to_local_map](uint32_t process_regnum) {
+  auto lldb_regit = remote_to_local_map.find(process_regnum);
+  return lldb_regit != remote_to_local_map.end() ? lldb_regit->second
+ : LLDB_INVALID_REGNUM;
+};
+
+llvm::transform(remote_reg_info.value_regs,
+remote_reg_info.value_regs.begin(), proc_to_lldb);
+llvm::transform(remote_reg_info.invalidate_regs,
+remote_reg_info.invalidate_regs.begin(), proc_to_lldb);
 
 auto regs_with_sentinel = [](std::vector &vec) -> uint32_t * {
   if (!vec.empty()) {
@@ -4612,7 +4634,8 @@
   remote_reg_info.byte_size, remote_reg_info.byte_offset,
   remote_reg_info.encoding, remote_reg_info.format,
   {remote_reg_info.regnum_ehframe, remote_reg_info.regnum_dwarf,
-   remote_reg_info.regnum_generic, remote_regnum++, local_regnum},
+   remote_reg_info.regnum_generic, remote_reg_info.regnum_remote,
+   local_regnum},
   regs_with_sentinel(remote_reg_info.value_regs),
   regs_with_sentinel(remote_reg_info.invalidate_regs),
   !remote_reg_info.dwarf_opcode_bytes.empty()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -253,7 +253,7 @@
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *prim_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
+GetRegisterInfo(eRegisterKindLLDB, prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
 else {
@@ -384,7 +384,7 @@
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *value_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, reg);
+GetRegisterInfo(eRegisterKindLLDB, reg);
 if (value_reg_info == nullptr)
   success = false;
 else
@@ -405,7 +405,7 @@
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx])
 SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-   eRegisterKindProcessPlugin, reg),
+   eRegisterKindLLDB, reg),
false);
 }
 
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -665,9 +665,7 @@
   if (reg.byte_offset == LLDB

[Lldb-commits] [lldb] bcb6b97 - Revert "[lldb] [gdb-remote] Refactor getting remote regs to use local vector"

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T18:17:09+02:00
New Revision: bcb6b97cde84b6acd67d5551302683234c09337c

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

LOG: Revert "[lldb] [gdb-remote] Refactor getting remote regs to use local 
vector"

This reverts commit b03e701c145365ba339657ead54a2e0cc5c02776.  This is
causing regressions when XML support is disabled.

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5b49c0c2c652a..7dd1f340cb331 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -454,7 +454,7 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) 
{
 return;
 
   char packet[128];
-  std::vector registers;
+  uint32_t reg_offset = LLDB_INVALID_INDEX32;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
StringExtractorGDBRemote::eResponse;
@@ -470,26 +470,53 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   if (response_type == StringExtractorGDBRemote::eResponse) {
 llvm::StringRef name;
 llvm::StringRef value;
-RemoteRegisterInfo reg_info;
+ConstString reg_name;
+ConstString alt_name;
+ConstString set_name;
+std::vector value_regs;
+std::vector invalidate_regs;
+std::vector dwarf_opcode_bytes;
+RegisterInfo reg_info = {
+nullptr,   // Name
+nullptr,   // Alt name
+0, // byte size
+reg_offset,// offset
+eEncodingUint, // encoding
+eFormatHex,// format
+{
+LLDB_INVALID_REGNUM, // eh_frame reg num
+LLDB_INVALID_REGNUM, // DWARF reg num
+LLDB_INVALID_REGNUM, // generic reg num
+reg_num, // process plugin reg num
+reg_num  // native register number
+},
+nullptr,
+nullptr,
+nullptr, // Dwarf expression opcode bytes pointer
+0// Dwarf expression opcode bytes length
+};
 
 while (response.GetNameColonValue(name, value)) {
   if (name.equals("name")) {
-reg_info.name.SetString(value);
+reg_name.SetString(value);
   } else if (name.equals("alt-name")) {
-reg_info.alt_name.SetString(value);
+alt_name.SetString(value);
   } else if (name.equals("bitsize")) {
-reg_info.byte_size =
-StringConvert::ToUInt32(value.data(), 0, 0) / CHAR_BIT;
+value.getAsInteger(0, reg_info.byte_size);
+reg_info.byte_size /= CHAR_BIT;
   } else if (name.equals("offset")) {
-reg_info.byte_offset =
-StringConvert::ToUInt32(value.data(), LLDB_INVALID_INDEX32, 0);
+if (value.getAsInteger(0, reg_offset))
+  reg_offset = UINT32_MAX;
   } else if (name.equals("encoding")) {
 const Encoding encoding = Args::StringToEncoding(value);
 if (encoding != eEncodingInvalid)
   reg_info.encoding = encoding;
   } else if (name.equals("format")) {
-if (!OptionArgParser::ToFormat(value.str().c_str(), 
reg_info.format, nullptr)
+Format format = eFormatInvalid;
+if (OptionArgParser::ToFormat(value.str().c_str(), format, nullptr)
 .Success())
+  reg_info.format = format;
+else {
   reg_info.format =
   llvm::StringSwitch(value)
   .Case("binary", eFormatBinary)
@@ -506,36 +533,59 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
+}
   } else if (name.equals("set")) {
-reg_info.set_name.SetString(value);
+set_name.SetString(value);
   } else if (name.equals("gcc") || name.equals("ehframe")) {
-reg_info.regnum_ehframe =
-StringConvert::ToUInt32(value.data(), LLDB_INVALID_REGNUM, 0);
+if (value.getAsInteger(0, reg_info.kinds[eRegisterKindEHFrame]))
+  reg_info.kinds[eRegisterKindEHFrame] = LLDB_INVALID_REGNUM;
   } else if (name.equals("dwarf")) {
-reg_info

[Lldb-commits] [lldb] 12504f5 - Revert "[lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs"

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T18:17:09+02:00
New Revision: 12504f50729a338fb37c1c1863e7125b607e11d7

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

LOG: Revert "[lldb] [gdb-remote] Use local regnos for 
value_regs/invalidate_regs"

This reverts commit 6fbed33d4a7de2229c40e6318f223092d3a23848.
The prerequisite commit is causing regressions.

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index 721a03745c3ef..2755e5f93d5ad 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -665,7 +665,9 @@ void DynamicRegisterInfo::ConfigureOffsets() {
   if (reg.byte_offset == LLDB_INVALID_INDEX32) {
 uint32_t value_regnum = reg.value_regs[0];
 if (value_regnum != LLDB_INVALID_INDEX32)
-  reg.byte_offset = GetRegisterInfoAtIndex(value_regnum)->byte_offset;
+  reg.byte_offset =
+  GetRegisterInfoAtIndex(remote_to_local_regnum_map[value_regnum])
+  ->byte_offset;
   }
 }
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 92a5227a9a3a8..df5d052d2e33b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -253,7 +253,7 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const 
RegisterInfo *reg_info) {
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *prim_reg_info =
-GetRegisterInfo(eRegisterKindLLDB, prim_reg);
+GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
 else {
@@ -384,7 +384,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *value_reg_info =
-GetRegisterInfo(eRegisterKindLLDB, reg);
+GetRegisterInfo(eRegisterKindProcessPlugin, reg);
 if (value_reg_info == nullptr)
   success = false;
 else
@@ -405,7 +405,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx])
 SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-   eRegisterKindLLDB, reg),
+   eRegisterKindProcessPlugin, reg),
false);
 }
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index eec0c55e75bed..5b49c0c2c652a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4591,35 +4591,13 @@ void ProcessGDBRemote::AddRemoteRegisters(
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
-  std::map remote_to_local_map;
   uint32_t remote_regnum = 0;
-  for (auto it : llvm::enumerate(registers)) {
-RemoteRegisterInfo &remote_reg_info = it.value();
-
-// Assign successive remote regnums if missing.
-if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
-  remote_reg_info.regnum_remote = remote_regnum;
-
-// Create a mapping from remote to local regnos.
-remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
-
-remote_regnum = remote_reg_info.regnum_remote + 1;
-  }
-
   for (auto it : llvm::enumerate(registers)) {
 uint32_t local_regnum = it.index();
 RemoteRegisterInfo &remote_reg_info = it.value();
-
-auto proc_to_lldb = [&remote_to_local_map](uint32_t process_regnum) {
-  auto lldb_regit = remote_to_local_map.find(process_regnum);
-  return lldb_regit != remote_to_local_map.end() ? lldb_regit->second
- : LLDB_INVALID_REGNUM;
-};
-
-llvm::transform(remote_reg_info.value_regs,
-remote_reg_info.value_regs.begin(), proc_to_lldb);
-llvm::transform(remote_reg_info.invalid

[Lldb-commits] [PATCH] D109908: [lldb] Show fix-it applied even if expression didn't evaluate succesfully

2021-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

My bad, I thought you wanted to print the fix-its if the 'fixed' expression 
failed to *parse* again, but this is about non-parsing errors. Ignore my other 
points, they were suggestions how to implemented what I thought this is about.

LGTM now. Just have some nits about the test but those don't need another round 
of review. Thanks for fixing this!




Comment at: lldb/test/API/commands/expression/fixits/TestFixIts.py:86
+def test_with_target_error_applies_fixit(self):
+""" Check that applying a Fix-it which fails to execute correctly 
still 
+ prints that the Fix-it was applied. """

Trailing white space here.



Comment at: lldb/test/API/commands/expression/fixits/TestFixIts.py:96
+result = self.dbg.GetCommandInterpreter().HandleCommand("expression 
null_pointer.first", ret_val)
+self.assertEqual(result, lldb.eReturnStatusFailed, ret_val.GetError())
+

I think you meant `ret_val.GetOutput()` here as message gets printed if `result 
!= eReturnStatusFailed`?

I anyway think you can just minimize everything from line 94 onwards with the 
normal `expect` routine:
```
lang=python
self.expect("expression -- null_pointer.first", error=True, substrs=[
"Fix-it applied, fixed expression was:", "null_pointer->first"])
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109908

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

Re: [Lldb-commits] [PATCH] D110269: Fix LLDB build on old Linux kernels

2021-09-23 Thread Walter via lldb-commits

I will! Thanks!

Il Gio 23 Set 2021, 12:35 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> labath added a comment.
>
> I'm pretty sure Caleb does not have commit access. Walter, would you do
> the honors?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D110269/new/
>
> https://reviews.llvm.org/D110269
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c223299 - [lldb] Add a C language REPL to test LLDB's REPL infrastructure

2021-09-23 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-09-23T19:31:02+02:00
New Revision: c22329972f02f9d51e2f9ea54d9075a4a808ffde

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

LOG: [lldb] Add a C language REPL to test LLDB's REPL infrastructure

LLDB has a bunch of code that implements REPL support, but all that code is
unreachable as no language in master currently has an implemented REPL backend.
The only REPL that exists is in the downstream Swift fork. All patches for this
generic REPL code therefore also only have tests downstream which is clearly not
a good situation.

This patch implements a basic C language REPL on top of LLDB's REPL framework.
Beside implementing the REPL interface and hooking it up into the plugin
manager, the only other small part of this patch is making the `--language` flag
of the expression command compatible with the `--repl` flag. The `--repl` flag
uses the value of `--language` to see which REPL should be started, but right
now the `--language` flag is only available in OptionGroups 1 and 2, but not in
OptionGroup 3 where the `--repl` flag is declared.

The REPL currently can currently only start if a running target exists. I'll add
the 'create and run a dummy executable' logic from Swift (which is requires when
doing `lldb --repl`) when I have time to translate all this logic to something
that will work with Clang.

I should point out that the REPL currently uses the C expression parser's
approach to persistent variables where only result variables and the ones
starting with a '$' are transferred between expressions. I'll fix that in a
follow up patch. Also the REPL currently doesn't work in a non-interactive
terminal. This seems to be fixed in the Swift fork, so I assume one of our many
REPL downstream changes addresses the issue.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D87281

Added: 
lldb/source/Plugins/REPL/CMakeLists.txt
lldb/source/Plugins/REPL/Clang/CMakeLists.txt
lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
lldb/source/Plugins/REPL/Clang/ClangREPL.h
lldb/test/API/repl/clang/Makefile
lldb/test/API/repl/clang/TestClangREPL.py
lldb/test/API/repl/clang/main.c

Modified: 
lldb/source/Commands/Options.td
lldb/source/Plugins/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 67cfc60f9d1b5..83df2ac22c578 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -355,7 +355,7 @@ let Command = "expression" in {
 Desc<"When specified, debug the JIT code by setting a breakpoint on the "
 "first instruction and forcing breakpoints to not be ignored (-i0) and no "
 "unwinding to happen on error (-u0).">;
-  def expression_options_language : Option<"language", "l">, Groups<[1,2]>,
+  def expression_options_language : Option<"language", "l">, Groups<[1,2,3]>,
 Arg<"Language">, Desc<"Specifies the Language to use when parsing the "
 "expression.  If not set the target.language setting is used.">;
   def expression_options_apply_fixits : Option<"apply-fixits", "X">,

diff  --git a/lldb/source/Plugins/CMakeLists.txt 
b/lldb/source/Plugins/CMakeLists.txt
index 9181a4e47675f..84cc065c3ca5a 100644
--- a/lldb/source/Plugins/CMakeLists.txt
+++ b/lldb/source/Plugins/CMakeLists.txt
@@ -14,6 +14,7 @@ add_subdirectory(ObjectFile)
 add_subdirectory(OperatingSystem)
 add_subdirectory(Platform)
 add_subdirectory(Process)
+add_subdirectory(REPL)
 add_subdirectory(ScriptInterpreter)
 add_subdirectory(StructuredData)
 add_subdirectory(SymbolFile)

diff  --git a/lldb/source/Plugins/REPL/CMakeLists.txt 
b/lldb/source/Plugins/REPL/CMakeLists.txt
new file mode 100644
index 0..17c40aee44cc2
--- /dev/null
+++ b/lldb/source/Plugins/REPL/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(Clang)

diff  --git a/lldb/source/Plugins/REPL/Clang/CMakeLists.txt 
b/lldb/source/Plugins/REPL/Clang/CMakeLists.txt
new file mode 100644
index 0..b995071235815
--- /dev/null
+++ b/lldb/source/Plugins/REPL/Clang/CMakeLists.txt
@@ -0,0 +1,17 @@
+add_lldb_library(lldbPluginClangREPL PLUGIN
+  ClangREPL.cpp
+
+  LINK_LIBS
+lldbCore
+lldbDataFormatters
+lldbHost
+lldbSymbol
+lldbTarget
+lldbUtility
+lldbPluginClangCommon
+lldbPluginCPPRuntime
+lldbPluginTypeSystemClang
+
+  LINK_COMPONENTS
+Support
+)

diff  --git a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp 
b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
new file mode 100644
index 0..5060dbb7ddba8
--- /dev/null
+++ b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
@@ -0,0 +1,102 @@
+//===-- ClangREPL.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache Lice

[Lldb-commits] [PATCH] D87281: [lldb] Add a C language REPL to test LLDB's REPL infrastructure

2021-09-23 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc22329972f02: [lldb] Add a C language REPL to test 
LLDB's REPL infrastructure (authored by teemperor).
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D87281?vs=323168&id=374615#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87281

Files:
  lldb/source/Commands/Options.td
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/REPL/CMakeLists.txt
  lldb/source/Plugins/REPL/Clang/CMakeLists.txt
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.h
  lldb/test/API/repl/clang/Makefile
  lldb/test/API/repl/clang/TestClangREPL.py
  lldb/test/API/repl/clang/main.c

Index: lldb/test/API/repl/clang/main.c
===
--- /dev/null
+++ lldb/test/API/repl/clang/main.c
@@ -0,0 +1,3 @@
+int main(int argc, char **argv) {
+  return 0;
+}
Index: lldb/test/API/repl/clang/TestClangREPL.py
===
--- /dev/null
+++ lldb/test/API/repl/clang/TestClangREPL.py
@@ -0,0 +1,54 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestCase(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def expect_repl(self, expr, substrs=[]):
+""" Evaluates the expression in the REPL and verifies that the list
+of substrs is in the REPL output."""
+# Only single line expressions supported.
+self.assertNotIn("\n", expr)
+self.child.send(expr + "\n")
+for substr in substrs:
+self.child.expect_exact(substr)
+# Look for the start of the next REPL input line.
+self.current_repl_line_number += 1
+self.child.expect_exact(str(self.current_repl_line_number) + ">")
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfEditlineSupportMissing
+def test_basic_completion(self):
+"""Test that we can complete a simple multiline expression"""
+self.build()
+self.current_repl_line_number = 1
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+# Try launching the REPL before we have a running target.
+self.expect("expression --repl -l c --", substrs=["REPL requires a running target process."])
+
+self.expect("b main", substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason = breakpoint 1"])
+
+# Start the REPL.
+self.child.send("expression --repl -l c --\n")
+self.child.expect_exact("1>")
+
+# Try evaluating a simple expression.
+self.expect_repl("3 + 3", substrs=["(int) $0 = 6"])
+
+# Try declaring a persistent variable.
+self.expect_repl("long $persistent = 7; 5",
+ substrs=["(int) $1 = 5",
+  "(long) $persistent = 7"])
+
+# Try using the persistent variable from before.
+self.expect_repl("$persistent + 10",
+ substrs=["(long) $2 = 17"])
+
+self.quit()
Index: lldb/test/API/repl/clang/Makefile
===
--- /dev/null
+++ lldb/test/API/repl/clang/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
Index: lldb/source/Plugins/REPL/Clang/ClangREPL.h
===
--- /dev/null
+++ lldb/source/Plugins/REPL/Clang/ClangREPL.h
@@ -0,0 +1,65 @@
+//===-- ClangREPL.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_PLUGINS_REPL_CLANG_CLANGREPL_H
+#define LLDB_SOURCE_PLUGINS_REPL_CLANG_CLANGREPL_H
+
+#include "lldb/Expression/REPL.h"
+
+namespace lldb_private {
+/// Implements a Clang-based REPL for C languages on top of LLDB's REPL
+/// framework.
+class ClangREPL : public REPL {
+public:
+  ClangREPL(lldb::LanguageType language, Target &target);
+
+  ~ClangREPL() override;
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::REPLSP CreateInstance(Status &error, lldb::LanguageType language,
+ Debugger *debugger, Target *target,
+ const char *repl_options);
+
+  static lldb_private::ConstString GetPluginNameStatic() {
+return ConstString("ClangREPL");
+  }
+
+protected:
+  Status DoIni

[Lldb-commits] [lldb] cc3c788 - [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T20:02:01+02:00
New Revision: cc3c788ad23636d16f1db2ae859315628783b0e8

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

LOG: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

Switch the gdb-remote client logic to use local (LLDB) register numbers
in value_regs/invalidate_regs rather than remote regnos. This involves
translating regnos received from lldb-server.

Differential Revision: https://reviews.llvm.org/D110027

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index 2755e5f93d5ad..721a03745c3ef 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -665,9 +665,7 @@ void DynamicRegisterInfo::ConfigureOffsets() {
   if (reg.byte_offset == LLDB_INVALID_INDEX32) {
 uint32_t value_regnum = reg.value_regs[0];
 if (value_regnum != LLDB_INVALID_INDEX32)
-  reg.byte_offset =
-  GetRegisterInfoAtIndex(remote_to_local_regnum_map[value_regnum])
-  ->byte_offset;
+  reg.byte_offset = GetRegisterInfoAtIndex(value_regnum)->byte_offset;
   }
 }
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index df5d052d2e33b..92a5227a9a3a8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -253,7 +253,7 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const 
RegisterInfo *reg_info) {
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *prim_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, prim_reg);
+GetRegisterInfo(eRegisterKindLLDB, prim_reg);
 if (prim_reg_info == nullptr)
   success = false;
 else {
@@ -384,7 +384,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
 // We have a valid primordial register as our constituent. Grab the
 // corresponding register info.
 const RegisterInfo *value_reg_info =
-GetRegisterInfo(eRegisterKindProcessPlugin, reg);
+GetRegisterInfo(eRegisterKindLLDB, reg);
 if (value_reg_info == nullptr)
   success = false;
 else
@@ -405,7 +405,7 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const 
RegisterInfo *reg_info,
reg != LLDB_INVALID_REGNUM;
reg = reg_info->invalidate_regs[++idx])
 SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(
-   eRegisterKindProcessPlugin, reg),
+   eRegisterKindLLDB, reg),
false);
 }
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 304fda0bc6d83..6209a45c4c784 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4588,13 +4588,35 @@ void ProcessGDBRemote::AddRemoteRegisters(
   // ABI is also potentially incorrect.
   ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
 
+  std::map remote_to_local_map;
   uint32_t remote_regnum = 0;
+  for (auto it : llvm::enumerate(registers)) {
+RemoteRegisterInfo &remote_reg_info = it.value();
+
+// Assign successive remote regnums if missing.
+if (remote_reg_info.regnum_remote == LLDB_INVALID_REGNUM)
+  remote_reg_info.regnum_remote = remote_regnum;
+
+// Create a mapping from remote to local regnos.
+remote_to_local_map[remote_reg_info.regnum_remote] = it.index();
+
+remote_regnum = remote_reg_info.regnum_remote + 1;
+  }
+
   for (auto it : llvm::enumerate(registers)) {
 uint32_t local_regnum = it.index();
 RemoteRegisterInfo &remote_reg_info = it.value();
-// Use remote regnum if available, previous remote regnum + 1 when not.
-if (remote_reg_info.regnum_remote != LLDB_INVALID_REGNUM)
-  remote_regnum = remote_reg_info.regnum_remote;
+
+auto proc_to_lldb = [&remote_to_local_map](uint32_t process_regnum) {
+  auto lldb_regit = remote_to_local_map.find(process_regnum);
+

[Lldb-commits] [lldb] fa45650 - [lldb] [gdb-remote] Refactor getting remote regs to use local vector

2021-09-23 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-09-23T20:02:00+02:00
New Revision: fa456505b80b0cf83647a1b26713e4d3b38eccc2

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

LOG: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

Refactor remote register getters to collect them into a local
std::vector rather than adding them straight into DynamicRegisterInfo.
The purpose of this change is to lay groundwork for switching value_regs
and invalidate_regs to use local LLDB register numbers rather than
remote numbers.

Differential Revision: https://reviews.llvm.org/D110025

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 7dd1f340cb331..304fda0bc6d83 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -454,7 +454,7 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) 
{
 return;
 
   char packet[128];
-  uint32_t reg_offset = LLDB_INVALID_INDEX32;
+  std::vector registers;
   uint32_t reg_num = 0;
   for (StringExtractorGDBRemote::ResponseType response_type =
StringExtractorGDBRemote::eResponse;
@@ -470,53 +470,25 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   if (response_type == StringExtractorGDBRemote::eResponse) {
 llvm::StringRef name;
 llvm::StringRef value;
-ConstString reg_name;
-ConstString alt_name;
-ConstString set_name;
-std::vector value_regs;
-std::vector invalidate_regs;
-std::vector dwarf_opcode_bytes;
-RegisterInfo reg_info = {
-nullptr,   // Name
-nullptr,   // Alt name
-0, // byte size
-reg_offset,// offset
-eEncodingUint, // encoding
-eFormatHex,// format
-{
-LLDB_INVALID_REGNUM, // eh_frame reg num
-LLDB_INVALID_REGNUM, // DWARF reg num
-LLDB_INVALID_REGNUM, // generic reg num
-reg_num, // process plugin reg num
-reg_num  // native register number
-},
-nullptr,
-nullptr,
-nullptr, // Dwarf expression opcode bytes pointer
-0// Dwarf expression opcode bytes length
-};
+RemoteRegisterInfo reg_info;
 
 while (response.GetNameColonValue(name, value)) {
   if (name.equals("name")) {
-reg_name.SetString(value);
+reg_info.name.SetString(value);
   } else if (name.equals("alt-name")) {
-alt_name.SetString(value);
+reg_info.alt_name.SetString(value);
   } else if (name.equals("bitsize")) {
-value.getAsInteger(0, reg_info.byte_size);
-reg_info.byte_size /= CHAR_BIT;
+if (!value.getAsInteger(0, reg_info.byte_size))
+  reg_info.byte_size /= CHAR_BIT;
   } else if (name.equals("offset")) {
-if (value.getAsInteger(0, reg_offset))
-  reg_offset = UINT32_MAX;
+value.getAsInteger(0, reg_info.byte_offset);
   } else if (name.equals("encoding")) {
 const Encoding encoding = Args::StringToEncoding(value);
 if (encoding != eEncodingInvalid)
   reg_info.encoding = encoding;
   } else if (name.equals("format")) {
-Format format = eFormatInvalid;
-if (OptionArgParser::ToFormat(value.str().c_str(), format, nullptr)
+if (!OptionArgParser::ToFormat(value.str().c_str(), 
reg_info.format, nullptr)
 .Success())
-  reg_info.format = format;
-else {
   reg_info.format =
   llvm::StringSwitch(value)
   .Case("binary", eFormatBinary)
@@ -533,59 +505,34 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool 
force) {
   .Case("vector-uint64", eFormatVectorOfUInt64)
   .Case("vector-uint128", eFormatVectorOfUInt128)
   .Default(eFormatInvalid);
-}
   } else if (name.equals("set")) {
-set_name.SetString(value);
+reg_info.set_name.SetString(value);
   } else if (name.equals("gcc") || name.equals("ehframe")) {
-if (value.getAsInteger(0, reg_info.kinds[eRegisterKindEHFrame]))
-  reg_info.kinds[eRegisterKindEHFrame] = LLDB_INVALID_REGNUM;
+value.getAsInteg

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

2021-09-23 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 374639.
bulbazord added a comment.

Rebasing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110115

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -130,6 +130,9 @@
   std::vector
   GenerateAlternateFunctionManglings(const ConstString mangled) const override;
 
+  ConstString FindBestAlternateFunctionMangledName(
+  const Mangled mangled, const SymbolContext &sym_ctx) const override;
+
   // PluginInterface protocol
   ConstString GetPluginName() override;
 };
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -20,12 +20,14 @@
 #include "llvm/Demangle/ItaniumDemangle.h"
 
 #include "lldb/Core/Mangled.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
 #include "lldb/DataFormatters/CXXFunctionPointer.h"
 #include "lldb/DataFormatters/DataVisualization.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/VectorType.h"
+#include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
@@ -478,6 +480,56 @@
   return alternates;
 }
 
+ConstString CPlusPlusLanguage::FindBestAlternateFunctionMangledName(
+const Mangled mangled, const SymbolContext &sym_ctx) const {
+  ConstString demangled = mangled.GetDemangledName();
+  if (!demangled)
+return ConstString();
+
+  CPlusPlusLanguage::MethodName cpp_name(demangled);
+  std::string scope_qualified_name = cpp_name.GetScopeQualifiedName();
+
+  if (!scope_qualified_name.size())
+return ConstString();
+
+  if (!sym_ctx.module_sp)
+return ConstString();
+
+  lldb_private::SymbolFile *sym_file = sym_ctx.module_sp->GetSymbolFile();
+  if (!sym_file)
+return ConstString();
+
+  std::vector alternates;
+  sym_file->GetMangledNamesForFunction(scope_qualified_name, alternates);
+
+  std::vector param_and_qual_matches;
+  std::vector param_matches;
+  for (size_t i = 0; i < alternates.size(); i++) {
+ConstString alternate_mangled_name = alternates[i];
+Mangled mangled(alternate_mangled_name);
+ConstString demangled = mangled.GetDemangledName();
+
+CPlusPlusLanguage::MethodName alternate_cpp_name(demangled);
+if (!cpp_name.IsValid())
+  continue;
+
+if (alternate_cpp_name.GetArguments() == cpp_name.GetArguments()) {
+  if (alternate_cpp_name.GetQualifiers() == cpp_name.GetQualifiers())
+param_and_qual_matches.push_back(alternate_mangled_name);
+  else
+param_matches.push_back(alternate_mangled_name);
+}
+  }
+
+  if (param_and_qual_matches.size())
+return param_and_qual_matches[0]; // It is assumed that there will be only
+  // one!
+  else if (param_matches.size())
+return param_matches[0]; // Return one of them as a best match
+  else
+return ConstString();
+}
+
 static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
 return;
Index: lldb/source/Expression/IRExecutionUnit.cpp
===
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/LanguageRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/DataBufferHeap.h"
@@ -33,7 +34,6 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 
-#include "lldb/../../source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "lldb/../../source/Plugins/ObjectFile/JIT/ObjectFileJIT.h"
 
 using namespace lldb_private;
@@ -652,52 +652,6 @@
   return return_value;
 }
 
-static ConstString FindBestAlternateMangledName(ConstString demangled,
-const SymbolContext &sym_ctx) {
-  CPlusPlusLanguage::MethodName cpp_name(demangled);
-  std::string scope_qualified_name = cpp_name.GetScopeQualifiedName();
-
-  if (!scope_qualified_name.size())
-return ConstString();
-
-  if (!sym_ctx.module_sp)
-return ConstString();

[Lldb-commits] [lldb] fbaf367 - [lldb] Show fix-it applied even if expression didn't evaluate succesfully

2021-09-23 Thread Augusto Noronha via lldb-commits


Author: Augusto Noronha
Date: 2021-09-23T16:45:04-03:00
New Revision: fbaf36721783c3bcbd45f81294e6980eaef165e4

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

LOG: [lldb] Show fix-it applied even if expression didn't evaluate succesfully

If we applied a fix-it before evaluating an expression and that
expression didn't evaluate correctly, we should still tell users about
the fix-it we applied since that may be the reason why it didn't work
correctly.

Differential Revision: https://reviews.llvm.org/D109908

Added: 


Modified: 
lldb/source/Commands/CommandObjectExpression.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py
lldb/test/API/commands/expression/fixits/main.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp 
b/lldb/source/Commands/CommandObjectExpression.cpp
index bf62f3f297cc..a93cc15b0ff3 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -421,9 +421,8 @@ bool 
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
   // We only tell you about the FixIt if we applied it.  The compiler errors
   // will suggest the FixIt if it parsed.
   if (!m_fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
-if (success == eExpressionCompleted)
-  error_stream.Printf("  Fix-it applied, fixed expression was: \n%s\n",
-  m_fixed_expression.c_str());
+error_stream.Printf("  Fix-it applied, fixed expression was: \n%s\n",
+m_fixed_expression.c_str());
   }
 
   if (result_valobj_sp) {

diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index a2e4564f7078..686f56ad7974 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -82,6 +82,22 @@ def test_with_target(self):
 error_string.find("my_pointer->second.a") != -1,
 "Fix was right")
 
+def test_with_target_error_applies_fixit(self):
+""" Check that applying a Fix-it which fails to execute correctly 
still 
+ prints that the Fix-it was applied. """
+self.build()
+(target, process, self.thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+'Stop here to evaluate expressions',
+ lldb.SBFileSpec("main.cpp"))
+# Enable fix-its as they were intentionally disabled by TestBase.setUp.
+self.runCmd("settings set target.auto-apply-fixits true")
+ret_val = lldb.SBCommandReturnObject()
+result = self.dbg.GetCommandInterpreter().HandleCommand("expression 
null_pointer.first", ret_val)
+self.assertEqual(result, lldb.eReturnStatusFailed, ret_val.GetError())
+
+self.assertIn("Fix-it applied, fixed expression was:", 
ret_val.GetError())
+self.assertIn("null_pointer->first", ret_val.GetError())
+
 # The final function call runs into SIGILL on aarch64-linux.
 @expectedFailureAll(archs=["aarch64"], oslist=["freebsd", "linux"],
 bugnumber="llvm.org/pr49407")

diff  --git a/lldb/test/API/commands/expression/fixits/main.cpp 
b/lldb/test/API/commands/expression/fixits/main.cpp
index 371d8333763b..0162ed24bd84 100644
--- a/lldb/test/API/commands/expression/fixits/main.cpp
+++ b/lldb/test/API/commands/expression/fixits/main.cpp
@@ -17,6 +17,7 @@ main()
 {
   struct MyStruct my_struct = {10, {20, 30}};
   struct MyStruct *my_pointer = &my_struct;
+  struct MyStruct *null_pointer = nullptr;
   printf ("Stop here to evaluate expressions: %d %d %p\n", my_pointer->first, 
my_pointer->second.a, my_pointer);
   return 0;
 }



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

[Lldb-commits] [PATCH] D109908: [lldb] Show fix-it applied even if expression didn't evaluate succesfully

2021-09-23 Thread Augusto Noronha via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfbaf36721783: [lldb] Show fix-it applied even if expression 
didn't evaluate succesfully (authored by augusto2112).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109908

Files:
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/test/API/commands/expression/fixits/TestFixIts.py
  lldb/test/API/commands/expression/fixits/main.cpp


Index: lldb/test/API/commands/expression/fixits/main.cpp
===
--- lldb/test/API/commands/expression/fixits/main.cpp
+++ lldb/test/API/commands/expression/fixits/main.cpp
@@ -17,6 +17,7 @@
 {
   struct MyStruct my_struct = {10, {20, 30}};
   struct MyStruct *my_pointer = &my_struct;
+  struct MyStruct *null_pointer = nullptr;
   printf ("Stop here to evaluate expressions: %d %d %p\n", my_pointer->first, 
my_pointer->second.a, my_pointer);
   return 0;
 }
Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -82,6 +82,22 @@
 error_string.find("my_pointer->second.a") != -1,
 "Fix was right")
 
+def test_with_target_error_applies_fixit(self):
+""" Check that applying a Fix-it which fails to execute correctly 
still 
+ prints that the Fix-it was applied. """
+self.build()
+(target, process, self.thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+'Stop here to evaluate expressions',
+ lldb.SBFileSpec("main.cpp"))
+# Enable fix-its as they were intentionally disabled by TestBase.setUp.
+self.runCmd("settings set target.auto-apply-fixits true")
+ret_val = lldb.SBCommandReturnObject()
+result = self.dbg.GetCommandInterpreter().HandleCommand("expression 
null_pointer.first", ret_val)
+self.assertEqual(result, lldb.eReturnStatusFailed, ret_val.GetError())
+
+self.assertIn("Fix-it applied, fixed expression was:", 
ret_val.GetError())
+self.assertIn("null_pointer->first", ret_val.GetError())
+
 # The final function call runs into SIGILL on aarch64-linux.
 @expectedFailureAll(archs=["aarch64"], oslist=["freebsd", "linux"],
 bugnumber="llvm.org/pr49407")
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -421,9 +421,8 @@
   // We only tell you about the FixIt if we applied it.  The compiler errors
   // will suggest the FixIt if it parsed.
   if (!m_fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
-if (success == eExpressionCompleted)
-  error_stream.Printf("  Fix-it applied, fixed expression was: \n%s\n",
-  m_fixed_expression.c_str());
+error_stream.Printf("  Fix-it applied, fixed expression was: \n%s\n",
+m_fixed_expression.c_str());
   }
 
   if (result_valobj_sp) {


Index: lldb/test/API/commands/expression/fixits/main.cpp
===
--- lldb/test/API/commands/expression/fixits/main.cpp
+++ lldb/test/API/commands/expression/fixits/main.cpp
@@ -17,6 +17,7 @@
 {
   struct MyStruct my_struct = {10, {20, 30}};
   struct MyStruct *my_pointer = &my_struct;
+  struct MyStruct *null_pointer = nullptr;
   printf ("Stop here to evaluate expressions: %d %d %p\n", my_pointer->first, my_pointer->second.a, my_pointer);
   return 0;
 }
Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -82,6 +82,22 @@
 error_string.find("my_pointer->second.a") != -1,
 "Fix was right")
 
+def test_with_target_error_applies_fixit(self):
+""" Check that applying a Fix-it which fails to execute correctly still 
+ prints that the Fix-it was applied. """
+self.build()
+(target, process, self.thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+'Stop here to evaluate expressions',
+ lldb.SBFileSpec("main.cpp"))
+# Enable fix-its as they were intentionally disabled by TestBase.setUp.
+self.runCmd("settings set target.auto-apply-fixits true")
+ret_val = lldb.SBCommandReturnObject()
+result = self.dbg.GetCommandInterpreter().HandleCommand("expression null_pointer.first", ret_

[Lldb-commits] [lldb] 953ddde - [lldb] Handle malformed qfThreadInfo reply

2021-09-23 Thread Ted Woodward via lldb-commits


Author: Ted Woodward
Date: 2021-09-23T17:03:47-05:00
New Revision: 953ddded1aa2b459a939e0f1649691c9086ba416

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

LOG: [lldb] Handle malformed qfThreadInfo reply

If the remote gdbserver's qfThreadInfo reply has a trailing comma,
GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs will return
an empty vector of thread ids. This will cause lldb to recurse through
three functions trying to get the list of threads, until it blows its
stack and crashes.

A trailing comma is a malformed response, but it shouldn't cause lldb to
crash. This patch will return the tids received before the malformed
response.

Reviewed By: clayborg, labath

Differential Revision: https://reviews.llvm.org/D109937

Added: 

lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py

Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d949cfe7a64e8..bf4baf7b7a266 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2908,8 +2908,12 @@ 
GDBRemoteCommunicationClient::GetCurrentProcessAndThreadIDs(
   if (ch == 'm') {
 do {
   auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID);
+  // If we get an invalid response, break out of the loop.
+  // If there are valid tids, they have been added to ids.
+  // If there are no valid tids, we'll fall through to the
+  // bare-iron target handling below.
   if (!pid_tid)
-return {};
+break;
 
   ids.push_back(pid_tid.getValue());
   ch = response.GetChar(); // Skip the command separator

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
 
b/lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
new file mode 100644
index 0..0035e1c06297f
--- /dev/null
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadInfoTrailingComma(GDBRemoteTestBase):
+
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return "T02thread:1"
+
+def qfThreadInfo(self):
+return "m1,2,3,4,"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 1)
+self.assertEqual(process.GetThreadAtIndex(1).GetThreadID(), 2)
+self.assertEqual(process.GetThreadAtIndex(2).GetThreadID(), 3)
+self.assertEqual(process.GetThreadAtIndex(3).GetThreadID(), 4)



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

[Lldb-commits] [PATCH] D109937: [lldb] Handle malformed qfThreadInfo reply

2021-09-23 Thread Ted Woodward via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG953ddded1aa2: [lldb] Handle malformed qfThreadInfo reply 
(authored by ted).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109937

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadInfoTrailingComma(GDBRemoteTestBase):
+
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return "T02thread:1"
+
+def qfThreadInfo(self):
+return "m1,2,3,4,"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 1)
+self.assertEqual(process.GetThreadAtIndex(1).GetThreadID(), 2)
+self.assertEqual(process.GetThreadAtIndex(2).GetThreadID(), 3)
+self.assertEqual(process.GetThreadAtIndex(3).GetThreadID(), 4)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2908,8 +2908,12 @@
   if (ch == 'm') {
 do {
   auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID);
+  // If we get an invalid response, break out of the loop.
+  // If there are valid tids, they have been added to ids.
+  // If there are no valid tids, we'll fall through to the
+  // bare-iron target handling below.
   if (!pid_tid)
-return {};
+break;
 
   ids.push_back(pid_tid.getValue());
   ch = response.GetChar(); // Skip the command separator


Index: lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestThreadInfoTrailingComma.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadInfoTrailingComma(GDBRemoteTestBase):
+
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return "T02thread:1"
+
+def qfThreadInfo(self):
+return "m1,2,3,4,"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 1)
+self.assertEqual(process.GetThreadAtIndex(1).GetThreadID(), 2)
+self.assertEqual(process.GetThreadAtIndex(2).GetThreadID(), 3)
+self.assertEqual(process.GetThreadAtIndex(3).GetThreadID(), 4)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2908,8 +2908,12 @@
   if (ch == 'm') {
 do {
   auto pid_tid = response.GetPidTid(LLDB_INVALID_PROCESS_ID);
+  // If we get an invalid response, break out of the loop.
+  // If there are valid tids, they have been added to ids.
+  // If there are no valid tids, we'll fall through to the
+  // bare-iron target handling below.
   if (!pid_tid)
-return {};
+break;
 
   ids.push_back(pid_tid.getValue());
   ch = response.GetChar(); // Skip the command separator
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

2021-09-23 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109975

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

[Lldb-commits] [PATCH] D109231: [lldb] Improve error handling around GetRngListData()

2021-09-23 Thread Kim-Anh Tran via Phabricator via lldb-commits

kimanh added inline comments.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:522
+entry->getContribution(llvm::DW_SECT_RNGLISTS)) {
+  Offset = contribution->Offset;
   return DWARFDataExtractor(data, contribution->Offset,

shafik wrote:
> If I am reading this correctly, it looks like this will only be set on the 
> non-error case which will leave `contribution_off` in the caller 
> uninitialized in the cases you care about logging.
Sorry for the very late reply (I had a long vacation)! And thanks for the 
comment.

If `GetRnglistData` didn't work, the error will be thrown below (line 526) in 
here. However, if it is successful, but another problem occurs (callers of 
`GetRnglistData` throw an error) the Offset will be initialized, and it can be 
logged. Example: `ParseListTableHeader` using it in ll. 548. Does that make 
sense?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109231

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

[Lldb-commits] [lldb] 5685eb9 - [lldb] Fix DomainSocket::GetSocketName for unnamed sockets

[Lldb-commits] [PATCH] D110269: Fix LLDB build on old Linux kernels

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Lldb-commits] [lldb] b03e701 - [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Lldb-commits] [lldb] 6fbed33 - [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Lldb-commits] [PATCH] D110027: [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Lldb-commits] [lldb] bcb6b97 - Revert "[lldb] [gdb-remote] Refactor getting remote regs to use local vector"

[Lldb-commits] [lldb] 12504f5 - Revert "[lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs"

[Lldb-commits] [PATCH] D109908: [lldb] Show fix-it applied even if expression didn't evaluate succesfully

Re: [Lldb-commits] [PATCH] D110269: Fix LLDB build on old Linux kernels

[Lldb-commits] [lldb] c223299 - [lldb] Add a C language REPL to test LLDB's REPL infrastructure

[Lldb-commits] [PATCH] D87281: [lldb] Add a C language REPL to test LLDB's REPL infrastructure

[Lldb-commits] [lldb] cc3c788 - [lldb] [gdb-remote] Use local regnos for value_regs/invalidate_regs

[Lldb-commits] [lldb] fa45650 - [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Lldb-commits] [PATCH] D110115: [lldb] Remove Expression's dependency on CPlusPlusLanguagePlugin

[Lldb-commits] [lldb] fbaf367 - [lldb] Show fix-it applied even if expression didn't evaluate succesfully

[Lldb-commits] [PATCH] D109908: [lldb] Show fix-it applied even if expression didn't evaluate succesfully

[Lldb-commits] [lldb] 953ddde - [lldb] Handle malformed qfThreadInfo reply

[Lldb-commits] [PATCH] D109937: [lldb] Handle malformed qfThreadInfo reply

[Lldb-commits] [PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

[Lldb-commits] [PATCH] D109231: [lldb] Improve error handling around GetRngListData()

27 matches

Site Navigation

Mail list logo

Footer information