[Lldb-commits] [lldb] ccb9d4d - [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T09:02:20+02:00
New Revision: ccb9d4d4addc2fb2aa94cf776d43d8be35365272

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

LOG: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

Try to always send vCont packets and include the PID in them if running
multiprocess.  This is necessary to ensure that with the upcoming full
multiprocess support always resumes the correct process without having
to resort to the legacy Hc packets.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131758

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 29417b35fde3a..11d0fec1926ab 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -181,6 +181,12 @@ bool 
GDBRemoteCommunicationClient::GetQXferSigInfoReadSupported() {
   return m_supports_qXfer_siginfo_read == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::GetMultiprocessSupported() {
+  if (m_supports_memory_tagging == eLazyBoolCalculate)
+GetRemoteQSupported();
+  return m_supports_multiprocess == eLazyBoolYes;
+}
+
 uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
 GetRemoteQSupported();
@@ -1514,7 +1520,7 @@ Status GDBRemoteCommunicationClient::Detach(bool 
keep_stopped,
 }
   }
 
-  if (m_supports_multiprocess) {
+  if (GetMultiprocessSupported()) {
 // Some servers (e.g. qemu) require specifying the PID even if only a 
single
 // process is running.
 if (pid == LLDB_INVALID_PROCESS_ID)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 3a62747603f6d..4e89ade772ad3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -339,6 +339,8 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   bool GetQXferSigInfoReadSupported();
 
+  bool GetMultiprocessSupported();
+
   LazyBool SupportsAllocDeallocMemory() // const
   {
 // Uncomment this to have lldb pretend the debug server doesn't respond to

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 30fb27932d192..0c876a8fd703e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1186,11 +1186,15 @@ Status ProcessGDBRemote::DoResume() {
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
+  std::string pid_prefix;
+  if (m_gdb_comm.GetMultiprocessSupported())
+pid_prefix = llvm::formatv("p{0:x-}.", GetID());
+
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing, just send a "c" packet
-continue_packet.PutCString("c");
+// All threads are continuing
+continue_packet.Format("vCont;c:{0}-1", pid_prefix);
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1200,7 +1204,7 @@ Status ProcessGDBRemote::DoResume() {
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
+  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1211,8 +1215,8 @@ Status ProcessGDBRemote::DoResume() {
  s_pos = m_continue_C_tids.begin(),
  s_end = m_continue_C_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Printf(";C%2.2x:%4.4" PRIx64, s_pos->second,
- s_pos->first);
+  continue_packet.Format(";C{0:x-2}:{1}{2:x-}", s_pos->second,
+ pid_prefix, s_pos->first);
   } else
 continue_packet

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
mgorny marked 2 inline comments as done.
Closed by commit rGccb9d4d4addc: [lldb] [gdb-remote] Include PID in vCont 
packets if multiprocess (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D131758?vs=452133&id=453901#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -0,0 +1,87 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestContinue(GDBRemoteTestBase):
+class BaseResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;multiprocess+"
+
+def qfThreadInfo(self):
+return "mp400.401"
+
+def haltReason(self):
+return "S13"
+
+def cont(self):
+return "W01"
+
+def other(self, packet):
+if packet == "vCont?":
+return "vCont;c;C;s;S"
+if packet.startswith("vCont;"):
+return "W00"
+return ""
+
+def test_continue_no_multiprocess(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertPacketLogContains(["vCont;C13:401"])
+
+def test_continue_no_vCont(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+def other(self, packet):
+return ""
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertPacketLogContains(["Hc401", "C13"])
+
+def test_continue_multiprocess(self):
+class MyResponder(self.BaseResponder):
+pass
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertPacketLogContains(["vCont;C13:p400.401"])
+
+def test_step_multiprocess(self):
+class MyResponder(self.BaseResponder):
+def other(self, packet):
+if packet == "vCont?":
+return "vCont;c;C;s;S"
+if packet.startswith("vCont;C"):
+return "S13"
+if packet.startswith("vCont;s"):
+return "W00"
+return ""
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+thread = process.GetSelectedThread()
+thread.StepInstruction(False)
+self.assertPacketLogContains(["vCont;s:p400.401"])
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
@@ -1186,11 +1186,15 @@
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
+  std::string pid_prefix;
+  if (m_gdb_comm.GetMultiprocessSupported())
+pid_prefix = llvm::formatv("p{0:x-}.", GetID());
+
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing, just send a "c" packet
-continue_packet.PutCString("c");
+// All threads are continuing
+continue_packet.Format("vCont;c:{0}-1", pid_prefix);
   } else {
 continue_

[Lldb-commits] [lldb] 5815708 - [lldb] [test] Skip step packet test on non-amd64

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T09:48:13+02:00
New Revision: 58157087b0a4fefeabd4cf6e4ee356d6152fd5a6

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

LOG: [lldb] [test] Skip step packet test on non-amd64

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

Removed: 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
index 14147fb5b2bf..9c3fa66608d0 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -67,6 +67,9 @@ class MyResponder(self.BaseResponder):
 process = self.connect(target)
 self.assertPacketLogContains(["vCont;C13:p400.401"])
 
+# uses 'S13' instead of 's' arm (the pc dance?), testing it on one
+# arch should be entirely sufficient
+@skipIf(archs=no_match(["x86_64"]))
 def test_step_multiprocess(self):
 class MyResponder(self.BaseResponder):
 def other(self, packet):



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

[Lldb-commits] [lldb] 8b50ffe - [lldb] [test] Remove test_step_multiprocess, it's unreliable

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T10:08:11+02:00
New Revision: 8b50ffe9fdc3cd457796857d3661e34490ef0490

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

LOG: [lldb] [test] Remove test_step_multiprocess, it's unreliable

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

Removed: 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
index 9c3fa66608d0..f0f0df81d321 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -66,25 +66,3 @@ class MyResponder(self.BaseResponder):
 target = self.createTarget("a.yaml")
 process = self.connect(target)
 self.assertPacketLogContains(["vCont;C13:p400.401"])
-
-# uses 'S13' instead of 's' arm (the pc dance?), testing it on one
-# arch should be entirely sufficient
-@skipIf(archs=no_match(["x86_64"]))
-def test_step_multiprocess(self):
-class MyResponder(self.BaseResponder):
-def other(self, packet):
-if packet == "vCont?":
-return "vCont;c;C;s;S"
-if packet.startswith("vCont;C"):
-return "S13"
-if packet.startswith("vCont;s"):
-return "W00"
-return ""
-
-self.server.responder = MyResponder()
-self.runCmd("platform select remote-linux")
-target = self.createTarget("a.yaml")
-process = self.connect(target)
-thread = process.GetSelectedThread()
-thread.StepInstruction(False)
-self.assertPacketLogContains(["vCont;s:p400.401"])



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

Re: [Lldb-commits] [lldb] 5815708 - [lldb] [test] Skip step packet test on non-amd64

[Pavel Labath via lldb-commits]

On 19/08/2022 09:48, Michał Górny via lldb-commits wrote:


Author: Michał Górny
Date: 2022-08-19T09:48:13+02:00
New Revision: 58157087b0a4fefeabd4cf6e4ee356d6152fd5a6

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

LOG: [lldb] [test] Skip step packet test on non-amd64

Sponsored by: The FreeBSD Foundation

Added:
 


Modified:
 lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

Removed:
 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
index 14147fb5b2bf..9c3fa66608d0 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -67,6 +67,9 @@ class MyResponder(self.BaseResponder):
  process = self.connect(target)
  self.assertPacketLogContains(["vCont;C13:p400.401"])
  
+# uses 'S13' instead of 's' arm (the pc dance?), testing it on one

+# arch should be entirely sufficient
Well, it means that people with (increasingly common) arm machines can't 
test it, but even more than that, I am troubled by the fact that the 
host architecture leaks into this test case. I think we either have a 
bug in lldb, or in the way we set up these tests.

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

[Lldb-commits] [PATCH] D131837: [lldb] [gdb-remote] Initial support for multiple ContinueDelegates

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131837#3734204 , @mgorny wrote:

> In D131837#3732971 , @labath wrote:
>
>> This sort of makes sense to me, but it's not clear to me how is this 
>> selection logic going to apply to packets other than stop-replies. All of 
>> the forked processes are going to share the same pty, so it's going to be 
>> literally impossible to distinguish the `O` packets, for instance. I'm 
>> wondering if it would make sense to split this interface into two, and have 
>> just one "preferred" recipient of `O` packets (and possibly others as well 
>> -- as we have no plans for supporting them right now), and have a separate 
>> list of recipients who would do things with the stop replies?
>
> Specifically about the `O` packets, I've been thinking that it doesn't really 
> make any difference which process receives them — after all, they will be 
> printed all the same, won't they?

Not exactly. That output gets routed through process-specific 
(SB)Process:GetSTDOUT functions. Of course, if you're using the command line, 
then all of these outputs will funnel into the lldb terminal anyway, but a 
scripting/gui user might be doing something different. I guess one of the goals 
of a future larger SB API redesign could be refactor this such that it is clear 
that forked processes will share the same terminal/stdout..




Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:23
 virtual ~ContinueDelegate();
-virtual void HandleAsyncStdout(llvm::StringRef out) = 0;
-virtual void HandleAsyncMisc(llvm::StringRef data) = 0;
-virtual void HandleStopReply() = 0;
+virtual void HandleAsyncStdout(llvm::StringRef out, bool &handled) = 0;
+virtual void HandleAsyncMisc(llvm::StringRef data, bool &handled) = 0;

mgorny wrote:
> labath wrote:
> > Why not just make this a regular return value?
> I've figured out an explicit variable makes its purpose clearer, rather than 
> arbitrary `return true/false`.
Maybe sometimes, but I don't think that's the case here. I mean, if you think 
this is particularly ambiguous, you could make a `{handled, not_handled}` enum 
and return that.


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

https://reviews.llvm.org/D131837

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

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added subscribers: kristof.beyls, arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Harmonize the status and error values of Communication::Read() when
running with and without read thread.  Prior to this change, Read()
would return eConnectionStatusSuccess if read thread was enabled
and the read timed out or reached end-of-file, rather than
the respective states that are returned if read thread was disabled.
Now, it correctly returns eConnectionStatusTimedOut
and eConnectionStatusEndOfFile, and sets the error respectively.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D132217

Files:
  lldb/source/Core/Communication.cpp
  lldb/unittests/Core/CommunicationTest.cpp

Index: lldb/unittests/Core/CommunicationTest.cpp
===
--- lldb/unittests/Core/CommunicationTest.cpp
+++ lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,115 @@
 
 using namespace lldb_private;
 
+TEST(CommunicationTest, Read) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Status error;
+  size_t bytes_written;
+  ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(),
+llvm::Succeeded());
+  ASSERT_EQ(bytes_written, 4U);
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+
+  // This read should wait for the data to become available and return it.
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char buf[16];
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 4U);
+  EXPECT_EQ(status, lldb::eConnectionStatusSuccess);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  buf[4] = 0;
+  EXPECT_STREQ(buf, "test");
+
+  // These reads should time out as there is no more data.
+  error.Clear();
+  EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status,
+  &error),
+0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // 0 is special-cased, so we test it separately.
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // This read should return EOF.
+  pipe.CloseWriteFileDescriptor();
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+
+  // JoinReadThread() should just return immediately since there was no read
+  // thread started.
+  EXPECT_TRUE(comm.JoinReadThread());
+}
+
+TEST(CommunicationTest, ReadThread) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Status error;
+  size_t bytes_written;
+  ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(),
+llvm::Succeeded());
+  ASSERT_EQ(bytes_written, 4U);
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+  ASSERT_TRUE(comm.StartReadThread());
+
+  // This read should wait for the data to become available and return it.
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char buf[16];
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 4U);
+  EXPECT_EQ(status, lldb::eConnectionStatusSuccess);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  buf[4] = 0;
+  EXPECT_STREQ(buf, "test");
+
+  // These reads should time out as there is no more data.
+  error.Clear();
+  EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status,
+  &error),
+0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // 0 is special-cased, so we test it separately.
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // This read should return EOF.
+  pipe.CloseWriteFileDescriptor();
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+
+  EXPECT_TRUE

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Cool stuff. Would it be possible to test for the two paths into a single 
function (with some `if`s, if necessary)? That would make it clearer what (if 
any) are the differences between the two modes of operation.


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

https://reviews.llvm.org/D132217

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

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D132217#3734749 , @labath wrote:

> Cool stuff. Would it be possible to test for the two paths into a single 
> function (with some `if`s, if necessary)? That would make it clearer what (if 
> any) are the differences between the two modes of operation.

I suppose this makes sense (why didn't I think of it?!). I was thinking of 
using parametrized tests but they seemed a bit too complex for two cases.


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

https://reviews.llvm.org/D132217

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

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 453950.
mgorny added a comment.

Use a common test function.


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

https://reviews.llvm.org/D132217

Files:
  lldb/source/Core/Communication.cpp
  lldb/unittests/Core/CommunicationTest.cpp

Index: lldb/unittests/Core/CommunicationTest.cpp
===
--- lldb/unittests/Core/CommunicationTest.cpp
+++ lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,72 @@
 
 using namespace lldb_private;
 
+static void CommunicationReadTest(bool use_read_thread) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Status error;
+  size_t bytes_written;
+  ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(),
+llvm::Succeeded());
+  ASSERT_EQ(bytes_written, 4U);
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+
+  if (use_read_thread)
+ASSERT_TRUE(comm.StartReadThread());
+
+  // This read should wait for the data to become available and return it.
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char buf[16];
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 4U);
+  EXPECT_EQ(status, lldb::eConnectionStatusSuccess);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  buf[4] = 0;
+  EXPECT_STREQ(buf, "test");
+
+  // These reads should time out as there is no more data.
+  error.Clear();
+  EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status,
+  &error),
+0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // 0 is special-cased, so we test it separately.
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // This read should return EOF.
+  pipe.CloseWriteFileDescriptor();
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+
+  // JoinReadThread() should just return immediately since there was no read
+  // thread started.
+  EXPECT_TRUE(comm.JoinReadThread());
+}
+
+TEST(CommunicationTest, Read) {
+  CommunicationReadTest(/*use_thread=*/false);
+}
+
+TEST(CommunicationTest, ReadThread) {
+  CommunicationReadTest(/*use_thread=*/true);
+}
+
 #ifndef _WIN32
 TEST(CommunicationTest, SynchronizeWhileClosing) {
   // Set up a communication object reading from a pipe.
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -132,10 +132,16 @@
   if (m_read_thread_enabled) {
 // We have a dedicated read thread that is getting data for us
 size_t cached_bytes = GetCachedBytes(dst, dst_len);
-if (cached_bytes > 0 || (timeout && timeout->count() == 0)) {
+if (cached_bytes > 0) {
   status = eConnectionStatusSuccess;
   return cached_bytes;
 }
+if (timeout && timeout->count() == 0) {
+  if (error_ptr)
+error_ptr->SetErrorString("Timed out.");
+  status = eConnectionStatusTimedOut;
+  return 0;
+}
 
 if (!m_connection_sp) {
   if (error_ptr)
@@ -155,11 +161,25 @@
   }
 
   if (event_type & eBroadcastBitReadThreadDidExit) {
+// If the thread exited of its own accord, it either means it
+// hit an end-of-file condition or lost connection
+// (we verified that we had an connection above).
+if (!m_connection_sp) {
+  if (error_ptr)
+error_ptr->SetErrorString("Lost connection.");
+  status = eConnectionStatusLostConnection;
+} else
+  status = eConnectionStatusEndOfFile;
+
 if (GetCloseOnEOF())
   Disconnect(nullptr);
-break;
+return 0;
   }
 }
+
+if (error_ptr)
+  error_ptr->SetErrorString("Timed out.");
+status = eConnectionStatusTimedOut;
 return 0;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c74c17f - [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-08-19T13:25:25+02:00
New Revision: c74c17f37aa9360492675e2aae3de99539eaf465

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

LOG: [lldb] Use WSAEventSelect for MainLoop polling on windows

This patch switches the MainLoop class to use the WSAEventSelect
mechanism to wait for multiple sockets to become readable. The
motivation for doing that is that this allows us to wait for other kinds
of events as well (as long as they can be converted to WSAEvents). This
will allow us to avoid (abstract away) pipe-based multiplexing
mechanisms in the generic code, since pipes cannot be combined with
sockets on windows.

Since the windows implementation will now look completely different than
the posix (file descriptor-based) implementations, I have split the
MainLoop class into two (MainLoopPosix and MainLoopWindows), with the
common code going into MainLoopBase.

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

Added: 
lldb/include/lldb/Host/posix/MainLoopPosix.h
lldb/include/lldb/Host/windows/MainLoopWindows.h
lldb/source/Host/common/MainLoopBase.cpp
lldb/source/Host/posix/MainLoopPosix.cpp
lldb/source/Host/windows/MainLoopWindows.cpp

Modified: 
lldb/include/lldb/Host/MainLoop.h
lldb/include/lldb/Host/MainLoopBase.h
lldb/source/Host/CMakeLists.txt

Removed: 
lldb/source/Host/common/MainLoop.cpp



diff  --git a/lldb/include/lldb/Host/MainLoop.h 
b/lldb/include/lldb/Host/MainLoop.h
index f04ac7359cc18..c88b9f8920ec4 100644
--- a/lldb/include/lldb/Host/MainLoop.h
+++ b/lldb/include/lldb/Host/MainLoop.h
@@ -9,114 +9,16 @@
 #ifndef LLDB_HOST_MAINLOOP_H
 #define LLDB_HOST_MAINLOOP_H
 
-#include "lldb/Host/Config.h"
-#include "lldb/Host/MainLoopBase.h"
-#include "llvm/ADT/DenseMap.h"
-#include 
-#include 
-#include 
-
-#if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
-#define SIGNAL_POLLING_UNSUPPORTED 1
-#endif
-
+#ifdef _WIN32
+#include "lldb/Host/windows/MainLoopWindows.h"
 namespace lldb_private {
-
-// Implementation of the MainLoopBase class. It can monitor file descriptors
-// for readability using ppoll, kqueue, poll or WSAPoll. On Windows it only
-// supports polling sockets, and will not work on generic file handles or
-// pipes. On systems without kqueue or ppoll handling singnals is not
-// supported. In addition to the common base, this class provides the ability
-// to invoke a given handler when a signal is received.
-//
-// Since this class is primarily intended to be used for single-threaded
-// processing, it does not attempt to perform any internal synchronisation and
-// any concurrent accesses must be protected  externally. However, it is
-// perfectly legitimate to have more than one instance of this class running on
-// separate threads, or even a single thread (with some limitations on signal
-// monitoring).
-// TODO: Add locking if this class is to be used in a multi-threaded context.
-class MainLoop : public MainLoopBase {
-private:
-  class SignalHandle;
-
-public:
-  typedef std::unique_ptr SignalHandleUP;
-
-  MainLoop();
-  ~MainLoop() override;
-
-  ReadHandleUP RegisterReadObject(const lldb::IOObjectSP &object_sp,
-  const Callback &callback,
-  Status &error) override;
-
-  // Listening for signals from multiple MainLoop instances is perfectly safe
-  // as long as they don't try to listen for the same signal. The callback
-  // function is invoked when the control returns to the Run() function, not
-  // when the hander is executed. This mean that you can treat the callback as
-  // a normal function and perform things which would not be safe in a signal
-  // handler. However, since the callback is not invoked synchronously, you
-  // cannot use this mechanism to handle SIGSEGV and the like.
-  SignalHandleUP RegisterSignal(int signo, const Callback &callback,
-Status &error);
-
-  // Add a pending callback that will be executed once after all the pending
-  // events are processed. The callback will be executed even if termination
-  // was requested.
-  void AddPendingCallback(const Callback &callback) override;
-
-  Status Run() override;
-
-  // This should only be performed from a callback. Do not attempt to terminate
-  // the processing from another thread.
-  // TODO: Add synchronization if we want to be terminated from another thread.
-  void RequestTermination() override { m_terminate_request = true; }
-
-protected:
-  void UnregisterReadObject(IOObject::WaitableHandle handle) override;
-
-  void UnregisterSignal(int signo, std::list::iterator callback_it);
-
-private:
-  void ProcessReadObject(IOObject::WaitableHandle handle);
-  void ProcessSignal(int signo);
-
-  class Signal

[Lldb-commits] [PATCH] D131159: [lldb] Use WSAEventSelect for MainLoop polling on windows

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc74c17f37aa9: [lldb] Use WSAEventSelect for MainLoop polling 
on windows (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131159

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/include/lldb/Host/windows/MainLoopWindows.h
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/MainLoop.cpp
  lldb/source/Host/common/MainLoopBase.cpp
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/source/Host/windows/MainLoopWindows.cpp

Index: lldb/source/Host/windows/MainLoopWindows.cpp
===
--- /dev/null
+++ lldb/source/Host/windows/MainLoopWindows.cpp
@@ -0,0 +1,115 @@
+//===-- MainLoopWindows.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/windows/MainLoopWindows.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/Config/llvm-config.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+
+llvm::Expected MainLoopWindows::Poll() {
+  std::vector read_events;
+  read_events.reserve(m_read_fds.size());
+  for (auto &[fd, info] : m_read_fds) {
+int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | FD_CLOSE);
+assert(result == 0);
+
+read_events.push_back(info.event);
+  }
+
+  DWORD result = WSAWaitForMultipleEvents(
+  read_events.size(), read_events.data(), FALSE, WSA_INFINITE, FALSE);
+
+  for (auto &fd : m_read_fds) {
+int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
+assert(result == 0);
+  }
+
+  if (result >= WSA_WAIT_EVENT_0 &&
+  result < WSA_WAIT_EVENT_0 + read_events.size())
+return result - WSA_WAIT_EVENT_0;
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "WSAWaitForMultipleEvents failed");
+}
+
+MainLoopWindows::ReadHandleUP
+MainLoopWindows::RegisterReadObject(const IOObjectSP &object_sp,
+const Callback &callback, Status &error) {
+  if (!object_sp || !object_sp->IsValid()) {
+error.SetErrorString("IO object is not valid.");
+return nullptr;
+  }
+  if (object_sp->GetFdType() != IOObject::eFDTypeSocket) {
+error.SetErrorString(
+"MainLoopWindows: non-socket types unsupported on Windows");
+return nullptr;
+  }
+
+  WSAEVENT event = WSACreateEvent();
+  if (event == WSA_INVALID_EVENT) {
+error.SetErrorStringWithFormat("Cannot create monitoring event.");
+return nullptr;
+  }
+
+  const bool inserted =
+  m_read_fds
+  .try_emplace(object_sp->GetWaitableHandle(), FdInfo{event, callback})
+  .second;
+  if (!inserted) {
+WSACloseEvent(event);
+error.SetErrorStringWithFormat("File descriptor %d already monitored.",
+   object_sp->GetWaitableHandle());
+return nullptr;
+  }
+
+  return CreateReadHandle(object_sp);
+}
+
+void MainLoopWindows::UnregisterReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  assert(it != m_read_fds.end());
+  BOOL result = WSACloseEvent(it->second.event);
+  assert(result == TRUE);
+  m_read_fds.erase(it);
+}
+
+void MainLoopWindows::ProcessReadObject(IOObject::WaitableHandle handle) {
+  auto it = m_read_fds.find(handle);
+  if (it != m_read_fds.end())
+it->second.callback(*this); // Do the work
+}
+
+Status MainLoopWindows::Run() {
+  m_terminate_request = false;
+
+  Status error;
+
+  // run until termination or until we run out of things to listen to
+  while (!m_terminate_request && !m_read_fds.empty()) {
+
+llvm::Expected signaled_event = Poll();
+if (!signaled_event)
+  return Status(signaled_event.takeError());
+
+auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
+
+ProcessReadObject(KV.first);
+ProcessPendingCallbacks();
+  }
+  return Status();
+}
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -1,4 +1,4 @@
-//===-- MainLoop.cpp --===//
+//===-- MainLoopPosix.cpp -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,12 +6,11 

[Lldb-commits] [PATCH] D131335: [lldb] abi_tag support 3/3 - Use mangle tree API to determine approximate mangled matches

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131335#3733117 , @Michael137 
wrote:

> In D131335#3732877 , @labath wrote:
>
>> FWIW, I would be all for attaching the abi tags to the clang declarations if 
>> they would be in some easily accessible form (e.g. a DWARF attribute). 
>> Parsing them out of the mangled name is somewhat dubious, but I am not 
>> entirely against that, if it is necessary for some use case. However, even 
>> if we did that, I'd still say that attaching the asm attribute is a good 
>> idea.
>
> We're actually thinking of maybe getting rid off the fallback logic for the 
> C++ plugin entirely. Only a handful of API tests seem to call into it (and 
> some of them don't even expect the symbol to be found). But maybe there are 
> some critical corner cases that require this that aren't covered by the tests.

I expect that this logic exists mostly for the benefit of low-quality or 
"minimal" debug info, at least that's what it looks like for me when I see the 
kinds of fallbacks that are used. For example the `char`->`signed char` 
substitution exists probably to work around the fact that DWARF does not really 
recognize `char` as a separate type (not in the same way that C does). So the 
type `char` could end up having using DW_ATE_signed_char (same as `signed 
char`) or DW_ATE_unsigned_char (same as `unsigned char`), depending on whether 
the type actually is signed or not. Makes sense if all you want is to provide a 
description of the type, but it does not sufficient for reconstructing mangled 
names. This is why our DWARF parsing code 
(TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize) needs to use string 
comparisons to get this right. One could say the same for int->long 
substitutions, etc.

That means that these things will only show up when using debug info produced 
by third-party compilers (I think even gcc should be fine), which is not 
something that we test regularly. That said, I don't think this is a good 
reason to keep this code around, and I'd hope that the asm labels will fix most 
of these issues (obviously, you won't be able to call the right overload based 
on the signedness of a char variable, but you couldn't do that before either). 
If it breaks anyones use case, we can figure out how to fix this in a better 
way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131335

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

[Lldb-commits] [PATCH] D131974: [lldb][ClangExpression] Add asm() label to all FunctionDecls we create from DWARF

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131974#3733024 , @Michael137 
wrote:

> In D131974#3732727 , @labath wrote:
>
>> Any particular reason for not removing the `SC_Extern` check? I'm pretty 
>> sure I could create a test case with a `static` abi-tagged function which we 
>> wouldn't be able to call with that check in place...
>
> Boiled down to this comment:
>
>> the local (indicated by the `L` in the name) mangled name is not necessarily 
>> unique
>
> Attaching it to non-external functions caused 
> `cpp/namespace/TestNamespaceLookup.py` to fail. With a non-static and a 
> static version of `func()` LLDB chose the wrong one. I can follow up on 
> whether this is a bug elsewhere and how feasible it would be to 
> unconditionally attach the label

Ah, this is a fun one. So this breaks the `test_scope_lookup_with_run_command` 
test in `TestNamespaceLookup.py`, but simultaneously "fixes"  
`test_file_scope_lookup_with_run_command`. I would say this is a pre-existing 
bug, that just happens to manifest itself differently with this change. The 
problem is that lldb is not able to correctly scope CU-local objects, as it 
ends up putting all files into one huge AST context. It was mostly accidental 
that lldb picked the external version of the function (because it happened to 
get its name mangling "right"). Now it happens to pick the static one as they 
both have the same signature (and I guess this is the one that gets parsed 
first).

I'd say we should just flip the xfails around, since this is already pretty 
broken, and the asm labels definitely help in some situations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131974

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

[Lldb-commits] [PATCH] D131437: Don't index the skeleton CU when we have a fission compile unit.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131437#3710203 , @clayborg wrote:

> In D131437#3709921 , @labath wrote:
>
>> Seems reasonable, but could use a test case, though I'm not sure what would 
>> be the best way to approach that. I suppose one could dump the index of one 
>> of these dwo-less files, and then make sure it's contents are right (empty?).
>
> That is what I was struggling with. I might be able to use lldb-test to dump 
> a type lookup on a name that used to appear in both the DWO file and in the 
> skeleton compile unit and make sure no error string from the parsing gets 
> emitted? I haven't used lldb-test much, but is this possible to expect output 
> and make sure the error that detected this issue is not emitted once it is 
> fixed? The test would create a binary with -fsplit-dwarf-inlining enabled and 
> make sure that the skeleton compile unit ends up having a type from a type 
> template that _could_ be found if this fix wasn't there, then make sure when 
> we do a type lookup we don't see the error message. Let me know if you have 
> any other ideas on how to do this.

Yeah, that's pretty much what I had in mind. I think it should be sufficient to 
run `lldb-test symbols` on this binary. Among other things, that will dump the 
contents of the dwarf index, and we can verify that it is empty. A good way to 
that is to use CHECK/CHECK-NEXT/EMPTY to match the lines before after the place 
where the output should appear. So, something like this might work:

  # CHECK: Function basenames:
  ## if the next line is empty then no entries have been printed
  # CHECK-EMPTY:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131437

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

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Michał Górny via Phabricator via lldb-commits]

mgorny marked an inline comment as done.
mgorny added a comment.

Ok, so I'm going to give it a shot today. Could you clarify one thing though?

> All it would take is to add some locking around the PendingCallback 
> manipulations

Are you thinking of locking directly in `MainLoop` classes, or in the caller? 
It'd feel a bit inconsistent if `MainLoop`s provided locking around pending 
callback manipulations and not other methods. However, I suppose making 
everything thread-safe wouldn't hurt either.


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

https://reviews.llvm.org/D131160

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

[Lldb-commits] [PATCH] D131974: [lldb][ClangExpression] Add asm() label to all FunctionDecls we create from DWARF

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D131974#3734849 , @labath wrote:

> In D131974#3733024 , @Michael137 
> wrote:
>
>> In D131974#3732727 , @labath wrote:
>>
>>> Any particular reason for not removing the `SC_Extern` check? I'm pretty 
>>> sure I could create a test case with a `static` abi-tagged function which 
>>> we wouldn't be able to call with that check in place...
>>
>> Boiled down to this comment:
>>
>>> the local (indicated by the `L` in the name) mangled name is not 
>>> necessarily unique
>>
>> Attaching it to non-external functions caused 
>> `cpp/namespace/TestNamespaceLookup.py` to fail. With a non-static and a 
>> static version of `func()` LLDB chose the wrong one. I can follow up on 
>> whether this is a bug elsewhere and how feasible it would be to 
>> unconditionally attach the label
>
> Ah, this is a fun one. So this breaks the 
> `test_scope_lookup_with_run_command` test in `TestNamespaceLookup.py`, but 
> simultaneously "fixes"  `test_file_scope_lookup_with_run_command`. I would 
> say this is a pre-existing bug, that just happens to manifest itself 
> differently with this change. The problem is that lldb is not able to 
> correctly scope CU-local objects, as it ends up putting all files into one 
> huge AST context. It was mostly accidental that lldb picked the external 
> version of the function (because it happened to get its name mangling 
> "right"). Now it happens to pick the static one as they both have the same 
> signature (and I guess this is the one that gets parsed first).

When I initially looked at, it seemed unintuitive for LLDB to pick the 
file-static function (from a context where it's usually not accessible) over 
the one in the header so I kept the check in.
But it's true that the storage-class just works around a different bug. There 
seem to always be cases where we pick an incorrect overload no matter how you 
flip it.
Happy to remove the check and flip the XFAIL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131974

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

[Lldb-commits] [lldb] d6e1e01 - [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T15:36:03+02:00
New Revision: d6e1e01da949c8cb4b869cee1953cf19a6d072c0

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

LOG: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

Harmonize the status and error values of Communication::Read() when
running with and without read thread.  Prior to this change, Read()
would return eConnectionStatusSuccess if read thread was enabled
and the read timed out or reached end-of-file, rather than
the respective states that are returned if read thread was disabled.
Now, it correctly returns eConnectionStatusTimedOut
and eConnectionStatusEndOfFile, and sets the error respectively.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D132217

Added: 


Modified: 
lldb/source/Core/Communication.cpp
lldb/unittests/Core/CommunicationTest.cpp

Removed: 




diff  --git a/lldb/source/Core/Communication.cpp 
b/lldb/source/Core/Communication.cpp
index d5f8f8922f5e1..56f0990d8c7f7 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -132,10 +132,16 @@ size_t Communication::Read(void *dst, size_t dst_len,
   if (m_read_thread_enabled) {
 // We have a dedicated read thread that is getting data for us
 size_t cached_bytes = GetCachedBytes(dst, dst_len);
-if (cached_bytes > 0 || (timeout && timeout->count() == 0)) {
+if (cached_bytes > 0) {
   status = eConnectionStatusSuccess;
   return cached_bytes;
 }
+if (timeout && timeout->count() == 0) {
+  if (error_ptr)
+error_ptr->SetErrorString("Timed out.");
+  status = eConnectionStatusTimedOut;
+  return 0;
+}
 
 if (!m_connection_sp) {
   if (error_ptr)
@@ -155,11 +161,25 @@ size_t Communication::Read(void *dst, size_t dst_len,
   }
 
   if (event_type & eBroadcastBitReadThreadDidExit) {
+// If the thread exited of its own accord, it either means it
+// hit an end-of-file condition or lost connection
+// (we verified that we had an connection above).
+if (!m_connection_sp) {
+  if (error_ptr)
+error_ptr->SetErrorString("Lost connection.");
+  status = eConnectionStatusLostConnection;
+} else
+  status = eConnectionStatusEndOfFile;
+
 if (GetCloseOnEOF())
   Disconnect(nullptr);
-break;
+return 0;
   }
 }
+
+if (error_ptr)
+  error_ptr->SetErrorString("Timed out.");
+status = eConnectionStatusTimedOut;
 return 0;
   }
 

diff  --git a/lldb/unittests/Core/CommunicationTest.cpp 
b/lldb/unittests/Core/CommunicationTest.cpp
index bb57a7b14670f..024684a486c2d 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,72 @@
 
 using namespace lldb_private;
 
+static void CommunicationReadTest(bool use_read_thread) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Status error;
+  size_t bytes_written;
+  ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(),
+llvm::Succeeded());
+  ASSERT_EQ(bytes_written, 4U);
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+
+  if (use_read_thread)
+ASSERT_TRUE(comm.StartReadThread());
+
+  // This read should wait for the data to become available and return it.
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char buf[16];
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 
4U);
+  EXPECT_EQ(status, lldb::eConnectionStatusSuccess);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  buf[4] = 0;
+  EXPECT_STREQ(buf, "test");
+
+  // These reads should time out as there is no more data.
+  error.Clear();
+  EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status,
+  &error),
+0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // 0 is special-cased, so we test it separately.
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 
0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // This read should return EOF.
+  pipe.CloseWriteFileDescriptor();
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 
0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile);
+  EXPECT_THAT_ERROR(err

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6e1e01da949: [lldb] [Core] Harmonize Communication::Read() 
returns w/ thread (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132217

Files:
  lldb/source/Core/Communication.cpp
  lldb/unittests/Core/CommunicationTest.cpp

Index: lldb/unittests/Core/CommunicationTest.cpp
===
--- lldb/unittests/Core/CommunicationTest.cpp
+++ lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,72 @@
 
 using namespace lldb_private;
 
+static void CommunicationReadTest(bool use_read_thread) {
+  Pipe pipe;
+  ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
+llvm::Succeeded());
+
+  Status error;
+  size_t bytes_written;
+  ASSERT_THAT_ERROR(pipe.Write("test", 4, bytes_written).ToError(),
+llvm::Succeeded());
+  ASSERT_EQ(bytes_written, 4U);
+
+  Communication comm("test");
+  comm.SetConnection(std::make_unique(
+  pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true));
+  comm.SetCloseOnEOF(true);
+
+  if (use_read_thread)
+ASSERT_TRUE(comm.StartReadThread());
+
+  // This read should wait for the data to become available and return it.
+  lldb::ConnectionStatus status = lldb::eConnectionStatusSuccess;
+  char buf[16];
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 4U);
+  EXPECT_EQ(status, lldb::eConnectionStatusSuccess);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  buf[4] = 0;
+  EXPECT_STREQ(buf, "test");
+
+  // These reads should time out as there is no more data.
+  error.Clear();
+  EXPECT_EQ(comm.Read(buf, sizeof(buf), std::chrono::microseconds(10), status,
+  &error),
+0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // 0 is special-cased, so we test it separately.
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(0), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusTimedOut);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Failed());
+
+  // This read should return EOF.
+  pipe.CloseWriteFileDescriptor();
+  error.Clear();
+  EXPECT_EQ(
+  comm.Read(buf, sizeof(buf), std::chrono::seconds(5), status, &error), 0U);
+  EXPECT_EQ(status, lldb::eConnectionStatusEndOfFile);
+  EXPECT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+
+  // JoinReadThread() should just return immediately since there was no read
+  // thread started.
+  EXPECT_TRUE(comm.JoinReadThread());
+}
+
+TEST(CommunicationTest, Read) {
+  CommunicationReadTest(/*use_thread=*/false);
+}
+
+TEST(CommunicationTest, ReadThread) {
+  CommunicationReadTest(/*use_thread=*/true);
+}
+
 #ifndef _WIN32
 TEST(CommunicationTest, SynchronizeWhileClosing) {
   // Set up a communication object reading from a pipe.
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -132,10 +132,16 @@
   if (m_read_thread_enabled) {
 // We have a dedicated read thread that is getting data for us
 size_t cached_bytes = GetCachedBytes(dst, dst_len);
-if (cached_bytes > 0 || (timeout && timeout->count() == 0)) {
+if (cached_bytes > 0) {
   status = eConnectionStatusSuccess;
   return cached_bytes;
 }
+if (timeout && timeout->count() == 0) {
+  if (error_ptr)
+error_ptr->SetErrorString("Timed out.");
+  status = eConnectionStatusTimedOut;
+  return 0;
+}
 
 if (!m_connection_sp) {
   if (error_ptr)
@@ -155,11 +161,25 @@
   }
 
   if (event_type & eBroadcastBitReadThreadDidExit) {
+// If the thread exited of its own accord, it either means it
+// hit an end-of-file condition or lost connection
+// (we verified that we had an connection above).
+if (!m_connection_sp) {
+  if (error_ptr)
+error_ptr->SetErrorString("Lost connection.");
+  status = eConnectionStatusLostConnection;
+} else
+  status = eConnectionStatusEndOfFile;
+
 if (GetCloseOnEOF())
   Disconnect(nullptr);
-break;
+return 0;
   }
 }
+
+if (error_ptr)
+  error_ptr->SetErrorString("Timed out.");
+status = eConnectionStatusTimedOut;
 return 0;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D131160#3735024 , @mgorny wrote:

> Ok, so I'm going to give it a shot today. Could you clarify one thing though?
>
>> All it would take is to add some locking around the PendingCallback 
>> manipulations
>
> Are you thinking of locking directly in `MainLoop` classes, or in the caller? 
> It'd feel a bit inconsistent if `MainLoop`s provided locking around pending 
> callback manipulations and not other methods. However, I suppose making 
> everything thread-safe wouldn't hurt either.

I'm not sure this can be done from the outside, as the callback adittions need 
to synchronize with the accesses that happen inside the Run function. It is 
somewhat inconsistent, but for example python has special 
`call_soon`/`call_soon_threadsafe`  versions for code which needs to schedule 
callbacks from other threads. We could do something like that. Or we could also 
just lock everything...


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

https://reviews.llvm.org/D131160

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

[Lldb-commits] [lldb] 7aadeca - [lldb] [test] Add synchronization to TestContinue

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T15:49:35+02:00
New Revision: 7aadecae404b8d47aabdd874e9018a76fd4d1ffa

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

LOG: [lldb] [test] Add synchronization to TestContinue

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

Removed: 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
index f0f0df81d321d..8d26cfbfdcc4e 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -38,6 +38,8 @@ def qfThreadInfo(self):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
 self.assertPacketLogContains(["vCont;C13:401"])
 
 def test_continue_no_vCont(self):
@@ -55,6 +57,8 @@ def other(self, packet):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
 self.assertPacketLogContains(["Hc401", "C13"])
 
 def test_continue_multiprocess(self):
@@ -65,4 +69,6 @@ class MyResponder(self.BaseResponder):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
 self.assertPacketLogContains(["vCont;C13:p400.401"])



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

[Lldb-commits] [PATCH] D132231: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: labath.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This check was put in place to prevent static functions
from translation units outside the one that the current
expression is evaluated from taking precedence over functions
in the global namespace. However, this is really a different
bug. LLDB lumps functions from all CUs into a single AST and
ends up picking the file-static even when C++ context rules
wouldn't allow that to happen.

This patch removes the check so we apply the AsmLabel to all
FunctionDecls we create from DWARF if we have a linkage name
available. This makes the code-path easier to reason about and
allows calling static functions in contexts where we previously
would've chosen the wrong function.

We also flip the XFAILs in the API test to reflect what effect
this change has.

**Testing**

- Fixed API tests and added XFAIL


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132231

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -36,6 +36,26 @@
 substrs=['stopped',
  'stop reason = breakpoint'])
 
+@skipIfWindows # This is flakey on Windows: llvm.org/pr38373
+@expectedFailure("CU-local objects incorrectly scoped")
+def test_scope_lookup_with_run_command(self):
+"""Test scope lookup of functions in lldb."""
+self.build()
+
+lldbutil.run_to_source_breakpoint(
+self,
+self.line_break_global_scope,
+lldb.SBFileSpec("ns.cpp"))
+
+# Evaluate func() - should call ::func()
+# FIXME: LLDB does not correctly scope CU-local objects.
+# LLDB currently lumps functions from all files into
+# a single AST and depending on the order with which
+# functions are considered, LLDB can incorrectly call
+# the static local ns.cpp::func() instead of the expected
+# ::func()
+self.expect_expr("func()", expect_type="int", expect_value="1")
+
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_with_run_command(self):
 """Test scope lookup of functions in lldb."""
@@ -81,8 +101,7 @@
 
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
-# Evaluate func() - should call ::func()
-self.expect("expr -- func()", startstr="(int) $0 = 1")
+
 # Evaluate A::B::func() - should call A::B::func()
 self.expect("expr -- A::B::func()", startstr="(int) $1 = 4")
 # Evaluate func(10) - should call ::func(int)
@@ -174,7 +193,6 @@
 # before functions.
 self.expect("expr -- foo()", startstr="(int) $2 = 42")
 
-@expectedFailure("lldb file scope lookup bugs")
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_file_scope_lookup_with_run_command(self):
 """Test file scope lookup in lldb."""
@@ -191,9 +209,7 @@
 # Run to BP_file_scope at file scope
 self.runToBkpt("run")
 # Evaluate func() - should call static ns2.cpp:func()
-# FIXME: This test fails because lldb doesn't know about file scopes so
-# finds the global ::func().
-self.expect("expr -- func()", startstr="(int) $0 = 2")
+self.expect_expr("func()", result_type="int", result_value="2")
 
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_before_using_with_run_command(self):
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1258,7 +1258,7 @@
   // example is generating calls to ABI-tagged template functions.
   // This is done separately for member functions in
   // AddMethodToCXXRecordType.
-  if (attrs.mangled_name && attrs.storage == clang::SC_Extern)
+  if (attrs.mangled_name)
 function_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(
 m_ast.getASTContext(), attrs.mangled_name, /*literal=*/false));
 


Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -36,6 +36,26 @@
 subst

[Lldb-commits] [PATCH] D132231: [lldb][ClangExpression] Remove storage-class check when creating AsmLabel

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py:212
 # Evaluate func() - should call static ns2.cpp:func()
-# FIXME: This test fails because lldb doesn't know about file scopes so
-# finds the global ::func().
-self.expect("expr -- func()", startstr="(int) $0 = 2")
+self.expect_expr("func()", result_type="int", result_value="2")
 

should probably move this to the appropriate test-function that isn't xfailed 
so we don't spin up an lldb instance just for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132231

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

[Lldb-commits] [PATCH] D131974: [lldb][ClangExpression] Add asm() label to all FunctionDecls we create from DWARF

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

https://reviews.llvm.org/D132231


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131974

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

[Lldb-commits] [PATCH] D132217: [lldb] [Core] Harmonize Communication::Read() returns w/ thread

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.
Herald added a subscriber: JDevlieghere.

@labath, any chance you could take a look at the failures on Windows? I'm 
wondering if this is because "pipes don't work like that on Windows" or if 
there's a way to make them work. For now I'm just going to skip these tests on 
win32.

https://lab.llvm.org/buildbot/#/builders/83/builds/22672


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132217

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

[Lldb-commits] [lldb] d38985a - [lldb] [test] Disable new CommunicationTests on Windows

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-19T16:23:39+02:00
New Revision: d38985a36be8b0165787f8893b3d2b0f831d1e83

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

LOG: [lldb] [test] Disable new CommunicationTests on Windows

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/unittests/Core/CommunicationTest.cpp

Removed: 




diff  --git a/lldb/unittests/Core/CommunicationTest.cpp 
b/lldb/unittests/Core/CommunicationTest.cpp
index 024684a486c2d..13ebfae772f47 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,7 @@
 
 using namespace lldb_private;
 
+#ifndef _WIN32
 static void CommunicationReadTest(bool use_read_thread) {
   Pipe pipe;
   ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
@@ -87,7 +88,6 @@ TEST(CommunicationTest, ReadThread) {
   CommunicationReadTest(/*use_thread=*/true);
 }
 
-#ifndef _WIN32
 TEST(CommunicationTest, SynchronizeWhileClosing) {
   // Set up a communication object reading from a pipe.
   Pipe pipe;



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

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 454015.
mgorny added a comment.

Rebase. Replace "events" with triggering pending callbacks.


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

https://reviews.llvm.org/D131160

Files:
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/include/lldb/Host/windows/MainLoopWindows.h
  lldb/source/Host/common/MainLoopBase.cpp
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/source/Host/windows/MainLoopWindows.cpp
  lldb/unittests/Host/MainLoopTest.cpp

Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -148,6 +148,27 @@
   ASSERT_EQ(3u, callback_count);
 }
 
+TEST_F(MainLoopTest, PendingCallbackTrigger) {
+  MainLoop loop;
+  bool callback_called = false;
+  loop.AddPendingCallback([&](MainLoopBase &loop) {
+callback_called = true;
+loop.RequestTermination();
+  });
+  Status error;
+  auto socket_handle = loop.RegisterReadObject(
+  socketpair[1], [](MainLoopBase &) {}, error);
+  ASSERT_TRUE(socket_handle);
+  ASSERT_THAT_ERROR(error.ToError(), llvm::Succeeded());
+  std::thread notifier([&]() {
+sleep(1);
+loop.TriggerPendingCallbacks();
+  });
+  ASSERT_THAT_ERROR(loop.Run().ToError(), llvm::Succeeded());
+  notifier.join();
+  ASSERT_TRUE(callback_called);
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/windows/MainLoopWindows.cpp
===
--- lldb/source/Host/windows/MainLoopWindows.cpp
+++ lldb/source/Host/windows/MainLoopWindows.cpp
@@ -21,18 +21,30 @@
 using namespace lldb;
 using namespace lldb_private;
 
+MainLoopWindows::MainLoopWindows() {
+  m_trigger_event = WSACreateEvent();
+  assert(m_trigger_event != WSA_INVALID_EVENT);
+}
+
+MainLoopWindows::~MainLoopWindows() {
+  assert(m_read_fds.empty());
+  BOOL result = WSACloseEvent(m_trigger_event);
+  assert(result == TRUE);
+}
+
 llvm::Expected MainLoopWindows::Poll() {
-  std::vector read_events;
-  read_events.reserve(m_read_fds.size());
+  std::vector events;
+  events.reserve(m_read_fds.size() + 1);
   for (auto &[fd, info] : m_read_fds) {
 int result = WSAEventSelect(fd, info.event, FD_READ | FD_ACCEPT | FD_CLOSE);
 assert(result == 0);
 
-read_events.push_back(info.event);
+events.push_back(info.event);
   }
+  events.push_back(loop.m_trigger_event);
 
   DWORD result = WSAWaitForMultipleEvents(
-  read_events.size(), read_events.data(), FALSE, WSA_INFINITE, FALSE);
+  events.size(), events.data(), FALSE, WSA_INFINITE, FALSE);
 
   for (auto &fd : m_read_fds) {
 int result = WSAEventSelect(fd.first, WSA_INVALID_EVENT, 0);
@@ -40,7 +52,7 @@
   }
 
   if (result >= WSA_WAIT_EVENT_0 &&
-  result < WSA_WAIT_EVENT_0 + read_events.size())
+  result <= WSA_WAIT_EVENT_0 + read_events.size())
 return result - WSA_WAIT_EVENT_0;
 
   return llvm::createStringError(llvm::inconvertibleErrorCode(),
@@ -106,10 +118,18 @@
 if (!signaled_event)
   return Status(signaled_event.takeError());
 
-auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
-
-ProcessReadObject(KV.first);
+if (*signaled_event < m_read_fds.size()) {
+  auto &KV = *std::next(m_read_fds.begin(), *signaled_event);
+  ProcessReadObject(KV.first);
+} else {
+  assert(*signaled_event == m_read_fds.size());
+  WSAResetEvent(m_trigger_event);
+}
 ProcessPendingCallbacks();
   }
   return Status();
 }
+
+void MainLoopWindows::TriggerPendingCallbacks() {
+  WSASetEvent(m_trigger_event);
+}
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/PosixApi.h"
 #include "lldb/Utility/Status.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Errno.h"
 #include 
 #include 
 #include 
@@ -222,6 +223,18 @@
 #endif
 
 MainLoopPosix::MainLoopPosix() {
+  Status error = m_trigger_pipe.CreateNew(/*child_process_inherit=*/false);
+  assert(error.Success());
+  const int trigger_pipe_fd = m_trigger_pipe.GetReadFileDescriptor();
+  m_read_fds.insert({trigger_pipe_fd, [trigger_pipe_fd](MainLoopBase &loop) {
+   char c;
+   ssize_t bytes_read = llvm::sys::RetryAfterSignal(
+   -1, ::read, trigger_pipe_fd, &c, 1);
+   assert(bytes_read == 1);
+   UNUSED_IF_ASSERT_DISABLED(bytes_read);
+   // NB: This implicitly causes another loop iteration
+   // and therefore the execution of pending callbacks.
+ }});
 #if HAVE_SYS_EVENT_H
   m_kqueue = kqueue();
   assert(m_kqueue >= 0);
@@ -232,6 +245,8 @@
 #if HAVE_SYS

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

@labath, what do you think of this?

Also I'm wondering about error handling. Should we introduce fallible 
constructor for this?


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

https://reviews.llvm.org/D131160

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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.
Herald added a subscriber: JDevlieghere.

This seems to cause all API tests to time out.

See LLDB Incremental buildbot: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Can we revert this until we know what the root cause is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [lldb] fe0f72d - Revert "[lldb] [test] Add synchronization to TestContinue"

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-08-19T09:23:18-07:00
New Revision: fe0f72d5c55a9b95c5564089e946e8f08112e995

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

LOG: Revert "[lldb] [test] Add synchronization to TestContinue"

This reverts commit 7aadecae404b8d47aabdd874e9018a76fd4d1ffa.

I'm reverting this commit because it appears to break the green dragon
incremental LLDB bot.

https://reviews.llvm.org/D131758

See LLDB Incremental buildbot: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

Removed: 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
index 8d26cfbfdcc4e..f0f0df81d321d 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -38,8 +38,6 @@ def qfThreadInfo(self):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
-lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
-  [lldb.eStateExited])
 self.assertPacketLogContains(["vCont;C13:401"])
 
 def test_continue_no_vCont(self):
@@ -57,8 +55,6 @@ def other(self, packet):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
-lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
-  [lldb.eStateExited])
 self.assertPacketLogContains(["Hc401", "C13"])
 
 def test_continue_multiprocess(self):
@@ -69,6 +65,4 @@ class MyResponder(self.BaseResponder):
 self.runCmd("platform select remote-linux")
 target = self.createTarget("a.yaml")
 process = self.connect(target)
-lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
-  [lldb.eStateExited])
 self.assertPacketLogContains(["vCont;C13:p400.401"])



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

[Lldb-commits] [lldb] 00c4852 - Revert "[lldb] [test] Disable new CommunicationTests on Windows"

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-08-19T09:23:17-07:00
New Revision: 00c4852561a7ea51e0514759efd745ff01137b4c

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

LOG: Revert "[lldb] [test] Disable new CommunicationTests on Windows"

This reverts commit d38985a36be8b0165787f8893b3d2b0f831d1e83.
because I'm also reverting D131758.

Added: 


Modified: 
lldb/unittests/Core/CommunicationTest.cpp

Removed: 




diff  --git a/lldb/unittests/Core/CommunicationTest.cpp 
b/lldb/unittests/Core/CommunicationTest.cpp
index 13ebfae772f47..024684a486c2d 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -21,7 +21,6 @@
 
 using namespace lldb_private;
 
-#ifndef _WIN32
 static void CommunicationReadTest(bool use_read_thread) {
   Pipe pipe;
   ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
@@ -88,6 +87,7 @@ TEST(CommunicationTest, ReadThread) {
   CommunicationReadTest(/*use_thread=*/true);
 }
 
+#ifndef _WIN32
 TEST(CommunicationTest, SynchronizeWhileClosing) {
   // Set up a communication object reading from a pipe.
   Pipe pipe;



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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D131758#3735621 , @Michael137 
wrote:

> This seems to cause all API tests to time out.
>
> See LLDB Incremental buildbot: 
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/
>
> Can we revert this until we know what the root cause is?

I reverted this and the follow-up commit that disables a test on Windows for 
now. Please let us know if you need any help in diagnosing the problem!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [PATCH] D132191: Treat a UUID of all zeros consistently to mean "Invalid UUID"

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D132191#3734014 , @clayborg wrote:

> Minidump files have UUID values that are zeroed out. We will need to do 
> something for these that can sense all zeroes and return a default 
> constructed one.

I'm confused.  The MinidumParser.cpp already used fromOptionalData and 
fromOptionalStringRef to create its UUID's.  So this patch won't change the 
Minidump behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132191

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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D131758#3735628 , @aprantl wrote:

> In D131758#3735621 , @Michael137 
> wrote:
>
>> This seems to cause all API tests to time out.
>>
>> See LLDB Incremental buildbot: 
>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/
>>
>> Can we revert this until we know what the root cause is?
>
> I reverted this and the follow-up commit that disables a test on Windows for 
> now. Please let us know if you need any help in diagnosing the problem!

Help would be most welcome. Unless my grep skills are failing me, the log 
doesn't contain anything but timeouts. I'd use at least some gdb-remote log. My 
only guess right now would be that debugserver doesn't support `vCont;c:-1`.

Alternatively, could you try reapplying the original commit but changing:

  continue_packet.Format("vCont;c:{0}-1", pid_prefix);

back to:

  continue_packet.PutCString("c");


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [PATCH] D132191: Treat a UUID of all zeros consistently to mean "Invalid UUID"

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Remember, all this patch does is make a universal rule that "All zero UUID's 
are placeholders and not meant to be matched against."  The only 
platforms/ObjectFile readers that weren't already following that rule were 
Darwin - where this was definitely just historical accident - and 
ObjectFileELF.  Then there were a few places in generic code where we seem to 
have tossed a coin for which behavior to use.  Windows & Breakpad were already 
calling the "Optional" version of the set functions.  So this is only a change 
in behavior for ELF.




Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:1392
   image_infos[i].SetName((const char *)name_data);
-  UUID uuid = UUID::fromOptionalData(extractor.GetData(&offset, 16), 16);
+  UUID uuid = UUID::fromData(extractor.GetData(&offset, 16), 16);
   image_infos[i].SetUUID(uuid);

clayborg wrote:
> Have you checked if the kernel ever just specifies all zeroes here? It it 
> does, this will change things right?
On Darwin, UUID's of all zeros always means "I had to put in a UUID, but I 
don't actually want you to use it".  I don't know whether the Kernel currently 
produces this or not - it's currently used for the stub binaries that 
Playgrounds uses.



Comment at: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp:70-72
   if (pdb70_uuid->Age != 0)
-return UUID::fromOptionalData(pdb70_uuid, sizeof(*pdb70_uuid));
-  return UUID::fromOptionalData(&pdb70_uuid->Uuid,
+return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+  return UUID::fromData(&pdb70_uuid->Uuid,

clayborg wrote:
> Fix the UUID UUID::fromCvRecord(UUID::CvRecordPdb70) function and call that 
> function from here.
Note that fromCvRecord already used the fromOptional version, so its behavior 
doesn't change with this patch.  I don't think anything else needs to be done 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132191

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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D131758#3735670 , @mgorny wrote:

> In D131758#3735628 , @aprantl wrote:
>
>> In D131758#3735621 , @Michael137 
>> wrote:
>>
>>> This seems to cause all API tests to time out.
>>>
>>> See LLDB Incremental buildbot: 
>>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/
>>>
>>> Can we revert this until we know what the root cause is?
>>
>> I reverted this and the follow-up commit that disables a test on Windows for 
>> now. Please let us know if you need any help in diagnosing the problem!
>
> Help would be most welcome. Unless my grep skills are failing me, the log 
> doesn't contain anything but timeouts. I'd use at least some gdb-remote log. 
> My only guess right now would be that debugserver doesn't support 
> `vCont;c:-1`.
>
> Alternatively, could you try reapplying the original commit but changing:
>
>   continue_packet.Format("vCont;c:{0}-1", pid_prefix);
>
> back to:
>
>   continue_packet.PutCString("c");

Can confirm this fixes the timeouts.

Will still revert for now and let you fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D131758#3735670 , @mgorny wrote:

> In D131758#3735628 , @aprantl wrote:
>
>> In D131758#3735621 , @Michael137 
>> wrote:
>>
>>> This seems to cause all API tests to time out.
>>>
>>> See LLDB Incremental buildbot: 
>>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46215/execution/node/70/log/
>>>
>>> Can we revert this until we know what the root cause is?
>>
>> I reverted this and the follow-up commit that disables a test on Windows for 
>> now. Please let us know if you need any help in diagnosing the problem!
>
> Help would be most welcome. Unless my grep skills are failing me, the log 
> doesn't contain anything but timeouts. I'd use at least some gdb-remote log. 
> My only guess right now would be that debugserver doesn't support 
> `vCont;c:-1`.
>
> Alternatively, could you try reapplying the original commit but changing:
>
>   continue_packet.Format("vCont;c:{0}-1", pid_prefix);
>
> back to:
>
>   continue_packet.PutCString("c");

@mgorny

Snippet of test log with timeout:

  
  UNSUPPORTED: lldb-api :: 
commands/expression/call-restarts/TestCallThatRestarts.py (64 of 1755)
  TIMEOUT: lldb-api :: sample_test/TestSampleInlineTest.py (65 of 1755)
   TEST 'lldb-api :: sample_test/TestSampleInlineTest.py' 
FAILED 
  Script:
  --
  
/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/bin/python3.8
 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/dotest.py
 -u CXXFLAGS -u CFLAGS --codesign-identity lldb_codesign --arch x86_64 
--build-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex
 -t --env TERM=vt100 --env 
LLVM_LIBS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib 
--env 
LLVM_INCLUDE_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/include
 --env 
LLVM_TOOLS_DIR=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin 
--hermetic-libcxx --arch x86_64 --build-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex
 --lldb-module-cache-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-lldb/lldb-api
 --clang-module-cache-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/lldb-test-build.noindex/module-cache-clang/lldb-api
 --executable 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/lldb --compiler 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/clang 
--dsymutil 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin/dsymutil 
--llvm-tools-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./bin --lldb-libs-dir 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/./lib 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/API/sample_test
 -p TestSampleInlineTest.py
  --
  Exit Code: -9
  Timeout: Reached timeout of 600 seconds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [PATCH] D132250: Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess"

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This reverts commit ccb9d4d4addc2fb2aa94cf776d43d8be35365272 
.

https://reviews.llvm.org/D131758


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132250

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  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
@@ -1186,15 +1186,11 @@
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
-  std::string pid_prefix;
-  if (m_gdb_comm.GetMultiprocessSupported())
-pid_prefix = llvm::formatv("p{0:x-}.", GetID());
-
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing
-continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+// All threads are continuing, just send a "c" packet
+continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1204,7 +1200,7 @@
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
+  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1215,8 +1211,8 @@
  s_pos = m_continue_C_tids.begin(),
  s_end = m_continue_C_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Format(";C{0:x-2}:{1}{2:x-}", s_pos->second,
- pid_prefix, s_pos->first);
+  continue_packet.Printf(";C%2.2x:%4.4" PRIx64, s_pos->second,
+ s_pos->first);
   } else
 continue_packet_error = true;
 }
@@ -1227,7 +1223,7 @@
  t_pos = m_continue_s_tids.begin(),
  t_end = m_continue_s_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Format(";s:{0}{1:x-}", pid_prefix, *t_pos);
+  continue_packet.Printf(";s:%4.4" PRIx64, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1238,8 +1234,8 @@
  s_pos = m_continue_S_tids.begin(),
  s_end = m_continue_S_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Format(";S{0:x-2}:{1}{2:x-}", s_pos->second,
- pid_prefix, s_pos->first);
+  continue_packet.Printf(";S%2.2x:%4.4" PRIx64, s_pos->second,
+ s_pos->first);
   } else
 continue_packet_error = true;
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -339,8 +339,6 @@
 
   bool GetQXferSigInfoReadSupported();
 
-  bool GetMultiprocessSupported();
-
   LazyBool SupportsAllocDeallocMemory() // const
   {
 // Uncomment this to have lldb pretend the debug server doesn't respond to
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
@@ -181,12 +181,6 @@
   return m_supports_qXfer_siginfo_read == eLazyBoolYes;
 }
 
-bool GDBRemoteCommunicationClient::GetMultiprocessSupported() {
-  if (m_supports_memory_tagging == eLazyBoolCalculate)
-GetRemoteQSupported();
-  return m_supports_multiprocess == eLazyBoolYes;
-}
-
 uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
 GetRemoteQSupported();
@@ -1520,7 +1514,7 @@
 }
   }
 
-  if (GetMultiprocessSupported()) {
+  if (m_supports_multiprocess) {
 // Some servers (e.g. qemu) require specifying the PID even if only a single
 // process is running.
 if (pid == LLDB_INVALID_PROCESS_ID)

[Lldb-commits] [lldb] 5517401 - Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess"

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-08-19T18:05:41+01:00
New Revision: 5517401f936ab5cb9db001030f027293d44aac92

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

LOG: Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess"

This reverts commit ccb9d4d4addc2fb2aa94cf776d43d8be35365272.

https://reviews.llvm.org/D131758

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 11d0fec1926ab..29417b35fde3a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -181,12 +181,6 @@ bool 
GDBRemoteCommunicationClient::GetQXferSigInfoReadSupported() {
   return m_supports_qXfer_siginfo_read == eLazyBoolYes;
 }
 
-bool GDBRemoteCommunicationClient::GetMultiprocessSupported() {
-  if (m_supports_memory_tagging == eLazyBoolCalculate)
-GetRemoteQSupported();
-  return m_supports_multiprocess == eLazyBoolYes;
-}
-
 uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
 GetRemoteQSupported();
@@ -1520,7 +1514,7 @@ Status GDBRemoteCommunicationClient::Detach(bool 
keep_stopped,
 }
   }
 
-  if (GetMultiprocessSupported()) {
+  if (m_supports_multiprocess) {
 // Some servers (e.g. qemu) require specifying the PID even if only a 
single
 // process is running.
 if (pid == LLDB_INVALID_PROCESS_ID)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 4e89ade772ad3..3a62747603f6d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -339,8 +339,6 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   bool GetQXferSigInfoReadSupported();
 
-  bool GetMultiprocessSupported();
-
   LazyBool SupportsAllocDeallocMemory() // const
   {
 // Uncomment this to have lldb pretend the debug server doesn't respond to

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 0c876a8fd703e..30fb27932d192 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1186,15 +1186,11 @@ Status ProcessGDBRemote::DoResume() {
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
-  std::string pid_prefix;
-  if (m_gdb_comm.GetMultiprocessSupported())
-pid_prefix = llvm::formatv("p{0:x-}.", GetID());
-
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing
-continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+// All threads are continuing, just send a "c" packet
+continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1204,7 +1200,7 @@ Status ProcessGDBRemote::DoResume() {
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
+  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1215,8 +1211,8 @@ Status ProcessGDBRemote::DoResume() {
  s_pos = m_continue_C_tids.begin(),
  s_end = m_continue_C_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Format(";C{0:x-2}:{1}{2:x-}", s_pos->second,
- pid_prefix, s_pos->first);
+  continue_packet.Printf(";C%2.2x:%4.4" PRIx64, s_pos->second,
+ s_pos->first);
   } else
 continue_packet_error = true;
 }
@@ -1227,7 +1223,7 @@ Status ProcessGDBRemote::DoResume() {
  t_pos = m_continue_s_tids.begin(),
  t_end = m_continue_s_tids.end();
  t_pos != t_end; ++t_pos)
-   

[Lldb-commits] [PATCH] D132250: Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess"

[Michael Buch via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5517401f936a: Revert "[lldb] [gdb-remote] Include PID 
in vCont packets if multiprocess" (authored by Michael137).
Herald added a subscriber: JDevlieghere.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132250

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  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
@@ -1186,15 +1186,11 @@
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
-  std::string pid_prefix;
-  if (m_gdb_comm.GetMultiprocessSupported())
-pid_prefix = llvm::formatv("p{0:x-}.", GetID());
-
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing
-continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+// All threads are continuing, just send a "c" packet
+continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1204,7 +1200,7 @@
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
+  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1215,8 +1211,8 @@
  s_pos = m_continue_C_tids.begin(),
  s_end = m_continue_C_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Format(";C{0:x-2}:{1}{2:x-}", s_pos->second,
- pid_prefix, s_pos->first);
+  continue_packet.Printf(";C%2.2x:%4.4" PRIx64, s_pos->second,
+ s_pos->first);
   } else
 continue_packet_error = true;
 }
@@ -1227,7 +1223,7 @@
  t_pos = m_continue_s_tids.begin(),
  t_end = m_continue_s_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Format(";s:{0}{1:x-}", pid_prefix, *t_pos);
+  continue_packet.Printf(";s:%4.4" PRIx64, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1238,8 +1234,8 @@
  s_pos = m_continue_S_tids.begin(),
  s_end = m_continue_S_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Format(";S{0:x-2}:{1}{2:x-}", s_pos->second,
- pid_prefix, s_pos->first);
+  continue_packet.Printf(";S%2.2x:%4.4" PRIx64, s_pos->second,
+ s_pos->first);
   } else
 continue_packet_error = true;
 }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -339,8 +339,6 @@
 
   bool GetQXferSigInfoReadSupported();
 
-  bool GetMultiprocessSupported();
-
   LazyBool SupportsAllocDeallocMemory() // const
   {
 // Uncomment this to have lldb pretend the debug server doesn't respond to
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
@@ -181,12 +181,6 @@
   return m_supports_qXfer_siginfo_read == eLazyBoolYes;
 }
 
-bool GDBRemoteCommunicationClient::GetMultiprocessSupported() {
-  if (m_supports_memory_tagging == eLazyBoolCalculate)
-GetRemoteQSupported();
-  return m_supports_multiprocess == eLazyBoolYes;
-}
-
 uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
 GetRemoteQSupported();
@@ -1520,7 +1514,7 @@
 }
   }
 
-  if (GetMultiprocessSupported()) {
+  if (m_supports_multiprocess) {
 // Some servers (e.g. qemu) require specifying the PID even if only 

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 454058.
mgorny added a comment.

@aprantl, @Michael137, could you confirm that this version works correctly?


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

https://reviews.llvm.org/D131758

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -0,0 +1,74 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestContinue(GDBRemoteTestBase):
+class BaseResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;multiprocess+"
+
+def qfThreadInfo(self):
+return "mp400.401"
+
+def haltReason(self):
+return "S13"
+
+def cont(self):
+return "W01"
+
+def other(self, packet):
+if packet == "vCont?":
+return "vCont;c;C;s;S"
+if packet.startswith("vCont;"):
+return "W00"
+return ""
+
+def test_continue_no_multiprocess(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["vCont;C13:401"])
+
+def test_continue_no_vCont(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+def other(self, packet):
+return ""
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["Hc401", "C13"])
+
+def test_continue_multiprocess(self):
+class MyResponder(self.BaseResponder):
+pass
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["vCont;C13:p400.401"])
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
@@ -1186,11 +1186,18 @@
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
+  std::string pid_prefix;
+  if (m_gdb_comm.GetMultiprocessSupported())
+pid_prefix = llvm::formatv("p{0:x-}.", GetID());
+
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing, just send a "c" packet
-continue_packet.PutCString("c");
+// All threads are continuing
+if (m_gdb_comm.GetMultiprocessSupported())
+  continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+else
+  continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1200,7 +1207,7 @@
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
+  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1211,8 +1218,8 @@
   

[Lldb-commits] [lldb] 1dc8fcf - Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess" - Part 2

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-08-19T19:31:14+01:00
New Revision: 1dc8fcff0e957ee137f59f94859a983425c1ba83

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

LOG: Revert "[lldb] [gdb-remote] Include PID in vCont packets if multiprocess" 
- Part 2

This reverts commit ccb9d4d4addc2fb2aa94cf776d43d8be35365272.

Reverts the associated tests

Added: 


Modified: 


Removed: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py



diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
deleted file mode 100644
index f0f0df81d321d..0
--- a/lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
+++ /dev/null
@@ -1,68 +0,0 @@
-import lldb
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test.decorators import *
-from lldbsuite.test.gdbclientutils import *
-from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
-
-
-class TestContinue(GDBRemoteTestBase):
-class BaseResponder(MockGDBServerResponder):
-def qSupported(self, client_supported):
-return "PacketSize=3fff;QStartNoAckMode+;multiprocess+"
-
-def qfThreadInfo(self):
-return "mp400.401"
-
-def haltReason(self):
-return "S13"
-
-def cont(self):
-return "W01"
-
-def other(self, packet):
-if packet == "vCont?":
-return "vCont;c;C;s;S"
-if packet.startswith("vCont;"):
-return "W00"
-return ""
-
-def test_continue_no_multiprocess(self):
-class MyResponder(self.BaseResponder):
-def qSupported(self, client_supported):
-return "PacketSize=3fff;QStartNoAckMode+"
-
-def qfThreadInfo(self):
-return "m401"
-
-self.server.responder = MyResponder()
-self.runCmd("platform select remote-linux")
-target = self.createTarget("a.yaml")
-process = self.connect(target)
-self.assertPacketLogContains(["vCont;C13:401"])
-
-def test_continue_no_vCont(self):
-class MyResponder(self.BaseResponder):
-def qSupported(self, client_supported):
-return "PacketSize=3fff;QStartNoAckMode+"
-
-def qfThreadInfo(self):
-return "m401"
-
-def other(self, packet):
-return ""
-
-self.server.responder = MyResponder()
-self.runCmd("platform select remote-linux")
-target = self.createTarget("a.yaml")
-process = self.connect(target)
-self.assertPacketLogContains(["Hc401", "C13"])
-
-def test_continue_multiprocess(self):
-class MyResponder(self.BaseResponder):
-pass
-
-self.server.responder = MyResponder()
-self.runCmd("platform select remote-linux")
-target = self.createTarget("a.yaml")
-process = self.connect(target)
-self.assertPacketLogContains(["vCont;C13:p400.401"])



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

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michael Buch via Phabricator via lldb-commits]

Michael137 accepted this revision.
Michael137 added a comment.

In D131758#3735763 , @mgorny wrote:

> @aprantl, @Michael137, could you confirm that this version works correctly?

`ninja check-lldb-api` runs fine with your patch


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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve created this revision.
fdeazeve added a reviewer: aprantl.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When running LLDB tests with a separately built copy of libcxx, it may
be necessary to also use the `nostdinc++` and `cxx-isystem` flags, so
that Clang doesn't keep multiple paths to the standard library in its
include search list. This is particularly important when also running
LLDB tests using older versions of clang, which may not deal with
multiple standard libraries properly.

These issues have all been seen in the LLDB matrix green dragon bot,
which ensures that tip-of-trunk LLDBs can debug code compiled with older
versions of Clang/libcxx.

Once this is merged, llvm-zorg can be updated to use the flags. Local
testing has been done by compiling the project with:

`-DLLDB_TEST_USER_ARGS="--custom-libcpp=`

We verified that all LLDB tests failed to find standard library files.
Then we fixed the invalid path to point to a proper libcxx, and the
tests passed.

A similar effect could have been accomplished by repurposing either
CXXFLAGS or CXXFLAGS_EXTRAS, but these are already for many different
purposes, so we choose to isolate the new behavior into a new
environment variable.

We also choose to keep the logic of formatting the flags inside the
Makefile, as that's where we can check which compiler is being used, as
it is already done in other places there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132257

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -402,6 +402,14 @@
endif
 endif
 
+ifdef CUSTOM_LIBCPP
+ifeq (,$(findstring clang,$(CC)))
+$(error "CUSTOM_LIBCPP only supported for Clang compilers")
+endif
+
+CXXFLAGS += -nostdinc++ -cxx-isystem $(CUSTOM_LIBCPP)
+endif
+
 #--
 # Additional system libraries
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -172,6 +172,11 @@
 type=str,
 metavar='A plugin whose tests will be enabled',
 help='A plugin whose tests will be enabled. The only currently 
supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcpp',
+metavar='custom-libcpp',
+dest='custom_libcpp',
+help='(Clang only) Specify path to a custom standard library to use. 
Disables search for other standard C++ libraries.')
 
 # Configuration options
 group = parser.add_argument_group('Remote platform options')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -335,6 +335,9 @@
 # that explicitly require no debug info.
 os.environ['CFLAGS'] = '-gdwarf-{}'.format(configuration.dwarf_version)
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+
 if args.settings:
 for setting in args.settings:
 if not len(setting) == 1 or not setting[0].count('='):


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -402,6 +402,14 @@
 	endif
 endif
 
+ifdef CUSTOM_LIBCPP
+ifeq (,$(findstring clang,$(CC)))
+$(error "CUSTOM_LIBCPP only supported for Clang compilers")
+endif
+
+CXXFLAGS += -nostdinc++ -cxx-isystem $(CUSTOM_LIBCPP)
+endif
+
 #--
 # Additional system libraries
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -172,6 +172,11 @@
 type=str,
 metavar='A plugin whose tests will be enabled',
 help='A plugin whose tests will be enabled. The only currently supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcp

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve updated this revision to Diff 454073.
fdeazeve added a comment.
Herald added a subscriber: JDevlieghere.

Fixed typo in commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -402,6 +402,14 @@
endif
 endif
 
+ifdef CUSTOM_LIBCPP
+ifeq (,$(findstring clang,$(CC)))
+$(error "CUSTOM_LIBCPP only supported for Clang compilers")
+endif
+
+CXXFLAGS += -nostdinc++ -cxx-isystem $(CUSTOM_LIBCPP)
+endif
+
 #--
 # Additional system libraries
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -172,6 +172,11 @@
 type=str,
 metavar='A plugin whose tests will be enabled',
 help='A plugin whose tests will be enabled. The only currently 
supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcpp',
+metavar='custom-libcpp',
+dest='custom_libcpp',
+help='(Clang only) Specify path to a custom standard library to use. 
Disables search for other standard C++ libraries.')
 
 # Configuration options
 group = parser.add_argument_group('Remote platform options')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -335,6 +335,9 @@
 # that explicitly require no debug info.
 os.environ['CFLAGS'] = '-gdwarf-{}'.format(configuration.dwarf_version)
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+
 if args.settings:
 for setting in args.settings:
 if not len(setting) == 1 or not setting[0].count('='):


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -402,6 +402,14 @@
 	endif
 endif
 
+ifdef CUSTOM_LIBCPP
+ifeq (,$(findstring clang,$(CC)))
+$(error "CUSTOM_LIBCPP only supported for Clang compilers")
+endif
+
+CXXFLAGS += -nostdinc++ -cxx-isystem $(CUSTOM_LIBCPP)
+endif
+
 #--
 # Additional system libraries
 #--
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -172,6 +172,11 @@
 type=str,
 metavar='A plugin whose tests will be enabled',
 help='A plugin whose tests will be enabled. The only currently supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcpp',
+metavar='custom-libcpp',
+dest='custom_libcpp',
+help='(Clang only) Specify path to a custom standard library to use. Disables search for other standard C++ libraries.')
 
 # Configuration options
 group = parser.add_argument_group('Remote platform options')
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -335,6 +335,9 @@
 # that explicitly require no debug info.
 os.environ['CFLAGS'] = '-gdwarf-{}'.format(configuration.dwarf_version)
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+
 if args.settings:
 for setting in args.settings:
 if not len(setting) == 1 or not setting[0].count('='):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Adrian Prantl via Phabricator via lldb-commits]

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

