[Lldb-commits] [lldb] df94877 - [lldb] [test] Implement getting thread ID on FreeBSD

2022-06-10 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2022-06-10T10:26:58+02:00
New Revision: df948771124953df9e69e44e3bc70ea7948e51f1

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

LOG: [lldb] [test] Implement getting thread ID on FreeBSD

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/thread.h

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/thread.h 
b/lldb/packages/Python/lldbsuite/test/make/thread.h
index 3cfa16b847614..053ba86dc9062 100644
--- a/lldb/packages/Python/lldbsuite/test/make/thread.h
+++ b/lldb/packages/Python/lldbsuite/test/make/thread.h
@@ -9,6 +9,8 @@ int pthread_threadid_np(pthread_t, __uint64_t *);
 #elif defined(__linux__)
 #include 
 #include 
+#elif defined(__FreeBSD__)
+#include 
 #elif defined(__NetBSD__)
 #include 
 #elif defined(_WIN32)
@@ -22,6 +24,8 @@ inline uint64_t get_thread_id() {
   return tid;
 #elif defined(__linux__)
   return syscall(__NR_gettid);
+#elif defined(__FreeBSD__)
+  return static_cast(pthread_getthreadid_np());
 #elif defined(__NetBSD__)
   // Technically lwpid_t is 32-bit signed integer
   return static_cast(_lwp_self());



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


[Lldb-commits] [PATCH] D126982: [lldb] [test] Implement getting thread ID on FreeBSD

2022-06-10 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf9487711249: [lldb] [test] Implement getting thread ID on 
FreeBSD (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126982

Files:
  lldb/packages/Python/lldbsuite/test/make/thread.h


Index: lldb/packages/Python/lldbsuite/test/make/thread.h
===
--- lldb/packages/Python/lldbsuite/test/make/thread.h
+++ lldb/packages/Python/lldbsuite/test/make/thread.h
@@ -9,6 +9,8 @@
 #elif defined(__linux__)
 #include 
 #include 
+#elif defined(__FreeBSD__)
+#include 
 #elif defined(__NetBSD__)
 #include 
 #elif defined(_WIN32)
@@ -22,6 +24,8 @@
   return tid;
 #elif defined(__linux__)
   return syscall(__NR_gettid);
+#elif defined(__FreeBSD__)
+  return static_cast(pthread_getthreadid_np());
 #elif defined(__NetBSD__)
   // Technically lwpid_t is 32-bit signed integer
   return static_cast(_lwp_self());


Index: lldb/packages/Python/lldbsuite/test/make/thread.h
===
--- lldb/packages/Python/lldbsuite/test/make/thread.h
+++ lldb/packages/Python/lldbsuite/test/make/thread.h
@@ -9,6 +9,8 @@
 #elif defined(__linux__)
 #include 
 #include 
+#elif defined(__FreeBSD__)
+#include 
 #elif defined(__NetBSD__)
 #include 
 #elif defined(_WIN32)
@@ -22,6 +24,8 @@
   return tid;
 #elif defined(__linux__)
   return syscall(__NR_gettid);
+#elif defined(__FreeBSD__)
+  return static_cast(pthread_getthreadid_np());
 #elif defined(__NetBSD__)
   // Technically lwpid_t is 32-bit signed integer
   return static_cast(_lwp_self());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D126080: Adapt C++ std::string dataformatter for D125496

2022-06-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

> How do we usually add tests for STL data structures?

I have https://reviews.llvm.org/D124155, which would allow testing of different 
string layouts in one go. It remains to be seen how maintainable will that test 
be when it starts supporting many string layouts. OTOH, one could say it will 
be as maintainable as a data formatter which supports all those string 
layouts...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126080

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


[Lldb-commits] [PATCH] D126770: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process

2022-06-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.
Herald added a subscriber: JDevlieghere.

This still leaves the possibility of a race where the process stops on its own 
while you're in the process of sending the signal, right?

I believe that on linux we handling this by checking the stopped events: If the 
application stopped due to a signal from us (siginfo->si_pid == getpid()) *and* 
we're not currently trying to stop a process, then we transparently resume it. 
Maybe you need to do the same here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126770

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


[Lldb-commits] [PATCH] D126770: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process

2022-06-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

(Actually, I don't think we do it for the main `Halt` operation (though maybe 
we should) -- only for the SIGSTOPs send to individual threads when we're 
performing an all-stop.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126770

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


[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-06-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a reviewer: jingham.
labath added a comment.

(I think Jim would be interested in following this.)

I'm not sure I understand what this does. Is it that, when the setting is 
enabled, lldb uses non-stop communication at the protocol level, but then 
forces an all-stop whenever one thread stops (so that the upper layers are 
happy)?

> API tests required changing the Process::Halt() API to inspect process'
> private state rather than the public state, that remains eStateUnloaded
> in gdb_remote_client tests.

I don't think that checking the private state is correct there. Doesn't that 
just mean that there's a bug in the tests that it's not simulating a real 
gdbserver sufficiently well? Or maybe (if the simulated responses are 
reasonable) lldb should be made to work (properly set the state)  for the 
simulated responses?




Comment at: lldb/packages/Python/lldbsuite/test/gdbclientutils.py:26
 
 Framing includes surrounding the message between $ and #, and appending
 a two character hex checksum.

I guess this comment needs updating. I've been also thinking whether this is a 
sufficiently obvious way to send notifications, but I kinda like the brevity of 
it. 



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:128
+PacketResult::Success) {
+  LLDB_LOGF(log, "GDBRemoteClientBase::%s () vCtrlC failed",
+__FUNCTION__);

not `vCont;t` ? Also, I think these log lines are unnecessarily verbose, and 
are obscuring the real code. I think a single log line (indicating failure) 
should be more than enough as more information (the kind of failure, for 
instance) can be determined by looking at the packet log.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:147
+// vCtrlC may not do anything, so timeout if we don't get notification
+if (ReadPacket(stop_response, milliseconds(500), false) ==
+PacketResult::Success) {

This is unfortunate. Isn't there a better way to do this? Why aren't you using 
vCtrlC as all the comments say? Did you find any parallel to this in gdb?



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:450
+  if (m_comm.GetNonStopProtocol())
+success = m_comm.SendPacketNoLock("vCtrlC") == PacketResult::Success;
+  else {

Isn't the vCtrlC packet supposed to have a response (OK or Exx according to 
) ?



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:62
 
+  bool GetNonStopProtocol() { return m_non_stop_protocol; }
+

Maybe call this `GetNonStopEnabled` or `GetNonStopInUse` ?



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py:686
+def QNonStop(self, val):
+assert val == 1
+return "OK"

This is not a good assertion. Maybe you could replace check that after-the-fact 
with `assertPacketLogContains`?



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py:717
+process.Stop()
+self.assertPacketLogContains(["vStopped", "vCont;t"])
+self.assertEqual(process.GetSelectedThread().GetStopReason(),

I think here you'll need to manually extract (and verify) the running/stopped 
events from the appropriate listener. In async mode, Continue and Stop return 
immediately, so there's no guarantee that anything has actually happened at 
this point.


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

https://reviews.llvm.org/D126614

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


[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

2022-06-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I don't really have the full context here, but I am wondering if we shouldn't 
somehow take the `DW_AT_declaration` attribute into account here. It seems like 
that should give a more definitive answer as to whether we can expect to see a 
full set of template parameters or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126668

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


[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 created this revision.
Michael137 added reviewers: aprantl, shafik.
Herald added a subscriber: mgorny.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a libcxx formatter for std::span. The
implementation is based on the libcxx formatter for
std::vector. The main difference is the fact that
std::span conditionally has a __size member based
on whether it has a static or dynamic extent.

Example output of formatted span:

  (std::span) $0 = size=6 {
[0] = 0
[1] = 1
[2] = 2
[3] = 3
[4] = 4
[5] = 5
  }

The second template parameter here is actually `std::dynamic_extent`,
but the type declaration we get back from the `TypeSystemClang` is the
actual value (which in this case is `(size_t)-1`). This is consistent
with diagnostics from clang, which doesn't desugar this value either.
E.g.,:

  span.cpp:30:31: error: implicit instantiation of undefined template
  'Undefined>'

Testing:

- Added API-tests
- Confirmed manually using LLDB cli that printing spans works in various 
scenarios


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127481

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
@@ -0,0 +1,58 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// TODO: use these
+
+template 
+void by_ref_and_ptr(std::span &ref, std::span *ptr) {
+  // Stop here to check by ref
+  return;
+}
+
+int main() {
+  std::array numbers = {1, 12, 123, 1234, 12345};
+
+  using dynamic_string_span = std::span;
+
+  // Test span of ints
+
+  //   Full view of numbers with static extent
+  std::span numbers_span = numbers;
+
+  printf("break here");
+
+  by_ref_and_ptr(numbers_span, &numbers_span);
+
+  // Test spans of strings
+  std::vector strings{"goofy", "is", "smart", "!!!"};
+  strings.reserve(strings.size() + 1);
+
+  //   Partial view of strings with dynamic extent
+  dynamic_string_span strings_span{std::span{strings}.subspan(2)};
+
+  auto strings_span_it = strings_span.begin();
+
+  printf("break here");
+
+  //   Vector size doesn't increase, span should
+  //   print unchanged and the strings_span_it
+  //   remains valid
+  strings.emplace_back("???");
+
+  printf("break here");
+
+  // Now some empty spans
+  std::span static_zero_span;
+  std::span dynamic_zero_span;
+
+  // Multiple spans
+  std::array span_arr{strings_span, strings_span};
+  std::span, 2> nested = span_arr;
+
+  printf("break here");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -0,0 +1,164 @@
+"""
+Test lldb data formatter subsystem for std::span
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxSpanDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_numbers(self, var_name):
+"""Helper to check that data formatter sees contents of std::span correctly"""
+self.expect("frame variable " + var_name,
+substrs=[var_name + ' = size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect("p " + var_name,
+substrs=['$', 'size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect_expr(var_name, result_summary="size=5", result_children=[
+ValueCheck(value="1"),
+ValueCheck(value="12"),
+ValueCheck(value="123"),
+ValueCheck(value="1234"),
+ValueCheck(value="12345"),
+])

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 435855.
Michael137 added a comment.
Herald added a subscriber: JDevlieghere.

Removed redundant comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
@@ -0,0 +1,56 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+template 
+void by_ref_and_ptr(std::span &ref, std::span *ptr) {
+  // Stop here to check by ref
+  return;
+}
+
+int main() {
+  std::array numbers = {1, 12, 123, 1234, 12345};
+
+  using dynamic_string_span = std::span;
+
+  // Test span of ints
+
+  //   Full view of numbers with static extent
+  std::span numbers_span = numbers;
+
+  printf("break here");
+
+  by_ref_and_ptr(numbers_span, &numbers_span);
+
+  // Test spans of strings
+  std::vector strings{"goofy", "is", "smart", "!!!"};
+  strings.reserve(strings.size() + 1);
+
+  //   Partial view of strings with dynamic extent
+  dynamic_string_span strings_span{std::span{strings}.subspan(2)};
+
+  auto strings_span_it = strings_span.begin();
+
+  printf("break here");
+
+  //   Vector size doesn't increase, span should
+  //   print unchanged and the strings_span_it
+  //   remains valid
+  strings.emplace_back("???");
+
+  printf("break here");
+
+  // Now some empty spans
+  std::span static_zero_span;
+  std::span dynamic_zero_span;
+
+  // Multiple spans
+  std::array span_arr{strings_span, strings_span};
+  std::span, 2> nested = span_arr;
+
+  printf("break here");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -0,0 +1,164 @@
+"""
+Test lldb data formatter subsystem for std::span
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxSpanDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_numbers(self, var_name):
+"""Helper to check that data formatter sees contents of std::span correctly"""
+self.expect("frame variable " + var_name,
+substrs=[var_name + ' = size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect("p " + var_name,
+substrs=['$', 'size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect_expr(var_name, result_summary="size=5", result_children=[
+ValueCheck(value="1"),
+ValueCheck(value="12"),
+ValueCheck(value="123"),
+ValueCheck(value="1234"),
+ValueCheck(value="12345"),
+])
+
+# check access-by-index
+self.expect("frame variable " + var_name + "[0]",
+substrs=['1'])
+self.expect("frame variable " + var_name + "[1]",
+substrs=['12'])
+self.expect("frame variable " + var_name + "[2]",
+substrs=['123'])
+self.expect("frame variable " + var_name + "[3]",
+substrs=['1234'])
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test that std::span variables are formatted correctly when printed."""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp", False))
+
+# This is the function to remove the custom formats in order to have a
+# clean slate for the next test case.
+def cleanup():
+self.runCmd('type format clear',

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile:5
+
+CXXFLAGS_EXTRAS := -std=c++20 -O0
+include Makefile.rules

Might need to have some compiler version checks here since by default we assume 
c++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

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


[Lldb-commits] [PATCH] D127497: [lldb] [llgs] Refactor SendStopReasonForState for multiprocess

2022-06-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Refactor GDBRemoteCommunicationServerLLGS::SendStopReasonForState()
to accept process as an argument rather than hardcoding
m_current_process, in order to make it work correctly for multiprocess
scenarios.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D127497

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

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -116,7 +116,8 @@
   PacketResult SendStopReplyPacketForThread(NativeProcessProtocol &process,
 lldb::tid_t tid);
 
-  PacketResult SendStopReasonForState(lldb::StateType process_state);
+  PacketResult SendStopReasonForState(NativeProcessProtocol &process,
+  lldb::StateType process_state);
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -993,7 +993,8 @@
   Log *log = GetLog(LLDBLog::Process);
   LLDB_LOGF(log, "GDBRemoteCommunicationServerLLGS::%s called", __FUNCTION__);
 
-  PacketResult result = SendStopReasonForState(StateType::eStateExited);
+  PacketResult result =
+  SendStopReasonForState(*process, StateType::eStateExited);
   if (result != PacketResult::Success) {
 LLDB_LOGF(log,
   "GDBRemoteCommunicationServerLLGS::%s failed to send stop "
@@ -1025,7 +1026,8 @@
 break;
   default:
 // In all other cases, send the stop reason.
-PacketResult result = SendStopReasonForState(StateType::eStateStopped);
+PacketResult result =
+SendStopReasonForState(*process, StateType::eStateStopped);
 if (result != PacketResult::Success) {
   LLDB_LOGF(log,
 "GDBRemoteCommunicationServerLLGS::%s failed to send stop "
@@ -1696,12 +1698,13 @@
   if (!m_current_process)
 return SendErrorResponse(02);
 
-  return SendStopReasonForState(m_current_process->GetState());
+  return SendStopReasonForState(*m_current_process,
+m_current_process->GetState());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::SendStopReasonForState(
-lldb::StateType process_state) {
+NativeProcessProtocol &process, lldb::StateType process_state) {
   Log *log = GetLog(LLDBLog::Process);
 
   switch (process_state) {
@@ -1717,22 +1720,21 @@
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
-assert(m_current_process != nullptr);
-lldb::tid_t tid = m_current_process->GetCurrentThreadID();
+lldb::tid_t tid = process.GetCurrentThreadID();
 // Make sure we set the current thread so g and p packets return the data
 // the gdb will expect.
 SetCurrentThreadID(tid);
-return SendStopReplyPacketForThread(*m_current_process, tid);
+return SendStopReplyPacketForThread(process, tid);
   }
 
   case eStateInvalid:
   case eStateUnloaded:
   case eStateExited:
-return SendWResponse(m_current_process);
+return SendWResponse(&process);
 
   default:
 LLDB_LOG(log, "pid {0}, current state reporting not handled: {1}",
- m_current_process->GetID(), process_state);
+ process.GetID(), process_state);
 break;
   }
 
@@ -3182,7 +3184,9 @@
   }
 
   // Notify we attached by sending a stop packet.
-  return SendStopReasonForState(m_current_process->GetState());
+  assert(m_current_process);
+  return SendStopReasonForState(*m_current_process,
+m_current_process->GetState());
 }
 
 GDBRemoteCommunication::PacketResult
@@ -3212,7 +3216,9 @@
   }
 
   // Notify we attached by sending a stop packet.
-  return SendStopReasonForState(m_current_process->GetState());
+  assert(m_current_process);
+  return SendStopReasonForState(*m_current_process,
+m_current_process->GetState());
 }
 
 GDBRemoteCommunication::PacketResult
@@ -3248,7 +3254,9 @@
   }
 
   // Notify we attached by sending a stop packet.
-  return SendStopReasonForState(m_current_process->GetState());
+  assert(m_current_process);
+  return SendStopReasonForState(*m_current_process,
+m_current_process->GetState());
 }
 
 GDBRemoteCommunication::PacketRes

[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

2022-06-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Modify the behavior of the `k` packet to kill all inferiors rather than
just the current one.  The specification leaves the exact behavior
of this packet up to the implementation but since vKill is specifically
meant to be used to kill a single process, it seems logical to use `k`
to provide the alternate function of killing all of them.

While at it, fix bugs that caused lldb-server to mishandle kills
and exits when more than one process is debugged.  That is:

1. Delay exiting lldb-server until all processes exit, rather than doing that 
on first exited process.  This requires us to actually remove exited processes 
from the process list in order to know whether we have any processes left.  The 
last exited process instance is moved onto a dedicated variable in order to 
prevent it from being destroyed while the notification is still being handled 
(i.e. effectively until the next process exits).

2. Fix enabling and disabling stdio forwarding to account for multiple 
processes running simultaneously.  Now, it is enabled when the first process is 
resumed, and remains enabled until the last one stops.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D127500

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -263,3 +263,44 @@
 "send packet: $Eff#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_kill_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+self.test_sequence.add_log_lines([
+# double-check our PIDs
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+"read packet: $QEnableErrorStrings#00",
+"send packet: $OK#00",
+# kill all processes
+"read packet: $k#00",
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -95,9 +95,11 @@
   std::recursive_mutex m_debugged_process_mutex;
   std::unordered_map>
   m_debugged_processes;
+  std::unique_ptr m_last_exited_process;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
+  int m_stdio_handle_refcount = 0;
 
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
   llvm::StringMap> m_xfer_buffer_map;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1002,13 +1002,26 @@
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m

[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

2022-06-10 Thread Will Hawkins via Phabricator via lldb-commits
hawkinsw added a comment.

In D126240#3564913 , @jgorbe wrote:

> I've been experimenting a little bit and it seems that I can avoid the 
> deadlocks I was seeing by applying only the modification to 
> `TypeCategoryMap::Add`. This would avoid the problem that @hawkinsw pointed 
> out in my change to `FormatManager::GetCategoryForLanguage`. However, the 
> following problem could still happen with the change to 
> `TypeCategoryMap::Add`:
>
> 1. Thread A acquires the lock, runs `m_map[key1] = value1`, and releases the 
> lock
> 2. Thread B acquires the lock, runs `m_map[key2] = value2`, and releases the 
> lock
> 3. Thread A runs `listener->Changed()`
> 4. Thread B runs `listener->Changed()`
>
> So we would have two changes, the listener would be called twice, but both 
> listener calls would see the same state with the two changes already applied, 
> instead of observing each change individually. I'll keep thinking to see if I 
> can fix this elsewhere.

I am *so* sorry I haven't had a chance to come back to work on this change 
until now, but I did have one thought that I will investigate this PM. I will 
keep you posted if I find anything. Again, I am sorry for the radio silence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126240

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


[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-06-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

Thanks for the review and welcome back!

In D126614#3572845 , @labath wrote:

> (I think Jim would be interested in following this.)

Ok, I've added him to all the related diffs.

> I'm not sure I understand what this does. Is it that, when the setting is 
> enabled, lldb uses non-stop communication at the protocol level, but then 
> forces an all-stop whenever one thread stops (so that the upper layers are 
> happy)?

Yes. This way, it preserves compatibility with genuine gdbserver that actually 
implements non-stop mode.

>> API tests required changing the Process::Halt() API to inspect process'
>> private state rather than the public state, that remains eStateUnloaded
>> in gdb_remote_client tests.
>
> I don't think that checking the private state is correct there. Doesn't that 
> just mean that there's a bug in the tests that it's not simulating a real 
> gdbserver sufficiently well? Or maybe (if the simulated responses are 
> reasonable) lldb should be made to work (properly set the state)  for the 
> simulated responses?

Yes, I think it's limitation of the test logic that public state isn't being 
updated. I haven't really managed to get past figuring that part out. Do you 
have any suggestion where to look for the problematic code?

That said, I'm currently focused on multiprocess support in lldb-server. While 
this is relevant, it isn't blocking server and I don't want to distract myself 
switching contexts right now but I'll look into implementing your suggestions 
later on.




Comment at: lldb/packages/Python/lldbsuite/test/gdbclientutils.py:26
 
 Framing includes surrounding the message between $ and #, and appending
 a two character hex checksum.

labath wrote:
> I guess this comment needs updating. I've been also thinking whether this is 
> a sufficiently obvious way to send notifications, but I kinda like the 
> brevity of it. 
Yeah, I can't say I'm very proud of it but I can't think of a way that would 
both be really clean and wouldn't involve major changes all around the place.

I suppose one option would be to wrap these packets in a minimal `Notification` 
class, and then do something like:

```
if isinstance(message, Notification):
  ...
```

and then pass `Notification("foo")` instead of `"%foo" but I'm not convinced 
it'd be actually more obvious.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:128
+PacketResult::Success) {
+  LLDB_LOGF(log, "GDBRemoteClientBase::%s () vCtrlC failed",
+__FUNCTION__);

labath wrote:
> not `vCont;t` ? Also, I think these log lines are unnecessarily verbose, and 
> are obscuring the real code. I think a single log line (indicating failure) 
> should be more than enough as more information (the kind of failure, for 
> instance) can be determined by looking at the packet log.
Good catch.

Just to be clear, do I understand that you're talking of having a single 
`LLDB_LOG` that covers all non-successful responses?



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:147
+// vCtrlC may not do anything, so timeout if we don't get notification
+if (ReadPacket(stop_response, milliseconds(500), false) ==
+PacketResult::Success) {

labath wrote:
> This is unfortunate. Isn't there a better way to do this? Why aren't you 
> using vCtrlC as all the comments say? Did you find any parallel to this in 
> gdb?
Yes, it is unfortunate. I'm going to think more and try to figure out a better 
way. I was originally thinking of simply not waiting for the response and 
instead letting it come after and then taking care of any pending notifications 
before sending the next continue packet (this would require merging D126655 
first) but I'm somewhat afraid of race conditions. Though maybe unnecessarily.

Though OTOH I guess 500 ms may be insufficient for debugging over the Internet, 
and this would lead to even worse results.

As for `vCtrlC`, I've changed the code to use `vCont;t` as the former didn't 
work well with gdbserver (and the latter is also more correct wrt the 
protocol). I've forgotten to update the comments.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:450
+  if (m_comm.GetNonStopProtocol())
+success = m_comm.SendPacketNoLock("vCtrlC") == PacketResult::Success;
+  else {

labath wrote:
> Isn't the vCtrlC packet supposed to have a response (OK or Exx according to 
> ) ?
Indeed you're correct. I guess I've missed it because stop reason handler took 
care of eating it.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:62
 
+  bool GetNonStopProtocol() { return m_non_stop_protocol; }
+
-

[Lldb-commits] [lldb] d36757b - [LLDB][Docs] Fix formatting of example code-block

2022-06-10 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2022-06-10T08:17:08-07:00
New Revision: d36757b511ea6ef466a8ce608a5431a5e6173632

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

LOG: [LLDB][Docs] Fix formatting of example code-block

Tested by building the docs-lldb-html target and
confirming the code-block renders properly with
the fix.

Patch by Michael Buch!

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

Added: 


Modified: 
lldb/docs/use/variable.rst

Removed: 




diff  --git a/lldb/docs/use/variable.rst b/lldb/docs/use/variable.rst
index c60095b2d83f0..e86b863f24856 100644
--- a/lldb/docs/use/variable.rst
+++ b/lldb/docs/use/variable.rst
@@ -808,6 +808,7 @@ of type names. This would let you rephrase the above 
example for arrays of type
 Simple [3] as:
 
 ::
+
(lldb) type summary add --summary-string "${var[].x}" -x "Simple \[[0-9]+\]"
(lldb) frame variable
(Simple [3]) sarray = [1,4,7]



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


[Lldb-commits] [PATCH] D127437: [LLDB][Docs] Fix formatting of example code-block

2022-06-10 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd36757b511ea: [LLDB][Docs] Fix formatting of example 
code-block (authored by aprantl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127437

Files:
  lldb/docs/use/variable.rst


Index: lldb/docs/use/variable.rst
===
--- lldb/docs/use/variable.rst
+++ lldb/docs/use/variable.rst
@@ -808,6 +808,7 @@
 Simple [3] as:
 
 ::
+
(lldb) type summary add --summary-string "${var[].x}" -x "Simple \[[0-9]+\]"
(lldb) frame variable
(Simple [3]) sarray = [1,4,7]


Index: lldb/docs/use/variable.rst
===
--- lldb/docs/use/variable.rst
+++ lldb/docs/use/variable.rst
@@ -808,6 +808,7 @@
 Simple [3] as:
 
 ::
+
(lldb) type summary add --summary-string "${var[].x}" -x "Simple \[[0-9]+\]"
(lldb) frame variable
(Simple [3]) sarray = [1,4,7]
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Thanks, this looks really good! I have a couple of small comments inline.




Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.h:77
 
+// libc++ std::span<>
+bool LibcxxSpanSummaryProvider(ValueObject &valobj, Stream &stream,

`/// Formatter for libc++ std::span<>.`



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:40
+private:
+  ValueObject *m_start = nullptr; ///< First element of span
+  CompilerType m_element_type{};  ///< Type of span elements

`///< First element of span.`



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:55
+~LibcxxStdSpanSyntheticFrontEnd() {
+  // this needs to stay around because it's a child object who will follow
+  // its parent's life cycle

// This ...



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:56
+  // this needs to stay around because it's a child object who will follow
+  // its parent's life cycle
+  // delete m_start;

.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:57
+  // its parent's life cycle
+  // delete m_start;
+}

I would just delete this last line IMHO, it's more confusing than helping. Or 
maybe just say that m_start is owned by s shared_ptr elsewhere.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:80
+
+/*
+ * std::span can either be instantiated with a compile-time known

I would move this into a doxygen comment on the function declaration.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:82
+ * std::span can either be instantiated with a compile-time known
+ * extent or a std::dynaic_extent (this is the default if only the
+ * type template argument is provided). The layout of std::span

dymanic



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:103
+bool lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
+  // Get element type
+  ValueObjectSP data_type_finder_sp(

// Get element type.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:111
+
+  // Get element size
+  if (llvm::Optional size = m_element_type.GetByteSize(nullptr)) {

dito



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp:116
+// Get data
+if (m_element_size > 0) {
+  m_start = data_type_finder_sp.get();

Technically LLVM doesn't use {} on single-statement bodies.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile:5
+
+CXXFLAGS_EXTRAS := -std=c++20 -O0
+include Makefile.rules

Michael137 wrote:
> Might need to have some compiler version checks here since by default we 
> assume c++11
The -O0 should not be necessary.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py:53
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test that std::span variables are formatted correctly when 
printed."""

We probably need to skip this on earlier compilers that don't support C++20.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py:71
+# Execute the cleanup function during test case tear down.
+self.addTearDownHook(cleanup)
+

I think the cleanups are no longer necessary because we run every test in 
isolation now.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp:10
+  // Stop here to check by ref
+  return;
+}

a more robust way to ensure code exists on this line would be 
`printf(" Stop here to check by ref")`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

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


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

@mib I tried but failed to answer that in the original commit description. I 
have reworded it to be more clear. Here's the relevant paragraph:

> For convenience, this change switches all SB classes that implement to
> `__str__` to instead implement `__repr__`. **The result is that `str()` and
> `repr` will produce the same output**. This is because `str` calls `__repr__`
> for classes that have  no `__str__` method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127500: [lldb] [llgs] Make `k` kill all processes, and fix multiple exits

2022-06-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 435939.
mgorny added a comment.

I've gotten so absorbed by fixing server code that I've forgotten to actually 
finish the test case ;-).


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

https://reviews.llvm.org/D127500

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -263,3 +263,37 @@
 "send packet: $Eff#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_kill_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+child_pid = ret["child_pid"]
+self.reset_test_sequence()
+
+exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+self.test_sequence.add_log_lines([
+# kill all processes
+"read packet: $k#00",
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertEqual(set([ret["pid1"], ret["pid2"]]),
+ set([parent_pid, child_pid]))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -95,9 +95,11 @@
   std::recursive_mutex m_debugged_process_mutex;
   std::unordered_map>
   m_debugged_processes;
+  std::unique_ptr m_last_exited_process;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
+  int m_stdio_handle_refcount = 0;
 
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
   llvm::StringMap> m_xfer_buffer_map;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1002,13 +1002,26 @@
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m_continue_process = nullptr;
+  auto process_it = m_debugged_processes.find(process->GetID());
+  assert(process_it != m_debugged_processes.end());
+  // Keep a reference to prevent the instance from being destroyed before
+  // state change notification finishes.
+  m_last_exited_process = std::move(process_it->second);
+  m_debugged_processes.erase(process_it);
 
-  // We are ready to exit the debug monitor.
-  m_exit_now = true;
-  m_mainloop.RequestTermination();
+  if (m_debugged_processes.empty()) {
+// Close the pipe to the inferior terminal i/o if we launched it and set one
+// up.
+MaybeCloseInferiorTerminalConnection();
+
+// We are ready to exit the debug monitor.
+m_exit_now = true;
+m_mainloop.RequestTermination();
+  }
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1171,12 +1184,19 @@
 }
 
 void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
+  Log *log = GetLog(LLDBLog::Process);
+
   // Don't forward if not connected (e.g. when attaching).
   if (!m_stdio_communication.IsConnected())
 return;
 
+  // If we're not debugging multiple processes, this should be called
+  // once.
+  if (!bool(m_extensions_supported &
+NativeProcessProtocol::Extension::multiprocess))
+assert(!m_stdio_handle_up);
+
   Status error;
-  lldbassert(!m_stdio_handle_up);
   m_stdio_handle_up = m_mainloop.RegisterReadObject(

[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Makes sense to me. Maybe add a small shell test that includes the example from 
the description?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.

Thanks for clarifying! LGTM with a test!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature

2022-06-10 Thread Dave Lee via Phabricator via lldb-commits
kastiglione accepted this revision.
kastiglione added a comment.
This revision is now accepted and ready to land.

thank you again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127038

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


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

Note also that the tests use `str()` in many places, so we can be sure this 
hasn't regressed that. I will add a test for `repr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature

2022-06-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.

Thanks Jim!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127038

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


[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

2022-06-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Is it easy to test this?

Ideally this behaviour works with a path to a file as well as `PATH` itself 
e.g. `C:\foo` becomes `C:\foo.exe` and you could write a test that only runs on 
Windows quite easily. If not I guess you could do a shell test that compiles a 
program then runs lldb with `PATH` set to the current dir? (though it sounds 
like you can use the current dir by default anyway)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 435972.
kastiglione added a comment.

add test/API/sanity/TestReprStrEquality.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

Files:
  lldb/bindings/macros.swig
  lldb/test/API/sanity/TestReprStrEquality.py


Index: lldb/test/API/sanity/TestReprStrEquality.py
===
--- /dev/null
+++ lldb/test/API/sanity/TestReprStrEquality.py
@@ -0,0 +1,18 @@
+"""
+This is a sanity check that verifies that `repr(sbobject)` and `str(sbobject)`
+produce the same string.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  def test(self):
+self.assertEqual(repr(self.dbg), str(self.dbg))
Index: lldb/bindings/macros.swig
===
--- lldb/bindings/macros.swig
+++ lldb/bindings/macros.swig
@@ -1,6 +1,6 @@
 %define STRING_EXTENSION_LEVEL(Class, Level)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream, Level);
 const char *desc = stream.GetData();
@@ -15,7 +15,7 @@
 
 %define STRING_EXTENSION(Class)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream);
 const char *desc = stream.GetData();


Index: lldb/test/API/sanity/TestReprStrEquality.py
===
--- /dev/null
+++ lldb/test/API/sanity/TestReprStrEquality.py
@@ -0,0 +1,18 @@
+"""
+This is a sanity check that verifies that `repr(sbobject)` and `str(sbobject)`
+produce the same string.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  def test(self):
+self.assertEqual(repr(self.dbg), str(self.dbg))
Index: lldb/bindings/macros.swig
===
--- lldb/bindings/macros.swig
+++ lldb/bindings/macros.swig
@@ -1,6 +1,6 @@
 %define STRING_EXTENSION_LEVEL(Class, Level)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream, Level);
 const char *desc = stream.GetData();
@@ -15,7 +15,7 @@
 
 %define STRING_EXTENSION(Class)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream);
 const char *desc = stream.GetData();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib accepted this revision.
mib added a comment.

Thanks @kastiglione !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 435980.
kastiglione added a comment.

update TestConvenienceVariables.test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

Files:
  lldb/bindings/macros.swig
  lldb/test/API/sanity/TestReprStrEquality.py
  lldb/test/Shell/Driver/Inputs/convenience.in
  lldb/test/Shell/Driver/TestConvenienceVariables.test


Index: lldb/test/Shell/Driver/TestConvenienceVariables.test
===
--- lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -6,10 +6,16 @@
 CHECK: stop reason = breakpoint 1.1
 CHECK: script print(lldb.debugger)
 CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
+CHECK: script lldb.debugger
+CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
 CHECK: script print(lldb.target)
 CHECK-NEXT: target.out
+CHECK: script lldb.target
+CHECK-NEXT: target.out
 CHECK: script print(lldb.process)
 CHECK-NEXT: SBProcess: pid = {{[0-9]+}}, state = stopped, threads = 
{{[0-9]+}}, executable = target.out
+CHECK: script lldb.process
+CHECK-NEXT: SBProcess: pid = {{[0-9]+}}, state = stopped, threads = 
{{[0-9]+}}, executable = target.out
 CHECK: script print(lldb.thread.GetStopDescription(100))
 CHECK-NEXT: breakpoint 1.1
 CHECK: script lldb.frame.GetLineEntry().GetLine()
Index: lldb/test/Shell/Driver/Inputs/convenience.in
===
--- lldb/test/Shell/Driver/Inputs/convenience.in
+++ lldb/test/Shell/Driver/Inputs/convenience.in
@@ -1,8 +1,11 @@
 breakpoint set -f hello.cpp -p Hello
 run
 script print(lldb.debugger)
+script lldb.debugger
 script print(lldb.target)
+script lldb.target
 script print(lldb.process)
+script lldb.process
 script print(lldb.thread.GetStopDescription(100))
 script lldb.frame.GetLineEntry().GetLine()
 script lldb.frame.GetLineEntry().GetFileSpec().GetFilename()
Index: lldb/test/API/sanity/TestReprStrEquality.py
===
--- /dev/null
+++ lldb/test/API/sanity/TestReprStrEquality.py
@@ -0,0 +1,18 @@
+"""
+This is a sanity check that verifies that `repr(sbobject)` and `str(sbobject)`
+produce the same string.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+
+class TestCase(TestBase):
+
+  mydir = TestBase.compute_mydir(__file__)
+
+  NO_DEBUG_INFO_TESTCASE = True
+
+  def test(self):
+self.assertEqual(repr(self.dbg), str(self.dbg))
Index: lldb/bindings/macros.swig
===
--- lldb/bindings/macros.swig
+++ lldb/bindings/macros.swig
@@ -1,6 +1,6 @@
 %define STRING_EXTENSION_LEVEL(Class, Level)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream, Level);
 const char *desc = stream.GetData();
@@ -15,7 +15,7 @@
 
 %define STRING_EXTENSION(Class)
 %extend {
-  std::string lldb:: ## Class ## ::__str__(){
+  std::string lldb:: ## Class ## ::__repr__(){
 lldb::SBStream stream;
 $self->GetDescription (stream);
 const char *desc = stream.GetData();


Index: lldb/test/Shell/Driver/TestConvenienceVariables.test
===
--- lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -6,10 +6,16 @@
 CHECK: stop reason = breakpoint 1.1
 CHECK: script print(lldb.debugger)
 CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
+CHECK: script lldb.debugger
+CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
 CHECK: script print(lldb.target)
 CHECK-NEXT: target.out
+CHECK: script lldb.target
+CHECK-NEXT: target.out
 CHECK: script print(lldb.process)
 CHECK-NEXT: SBProcess: pid = {{[0-9]+}}, state = stopped, threads = {{[0-9]+}}, executable = target.out
+CHECK: script lldb.process
+CHECK-NEXT: SBProcess: pid = {{[0-9]+}}, state = stopped, threads = {{[0-9]+}}, executable = target.out
 CHECK: script print(lldb.thread.GetStopDescription(100))
 CHECK-NEXT: breakpoint 1.1
 CHECK: script lldb.frame.GetLineEntry().GetLine()
Index: lldb/test/Shell/Driver/Inputs/convenience.in
===
--- lldb/test/Shell/Driver/Inputs/convenience.in
+++ lldb/test/Shell/Driver/Inputs/convenience.in
@@ -1,8 +1,11 @@
 breakpoint set -f hello.cpp -p Hello
 run
 script print(lldb.debugger)
+script lldb.debugger
 script print(lldb.target)
+script lldb.target
 script print(lldb.process)
+script lldb.process
 script print(lldb.thread.GetStopDescription(100))
 script lldb.frame.GetLineEntry().GetLine()
 script lldb.frame.GetLineEntry().GetFileSpec().GetFilename()
Index: lldb/test/API/sanity/TestReprStrEquality.py
===
--- /dev/null

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

2022-06-10 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

I suppose it is possible to have a Windows-only test for this. (Someone will 
need to verify them for me though as I don't have a build that runs tests on 
Windows.)

This doesn't currently work with a path without the suffix though (like 
`C:\Windows\System32\notepad` would not work). `ResolveExecutableLocation` 
calls `llvm::sys::findProgramByName` (from 
`llvm/lib/Support/Windows/Program.inc`), which doesn't do anything if the 
argument is a path. I can change this behaviour, but it may affect more than 
just `target create`... I don't feel comfortable making this change here for 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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


[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 435985.
Michael137 added a comment.

- Update to use Doxygen-style comments
- Improve test robustness


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
@@ -0,0 +1,56 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+template 
+void by_ref_and_ptr(std::span &ref, std::span *ptr) {
+  printf("Stop here to check by ref");
+  return;
+}
+
+int main() {
+  std::array numbers = {1, 12, 123, 1234, 12345};
+
+  using dynamic_string_span = std::span;
+
+  // Test span of ints
+
+  //   Full view of numbers with static extent
+  std::span numbers_span = numbers;
+
+  printf("break here");
+
+  by_ref_and_ptr(numbers_span, &numbers_span);
+
+  // Test spans of strings
+  std::vector strings{"goofy", "is", "smart", "!!!"};
+  strings.reserve(strings.size() + 1);
+
+  //   Partial view of strings with dynamic extent
+  dynamic_string_span strings_span{std::span{strings}.subspan(2)};
+
+  auto strings_span_it = strings_span.begin();
+
+  printf("break here");
+
+  //   Vector size doesn't increase, span should
+  //   print unchanged and the strings_span_it
+  //   remains valid
+  strings.emplace_back("???");
+
+  printf("break here");
+
+  // Now some empty spans
+  std::span static_zero_span;
+  std::span dynamic_zero_span;
+
+  // Multiple spans
+  std::array span_arr{strings_span, strings_span};
+  std::span, 2> nested = span_arr;
+
+  printf("break here");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -0,0 +1,152 @@
+"""
+Test lldb data formatter subsystem for std::span
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxSpanDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_numbers(self, var_name):
+"""Helper to check that data formatter sees contents of std::span correctly"""
+self.expect("frame variable " + var_name,
+substrs=[var_name + ' = size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect("p " + var_name,
+substrs=['$', 'size=5',
+ '[0] = 1',
+ '[1] = 12',
+ '[2] = 123',
+ '[3] = 1234',
+ '[4] = 12345',
+ '}'])
+
+self.expect_expr(var_name, result_summary="size=5", result_children=[
+ValueCheck(value="1"),
+ValueCheck(value="12"),
+ValueCheck(value="123"),
+ValueCheck(value="1234"),
+ValueCheck(value="12345"),
+])
+
+# check access-by-index
+self.expect("frame variable " + var_name + "[0]",
+substrs=['1'])
+self.expect("frame variable " + var_name + "[1]",
+substrs=['12'])
+self.expect("frame variable " + var_name + "[2]",
+substrs=['123'])
+self.expect("frame variable " + var_name + "[3]",
+substrs=['1234'])
+
+@add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '11.0'])
+def test_with_run_command(self):
+"""Test that std::span variables are formatted correctly when printed."""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.cpp", False))
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+# std::span of std::array with extents known at compile-time

[Lldb-commits] [PATCH] D127436: [lldb] Resolve exe location for `target create`

2022-06-10 Thread Alvin Wong via Phabricator via lldb-commits
alvinhochun added a comment.

Does this test look good:

  diff
  diff --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test 
b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
  new file mode 100644
  index ..3dfa7f0d9853
  --- /dev/null
  +++ b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
  @@ -0,0 +1,22 @@
  +# REQUIRES: system-windows
  +# RUN: mkdir "%t.dir"
  +# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir\testmain.exe
  +
  +## Test with full path to exe
  +# RUN: %lldb %t.dir\testmain.exe -b | FileCheck %s
  +
  +## Test with exe on path, with .exe suffix
  +# RUN: set "PATH=%t.dir;%%PATH%%" && %lldb testmain.exe -b | FileCheck %s
  +
  +## Test with exe on path, without .exe suffix
  +# RUN: set "PATH=%t.dir;%%PATH%%" && %lldb testmain -b | FileCheck %s
  +
  +## Test with in cwd, with .exe suffix
  +# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
  +
  +## Test with in cwd, without .exe suffix
  +# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
  +
  +# LABEL: target create
  +# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
  +# CHECK-NOT: Error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

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


[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py:23
+ '[4] = 12345',
+ '}'])
+

Something you could consider for a follow-up commit:
A more robust and canonical way to test for synthetic children would be to 
check the SBAPI directly.
https://lldb.llvm.org/python_api.html
```
var = frame.FindVariable(var_name)
self.assertEquals(var.GetNumChildren(), 5)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

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


[Lldb-commits] [lldb] 5a9fa21 - [lldb/crashlog] Show help when the command is called without any argument

2022-06-10 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-06-10T13:44:43-07:00
New Revision: 5a9fa21ce82fc04d77d52eb1255dd3cf0292d573

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

LOG: [lldb/crashlog] Show help when the command is called without any argument

This patch changes the `crashlog` command behavior to print the help
message if no argument was provided with the command.

rdar://94576026

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

Signed-off-by: Med Ismail Bennani 

Added: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test

Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index b2145762d7a8f..c30a0ec0a1e83 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -1199,6 +1199,11 @@ def CrashLogOptionParser():
 
 def SymbolicateCrashLogs(debugger, command_args):
 option_parser = CrashLogOptionParser()
+
+if not len(command_args):
+option_parser.print_help()
+return
+
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
new file mode 100644
index 0..d35e62391af8d
--- /dev/null
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
@@ -0,0 +1,9 @@
+# RUN: %lldb -o 'command script import lldb.macosx.crashlog' -o 'crashlog' 
2>&1 | FileCheck %s
+
+# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" 
options on these commands
+
+# CHECK: Usage: crashlog [options]  [FILE ...]
+# CHECK: Symbolicate one or more darwin crash log files to provide source file 
and line
+# CHECK: Options:
+# CHECK:  -h, --helpshow this help message and exit
+



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


[Lldb-commits] [PATCH] D127362: [lldb/crashlog] Show help when the command is called without any argument

2022-06-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a9fa21ce82f: [lldb/crashlog] Show help when the command is 
called without any argument (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127362

Files:
  lldb/examples/python/crashlog.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test


Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
@@ -0,0 +1,9 @@
+# RUN: %lldb -o 'command script import lldb.macosx.crashlog' -o 'crashlog' 
2>&1 | FileCheck %s
+
+# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" 
options on these commands
+
+# CHECK: Usage: crashlog [options]  [FILE ...]
+# CHECK: Symbolicate one or more darwin crash log files to provide source file 
and line
+# CHECK: Options:
+# CHECK:  -h, --helpshow this help message and exit
+
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1199,6 +1199,11 @@
 
 def SymbolicateCrashLogs(debugger, command_args):
 option_parser = CrashLogOptionParser()
+
+if not len(command_args):
+option_parser.print_help()
+return
+
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:


Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/no-args.test
@@ -0,0 +1,9 @@
+# RUN: %lldb -o 'command script import lldb.macosx.crashlog' -o 'crashlog' 2>&1 | FileCheck %s
+
+# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
+
+# CHECK: Usage: crashlog [options]  [FILE ...]
+# CHECK: Symbolicate one or more darwin crash log files to provide source file and line
+# CHECK: Options:
+# CHECK:  -h, --helpshow this help message and exit
+
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -1199,6 +1199,11 @@
 
 def SymbolicateCrashLogs(debugger, command_args):
 option_parser = CrashLogOptionParser()
+
+if not len(command_args):
+option_parser.print_help()
+return
+
 try:
 (options, args) = option_parser.parse_args(command_args)
 except:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D127458: [lldb][bindings] Implement __repr__ instead of __str__

2022-06-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.

This looks good to me as well. The job of "recreating the object from an eval 
of the repr output" is not even possible for many SB objects, by the time the 
object was made it's lost the factory invocation that produced it.  And anyway, 
I also can't see a good use case for this.  The fact that this gives us good 
output in lists of objects is also pretty persuasive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127458

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


[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 436077.
Michael137 added a comment.

- Updated tests to avoid using `self.expect` and instead the more specific 
assertions from `lldbtest` and `SB API`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
@@ -0,0 +1,56 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+template 
+void by_ref_and_ptr(std::span &ref, std::span *ptr) {
+  printf("Stop here to check by ref");
+  return;
+}
+
+int main() {
+  std::array numbers = {1, 12, 123, 1234, 12345};
+
+  using dynamic_string_span = std::span;
+
+  // Test span of ints
+
+  //   Full view of numbers with static extent
+  std::span numbers_span = numbers;
+
+  printf("break here");
+
+  by_ref_and_ptr(numbers_span, &numbers_span);
+
+  // Test spans of strings
+  std::vector strings{"goofy", "is", "smart", "!!!"};
+  strings.reserve(strings.size() + 1);
+
+  //   Partial view of strings with dynamic extent
+  dynamic_string_span strings_span{std::span{strings}.subspan(2)};
+
+  auto strings_span_it = strings_span.begin();
+
+  printf("break here");
+
+  //   Vector size doesn't increase, span should
+  //   print unchanged and the strings_span_it
+  //   remains valid
+  strings.emplace_back("???");
+
+  printf("break here");
+
+  // Now some empty spans
+  std::span static_zero_span;
+  std::span dynamic_zero_span;
+
+  // Multiple spans
+  std::array span_arr{strings_span, strings_span};
+  std::span, 2> nested = span_arr;
+
+  printf("break here");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -0,0 +1,153 @@
+"""
+Test lldb data formatter subsystem for std::span
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxSpanDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def findVariable(self, name):
+var = self.frame().FindVariable(name)
+self.assertTrue(var.IsValid())
+return var
+
+def check_size(self, var_name, size):
+var = self.findVariable(var_name)
+self.assertEqual(var.GetNumChildren(), size)
+
+def check_numbers(self, var_name):
+"""Helper to check that data formatter sees contents of std::span correctly"""
+
+expectedSize = 5
+self.check_size(var_name, expectedSize)
+
+self.expect_expr(
+var_name, result_type=f'std::span',
+result_summary=f'size={expectedSize}',
+result_children=[
+ValueCheck(name='[0]', value='1'),
+ValueCheck(name='[1]', value='12'),
+ValueCheck(name='[2]', value='123'),
+ValueCheck(name='[3]', value='1234'),
+ValueCheck(name='[4]', value='12345')
+])
+
+# check access-by-index
+self.expect_var_path(f'{var_name}[0]', type='int', value='1')
+self.expect_var_path(f'{var_name}[1]', type='int', value='12')
+self.expect_var_path(f'{var_name}[2]', type='int', value='123')
+self.expect_var_path(f'{var_name}[3]', type='int', value='1234')
+self.expect_var_path(f'{var_name}[4]', type='int', value='12345')
+
+@add_test_categories(['libc++'])
+@skipIf(compiler='clang', compiler_version=['<', '11.0'])
+def test_with_run_command(self):
+"""Test that std::span variables are formatted correctly when printed."""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.cpp', False))
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+# std::span of std::array with extents known at compile-time
+self.check_numbers('numbers_span')
+
+# check access to synthetic children for static span

[Lldb-commits] [PATCH] D127481: [LLDB][formatters] Add formatter for libc++'s std::span

2022-06-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 436083.
Michael137 added a comment.

Redundant destructor definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127481

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/main.cpp
@@ -0,0 +1,56 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+template 
+void by_ref_and_ptr(std::span &ref, std::span *ptr) {
+  printf("Stop here to check by ref");
+  return;
+}
+
+int main() {
+  std::array numbers = {1, 12, 123, 1234, 12345};
+
+  using dynamic_string_span = std::span;
+
+  // Test span of ints
+
+  //   Full view of numbers with static extent
+  std::span numbers_span = numbers;
+
+  printf("break here");
+
+  by_ref_and_ptr(numbers_span, &numbers_span);
+
+  // Test spans of strings
+  std::vector strings{"goofy", "is", "smart", "!!!"};
+  strings.reserve(strings.size() + 1);
+
+  //   Partial view of strings with dynamic extent
+  dynamic_string_span strings_span{std::span{strings}.subspan(2)};
+
+  auto strings_span_it = strings_span.begin();
+
+  printf("break here");
+
+  //   Vector size doesn't increase, span should
+  //   print unchanged and the strings_span_it
+  //   remains valid
+  strings.emplace_back("???");
+
+  printf("break here");
+
+  // Now some empty spans
+  std::span static_zero_span;
+  std::span dynamic_zero_span;
+
+  // Multiple spans
+  std::array span_arr{strings_span, strings_span};
+  std::span, 2> nested = span_arr;
+
+  printf("break here");
+
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -0,0 +1,153 @@
+"""
+Test lldb data formatter subsystem for std::span
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxSpanDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def findVariable(self, name):
+var = self.frame().FindVariable(name)
+self.assertTrue(var.IsValid())
+return var
+
+def check_size(self, var_name, size):
+var = self.findVariable(var_name)
+self.assertEqual(var.GetNumChildren(), size)
+
+def check_numbers(self, var_name):
+"""Helper to check that data formatter sees contents of std::span correctly"""
+
+expectedSize = 5
+self.check_size(var_name, expectedSize)
+
+self.expect_expr(
+var_name, result_type=f'std::span',
+result_summary=f'size={expectedSize}',
+result_children=[
+ValueCheck(name='[0]', value='1'),
+ValueCheck(name='[1]', value='12'),
+ValueCheck(name='[2]', value='123'),
+ValueCheck(name='[3]', value='1234'),
+ValueCheck(name='[4]', value='12345')
+])
+
+# check access-by-index
+self.expect_var_path(f'{var_name}[0]', type='int', value='1')
+self.expect_var_path(f'{var_name}[1]', type='int', value='12')
+self.expect_var_path(f'{var_name}[2]', type='int', value='123')
+self.expect_var_path(f'{var_name}[3]', type='int', value='1234')
+self.expect_var_path(f'{var_name}[4]', type='int', value='12345')
+
+@add_test_categories(['libc++'])
+@skipIf(compiler='clang', compiler_version=['<', '11.0'])
+def test_with_run_command(self):
+"""Test that std::span variables are formatted correctly when printed."""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, 'break here', lldb.SBFileSpec('main.cpp', False))
+
+lldbutil.continue_to_breakpoint(process, bkpt)
+
+# std::span of std::array with extents known at compile-time
+self.check_numbers('numbers_span')
+
+# check access to synthetic children for static spans
+self.runCmd('type summary add --summary-string "item 0 is ${var[0]}" -x "