[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-08 Thread Caroline Tice via Phabricator via lldb-commits
cmtice updated this revision to Diff 336276.
cmtice added a comment.
Herald added a reviewer: alexshap.

Add a test case.


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

https://reviews.llvm.org/D97786

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
@@ -0,0 +1,390 @@
+# Test to verify LLDB searches for dwos with relative paths relative to the
+# binary location, not relative to LLDB's launch location.
+
+# RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
+# RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o
+
+# RUN: ld.lld %t.o -o %T/dwo-relative-path
+
+# RUN: cd ../..
+
+# RUN: %lldb %T/dwo-relative-path -o "br set -n main" -o run -o step -o "frame var x" -b 2>&1 | FileCheck %s
+# CHECK: stop reason = breakpoint
+# CHECK: x = 10
+
+	.text
+	.file	"dwo-relative-path.cpp"
+	.file	0 "." "dwo-relative-path.cpp" md5 0x9dfc8a64128702e53fdbd698fad5ffa8
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 12 0  # dwo-relative-path.cpp:12:0
+	.cfi_startproc
+# %bb.0:# %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movl	$0, -4(%rbp)
+	movl	%edi, -8(%rbp)
+	movq	%rsi, -16(%rbp)
+.Ltmp0:
+	.loc	0 13 7 prologue_end # dwo-relative-path.cpp:13:7
+	movl	$10, -20(%rbp)
+	.loc	0 14 8  # dwo-relative-path.cpp:14:8
+	movw	.L__const.main.y, %ax
+	movw	%ax, -23(%rbp)
+	movb	.L__const.main.y+2, %al
+	movb	%al, -21(%rbp)
+	.loc	0 16 14 # dwo-relative-path.cpp:16:14
+	movsbl	-23(%rbp), %eax
+	.loc	0 16 27 is_stmt 0   # dwo-relative-path.cpp:16:27
+	movsbl	-22(%rbp), %ecx
+	.loc	0 16 19 # dwo-relative-path.cpp:16:19
+	addl	%ecx, %eax
+	.loc	0 16 5  # dwo-relative-path.cpp:16:5
+	addl	-20(%rbp), %eax
+	movl	%eax, -20(%rbp)
+	.loc	0 18 3 is_stmt 1# dwo-relative-path.cpp:18:3
+	xorl	%eax, %eax
+	popq	%rbp
+	.cfi_def_cfa %rsp, 8
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.type	.L__const.main.y,@object# @__const.main.y
+	.section	.rodata,"a",@progbits
+.L__const.main.y:
+	.ascii	"abc"
+	.size	.L__const.main.y, 3
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	74  # DW_TAG_skeleton_unit
+	.byte	0   # DW_CHILDREN_no
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.ascii	"\264B" # DW_AT_GNU_pubnames
+	.byte	25  # DW_FORM_flag_present
+	.byte	118 # DW_AT_dwo_name
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	0   # EOM(3)
+	.section	.debug_info,"",@progbits
+.Lcu_begin0:
+	.long	.Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+	.short	5   # DWARF version number
+	.byte	4   # DWARF Unit Type
+	.byte	8   # Address Size (in bytes)
+	.long	.debug_abbrev   # Offset Into Abbrev. Section
+	.quad	6502600918997875500
+	.byte	1   # Abbrev [1] 0x14:0x14 DW_TAG_skeleton_unit
+	.long	.Lline_table_start0 # DW_AT_stmt_list
+	.long	.Lstr_offsets_base0 # DW_AT_str_offsets_base
+	.byte	0   # DW_AT_comp_dir
+# DW_AT_GNU_pubnames
+	.byte	1   # DW_AT_dwo_name
+	.byte	0   # DW_AT_low_pc
+	.long	.Lfunc_end0-.Lfunc_begin0   # DW_AT_high_pc
+	.long	.Laddr_table_base0  # 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 336263.
mgorny added a comment.

Add to `_KNOWN_QSUPPORTED_STUB_FEATURES` and clang-format.


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

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  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
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol& process);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
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
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,49 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" &&
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags;
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
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
@@ -353,7 +353,9 @@
 
   // build the qSupported packet
   std::vector features = {"xmlRegisters=i386,arm,mips,arc",
-   "multiprocess+"};
+   "multiprocess+",
+   "fork-events+",
+   "vfork-events+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

Note that I've included full `fork-events+` and `vfork-events+` as demo of the 
API. I can split them further and/or move to more relevant commits as I 
progress with the split.


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

https://reviews.llvm.org/D100153

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


[Lldb-commits] [PATCH] D99864: [lldb] Fork/vfork support via gdb-remote protocol [WIP]

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

And now moved Extension API to D100153 . I've 
added an API to query features supported by platform plugin, so now server 
doesn't report them unless they're actually going to be used.


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

https://reviews.llvm.org/D99864

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


[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

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

Introduce a NativeProcessProtocol API for indicating support for
protocol extensions and enabling them.  LLGS calls
GetSupportedExtensions() method on the process factory to determine
which extensions are supported by the plugin.  If the future is both
supported by the plugin and reported as supported by the client, LLGS
enables it and reports to the client as supported by the server.

The extension is enabled on the process instance by calling
SetEnabledExtensions() method.  This is done after qSupported exchange
(if the debugger is attached to any process), as well as after launching
or attaching to a new inferior.

The patch adds 'fork' extension corresponding to 'fork-events+'
qSupported feature and 'vfork' extension for 'vfork-events+'.


https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  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
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol& process);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
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
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,45 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" && (process_features & NativeProcessProtocol::Extension::fork) == NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" && (process_features & NativeProcessProtocol::Extension::vfork) == NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags;
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- 

[Lldb-commits] [PATCH] D99812: [PowerPC] [GlobalISel] Implementation of formal arguments lowering in the IRTranslator for the PPC backend

2021-04-08 Thread Anshil Gandhi via Phabricator via lldb-commits
gandhi21299 added a comment.

@arsenm Yea, I am sorry about that. Looks like I am still not using arc properly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99812

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


[Lldb-commits] [PATCH] D99812: [PowerPC] [GlobalISel] Implementation of formal arguments lowering in the IRTranslator for the PPC backend

2021-04-08 Thread Matt Arsenault via Phabricator via lldb-commits
arsenm added a comment.

Attached wrong diff? There are a lot of unrelated files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99812

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D98822#2677961 , @JDevlieghere 
wrote:

> `test/Shell/Subprocess/vfork-follow-parent-wp.test` is failing on GreenDragon:
>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31016/testReport/junit/lldb-shell/Subprocess/vfork_follow_parent_wp_test/
>
> I'm going to temporarily disable the test on Darwin. Happy to provide help to 
> investigate :-)

I guess that `vfork()`-ed process doesn't start with cleared dbregs on Darwin. 
I suppose you don't handle forks at all, so I suppose skipping/xfailing the 
test is the thing to do — knowing that there's a potential issue but extremely 
unlikely in real life.

Forks are handled explicitly on FreeBSD, Linux and NetBSD right now but I've 
left these tests open to all platforms since they merely verify that LLDB 
doesn't explode when the debugged program forks, and that it (probably) doesn't 
try to debug the child accidentally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D99864: [lldb] Fork/vfork support via gdb-remote protocol [WIP]

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked an inline comment as done.
mgorny added a comment.

Client-side qSupported processing is now refactored in D100146 
.


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

https://reviews.llvm.org/D99864

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


[Lldb-commits] [PATCH] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

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

Refactor the qSupported handler to split the reply into an array,
and identify features within the array rather than searching the string
for partial matches.  While at it, use StringRef.split() to process
the compression list instead of reinventing the wheel.

Switch the arguments to MaybeEnableCompression() to use an ArrayRef
of StringRefs to simplify parameter passing from GetRemoteQSupported().


https://reviews.llvm.org/D100146

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

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
@@ -601,7 +601,8 @@
 
   // Given the list of compression types that the remote debug stub can support,
   // possibly enable compression if we find an encoding we can handle.
-  void MaybeEnableCompression(std::vector supported_compressions);
+  void MaybeEnableCompression(
+  llvm::ArrayRef supported_compressions);
 
   bool DecodeProcessInfoResponse(StringExtractorGDBRemote ,
  ProcessInstanceInfo _info);
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
@@ -345,6 +345,9 @@
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
+  m_supports_qEcho = eLazyBoolNo;
+  m_supports_QPassSignals = eLazyBoolNo;
+
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
 