That looks nice and clean!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+

Please don't rely on environment variables to pass arguments to the Make 
invocation. This makes it really tedious to debug make invocations. Instead, 
pass these explicitly as part of the make invocation from the builders 
(`packages/Python/lldbsuite/test/builders/`). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+

JDevlieghere wrote:
> Please don't rely on environment variables to pass arguments to the Make 
> invocation. This makes it really tedious to debug make invocations. Instead, 
> pass these explicitly as part of the make invocation from the builders 
> (`packages/Python/lldbsuite/test/builders/`). 
That is a very good point. Maybe we should just fix the -E option, which 
doesn't work anyway due to Makefiles setting CFLAGS_EXTRAS and use that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+

aprantl wrote:
> JDevlieghere wrote:
> > Please don't rely on environment variables to pass arguments to the Make 
> > invocation. This makes it really tedious to debug make invocations. 
> > Instead, pass these explicitly as part of the make invocation from the 
> > builders (`packages/Python/lldbsuite/test/builders/`). 
> That is a very good point. Maybe we should just fix the -E option, which 
> doesn't work anyway due to Makefiles setting CFLAGS_EXTRAS and use that.
I think a specific flag is fine, we already have something similar for using 
the "hermetic" libc++ (I left a comment below about that). We should see if we 
can unify this.



Comment at: lldb/packages/Python/lldbsuite/test/dotest_args.py:175
 help='A plugin whose tests will be enabled. The only currently 
supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcpp',

We should also think about how this interacts with `--hermetic-libcxx` (and 
make the name of the options consistent). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+

JDevlieghere wrote:
> aprantl wrote:
> > JDevlieghere wrote:
> > > Please don't rely on environment variables to pass arguments to the Make 
> > > invocation. This makes it really tedious to debug make invocations. 
> > > Instead, pass these explicitly as part of the make invocation from the 
> > > builders (`packages/Python/lldbsuite/test/builders/`). 
> > That is a very good point. Maybe we should just fix the -E option, which 
> > doesn't work anyway due to Makefiles setting CFLAGS_EXTRAS and use that.
> I think a specific flag is fine, we already have something similar for using 
> the "hermetic" libc++ (I left a comment below about that). We should see if 
> we can unify this.
It looks like (apart from making -E not use CFLAGS_EXTRAS but a fresh variable) 
we should use that mechanism for all the options passed using os.environ().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:338-339
 
+if args.custom_libcpp:
+os.environ['CUSTOM_LIBCPP'] = args.custom_libcpp
+

aprantl wrote:
> JDevlieghere wrote:
> > aprantl wrote:
> > > JDevlieghere wrote:
> > > > Please don't rely on environment variables to pass arguments to the 
> > > > Make invocation. This makes it really tedious to debug make 
> > > > invocations. Instead, pass these explicitly as part of the make 
> > > > invocation from the builders 
> > > > (`packages/Python/lldbsuite/test/builders/`). 
> > > That is a very good point. Maybe we should just fix the -E option, which 
> > > doesn't work anyway due to Makefiles setting CFLAGS_EXTRAS and use that.
> > I think a specific flag is fine, we already have something similar for 
> > using the "hermetic" libc++ (I left a comment below about that). We should 
> > see if we can unify this.
> It looks like (apart from making -E not use CFLAGS_EXTRAS but a fresh 
> variable) we should use that mechanism for all the options passed using 
> os.environ().
I'm a bit puzzled by the Builder class, since I don't recall encountering it 
while tracing the variables from CMake to the final Makefile invocation. I'll 
try to figure this out based on your patch.