@@ -362,97 +365,51 @@
   if (SendPacketAndWaitForResponse(packet.GetString(), response,
/*send_async=*/false) ==
   PacketResult::Success) {
-const char *response_cstr = response.GetStringRef().data();
-
 // Hang on to the qSupported packet, so that platforms can do custom
 // configuration of the transport before attaching/launching the process.
-m_qSupported_response = response_cstr;
-
-if (::strstr(response_cstr, "qXfer:auxv:read+"))
-  m_supports_qXfer_auxv_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:libraries-svr4:read+"))
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
-if (::strstr(response_cstr, "augmented-libraries-svr4-read")) {
-  m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
-  m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
-}
-if (::strstr(response_cstr, "qXfer:libraries:read+"))
-  m_supports_qXfer_libraries_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:features:read+"))
-  m_supports_qXfer_features_read = eLazyBoolYes;
-if (::strstr(response_cstr, "qXfer:memory-map:read+"))
-  m_supports_qXfer_memory_map_read = eLazyBoolYes;
-
-// Look for a list of compressions in the features list e.g.
-// qXfer:features:read+;PacketSize=2;qEcho+;SupportedCompressions=zlib-
-// deflate,lzma
-const char *features_list = ::strstr(response_cstr, "qXfer:features:");
-if (features_list) {
-  const char *compressions =
-  ::strstr(features_list, "SupportedCompressions=");
-  if (compressions) {
-std::vector supported_compressions;
-compressions += sizeof("SupportedCompressions=") - 1;
-const char *end_of_compressions = strchr(compressions, ';');
-if (end_of_compressions == nullptr) {
-  end_of_compressions = strchr(compressions, '\0');
-}
-const char *current_compression = compressions;
-while (current_compression < end_of_compressions) {
-  const char *next_compression_name = strchr(current_compression, ',');
-  const char *end_of_this_word = next_compression_name;
-  if (next_compression_name == nullptr ||
-  end_of_compressions < next_compression_name) {
-end_of_this_word = end_of_compressions;
-  }
-
-  if (end_of_this_word) {
-if (end_of_this_word == current_compression) {
-  current_compression++;
-} else {
-  std::string this_compression(
-  current_compression, end_of_this_word - current_compression);
-  supported_compressions.push_back(this_compression);
-  current_compression = end_of_this_word + 1;
-}
-  } else {
-

[Lldb-commits] [lldb] e761b6b - [lldb] (Temporarily) disable vfork-follow-parent-wp.test on Darwin

2021-04-08 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2021-04-08T15:05:27-07:00
New Revision: e761b6b4c58d4f7ae1073d925d7cb321d68ee93a

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

LOG: [lldb] (Temporarily) disable vfork-follow-parent-wp.test on Darwin

The test is failing on GreenDragon. Pinged Michał in D98822.

Added: 


Modified: 
lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test

Removed: 




diff  --git a/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test 
b/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
index d73b854b6647d..8c74e28dbc199 100644
--- a/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
+++ b/lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -1,5 +1,6 @@
 # REQUIRES: native && dbregs-set
 # UNSUPPORTED: system-windows
+# UNSUPPORTED: system-darwin
 # RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch -s



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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

`test/Shell/Subprocess/vfork-follow-parent-wp.test` is failing on GreenDragon:

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31016/testReport/junit/lldb-shell/Subprocess/vfork_follow_parent_wp_test/

I'm going to temporarily disable the test on Darwin. Happy to provide help to 
investigate :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D99864: [lldb] Fork/vfork support via gdb-remote protocol [WIP]

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked 2 inline comments as done.
mgorny added a comment.

The server-side `qSupported` handling is now split into D100140 
, and all relevant remarks were addressed 
there.


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

https://reviews.llvm.org/D99864

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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

This is split from D99864  as a minimal 
independent change.


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

https://reviews.llvm.org/D100140

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


[Lldb-commits] [PATCH] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

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

Refactor handling qSupported to use a virtual HandleFeatures() method.
The client-provided features are split into an array and passed
to the method.  The method returns an array of server features that are
concatenated into the qSupported response to the server.

The base implementation of HandleFeatures()
in GDBRemoteCommunicationServerCommon now includes only flags common
to both platform server and llgs, while llgs-specific flags are inserted
in GDBRemoteCommunicationServerLLGS.


https://reviews.llvm.org/D100140

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  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
@@ -219,6 +219,9 @@
 
   static std::string XMLEncodeAttributeValue(llvm::StringRef value);
 
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features) override;
+
 private:
   llvm::Expected> BuildTargetXml();
 
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
@@ -3534,3 +3534,16 @@
 
   return tid;
 }
+
+std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  auto ret =
+  GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
+  ret.insert(ret.end(), {
+"qXfer:features:read+", "multiprocess+",
+#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
+"QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
+#endif
+  });
+  return ret;
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -145,6 +145,11 @@
   virtual FileSpec FindModuleFile(const std::string _path,
   const ArchSpec );
 
+  // Process client_features (qSupported) and return an array of server features
+  // to be returned in response.
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+
 private:
   ModuleSpec GetModuleInfo(llvm::StringRef module_path, llvm::StringRef triple);
 };
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -831,26 +831,11 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_qSupported(
 StringExtractorGDBRemote ) {
-  StreamGDBRemote response;
-
-  // Features common to lldb-platform and llgs.
-  uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
- // size--debugger can always use less
-  response.Printf("PacketSize=%x", max_packet_size);
-
-  response.PutCString(";QStartNoAckMode+");
-  response.PutCString(";QThreadSuffixSupported+");
-  response.PutCString(";QListThreadsInStopReply+");
-  response.PutCString(";qEcho+");
-  response.PutCString(";qXfer:features:read+");
-#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
-  response.PutCString(";QPassSignals+");
-  response.PutCString(";qXfer:auxv:read+");
-  response.PutCString(";qXfer:libraries-svr4:read+");
-#endif
-  response.PutCString(";multiprocess+");
-
-  return SendPacketNoLock(response.GetString());
+  // Parse client-indicated features.
+  llvm::StringRef view = packet.GetStringRef();
+  llvm::SmallVector client_features;
+  view.split(client_features, ';');
+  return SendPacketNoLock(llvm::join(HandleFeatures(client_features), ";"));
 }
 
 GDBRemoteCommunication::PacketResult
@@ -1312,3 +1297,17 @@
 
   return matched_module_spec;
 }
+
+std::vector GDBRemoteCommunicationServerCommon::HandleFeatures(
+const llvm::ArrayRef client_features) {
+  // 128KBytes is a reasonable max packet size--debugger can always use less.
+  constexpr uint32_t max_packet_size = 128 * 1024;
+
+  // Features common to platform 

[Lldb-commits] [PATCH] D99828: Create setting to disable LanguageRuntime provided UnwindPlans

2021-04-08 Thread Jason Molenda 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 rGdd453a1389b6: Add setting to disable LanguageRuntime 
UnwindPlans (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99828

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/LanguageRuntime.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -206,6 +206,10 @@
 Global,
 DefaultFalse,
 Desc<"If true, stop when a shared library is loaded or unloaded.">;
+  def DisableLangRuntimeUnwindPlans: 
Property<"disable-language-runtime-unwindplans", "Boolean">,
+Global,
+DefaultFalse,
+Desc<"If true, LanguageRuntime plugins' UnwindPlans will not be used when 
backtracing.">;
   def DetachKeepsStopped: Property<"detach-keeps-stopped", "Boolean">,
 Global,
 DefaultFalse,
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -238,6 +238,18 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, stop);
 }
 
+bool ProcessProperties::GetDisableLangRuntimeUnwindPlans() const {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
+void ProcessProperties::SetDisableLangRuntimeUnwindPlans(bool disable) {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, disable);
+  m_process->Flush();
+}
+
 bool ProcessProperties::GetDetachKeepsStopped() const {
   const uint32_t idx = ePropertyDetachKeepsStopped;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Target/LanguageRuntime.cpp
===
--- lldb/source/Target/LanguageRuntime.cpp
+++ lldb/source/Target/LanguageRuntime.cpp
@@ -265,6 +265,8 @@
   ProcessSP process_sp = thread.GetProcess();
   if (!process_sp.get())
 return UnwindPlanSP();
+  if (process_sp->GetDisableLangRuntimeUnwindPlans() == true)
+return UnwindPlanSP();
   for (const lldb::LanguageType lang_type : Language::GetSupportedLanguages()) 
{
 if (LanguageRuntime *runtime = process_sp->GetLanguageRuntime(lang_type)) {
   UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -84,6 +84,8 @@
   void SetUnwindOnErrorInExpressions(bool ignore);
   bool GetStopOnSharedLibraryEvents() const;
   void SetStopOnSharedLibraryEvents(bool stop);
+  bool GetDisableLangRuntimeUnwindPlans() const;
+  void SetDisableLangRuntimeUnwindPlans(bool disable);
   bool GetDetachKeepsStopped() const;
   void SetDetachKeepsStopped(bool keep_stopped);
   bool GetWarningsOptimization() const;


Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -206,6 +206,10 @@
 Global,
 DefaultFalse,
 Desc<"If true, stop when a shared library is loaded or unloaded.">;
+  def DisableLangRuntimeUnwindPlans: Property<"disable-language-runtime-unwindplans", "Boolean">,
+Global,
+DefaultFalse,
+Desc<"If true, LanguageRuntime plugins' UnwindPlans will not be used when backtracing.">;
   def DetachKeepsStopped: Property<"detach-keeps-stopped", "Boolean">,
 Global,
 DefaultFalse,
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -238,6 +238,18 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, stop);
 }
 
+bool ProcessProperties::GetDisableLangRuntimeUnwindPlans() const {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
+void ProcessProperties::SetDisableLangRuntimeUnwindPlans(bool disable) {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, disable);
+  m_process->Flush();
+}
+
 bool ProcessProperties::GetDetachKeepsStopped() const {
   const uint32_t idx = ePropertyDetachKeepsStopped;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: 

[Lldb-commits] [lldb] dd453a1 - Add setting to disable LanguageRuntime UnwindPlans

2021-04-08 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-04-08T13:28:59-07:00
New Revision: dd453a1389b6a7e6d9214b449d3c54981b1a89b6

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

LOG: Add setting to disable LanguageRuntime UnwindPlans

When debugging LanguageRuntime unwindplans, it can be
helpful to disable their use and see the normal
stack walk.  Add a setting for this.

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/LanguageRuntime.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index dbe61d892c499..abd0d39fe9699 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -84,6 +84,8 @@ class ProcessProperties : public Properties {
   void SetUnwindOnErrorInExpressions(bool ignore);
   bool GetStopOnSharedLibraryEvents() const;
   void SetStopOnSharedLibraryEvents(bool stop);
+  bool GetDisableLangRuntimeUnwindPlans() const;
+  void SetDisableLangRuntimeUnwindPlans(bool disable);
   bool GetDetachKeepsStopped() const;
   void SetDetachKeepsStopped(bool keep_stopped);
   bool GetWarningsOptimization() const;

diff  --git a/lldb/source/Target/LanguageRuntime.cpp 
b/lldb/source/Target/LanguageRuntime.cpp
index 1fbe94b7f535e..be878d69fa00d 100644
--- a/lldb/source/Target/LanguageRuntime.cpp
+++ b/lldb/source/Target/LanguageRuntime.cpp
@@ -265,6 +265,8 @@ LanguageRuntime::GetRuntimeUnwindPlan(Thread , 
RegisterContext *regctx,
   ProcessSP process_sp = thread.GetProcess();
   if (!process_sp.get())
 return UnwindPlanSP();
+  if (process_sp->GetDisableLangRuntimeUnwindPlans() == true)
+return UnwindPlanSP();
   for (const lldb::LanguageType lang_type : Language::GetSupportedLanguages()) 
{
 if (LanguageRuntime *runtime = process_sp->GetLanguageRuntime(lang_type)) {
   UnwindPlanSP plan_sp = runtime->GetRuntimeUnwindPlan(

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6db410fa6bf0e..4e01c3bbbf4f7 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -238,6 +238,18 @@ void ProcessProperties::SetStopOnSharedLibraryEvents(bool 
stop) {
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, stop);
 }
 
+bool ProcessProperties::GetDisableLangRuntimeUnwindPlans() const {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
+void ProcessProperties::SetDisableLangRuntimeUnwindPlans(bool disable) {
+  const uint32_t idx = ePropertyDisableLangRuntimeUnwindPlans;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, disable);
+  m_process->Flush();
+}
+
 bool ProcessProperties::GetDetachKeepsStopped() const {
   const uint32_t idx = ePropertyDetachKeepsStopped;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(

diff  --git a/lldb/source/Target/TargetProperties.td 
b/lldb/source/Target/TargetProperties.td
index 030c0a7917e82..e22f94069131a 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -206,6 +206,10 @@ let Definition = "process" in {
 Global,
 DefaultFalse,
 Desc<"If true, stop when a shared library is loaded or unloaded.">;
+  def DisableLangRuntimeUnwindPlans: 
Property<"disable-language-runtime-unwindplans", "Boolean">,
+Global,
+DefaultFalse,
+Desc<"If true, LanguageRuntime plugins' UnwindPlans will not be used when 
backtracing.">;
   def DetachKeepsStopped: Property<"detach-keeps-stopped", "Boolean">,
 Global,
 DefaultFalse,



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


[Lldb-commits] [lldb] d01bff8 - [lldb] [test] Skip clone() tests on Linux/aarch64

2021-04-08 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-04-08T20:03:45+02:00
New Revision: d01bff8cbdc98fb8751f7bf10af19b47ae5c445d

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

LOG: [lldb] [test] Skip clone() tests on Linux/aarch64

Added: 


Modified: 
lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
lldb/test/Shell/Subprocess/clone-follow-parent.test

Removed: 




diff  --git a/lldb/test/Shell/Subprocess/clone-follow-parent-wp.test 
b/lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
index 64ae20d7e363..67d94af3ca10 100644
--- a/lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
+++ b/lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
@@ -1,4 +1,6 @@
 # REQUIRES: native && (system-linux || system-netbsd) && dbregs-set
+# clone() tests fails on arm64 Linux, PR #49899
+# UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_CLONE -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 process launch -s

diff  --git a/lldb/test/Shell/Subprocess/clone-follow-parent.test 
b/lldb/test/Shell/Subprocess/clone-follow-parent.test
index 863531af9a4d..3d89279bbb29 100644
--- a/lldb/test/Shell/Subprocess/clone-follow-parent.test
+++ b/lldb/test/Shell/Subprocess/clone-follow-parent.test
@@ -1,4 +1,6 @@
 # REQUIRES: native && (system-linux || system-netbsd)
+# clone() tests fails on arm64 Linux, PR #49899
+# UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 b parent_func



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


[Lldb-commits] [PATCH] D100053: Fixed bug issue #42017

2021-04-08 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb68545acf977: [lldb] Improve the documentation (#42017) 
(authored by sushmaunnibhavi, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100053

Files:
  lldb/docs/use/tutorial.rst


Index: lldb/docs/use/tutorial.rst
===
--- lldb/docs/use/tutorial.rst
+++ lldb/docs/use/tutorial.rst
@@ -28,7 +28,7 @@
 
 Options can be placed anywhere on the command line, but if the arguments begin
 with a "-" then you have to tell lldb that you're done with options for the
-current command by adding an option termination: "--" So for instance if you
+current command by adding an option termination: "--". So for instance, if you
 want to launch a process and give the "process launch" command the
 "--stop-at-entry" option, yet you want the process you are about to launch to
 be launched with the arguments "-program_arg value", you would type:
@@ -121,7 +121,7 @@
 
 lldb also supports command completion for source file names, symbol names, file
 names, etc. Completion is initiated by a hitting a TAB. Individual options in a
-command can have different completers, so for instance the "--file "
+command can have different completers, so for instance, the "--file "
 option in "breakpoint" completes to source files, the "--shlib " option
 to currently loaded shared libraries, etc. We can even do things like if you
 specify "--shlib ", and are completing on "--file ", we will only
@@ -134,7 +134,7 @@
 for each matching command.
 
 Finally, there is a mechanism to construct aliases for commonly used commands.
-So for instance if you get annoyed typing:
+For instance, if you get annoyed typing:
 
 ::
 
@@ -162,10 +162,10 @@
 One alias of note that we do include by popular demand is a weak emulator of
 gdb's "break" command. It doesn't try to do everything that gdb's break command
 does (for instance, it doesn't handle foo.c::bar. But it mostly works, and
-makes the transition easier. Also by popular demand, it is aliased to b. If you
+makes the transition easier. Also, by popular demand, it is aliased to b. If 
you
 actually want to learn the lldb command set natively, that means it will get in
 the way of the rest of the breakpoint commands. Fortunately, if you don't like
-one of our aliases, you an easily get rid of it by running (for example):
+one of our aliases, you can easily get rid of it by running (for example):
 
 ::
 
@@ -183,7 +183,7 @@
 options are stripped off, the rest of the command string is passed
 uninterpreted to the command. This is convenient for commands whose arguments
 might be some complex expression that would be painful to backslash protect.
-For instance the "expression" command is a "raw" command for obvious reasons.
+For instance, the "expression" command is a "raw" command for obvious reasons.
 The "help" output for a command will tell you if it is "raw" or not, so you
 know what to expect. The one thing you have to watch out for is that since raw
 commands still can have options, if your command string has dashes in it,
@@ -248,9 +248,9 @@
 
 The logical breakpoint has an integer id, and its locations have an id within
 their parent breakpoint (the two are joined by a ".", e.g. 1.1 in the example
-above.)
+above).
 
-Also the logical breakpoints remain live so that if another shared library were
+Also, the logical breakpoints remain live so that if another shared library 
were
 to be loaded that had another implementation of the "alignLeftEdges:" selector,
 the new location would be added to breakpoint 1 (e.g. a "1.2" breakpoint would
 be set on the newly loaded selector).
@@ -284,7 +284,7 @@
 
 You can delete, disable, set conditions and ignore counts either on all the
 locations generated by your logical breakpoint, or on any one of the particular
-locations your specification resolved to. For instance if we wanted to add a
+locations your specification resolved to. For instance, if we wanted to add a
 command to print a backtrace when we hit this breakpoint we could do:
 
 ::
@@ -299,7 +299,7 @@
 option. Use "--script" if you want to implement your breakpoint command using
 the Python script instead.
 
-This is an convenient point to bring up another feature of the lldb command
+This is a convenient point to bring up another feature of the lldb command
 help. Do:
 
 ::


Index: lldb/docs/use/tutorial.rst
===
--- lldb/docs/use/tutorial.rst
+++ lldb/docs/use/tutorial.rst
@@ -28,7 +28,7 @@
 
 Options can be placed anywhere on the command line, but if the arguments begin
 with a "-" then you have to tell lldb that you're done with options for the
-current command by adding an option termination: "--" So for instance if you
+current command by adding an option termination: 

[Lldb-commits] [lldb] b68545a - [lldb] Improve the documentation (#42017)

2021-04-08 Thread Jonas Devlieghere via lldb-commits

Author: Sushma Unnibhavi
Date: 2021-04-08T10:58:02-07:00
New Revision: b68545acf9771b6b205647b8028d9e042ede8838

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

LOG: [lldb] Improve the documentation (#42017)

Added punctuation and changed "So for instance" to "For instance".

Fixes https://llvm.org/PR42017

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

Added: 


Modified: 
lldb/docs/use/tutorial.rst

Removed: 




diff  --git a/lldb/docs/use/tutorial.rst b/lldb/docs/use/tutorial.rst
index 30c25fa4381fa..3c552a341e024 100644
--- a/lldb/docs/use/tutorial.rst
+++ b/lldb/docs/use/tutorial.rst
@@ -28,7 +28,7 @@ may have to quote some arguments in lldb that you wouldn't in 
gdb.
 
 Options can be placed anywhere on the command line, but if the arguments begin
 with a "-" then you have to tell lldb that you're done with options for the
-current command by adding an option termination: "--" So for instance if you
+current command by adding an option termination: "--". So for instance, if you
 want to launch a process and give the "process launch" command the
 "--stop-at-entry" option, yet you want the process you are about to launch to
 be launched with the arguments "-program_arg value", you would type:
@@ -121,7 +121,7 @@ command:
 
 lldb also supports command completion for source file names, symbol names, file
 names, etc. Completion is initiated by a hitting a TAB. Individual options in a
-command can have 
diff erent completers, so for instance the "--file "
+command can have 
diff erent completers, so for instance, the "--file "
 option in "breakpoint" completes to source files, the "--shlib " option
 to currently loaded shared libraries, etc. We can even do things like if you
 specify "--shlib ", and are completing on "--file ", we will only
@@ -134,7 +134,7 @@ help text for all commands for a particular word and dump a 
summary help string
 for each matching command.
 
 Finally, there is a mechanism to construct aliases for commonly used commands.
-So for instance if you get annoyed typing:
+For instance, if you get annoyed typing:
 
 ::
 
@@ -162,10 +162,10 @@ set up.
 One alias of note that we do include by popular demand is a weak emulator of
 gdb's "break" command. It doesn't try to do everything that gdb's break command
 does (for instance, it doesn't handle foo.c::bar. But it mostly works, and
-makes the transition easier. Also by popular demand, it is aliased to b. If you
+makes the transition easier. Also, by popular demand, it is aliased to b. If 
you
 actually want to learn the lldb command set natively, that means it will get in
 the way of the rest of the breakpoint commands. Fortunately, if you don't like
-one of our aliases, you an easily get rid of it by running (for example):
+one of our aliases, you can easily get rid of it by running (for example):
 
 ::
 
@@ -183,7 +183,7 @@ The lldb command parser also supports "raw" commands, 
where, after command
 options are stripped off, the rest of the command string is passed
 uninterpreted to the command. This is convenient for commands whose arguments
 might be some complex expression that would be painful to backslash protect.
-For instance the "expression" command is a "raw" command for obvious reasons.
+For instance, the "expression" command is a "raw" command for obvious reasons.
 The "help" output for a command will tell you if it is "raw" or not, so you
 know what to expect. The one thing you have to watch out for is that since raw
 commands still can have options, if your command string has dashes in it,
@@ -248,9 +248,9 @@ locations if that file and line were inlined in 
diff erent places in your code.
 
 The logical breakpoint has an integer id, and its locations have an id within
 their parent breakpoint (the two are joined by a ".", e.g. 1.1 in the example
-above.)
+above).
 
-Also the logical breakpoints remain live so that if another shared library were
+Also, the logical breakpoints remain live so that if another shared library 
were
 to be loaded that had another implementation of the "alignLeftEdges:" selector,
 the new location would be added to breakpoint 1 (e.g. a "1.2" breakpoint would
 be set on the newly loaded selector).
@@ -284,7 +284,7 @@ locations were found:
 
 You can delete, disable, set conditions and ignore counts either on all the
 locations generated by your logical breakpoint, or on any one of the particular
-locations your specification resolved to. For instance if we wanted to add a
+locations your specification resolved to. For instance, if we wanted to add a
 command to print a backtrace when we hit this breakpoint we could do:
 
 ::
@@ -299,7 +299,7 @@ commands. You can also specify this explicitly by passing 
the "--command"
 option. Use "--script" if you want to 

[Lldb-commits] [PATCH] D99864: [lldb] Fork/vfork support via gdb-remote protocol [WIP]

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D99864#2676389 , @labath wrote:

> I'm going to have more questions, but let me start with this: What should the 
> (v)fork-events extension actually mean? In the client, I guess it 
> demonstrates its willingness to receive the appropriate type of event. But 
> what should it mean on the server? Is it like "I promise to stop on every 
> (v)fork", or just something like "hey, I know this word"? The second meaning 
> only makes sense if these (v)fork-events come with some new protocol 
> extensions/packets that the client can use when communicating with the 
> server. Such is the case with the multiprocess extension (new thread id 
> syntax), but I am not aware of anything similar in for these features. Are 
> there any? The gdb documentation (//This feature indicates whether GDB 
> supports fork event extensions to the remote protocol. GDB does not use such 
> extensions unless the stub also reports that it supports them by including 
> ‘fork-events+’ in its ‘qSupported’ reply. //) seems to indicate they might 
> exist, but I am not sure what would they be...
>
> If there aren't any, then it would probably be better to use the first 
> "definition" of the feature (or something similar), as the client can 
> legitimately be interested in whether it will be able to intercept forks or 
> not. And if that's the case, then we should first consult the relevant 
> NativeProcess class before we reply (v)fork-events+.

Reading GDB code, the server indicates `fork-events+` only if the target 
explicitly supports it. In case of Linux, it even goes as far as to perform a 
runtime test for fork reporting but I don't think we want to go that far. The 
support for particular kind of events is reported independently of whether 
client indicated support.

On client end, the server indication seems to be used to determine whether 
'inserting' fork catchpoint succeeds or fails (it does not perform any comm 
itself since catchpoints just react to fork events from server). If I'm reading 
the code correctly, it also ignores fork/vfork events if `qSupported` did not 
indicate support earlier, though tbh I don't think that makes much sense — I 
suppose it just leads to UB since server sent a stop and we do nothing about it.


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

https://reviews.llvm.org/D99864

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa345419ee030: [lldb] [Process] Watch for fork/vfork 
notifications (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98822

Files:
  lldb/include/lldb/Host/linux/Host.h
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
  lldb/test/Shell/Subprocess/Inputs/fork.cpp
  lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
  lldb/test/Shell/Subprocess/clone-follow-parent.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native && (system-linux || system-netbsd)
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native && (system-linux || system-netbsd) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w 

[Lldb-commits] [lldb] a345419 - [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-04-08T18:49:50+02:00
New Revision: a345419ee03095c8cdfbe1c2728467c4da8fa0a4

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

LOG: [lldb] [Process] Watch for fork/vfork notifications

Watch for fork(2)/vfork(2) (also fork/vfork-style clone(2) on Linux)
notifications and explicitly detach the forked child process, and add
initial tests for these cases.  The code covers FreeBSD, Linux
and NetBSD process plugins.  There is no new user-visible functionality
provided -- this change lays foundations over subsequent work on fork
support.

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

Added: 
lldb/include/lldb/Host/linux/Host.h
lldb/test/Shell/Subprocess/Inputs/fork.cpp
lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
lldb/test/Shell/Subprocess/clone-follow-parent.test
lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
lldb/test/Shell/Subprocess/fork-follow-parent.test
lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
lldb/test/Shell/Subprocess/vfork-follow-parent.test

Modified: 
lldb/source/Host/linux/Host.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py

Removed: 




diff  --git a/lldb/include/lldb/Host/linux/Host.h 
b/lldb/include/lldb/Host/linux/Host.h
new file mode 100644
index ..409a9fc9b322
--- /dev/null
+++ b/lldb/include/lldb/Host/linux/Host.h
@@ -0,0 +1,22 @@
+//===-- Host.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_HOST_LINUX_HOST_H
+#define LLDB_HOST_LINUX_HOST_H
+
+#include "lldb/lldb-types.h"
+#include "llvm/ADT/Optional.h"
+
+namespace lldb_private {
+
+// Get PID (i.e. the primary thread ID) corresponding to the specified TID.
+llvm::Optional getPIDForTID(lldb::pid_t tid);
+
+} // namespace lldb_private
+
+#endif // #ifndef LLDB_HOST_LINUX_HOST_H

diff  --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 520a00df35f6..334634a357f5 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -27,6 +27,7 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/linux/Host.h"
 #include "lldb/Host/linux/Support.h"
 #include "lldb/Utility/DataExtractor.h"
 
@@ -53,7 +54,8 @@ class ProcessLaunchInfo;
 }
 
 static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo ,
-  ProcessState , ::pid_t ) {
+  ProcessState , ::pid_t ,
+  ::pid_t ) {
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
 
   auto BufferOrError = getProcFile(Pid, "status");
@@ -107,6 +109,9 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
,
 } else if (Line.consume_front("TracerPid:")) {
   Line = Line.ltrim();
   Line.consumeInteger(10, TracerPid);
+} else if (Line.consume_front("Tgid:")) {
+  Line = Line.ltrim();
+  Line.consumeInteger(10, Tgid);
 }
   }
   return true;
@@ -204,6 +209,7 @@ static void GetProcessEnviron(::pid_t pid, 
ProcessInstanceInfo _info) {
 static bool GetProcessAndStatInfo(::pid_t pid,
   ProcessInstanceInfo _info,
   ProcessState , ::pid_t ) {
+  ::pid_t tgid;
   tracerpid = 0;
   process_info.Clear();
 
@@ -214,7 +220,7 @@ static bool GetProcessAndStatInfo(::pid_t pid,
   GetProcessEnviron(pid, process_info);
 
   // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+  if (!GetStatusInfo(pid, process_info, State, tracerpid, tgid))
 return false;
 
   return true;
@@ -308,3 +314,14 @@ Environment Host::GetEnvironment() { return 

[Lldb-commits] [PATCH] D100053: Fixed bug issue #42017

2021-04-08 Thread Sushma Unnibhavi via Phabricator via lldb-commits
sushmaunnibhavi updated this revision to Diff 336124.
sushmaunnibhavi edited the summary of this revision.

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

https://reviews.llvm.org/D100053

Files:
  lldb/docs/use/tutorial.rst


Index: lldb/docs/use/tutorial.rst
===
--- lldb/docs/use/tutorial.rst
+++ lldb/docs/use/tutorial.rst
@@ -28,7 +28,7 @@
 
 Options can be placed anywhere on the command line, but if the arguments begin
 with a "-" then you have to tell lldb that you're done with options for the
-current command by adding an option termination: "--" So for instance if you
+current command by adding an option termination: "--". So for instance, if you
 want to launch a process and give the "process launch" command the
 "--stop-at-entry" option, yet you want the process you are about to launch to
 be launched with the arguments "-program_arg value", you would type:
@@ -121,7 +121,7 @@
 
 lldb also supports command completion for source file names, symbol names, file
 names, etc. Completion is initiated by a hitting a TAB. Individual options in a
-command can have different completers, so for instance the "--file "
+command can have different completers, so for instance, the "--file "
 option in "breakpoint" completes to source files, the "--shlib " option
 to currently loaded shared libraries, etc. We can even do things like if you
 specify "--shlib ", and are completing on "--file ", we will only
@@ -134,7 +134,7 @@
 for each matching command.
 
 Finally, there is a mechanism to construct aliases for commonly used commands.
-So for instance if you get annoyed typing:
+For instance, if you get annoyed typing:
 
 ::
 
@@ -162,10 +162,10 @@
 One alias of note that we do include by popular demand is a weak emulator of
 gdb's "break" command. It doesn't try to do everything that gdb's break command
 does (for instance, it doesn't handle foo.c::bar. But it mostly works, and
-makes the transition easier. Also by popular demand, it is aliased to b. If you
+makes the transition easier. Also, by popular demand, it is aliased to b. If 
you
 actually want to learn the lldb command set natively, that means it will get in
 the way of the rest of the breakpoint commands. Fortunately, if you don't like
-one of our aliases, you an easily get rid of it by running (for example):
+one of our aliases, you can easily get rid of it by running (for example):
 
 ::
 
@@ -183,7 +183,7 @@
 options are stripped off, the rest of the command string is passed
 uninterpreted to the command. This is convenient for commands whose arguments
 might be some complex expression that would be painful to backslash protect.
-For instance the "expression" command is a "raw" command for obvious reasons.
+For instance, the "expression" command is a "raw" command for obvious reasons.
 The "help" output for a command will tell you if it is "raw" or not, so you
 know what to expect. The one thing you have to watch out for is that since raw
 commands still can have options, if your command string has dashes in it,
@@ -248,9 +248,9 @@
 
 The logical breakpoint has an integer id, and its locations have an id within
 their parent breakpoint (the two are joined by a ".", e.g. 1.1 in the example
-above.)
+above).
 
-Also the logical breakpoints remain live so that if another shared library were
+Also, the logical breakpoints remain live so that if another shared library 
were
 to be loaded that had another implementation of the "alignLeftEdges:" selector,
 the new location would be added to breakpoint 1 (e.g. a "1.2" breakpoint would
 be set on the newly loaded selector).
@@ -284,7 +284,7 @@
 
 You can delete, disable, set conditions and ignore counts either on all the
 locations generated by your logical breakpoint, or on any one of the particular
-locations your specification resolved to. For instance if we wanted to add a
+locations your specification resolved to. For instance, if we wanted to add a
 command to print a backtrace when we hit this breakpoint we could do:
 
 ::
@@ -299,7 +299,7 @@
 option. Use "--script" if you want to implement your breakpoint command using
 the Python script instead.
 
-This is an convenient point to bring up another feature of the lldb command
+This is a convenient point to bring up another feature of the lldb command
 help. Do:
 
 ::


Index: lldb/docs/use/tutorial.rst
===
--- lldb/docs/use/tutorial.rst
+++ lldb/docs/use/tutorial.rst
@@ -28,7 +28,7 @@
 
 Options can be placed anywhere on the command line, but if the arguments begin
 with a "-" then you have to tell lldb that you're done with options for the
-current command by adding an option termination: "--" So for instance if you
+current command by adding an option termination: "--". So for instance, if you
 want to launch a process and give the "process launch" command the
 "--stop-at-entry" option, yet you want the 

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

ship it


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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 336107.
mgorny added a comment.

Implemented @labath's requests.


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

https://reviews.llvm.org/D98822

Files:
  lldb/include/lldb/Host/linux/Host.h
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
  lldb/test/Shell/Subprocess/Inputs/fork.cpp
  lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
  lldb/test/Shell/Subprocess/clone-follow-parent.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,11 @@
+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,13 @@
+# REQUIRES: native && dbregs-set
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native && (system-linux || system-netbsd)
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native && (system-linux || system-netbsd) && dbregs-set
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: 

[Lldb-commits] [lldb] 2ecf928 - [lldb/DWARF] Fix a crash parsing invalid dwarf (pr49678)

2021-04-08 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-04-08T16:48:02+02:00
New Revision: 2ecf928153fc56dcb6bb0bd910584eac86bc23bd

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

LOG: [lldb/DWARF] Fix a crash parsing invalid dwarf (pr49678)

If the debug info is missing the terminating null die, we would crash
when trying to access the nonexisting children/siblings. This was
discovered because the test case for D98619 accidentaly produced such
input.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 86b18615da7df..ea10ba75afa84 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -236,6 +236,11 @@ void DWARFUnit::ExtractDIEsRWLocked() {
   }
 
   if (!m_die_array.empty()) {
+// The last die cannot have children (if it did, it wouldn't be the last 
one).
+// This only makes a 
diff erence for malformed dwarf that does not have a
+// terminating null die.
+m_die_array.back().SetHasChildren(false);
+
 if (m_first_die) {
   // Only needed for the assertion.
   m_first_die.SetHasChildren(m_die_array.front().HasChildren());

diff  --git a/lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp 
b/lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp
index 3a4b1cfd8ba70..f5cfd1e61120b 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFUnitTest.cpp
@@ -47,3 +47,40 @@ TEST(DWARFUnitTest, NullUnitDie) {
   ASSERT_NE(die_first, nullptr);
   EXPECT_TRUE(die_first->IsNULL());
 }
+
+TEST(DWARFUnitTest, MissingSentinel) {
+  // Make sure we don't crash if the debug info is missing a null DIE sentinel.
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+- Table:
+- Code:0x0001
+  Tag: DW_TAG_compile_unit
+  Children:DW_CHILDREN_yes
+  Attributes:
+- Attribute:   DW_AT_language
+  Form:DW_FORM_data2
+  debug_info:
+- Version: 4
+  AddrSize:8
+  Entries:
+- AbbrCode:0x0001
+  Values:
+- Value:   0x000C
+)";
+
+  YAMLModuleTester t(yamldata);
+  ASSERT_TRUE((bool)t.GetDwarfUnit());
+
+  DWARFUnit *unit = t.GetDwarfUnit();
+  const DWARFDebugInfoEntry *die_first = unit->DIE().GetDIE();
+  ASSERT_NE(die_first, nullptr);
+  EXPECT_EQ(die_first->GetFirstChild(), nullptr);
+  EXPECT_EQ(die_first->GetSibling(), nullptr);
+}



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


[Lldb-commits] [lldb] 1e511bb - [lldb] Re-skip TestVSCode_launch

2021-04-08 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2021-04-08T16:31:18+02:00
New Revision: 1e511bb1be718657a5584ccb792723d9ddac481f

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

LOG: [lldb] Re-skip TestVSCode_launch

The test is flaky (sleeps didn't help).

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index e69003c135b00..1d99ecf87c4e0 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -454,6 +454,7 @@ def test_terminate_commands(self):
 
 @skipIfWindows
 @skipIfRemote
+@skipIf(oslist=["linux"])
 def test_progress_events(self):
 '''
 Tests the progress events to ensure we are receiving them.



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


[Lldb-commits] [PATCH] D99497: [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.
Herald added a subscriber: JDevlieghere.

The test is still flaky (http://lab.llvm.org:8011/#/builders/68/builds/10124, 
http://lab.llvm.org:8011/#/builders/68/builds/10118, 
http://lab.llvm.org:8011/#/builders/68/builds/10113, ...) In fact, I think it 
has gotten flakier since the timeouts were added... I'm disabling it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99497

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


[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Setting the address size from the register context is a bit awkward. We have 
one register context per thread, and they all will be competing to set the 
value (it should always be the same value under normal circumstances, but it 
still makes for a strange relationship). It would be better if all of this 
could be contained in the ABI class. The ABI classes have intimate knowledge of 
the registers anyway, so it would not be unusual for it to access a specific 
register to get the mask (here I am assuming that the mask is actually stored 
in a register).

Then we'd have no need for Process::GetAddressBitsInUse and worry whether it 
has been initialized, as the abi class could fetch the value when it gets 
constructed, or something...

Do you envision using this in some other way than through the ABI class?




Comment at: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp:41-42
+  uint32_t address_shift_width = GetProcessSP()->GetAddressBitsInUse();
+  if (address_shift_width &&
+  (address_shift_width < (sizeof(lldb::addr_t) * CHAR_BIT))) {
+lldb::addr_t sign = (lldb::addr_t)1 << (address_shift_width - 1);

I think this should be an assert (here, or even somewhere earlier).



Comment at: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp:43-45
+lldb::addr_t sign = (lldb::addr_t)1 << (address_shift_width - 1);
+pc &= ((lldb::addr_t)1 << address_shift_width) - 1;
+pc = (pc ^ sign) - sign;

return llvm::SignExtend64(pc, address_shift_width)


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

https://reviews.llvm.org/D99944

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked 10 inline comments as done.
mgorny added a comment.

Ok, I think I've implemented all the requested changes. I'm going to test it on 
all three platforms now and attach the patch if it miraculously still works ;-).




Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:981-987
+  switch (event) {
+  case PL_FLAG_FORKED:
+  case PL_FLAG_FORKED | PL_FLAG_VFORKED:
+break;
+  default:
+assert(false && "unknown clone_info.event");
+  }

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > `assert(event_FLAG_FORKED)` would be shorter, but I am not sure if 
> > > > we need even that, as this is already checked in the caller. You could 
> > > > just drop the entire `event` argument, if it is unused..
> > > The distinction is necessary to know how to deal with software 
> > > breakpoints, and it will be used in the followup patch (D99864, when I 
> > > extend it to non-Linux targets). I think removing `event` at this point 
> > > to reintroduce it would be a wasted effort.
> > > 
> > > That said, I guess this is yet another case for `llvm_unreachable()`. 
> > > Also, I think it's worth keeping `assert()`s like this as they make sure 
> > > we don't break stuff accidentally when changing code on the caller's end.
> > True, but it's even better when the code is written such that the 
> > asserts/unreachables are unnecessary. One way to do that might be to do the 
> > decoding in the caller:
> > ```
> >   if (info.pl_flags & PL_FLAG_FORKED) {
> > MonitorClone(info.pl_child_pid, /*bool is_vfork*/ fork_flags & 
> > PL_FLAG_VFORKED);
> > return;
> >   }
> > ```
> > It's hard to say whether that would work without seeing the real 
> > implementation.
> In the long run, we would probably also want a separate event for 
> `posix_spawn` on NetBSD. I've figured out that reusing native constants 
> avoids reinventing the wheel unnecessarily.
Hmm, though I guess it doesn't matter for FreeBSD right now and since this is 
entirely platform-specific code, I'll simplify it for now.


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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:981-987
+  switch (event) {
+  case PL_FLAG_FORKED:
+  case PL_FLAG_FORKED | PL_FLAG_VFORKED:
+break;
+  default:
+assert(false && "unknown clone_info.event");
+  }

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > `assert(event_FLAG_FORKED)` would be shorter, but I am not sure if we 
> > > need even that, as this is already checked in the caller. You could just 
> > > drop the entire `event` argument, if it is unused..
> > The distinction is necessary to know how to deal with software breakpoints, 
> > and it will be used in the followup patch (D99864, when I extend it to 
> > non-Linux targets). I think removing `event` at this point to reintroduce 
> > it would be a wasted effort.
> > 
> > That said, I guess this is yet another case for `llvm_unreachable()`. Also, 
> > I think it's worth keeping `assert()`s like this as they make sure we don't 
> > break stuff accidentally when changing code on the caller's end.
> True, but it's even better when the code is written such that the 
> asserts/unreachables are unnecessary. One way to do that might be to do the 
> decoding in the caller:
> ```
>   if (info.pl_flags & PL_FLAG_FORKED) {
> MonitorClone(info.pl_child_pid, /*bool is_vfork*/ fork_flags & 
> PL_FLAG_VFORKED);
> return;
>   }
> ```
> It's hard to say whether that would work without seeing the real 
> implementation.
In the long run, we would probably also want a separate event for `posix_spawn` 
on NetBSD. I've figured out that reusing native constants avoids reinventing 
the wheel unnecessarily.


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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D99941: [LLDB] Support AArch64 PAC elf-core register read

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Seems reasonable to me. I'm happy if @DavidSpickett is.


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

https://reviews.llvm.org/D99941

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


[Lldb-commits] [PATCH] D99864: [lldb] Fork/vfork support via gdb-remote protocol [WIP]

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This is going to be pretty big, so I'd say we start splitting this up. The 
client & server bits can go into separate patches, and it would make things 
simpler if the qSupported emission and parsing also got its own patches, as 
that is going to be a lot less controversial.

I'm going to have more questions, but let me start with this: What should the 
(v)fork-events extension actually mean? In the client, I guess it demonstrates 
its willingness to receive the appropriate type of event. But what should it 
mean on the server? Is it like "I promise to stop on every (v)fork", or just 
something like "hey, I know this word"? The second meaning only makes sense if 
these (v)fork-events come with some new protocol extensions/packets that the 
client can use when communicating with the server. Such is the case with the 
multiprocess extension (new thread id syntax), but I am not aware of anything 
similar in for these features. Are there any? The gdb documentation (//This 
feature indicates whether GDB supports fork event extensions to the remote 
protocol. GDB does not use such extensions unless the stub also reports that it 
supports them by including ‘fork-events+’ in its ‘qSupported’ reply. //) seems 
to indicate they might exist, but I am not sure what would they be...

If there aren't any, then it would probably be better to use the first 
"definition" of the feature (or something similar), as the client can 
legitimately be interested in whether it will be able to intercept forks or 
not. And if that's the case, then we should first consult the relevant 
NativeProcess class before we reply (v)fork-events+.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:455
 
+if (::strstr(response_cstr, "fork-events+"))
+  m_supports_fork_events = eLazyBoolYes;

mgorny wrote:
> Kamil noticed that this will also catch `vfork-events+`. I suppose I'll 
> rewrite this function to iterate over split data as well.
Yeah, that should have been done a long time ago.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1314-1322
+"qXfer:features:read+",
+#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
+"QPassSignals+",
+"qXfer:auxv:read+",
+"qXfer:libraries-svr4:read+",
+#endif
+"multiprocess+",

Ideally, all of these would be handled in the GDBRemoteCommunicationServerLLGS 
class, since these features are meaningless for the platform server. Then you 
also wouldn't need to define the m_(v)fork_events_supported members in this 
class.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h:152
+  // to be returned in response.
+  virtual llvm::SmallVector
+  HandleFeatures(const llvm::ArrayRef client_features);

A SmallVector as a return type is fairly odd. One tends to use SmallVectorImpl 
by-ref argument in this case, but a plain std::vector result is also fine.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2342-2344
+StreamString ostr;
+ostr.Printf("%" PRIu64 " %" PRIu64, pid_tid->first, pid_tid->second);
+description = std::string(ostr.GetString());

description = llvm::formatv("{0} {1}", pid_tid->first, pid_tid->second).str()


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

https://reviews.llvm.org/D99864

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


[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb601c6719226: [lldb] [client] Support for multiprocess 
extension (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99603

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/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
@@ -0,0 +1,46 @@
+from __future__ import print_function
+import lldb
+import unittest
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestMultiprocess(GDBRemoteTestBase):
+def test_qfThreadInfo(self):
+class MyResponder(MockGDBServerResponder):
+def qfThreadInfo(self):
+return "mp400.10200,p400.10204,p401.10300,p400.10208"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+self.assertEqual(process.id, 0x400)
+self.assertEqual(
+[process.threads[i].id for i in range(process.num_threads)],
+[0x10200, 0x10204, 0x10208])
+
+def test_stop_reason(self):
+class MyResponder(MockGDBServerResponder):
+def qfThreadInfo(self):
+return "mp400.10200,p400.10204"
+
+def cont(self):
+return "S02thread:p400.10200;"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+process.Continue()
+self.assertEqual(process.GetThreadByID(0x10200).stop_reason,
+ lldb.eStopReasonSignal)
+self.assertEqual(process.GetThreadByID(0x10204).stop_reason,
+ lldb.eStopReasonNone)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -335,7 +335,7 @@
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string );
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string );
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote );
 
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
@@ -1487,22 +1487,22 @@
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string ) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-// thread in big endian hex
-tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-if (tid != LLDB_INVALID_THREAD_ID)
-  m_thread_ids.push_back(tid);
-value.erase(0, comma_pos + 1);
-  }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-m_thread_ids.push_back(tid);
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+auto pid_tid = thread_ids.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid) {
+  lldb::tid_t tid = pid_tid->second;
+  if (tid != LLDB_INVALID_THREAD_ID &&
+  tid != StringExtractorGDBRemote::AllProcesses)
+m_thread_ids.push_back(tid);
+}
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1519,7 +1519,7 @@
 value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != 

[Lldb-commits] [lldb] b601c67 - [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2021-04-08T13:45:07+02:00
New Revision: b601c6719226fb83c43dae62a581e5ee08bfb169

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

LOG: [lldb] [client] Support for multiprocess extension

Add a minimal support for the multiprocess extension in gdb-remote
client.  It accepts PIDs as part of thread-ids, and rejects PIDs that
do not match the current inferior.

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

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.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
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 22e05c04b3a0..c032fc20b5f3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -89,6 +89,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
   m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
   m_supports_QPassSignals(eLazyBoolCalculate),
   m_supports_error_string_reply(eLazyBoolCalculate),
+  m_supports_multiprocess(eLazyBoolCalculate),
   m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
   m_supports_qUserName(true), m_supports_qGroupName(true),
   m_supports_qThreadStopInfo(true), m_supports_z0(true),
@@ -292,6 +293,7 @@ void 
GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
 m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
 m_attach_or_wait_reply = eLazyBoolCalculate;
 m_avoid_g_packets = eLazyBoolCalculate;
+m_supports_multiprocess = eLazyBoolCalculate;
 m_supports_qXfer_auxv_read = eLazyBoolCalculate;
 m_supports_qXfer_libraries_read = eLazyBoolCalculate;
 m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
@@ -342,11 +344,13 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
+  m_supports_multiprocess = eLazyBoolNo;
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
 
   // build the qSupported packet
-  std::vector features = {"xmlRegisters=i386,arm,mips,arc"};
+  std::vector features = {"xmlRegisters=i386,arm,mips,arc",
+   "multiprocess+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {
@@ -433,6 +437,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
 else
   m_supports_QPassSignals = eLazyBoolNo;
 
+if (::strstr(response_cstr, "multiprocess+"))
+  m_supports_multiprocess = eLazyBoolYes;
+else
+  m_supports_multiprocess = eLazyBoolNo;
+
 const char *packet_size_str = ::strstr(response_cstr, "PacketSize=");
 if (packet_size_str) {
   StringExtractorGDBRemote packet_response(packet_size_str +
@@ -741,12 +750,14 @@ lldb::pid_t 
GDBRemoteCommunicationClient::GetCurrentProcessID(bool allow_lazy) {
 // If we don't get a response for $qC, check if $qfThreadID gives us a
 // result.
 if (m_curr_pid == LLDB_INVALID_PROCESS_ID) {
-  std::vector thread_ids;
   bool sequence_mutex_unavailable;
-  size_t size;
-  size = GetCurrentThreadIDs(thread_ids, sequence_mutex_unavailable);
-  if (size && !sequence_mutex_unavailable) {
-m_curr_pid = thread_ids.front();
+  auto ids = GetCurrentProcessAndThreadIDs(sequence_mutex_unavailable);
+  if (!ids.empty() && !sequence_mutex_unavailable) {
+// If server returned an explicit PID, use that.
+m_curr_pid = ids.front().first;
+// Otherwise, use the TID of the first thread (Linux hack).
+if (m_curr_pid == LLDB_INVALID_PROCESS_ID)
+  m_curr_pid = ids.front().second;
 m_curr_pid_is_valid = eLazyBoolYes;
 return m_curr_pid;
   }
@@ -1125,8 +1136,23 @@ bool 
GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t ) {
   if (!response.IsNormalResponse())
 return false;
 
-  if (response.GetChar() == 'Q' && response.GetChar() == 'C')
-tid = response.GetHexMaxU64(true, -1);
+  if (response.GetChar() == 'Q' && response.GetChar() == 'C') {
+auto pid_tid = response.GetPidTid(0);
+if (!pid_tid)
+  return false;
+

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:723
   // Process all pending waitpid notifications.
-  int status;
-  ::pid_t wait_pid =
-  llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), , WNOHANG);
+  while (true) {
+int status;

mgorny wrote:
> labath wrote:
> > Is the loop still necessary?
> I am not sure but I think there's no harm in keeping it, and it might save us 
> from some hard-to-debug corner conditions in the future.
I don't  mind that much, though I would be surprised if this can help with 
anything. IIUC, the BSD kernel always sends one notification when the process 
stops, and I believe this code is written in a way which expects that (only 
linux has special code to delay some actions until every thread registers as 
stopped). If, for whatever reason, we end up getting two notifications here, I 
expect things would blow up spectacularly...



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:981-987
+  switch (event) {
+  case PL_FLAG_FORKED:
+  case PL_FLAG_FORKED | PL_FLAG_VFORKED:
+break;
+  default:
+assert(false && "unknown clone_info.event");
+  }

mgorny wrote:
> labath wrote:
> > `assert(event_FLAG_FORKED)` would be shorter, but I am not sure if we 
> > need even that, as this is already checked in the caller. You could just 
> > drop the entire `event` argument, if it is unused..
> The distinction is necessary to know how to deal with software breakpoints, 
> and it will be used in the followup patch (D99864, when I extend it to 
> non-Linux targets). I think removing `event` at this point to reintroduce it 
> would be a wasted effort.
> 
> That said, I guess this is yet another case for `llvm_unreachable()`. Also, I 
> think it's worth keeping `assert()`s like this as they make sure we don't 
> break stuff accidentally when changing code on the caller's end.
True, but it's even better when the code is written such that the 
asserts/unreachables are unnecessary. One way to do that might be to do the 
decoding in the caller:
```
  if (info.pl_flags & PL_FLAG_FORKED) {
MonitorClone(info.pl_child_pid, /*bool is_vfork*/ fork_flags & 
PL_FLAG_VFORKED);
return;
  }
```
It's hard to say whether that would work without seeing the real implementation.


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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny marked 3 inline comments as done.
mgorny added a comment.

Answered where answer was due, will update the rest once I finish retesting the 
multiprocess patch.




Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:723
   // Process all pending waitpid notifications.
-  int status;
-  ::pid_t wait_pid =
-  llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), , WNOHANG);
+  while (true) {
+int status;

labath wrote:
> Is the loop still necessary?
I am not sure but I think there's no harm in keeping it, and it might save us 
from some hard-to-debug corner conditions in the future.



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:981-987
+  switch (event) {
+  case PL_FLAG_FORKED:
+  case PL_FLAG_FORKED | PL_FLAG_VFORKED:
+break;
+  default:
+assert(false && "unknown clone_info.event");
+  }

labath wrote:
> `assert(event_FLAG_FORKED)` would be shorter, but I am not sure if we need 
> even that, as this is already checked in the caller. You could just drop the 
> entire `event` argument, if it is unused..
The distinction is necessary to know how to deal with software breakpoints, and 
it will be used in the followup patch (D99864, when I extend it to non-Linux 
targets). I think removing `event` at this point to reintroduce it would be a 
wasted effort.

That said, I guess this is yet another case for `llvm_unreachable()`. Also, I 
think it's worth keeping `assert()`s like this as they make sure we don't break 
stuff accidentally when changing code on the caller's end.


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

https://reviews.llvm.org/D98822

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


[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yea, I think it should be fine.


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

https://reviews.llvm.org/D99603

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


[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D98822#2666257 , @mgorny wrote:

> @labath, I think I've made all the requested changes.

This looks pretty good now. Just some small nits inline...




Comment at: lldb/include/lldb/Host/linux/Host.h:13
+#include "llvm/ADT/Optional.h"
+
+#include "lldb/lldb-types.h"

mgorny wrote:
> labath wrote:
> > These spaces are pretty common in lldb, but that's not actually consistent 
> > with how the rest of llvm formats them...
> Will remove. However, note that this causes `clang-format` to reorder the 
> includes.
And it should do that -- that's part of the point :)



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:723
   // Process all pending waitpid notifications.
-  int status;
-  ::pid_t wait_pid =
-  llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), , WNOHANG);
+  while (true) {
+int status;

Is the loop still necessary?



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp:981-987
+  switch (event) {
+  case PL_FLAG_FORKED:
+  case PL_FLAG_FORKED | PL_FLAG_VFORKED:
+break;
+  default:
+assert(false && "unknown clone_info.event");
+  }

`assert(event_FLAG_FORKED)` would be shorter, but I am not sure if we need 
even that, as this is already checked in the caller. You could just drop the 
entire `event` argument, if it is unused..



Comment at: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h:124
+
+  void MonitorClone(::pid_t child_pid, int event);
 };

move this next to the other `Monitor` functions ?



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:670
+  data = 0;
+
+MonitorFork(data, true, PTRACE_EVENT_FORK);

mgorny wrote:
> labath wrote:
> > I guess we should also do something like the `WaitForNewThread` call in the 
> > clone case.
> Yeah, I was wondering about it. On the other hand, won't the main loop handle 
> it anyway?
> 
> Does doing it explicitly have any real gain? One thing I was worried about is 
> that the case of parent signal first is much more common than child signal 
> first, so having a different logic on both branches would make it more likely 
> for bugs in the child-first branch not to be noticed.
Maybe obsolete by now, but I just realized what you actually meant here.

There will always be some divergence between the two sequence of events. 
Letting this be handled from the main loop may make it smaller though. I don't 
really mind doing it that way, as long as the (v)fork and clone cases do it the 
same way.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:760
   // Process all pending waitpid notifications.
-  int status;
-  ::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), ,
- WALLSIG | WNOHANG);
-
-  if (wait_pid == 0)
-return; // We are done.
+  while (true) {
+int status;

same here



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:784
+int status;
+::pid_t wait_pid = llvm::sys::RetryAfterSignal(-1, waitpid, -1, ,
+   WALLSIG | WNOHANG);

mgorny wrote:
> labath wrote:
> > For NetBSD, it might be cleaner to keep waiting for the parent only, and 
> > then do a separate waitpid for the child once you get its process id. That 
> > would make the sequence of events (and the relevant code) predictable.
> > 
> > I think this is how this interface was meant to be used, even on linux, but 
> > linux makes it tricky because it's not possible to wait for all threads 
> > belonging to a given process in a single call -- one would have to iterate 
> > through the threads and wait for each one separately (it might be 
> > interesting to try to implement and benchmark that).
> I've been thinking about it and I suppose it makes sense. I was considering 
> what would be better to future-proof it for the future full multiprocess 
> support but I suppose we could just loop over all process classes and let 
> every one wait for its own process rather than creating a dedicated 
> dispatcher that calls `waitpid(-1...)`.
I think that the looping would be the cleanest solution for the BSDs. For 
linux, we will need to call waitpid centrally, and then somehow choose which 
instance to notify.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h:117
+
+  void MonitorClone(::pid_t child_pid, int event);
 };

same here



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py:3
 import lldb
+<<< HEAD
 import unittest

bad merge



Comment at: lldb/test/Shell/Subprocess/Inputs/fork.cpp:30-42
+#if 

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

@labath, any further comments or is this good to go then?


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

https://reviews.llvm.org/D99603

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


[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 336052.
mgorny added a comment.

Removed the hanging test.


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

https://reviews.llvm.org/D99603

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/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestMultiprocess.py
@@ -0,0 +1,46 @@
+from __future__ import print_function
+import lldb
+import unittest
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestMultiprocess(GDBRemoteTestBase):
+def test_qfThreadInfo(self):
+class MyResponder(MockGDBServerResponder):
+def qfThreadInfo(self):
+return "mp400.10200,p400.10204,p401.10300,p400.10208"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+self.assertEqual(process.id, 0x400)
+self.assertEqual(
+[process.threads[i].id for i in range(process.num_threads)],
+[0x10200, 0x10204, 0x10208])
+
+def test_stop_reason(self):
+class MyResponder(MockGDBServerResponder):
+def qfThreadInfo(self):
+return "mp400.10200,p400.10204"
+
+def cont(self):
+return "S02thread:p400.10200;"
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+process.Continue()
+self.assertEqual(process.GetThreadByID(0x10200).stop_reason,
+ lldb.eStopReasonSignal)
+self.assertEqual(process.GetThreadByID(0x10204).stop_reason,
+ lldb.eStopReasonNone)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -335,7 +335,7 @@
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string );
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string );
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote );
 
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
@@ -1487,22 +1487,22 @@
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string ) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-// thread in big endian hex
-tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-if (tid != LLDB_INVALID_THREAD_ID)
-  m_thread_ids.push_back(tid);
-value.erase(0, comma_pos + 1);
-  }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-m_thread_ids.push_back(tid);
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+auto pid_tid = thread_ids.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid) {
+  lldb::tid_t tid = pid_tid->second;
+  if (tid != LLDB_INVALID_THREAD_ID &&
+  tid != StringExtractorGDBRemote::AllProcesses)
+m_thread_ids.push_back(tid);
+}
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1519,7 +1519,7 @@
 value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_THREAD_ID)
+  if (pc != LLDB_INVALID_ADDRESS)
 m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
@@ -2141,6 +2141,7 @@
 }
 
 StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor _packet) {
+  

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Yeah, that's tricky. For a inconsistent/unsupported response like this, 
probably the only reasonable thing we can do print some error message, 
terminate the connection and put the inferior into some terminal state. 
However, I don't think we have anything like that (and I suspect there are 
plenty of other ways one could trigger a loop like this). I'd say we should 
just delete the test right now.


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

https://reviews.llvm.org/D99603

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


[Lldb-commits] [PATCH] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

2021-04-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

If gdb does it, then I don't have any issues with this functionality. It could 
use a test case though. You can try rewriting that gdb test case for lldb -- we 
don't have fancy dwarf assemblers (cool stuff, btw), we just use asm (you could 
look at test/Shell/SymbolFile/DWARF/dwo-type-in-main-file.s for inspiration). 
However, generating the inputs via clang should be fine as well..


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

https://reviews.llvm.org/D97786

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


[Lldb-commits] [lldb] d9773c1 - Reorg firmware corefile tests; add test for OS plugin loading

2021-04-08 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2021-04-08T01:45:25-07:00
New Revision: d9773c1b4eb14c5f38d23323270ef67b4c528844

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

LOG: Reorg firmware corefile tests; add test for OS plugin loading

A little cleanup to how these firmware corefile tests are done; add
a test that loads a dSYM that loads an OS plugin, and confirm that
the OS plugin's threads are created.

Added: 
lldb/test/API/macosx/lc-note/firmware-corefile/operating_system.py

Modified: 
lldb/test/API/macosx/lc-note/firmware-corefile/Makefile
lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
lldb/test/API/macosx/lc-note/firmware-corefile/main.c

Removed: 
lldb/test/API/macosx/lc-note/firmware-corefile/bout.mk



diff  --git a/lldb/test/API/macosx/lc-note/firmware-corefile/Makefile 
b/lldb/test/API/macosx/lc-note/firmware-corefile/Makefile
index faa5ef2f29f43..05020112520e8 100644
--- a/lldb/test/API/macosx/lc-note/firmware-corefile/Makefile
+++ b/lldb/test/API/macosx/lc-note/firmware-corefile/Makefile
@@ -1,14 +1,11 @@
 MAKE_DSYM := NO
 C_SOURCES := main.c
-CFLAGS_EXTRAS := -Wl,-random_uuid
+#CFLAGS_EXTRAS := 
 
-all: a.out b.out create-empty-corefile
+all: a.out create-empty-corefile
 
 create-empty-corefile:
-   $(MAKE) -f $(MAKEFILE_RULES) EXE=create-empty-corefile \
+   "$(MAKE)" -f "$(MAKEFILE_RULES)" EXE=create-empty-corefile \
C_SOURCES=create-empty-corefile.c
 
-b.out:
-   $(MAKE)  VPATH=$(SRCDIR)/$* -I $(SRCDIR) -f $(SRCDIR)/bout.mk
-
 include Makefile.rules

diff  --git 
a/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py 
b/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
index 068848be06eaf..a28b01bb1789e 100644
--- a/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
+++ b/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
@@ -16,25 +16,37 @@ class TestFirmwareCorefiles(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@skipIf(debug_info=no_match(["dsym"]), bugnumber="This test is looking 
explicitly for a dSYM")
-@skipIf(archs=no_match(['x86_64']))
-@skipUnlessDarwin
-def test_lc_note(self):
+def initial_setup(self):
 self.build()
 self.aout_exe = self.getBuildArtifact("a.out")
-self.bout_exe = self.getBuildArtifact("b.out")
+self.aout_exe_basename = "a.out"
 self.create_corefile = self.getBuildArtifact("create-empty-corefile")
 self.dsym_for_uuid = self.getBuildArtifact("dsym-for-uuid.sh")
-self.aout_corefile = self.getBuildArtifact("aout.core")
-self.aout_corefile_addr = self.getBuildArtifact("aout.core_addr")
-self.bout_corefile = self.getBuildArtifact("bout.core")
-self.bout_corefile_addr = self.getBuildArtifact("bout.core_addr")
+self.verstr_corefile = self.getBuildArtifact("verstr.core")
+self.verstr_corefile_addr = self.getBuildArtifact("verstr-addr.core")
+self.binspec_corefile = self.getBuildArtifact("binspec.core")
+self.binspec_corefile_addr = self.getBuildArtifact("binspec-addr.core")
 
 ## We can hook in our dsym-for-uuid shell script to lldb with this env
 ## var instead of requiring a defaults write.
 os.environ['LLDB_APPLE_DSYMFORUUID_EXECUTABLE'] = self.dsym_for_uuid
 self.addTearDownHook(lambda: 
os.environ.pop('LLDB_APPLE_DSYMFORUUID_EXECUTABLE', None))
 
+self.runCmd("settings set target.load-script-from-symbol-file true")
+self.addTearDownHook(lambda: self.runCmd("settings set 
target.load-script-from-symbol-file false"))
+
+dsym_python_dir = '%s.dSYM/Contents/Resources/Python' % (self.aout_exe)
+os.makedirs(dsym_python_dir)
+python_os_plugin_path = os.path.join(self.getSourceDir(),
+ 'operating_system.py')
+python_init = [
+'def __lldb_init_module(debugger, internal_dict):',
+'  debugger.HandleCommand(\'settings set 
target.process.python-os-plugin-path %s\')' % python_os_plugin_path,
+]
+with open(dsym_python_dir + "/a_out.py", "w") as writer:
+for l in python_init:
+writer.write(l + '\n')
+
 dwarfdump_uuid_regex = re.compile(
 'UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*')
 dwarfdump_cmd_output = subprocess.check_output(
@@ -46,17 +58,7 @@ def test_lc_note(self):
 aout_uuid = match.group(1)
 self.assertNotEqual(aout_uuid, None, "Could not get uuid of built 
a.out")
 
-dwarfdump_cmd_output = subprocess.check_output(
-('/usr/bin/dwarfdump --uuid "%s"' % self.bout_exe),