Comment at: lldb/packages/Python/lldbsuite/test/dotest_args.py:175
 help='A plugin whose tests will be enabled. The only currently 
supported plugin is intel-pt.')
+group.add_argument(
+'--custom-libcpp',

JDevlieghere wrote:
> We should also think about how this interacts with `--hermetic-libcxx` (and 
> make the name of the options consistent). 
If I understand correctly, that flag is a more restricted behaviour of what is 
being implemented here: `hermetic-libcxx` is _only_ about looking for a libcxx 
built alongside LLDB / Clang, whereas this patch allows an arbitrary libcxx to 
be used (the use cases we're aiming  for don't even use a Clang built alongside 
LLDB).

I believe `hermetic-libcxx` can be implemented in terms of the new 
functionality being added here, but I'll look some more at your patch to make 
sure that's the case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [PATCH] D132263: [lldb] Support specifying a custom libcxx for the API tests

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: fdeazeve, aprantl, labath.
Herald added a project: All.
JDevlieghere requested review of this revision.

D129166  was an attempt to ensure that by 
default we use the libc++ from the build dir instead of potentially picking up 
the system libc++. While I was reviewing D132257 
, I realized that part of the patch (i.e. the 
change to `Makefile.rules`) got lost. As I was reviving the code, I realized 
that it would be relatively straightforward to merge my change with what Felipe 
was trying to achieve. Instead of telling dotest to use the libc++ form the 
build dir and computing the path to the current build's libc++  (as was the 
case in D129166 ), we can specify the libc++ 
include and library dir to dotest (as was the case in D132257 
) and have lit compute the path to the libc++ 
in the build dir, achieving both objectives.


https://reviews.llvm.org/D132263

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/lit.cfg.py

Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -172,7 +172,9 @@
 
 # If we have a just-built libcxx, prefer it over the system one.
 if is_configured('has_libcxx') and platform.system() != 'Windows':
-  dotest_cmd += ['--hermetic-libcxx']
+  if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
+dotest_cmd += ['--libcxx-include-dir', os.path.join(config.llvm_include_dir, 'c++', 'v1')]
+dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
 
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -388,16 +388,21 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Android"
-		# Nothing to do, this is already handled in
-		# Android.rules.
+	ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),)
+		CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR)
+		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++
 	else
-		CXXFLAGS += -stdlib=libc++
-		LDFLAGS += -stdlib=libc++
-	endif
-	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
-		ifneq (,$(LLVM_LIBS_DIR))
-			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		ifeq "$(OS)" "Android"
+# Nothing to do, this is already handled in
+# Android.rules.
+		else
+CXXFLAGS += -stdlib=libc++
+LDFLAGS += -stdlib=libc++
+		endif
+		ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+ifneq (,$(LLVM_LIBS_DIR))
+LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+endif
 		endif
 	endif
 endif
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -43,8 +43,8 @@
 if sys.platform == 'darwin':
 group.add_argument('--apple-sdk', metavar='apple_sdk', dest='apple_sdk', default="", help=textwrap.dedent(
 '''Specify the name of the Apple SDK (macosx, macosx.internal, iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from that SDK's toolchain.'''))
-group.add_argument('--hermetic-libcxx', action='store_true', help=textwrap.dedent(
-'''Force the just-built libcxx to be used for the libc++ formatter tests.'''))
+group.add_argument('--libcxx-include-dir', help=textwrap.dedent('Specify the path to a custom libc++ include directory. Must be used in conjunction with --libcxx-library-dir.'))
+group.add_argument('--libcxx-library-dir', help=textwrap.dedent('Specify the path to a custom libc++ library directory. Must be used in conjunction with --libcxx-include-dir.'))
 # FIXME? This won't work for different extra flags according to each arch.
 group.add_argument(
 '-E',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,10 +280,17 @@
 logging.warning('No valid FileCheck executable; some tests may fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to dotest.py)')
 
-configuration.hermetic_libcxx = args.hermetic_libcxx
-if configuration.hermetic_libcx

[Lldb-commits] [PATCH] D132263: [lldb] Support specifying a custom libcxx for the API tests

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

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

Thanks for merging the two patches! This LGTM!


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

https://reviews.llvm.org/D132263

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

[Lldb-commits] [PATCH] D132257: [lldb] Create flag to use a custom libcxx in tests

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve abandoned this revision.
fdeazeve added a comment.

Abandoning in favour of D132263 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132257

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

[Lldb-commits] [lldb] cc0b5eb - [lldb] Support specifying a custom libcxx for the API tests

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-08-19T15:20:41-07:00
New Revision: cc0b5ebf7fc8beb8fa907730e2d8f52d4c31bdc7

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

LOG: [lldb] Support specifying a custom libcxx for the API tests

This patch combines D129166 (to always pick the just-built libc++) and
D132257 (to allow customizing the libc++ for testing). The common goal
is to avoid picking up an unexpected libc++ for testing.

Differential revision: https://reviews.llvm.org/D132263

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test/make/Makefile.rules
lldb/test/API/lit.cfg.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index cb82dd4c98817..e260431cad812 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -121,8 +121,9 @@ def getModuleCacheSpec(self):
 return []
 
 def getLibCxxArgs(self):
-if configuration.hermetic_libcxx:
-return ["USE_HERMETIC_LIBCPP=1"]
+if configuration.libcxx_include_dir and 
configuration.libcxx_library_dir:
+return 
["LIBCPP_INCLUDE_DIR={}".format(configuration.libcxx_include_dir),
+
"LIBCPP_LIBRARY_DIR={}".format(configuration.libcxx_library_dir)]
 return []
 
 def _getDebugInfoArgs(self, debug_info):

diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index c29a44e70638a..5133dedbe8524 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -124,8 +124,8 @@
 # LLDB library directory.
 lldb_libs_dir = None
 
-# Force us to use the just-built libcxx
-hermetic_libcxx = False
+libcxx_include_dir = None
+libcxx_library_dir = None
 
 # A plugin whose tests will be enabled, like intel-pt.
 enabled_plugins = []

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index fd212631d3e27..37842c96ba388 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,10 +280,17 @@ def parseOptionsAndInitTestdirs():
 logging.warning('No valid FileCheck executable; some tests may 
fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to 
dotest.py)')
 
-configuration.hermetic_libcxx = args.hermetic_libcxx
-if configuration.hermetic_libcxx and args.lldb_platform_name:
-configuration.hermetic_libcxx = False
-logging.warning('Hermetic libc++ is not supported for remote runs: 
ignoring --hermetic-libcxx')
+configuration.libcxx_include_dir = args.libcxx_include_dir
+configuration.libcxx_library_dir = args.libcxx_library_dir
+if args.libcxx_include_dir or args.libcxx_library_dir:
+if args.lldb_platform_name:
+logging.warning('Custom libc++ is not supported for remote runs: 
ignoring --libcxx arguments')
+elif args.libcxx_include_dir and args.libcxx_library_dir:
+configuration.libcxx_include_dir = args.libcxx_include_dir
+configuration.libcxx_library_dir = args.libcxx_library_dir
+else:
+logging.error('Custom libc++ requires both --libcxx-include-dir 
and --libcxx-library-dir')
+sys.exit(-1)
 
 if args.channels:
 lldbtest_config.channels = args.channels

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index 768b052885074..3bd523ac579bc 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -43,8 +43,8 @@ def create_parser():
 if sys.platform == 'darwin':
 group.add_argument('--apple-sdk', metavar='apple_sdk', 
dest='apple_sdk', default="", help=textwrap.dedent(
 '''Specify the name of the Apple SDK (macosx, macosx.internal, 
iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from 
that SDK's toolchain.'''))
-group.add_argument('--hermetic-libcxx', action='store_true', 
help=textwrap.dedent(
-'''Force the just-built libcxx to be used for the libc++ formatter 
tests.'''))
+group.add_argument('--libcxx-include-dir', help=textwrap.dedent('Specify 
the path to a custom libc++ include directory. Must be used in conjunction with 
--libcxx-library-dir.'))
+group.add_argument('--

[Lldb-commits] [PATCH] D132263: [lldb] Support specifying a custom libcxx for the API tests

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc0b5ebf7fc8: [lldb] Support specifying a custom libcxx for 
the API tests (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132263

Files:
  lldb/packages/Python/lldbsuite/test/builders/builder.py
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules
  lldb/test/API/lit.cfg.py

Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -172,7 +172,9 @@
 
 # If we have a just-built libcxx, prefer it over the system one.
 if is_configured('has_libcxx') and platform.system() != 'Windows':
-  dotest_cmd += ['--hermetic-libcxx']
+  if is_configured('llvm_include_dir') and is_configured('llvm_libs_dir'):
+dotest_cmd += ['--libcxx-include-dir', os.path.join(config.llvm_include_dir, 'c++', 'v1')]
+dotest_cmd += ['--libcxx-library-dir', config.llvm_libs_dir]
 
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -388,16 +388,21 @@
 
 ifeq (1,$(USE_LIBCPP))
 	CXXFLAGS += -DLLDB_USING_LIBCPP
-	ifeq "$(OS)" "Android"
-		# Nothing to do, this is already handled in
-		# Android.rules.
+	ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),)
+		CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR)
+		LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++
 	else
-		CXXFLAGS += -stdlib=libc++
-		LDFLAGS += -stdlib=libc++
-	endif
-	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
-		ifneq (,$(LLVM_LIBS_DIR))
-			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+		ifeq "$(OS)" "Android"
+# Nothing to do, this is already handled in
+# Android.rules.
+		else
+CXXFLAGS += -stdlib=libc++
+LDFLAGS += -stdlib=libc++
+		endif
+		ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
+ifneq (,$(LLVM_LIBS_DIR))
+LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+endif
 		endif
 	endif
 endif
Index: lldb/packages/Python/lldbsuite/test/dotest_args.py
===
--- lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -43,8 +43,8 @@
 if sys.platform == 'darwin':
 group.add_argument('--apple-sdk', metavar='apple_sdk', dest='apple_sdk', default="", help=textwrap.dedent(
 '''Specify the name of the Apple SDK (macosx, macosx.internal, iphoneos, iphoneos.internal, or path to SDK) and use the appropriate tools from that SDK's toolchain.'''))
-group.add_argument('--hermetic-libcxx', action='store_true', help=textwrap.dedent(
-'''Force the just-built libcxx to be used for the libc++ formatter tests.'''))
+group.add_argument('--libcxx-include-dir', help=textwrap.dedent('Specify the path to a custom libc++ include directory. Must be used in conjunction with --libcxx-library-dir.'))
+group.add_argument('--libcxx-library-dir', help=textwrap.dedent('Specify the path to a custom libc++ library directory. Must be used in conjunction with --libcxx-include-dir.'))
 # FIXME? This won't work for different extra flags according to each arch.
 group.add_argument(
 '-E',
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,10 +280,17 @@
 logging.warning('No valid FileCheck executable; some tests may fail...')
 logging.warning('(Double-check the --llvm-tools-dir argument to dotest.py)')
 
-configuration.hermetic_libcxx = args.hermetic_libcxx
-if configuration.hermetic_libcxx and args.lldb_platform_name:
-configuration.hermetic_libcxx = False
-logging.warning('Hermetic libc++ is not supported for remote runs: ignoring --hermetic-libcxx')
+configuration.libcxx_include_dir = args.libcxx_include_dir
+configuration.libcxx_library_dir = args.libcxx_library_dir
+if args.libcxx_include_dir or args.libcxx_library_dir:
+if args.lldb_platform_name:
+logging.warning('Custom libc++ is not supported for remote runs: ignoring --libcxx arguments')
+elif args.libcxx_include_dir and args.libcxx_library_dir:
+configuration.libcxx_include_dir = args.libcxx_include_dir
+configuration.libcxx_library_dir = args.li

[Lldb-commits] [PATCH] D132271: [lldb][Test] Replace expect() with expect_expr() in TestNamespaceLookup.py

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This will be useful in preparation for some reshuffling
of assertions in this file since we won't have to
adjust the persitent variable names during the process.

sed commands:

  s/expect("expr -- /expect_expr("/g
  s/startstr="(int) [$0-9]* = /result_type="int", result_value="/g

**Testing**

- API tests still pass


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132271

Files:
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -82,60 +82,60 @@
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
 # Evaluate func() - should call ::func()
-self.expect("expr -- func()", startstr="(int) $0 = 1")
+self.expect_expr("func()", result_type="int", result_value="1")
 # Evaluate A::B::func() - should call A::B::func()
-self.expect("expr -- A::B::func()", startstr="(int) $1 = 4")
+self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
-self.expect("expr -- func(10)", startstr="(int) $2 = 11")
+self.expect_expr("func(10)", result_type="int", result_value="11")
 # Evaluate ::func() - should call A::func()
-self.expect("expr -- ::func()", startstr="(int) $3 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
-self.expect("expr -- A::foo()", startstr="(int) $4 = 42")
+self.expect_expr("A::foo()", result_type="int", result_value="42")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
-self.expect("expr -- func(10)", startstr="(int) $5 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $6 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 # Evaluate func() - should call A::func()
-self.expect("expr -- func()", startstr="(int) $7 = 3")
+self.expect_expr("func()", result_type="int", result_value="3")
 
 # Continue to BP_nested_ns_scope at nested ns scope
 self.runToBkpt("continue")
 # Evaluate func() - should call A::B::func()
-self.expect("expr -- func()", startstr="(int) $8 = 4")
+self.expect_expr("func()", result_type="int", result_value="4")
 # Evaluate A::func() - should call A::func()
-self.expect("expr -- A::func()", startstr="(int) $9 = 3")
+self.expect_expr("A::func()", result_type="int", result_value="3")
 
 # Evaluate func(10) - should call A::func(10)
 # NOTE: Under the rules of C++, this test would normally get an error
 # because A::B::func() hides A::func(), but lldb intentionally
 # disobeys these rules so that the intended overload can be found
 # by only removing duplicates if they have the same type.
-self.expect("expr -- func(10)", startstr="(int) $10 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 
 # Continue to BP_nested_ns_scope_after_using at nested ns scope after
 # using declaration
 self.runToBkpt("continue")
 # Evaluate A::func(10) - should call A::func(int)
-self.expect("expr -- A::func(10)", startstr="(int) $11 = 13")
+self.expect_expr("A::func(10)", result_type="int", result_value="13")
 
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $12 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $13 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $14 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $15 = 

[Lldb-commits] [PATCH] D132271: [lldb][Test] Replace expect() with expect_expr() in TestNamespaceLookup.py

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 454145.
Michael137 added a comment.

- Don't need replacement on one of the assertions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132271

Files:
  lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py

Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -82,60 +82,60 @@
 # Run to BP_global_scope at global scope
 self.runToBkpt("run")
 # Evaluate func() - should call ::func()
-self.expect("expr -- func()", startstr="(int) $0 = 1")
+self.expect_expr("func()", result_type="int", result_value="1")
 # Evaluate A::B::func() - should call A::B::func()
-self.expect("expr -- A::B::func()", startstr="(int) $1 = 4")
+self.expect_expr("A::B::func()", result_type="int", result_value="4")
 # Evaluate func(10) - should call ::func(int)
-self.expect("expr -- func(10)", startstr="(int) $2 = 11")
+self.expect_expr("func(10)", result_type="int", result_value="11")
 # Evaluate ::func() - should call A::func()
-self.expect("expr -- ::func()", startstr="(int) $3 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate A::foo() - should call A::foo()
-self.expect("expr -- A::foo()", startstr="(int) $4 = 42")
+self.expect_expr("A::foo()", result_type="int", result_value="42")
 
 # Continue to BP_ns_scope at ns scope
 self.runToBkpt("continue")
 # Evaluate func(10) - should call A::func(int)
-self.expect("expr -- func(10)", startstr="(int) $5 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $6 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 # Evaluate func() - should call A::func()
-self.expect("expr -- func()", startstr="(int) $7 = 3")
+self.expect_expr("func()", result_type="int", result_value="3")
 
 # Continue to BP_nested_ns_scope at nested ns scope
 self.runToBkpt("continue")
 # Evaluate func() - should call A::B::func()
-self.expect("expr -- func()", startstr="(int) $8 = 4")
+self.expect_expr("func()", result_type="int", result_value="4")
 # Evaluate A::func() - should call A::func()
-self.expect("expr -- A::func()", startstr="(int) $9 = 3")
+self.expect_expr("A::func()", result_type="int", result_value="3")
 
 # Evaluate func(10) - should call A::func(10)
 # NOTE: Under the rules of C++, this test would normally get an error
 # because A::B::func() hides A::func(), but lldb intentionally
 # disobeys these rules so that the intended overload can be found
 # by only removing duplicates if they have the same type.
-self.expect("expr -- func(10)", startstr="(int) $10 = 13")
+self.expect_expr("func(10)", result_type="int", result_value="13")
 
 # Continue to BP_nested_ns_scope_after_using at nested ns scope after
 # using declaration
 self.runToBkpt("continue")
 # Evaluate A::func(10) - should call A::func(int)
-self.expect("expr -- A::func(10)", startstr="(int) $11 = 13")
+self.expect_expr("A::func(10)", result_type="int", result_value="13")
 
 # Continue to BP_before_using_directive at global scope before using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $12 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $13 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 # Continue to BP_after_using_directive at global scope after using
 # declaration
 self.runToBkpt("continue")
 # Evaluate ::func() - should call ::func()
-self.expect("expr -- ::func()", startstr="(int) $14 = 1")
+self.expect_expr("::func()", result_type="int", result_value="1")
 # Evaluate B::func() - should call B::func()
-self.expect("expr -- B::func()", startstr="(int) $15 = 4")
+self.expect_expr("B::func()", result_type="int", result_value="4")
 
 @expectedFailure("lldb scope lookup of functions bugs")
 def test_function_scope_lookup_with_run_command(self):
@@ -161,18 +161,18 @@
 # Evaluate foo() - should call ::foo()
 # FIXME: lldb finds Y::foo because lookup for variables is do

[Lldb-commits] [lldb] c38bc42 - [lldb] Use Optional::value instead of Optional::getValue (NFC)

[Kazu Hirata via lldb-commits]

Author: Kazu Hirata
Date: 2022-08-19T21:40:48-07:00
New Revision: c38bc421b1268ca371cdbb54e470d758968ce67a

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

LOG: [lldb] Use Optional::value instead of Optional::getValue (NFC)

Added: 


Modified: 
lldb/source/Host/common/Editline.cpp
lldb/tools/lldb-test/lldb-test.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Editline.cpp 
b/lldb/source/Host/common/Editline.cpp
index cee7a3655046a..c46e69d9be7ff 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1086,7 +1086,7 @@ unsigned char Editline::TypedCharacter(int ch) {
   m_color_prompts ? m_suggestion_ansi_suffix.c_str() : "";
 
   if (llvm::Optional to_add = m_suggestion_callback(line)) {
-std::string to_add_color = ansi_prefix + to_add.getValue() + ansi_suffix;
+std::string to_add_color = ansi_prefix + to_add.value() + ansi_suffix;
 fputs(typed.c_str(), m_output_file);
 fputs(to_add_color.c_str(), m_output_file);
 size_t new_autosuggestion_size = line.size() + to_add->length();

diff  --git a/lldb/tools/lldb-test/lldb-test.cpp 
b/lldb/tools/lldb-test/lldb-test.cpp
index 466e082534e58..b3651314b7f22 100644
--- a/lldb/tools/lldb-test/lldb-test.cpp
+++ b/lldb/tools/lldb-test/lldb-test.cpp
@@ -884,7 +884,7 @@ static Mangled::NamePreference 
opts::symtab::getNamePreference() {
 
 int opts::symtab::handleSymtabCommand(Debugger &Dbg) {
   if (auto error = validate()) {
-logAllUnhandledErrors(std::move(error.getValue()), WithColor::error(), "");
+logAllUnhandledErrors(std::move(error.value()), WithColor::error(), "");
 return 1;
   }
 



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

[Lldb-commits] [lldb] 2b46625 - [lldb] Use Optional::transform instead of Optional::map (NFC)

[Kazu Hirata via lldb-commits]

Author: Kazu Hirata
Date: 2022-08-19T21:40:47-07:00
New Revision: 2b4662551603d59e1d425494cd13452c42e5c366

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

LOG: [lldb] Use Optional::transform instead of Optional::map (NFC)

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 03cfb1e0a3eee..0ce901e7becb6 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -251,7 +251,7 @@ Expected 
IntelPTSingleBufferTrace::Start(
   (request.ipt_trace_size + page_size - 1) / page_size));
 
   Expected attr = CreateIntelPTPerfEventConfiguration(
-  request.enable_tsc, request.psb_period.map([](int value) {
+  request.enable_tsc, request.psb_period.transform([](int value) {
 return static_cast(value);
   }));
   if (!attr)



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

[Lldb-commits] [lldb] 2f50883 - [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-20T08:21:32+02:00
New Revision: 2f50883c132879395c2cb45c458a4ec132309b32

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

LOG: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

Try to always send vCont packets and include the PID in them if running
multiprocess.  This is necessary to ensure that with the upcoming full
multiprocess support always resumes the correct process without having
to resort to the legacy Hc packets.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131758

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestContinue.py

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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 29417b35fde3..11d0fec1926a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -181,6 +181,12 @@ bool 
GDBRemoteCommunicationClient::GetQXferSigInfoReadSupported() {
   return m_supports_qXfer_siginfo_read == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::GetMultiprocessSupported() {
+  if (m_supports_memory_tagging == eLazyBoolCalculate)
+GetRemoteQSupported();
+  return m_supports_multiprocess == eLazyBoolYes;
+}
+
 uint64_t GDBRemoteCommunicationClient::GetRemoteMaxPacketSize() {
   if (m_max_packet_size == 0) {
 GetRemoteQSupported();
@@ -1514,7 +1520,7 @@ Status GDBRemoteCommunicationClient::Detach(bool 
keep_stopped,
 }
   }
 
-  if (m_supports_multiprocess) {
+  if (GetMultiprocessSupported()) {
 // Some servers (e.g. qemu) require specifying the PID even if only a 
single
 // process is running.
 if (pid == LLDB_INVALID_PROCESS_ID)

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 3a62747603f6..4e89ade772ad 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -339,6 +339,8 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
 
   bool GetQXferSigInfoReadSupported();
 
+  bool GetMultiprocessSupported();
+
   LazyBool SupportsAllocDeallocMemory() // const
   {
 // Uncomment this to have lldb pretend the debug server doesn't respond to

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 30fb27932d19..fbc7ab9180e7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1186,11 +1186,18 @@ Status ProcessGDBRemote::DoResume() {
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
+  std::string pid_prefix;
+  if (m_gdb_comm.GetMultiprocessSupported())
+pid_prefix = llvm::formatv("p{0:x-}.", GetID());
+
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing, just send a "c" packet
-continue_packet.PutCString("c");
+// All threads are continuing
+if (m_gdb_comm.GetMultiprocessSupported())
+  continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+else
+  continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1200,7 +1207,7 @@ Status ProcessGDBRemote::DoResume() {
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
+  continue_packet.Format(";c:{0}{1:x-}", pid_prefix, *t_pos);
   } else
 continue_packet_error = true;
 }
@@ -1211,8 +1218,8 @@ Status ProcessGDBRemote::DoResume() {
  s_pos = m_continue_C_tids.begin(),
  s_end = m_continue_C_tids.end();
  s_pos != s_end; ++s_pos)
-  continue_packet.Printf(";C%2.2x:%4.4" PRIx64, s_pos->second,
- s_pos->first);
+  continue_packet.Format(";C{0:x-2}:{1}{2:x-}", s_pos->second,
+

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[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 rG2f50883c1328: [lldb] [gdb-remote] Include PID in vCont 
packets if multiprocess (authored by mgorny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestContinue.py
@@ -0,0 +1,74 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestContinue(GDBRemoteTestBase):
+class BaseResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;multiprocess+"
+
+def qfThreadInfo(self):
+return "mp400.401"
+
+def haltReason(self):
+return "S13"
+
+def cont(self):
+return "W01"
+
+def other(self, packet):
+if packet == "vCont?":
+return "vCont;c;C;s;S"
+if packet.startswith("vCont;"):
+return "W00"
+return ""
+
+def test_continue_no_multiprocess(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["vCont;C13:401"])
+
+def test_continue_no_vCont(self):
+class MyResponder(self.BaseResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+"
+
+def qfThreadInfo(self):
+return "m401"
+
+def other(self, packet):
+return ""
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["Hc401", "C13"])
+
+def test_continue_multiprocess(self):
+class MyResponder(self.BaseResponder):
+pass
+
+self.server.responder = MyResponder()
+self.runCmd("platform select remote-linux")
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+  [lldb.eStateExited])
+self.assertPacketLogContains(["vCont;C13:p400.401"])
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
@@ -1186,11 +1186,18 @@
 StreamString continue_packet;
 bool continue_packet_error = false;
 if (m_gdb_comm.HasAnyVContSupport()) {
+  std::string pid_prefix;
+  if (m_gdb_comm.GetMultiprocessSupported())
+pid_prefix = llvm::formatv("p{0:x-}.", GetID());
+
   if (m_continue_c_tids.size() == num_threads ||
   (m_continue_c_tids.empty() && m_continue_C_tids.empty() &&
m_continue_s_tids.empty() && m_continue_S_tids.empty())) {
-// All threads are continuing, just send a "c" packet
-continue_packet.PutCString("c");
+// All threads are continuing
+if (m_gdb_comm.GetMultiprocessSupported())
+  continue_packet.Format("vCont;c:{0}-1", pid_prefix);
+else
+  continue_packet.PutCString("c");
   } else {
 continue_packet.PutCString("vCont");
 
@@ -1200,7 +1207,7 @@
  t_pos = m_continue_c_tids.begin(),
  t_end = m_continue_c_tids.end();
  t_pos != t_end; ++t_pos)
-  continue_packet.Printf(";c:%4.4" PRIx64, *t_pos);
+  continue_packet.Format(";c

[Lldb-commits] [PATCH] D131758: [lldb] [gdb-remote] Include PID in vCont packets if multiprocess

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

@aprantl, why did you revert d38985a36be8b0165787f8893b3d2b0f831d1e83 
? This 
change had nothing to do with this patch, and you broke Windows buildbot again. 
If you really can't wait a few hours for me to able to show up, I'd really 
appreciate if you took responsibility for the things you've broken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131758

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

[Lldb-commits] [lldb] d92f49f - Reland "[lldb] [test] Disable new CommunicationTests on Windows"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-08-20T08:32:55+02:00
New Revision: d92f49fc4b22a738bd39e4588543b0a23037bd69

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

LOG: Reland "[lldb] [test] Disable new CommunicationTests on Windows"

This change was wrongly reverted and re-broke the Windows buildbot.

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/unittests/Core/CommunicationTest.cpp

Removed: 




diff  --git a/lldb/unittests/Core/CommunicationTest.cpp 
b/lldb/unittests/Core/CommunicationTest.cpp
index 024684a486c2d..13ebfae772f47 100644
--- a/lldb/unittests/Core/CommunicationTest.cpp
+++ b/lldb/unittests/Core/CommunicationTest.cpp
@@ -21,6 +21,7 @@
 
 using namespace lldb_private;
 
+#ifndef _WIN32
 static void CommunicationReadTest(bool use_read_thread) {
   Pipe pipe;
   ASSERT_THAT_ERROR(pipe.CreateNew(/*child_process_inherit=*/false).ToError(),
@@ -87,7 +88,6 @@ TEST(CommunicationTest, ReadThread) {
   CommunicationReadTest(/*use_thread=*/true);
 }
 
-#ifndef _WIN32
 TEST(CommunicationTest, SynchronizeWhileClosing) {
   // Set up a communication object reading from a pipe.
   Pipe pipe;



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