[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Neal via Phabricator via lldb-commits]

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

Thanks for the review, and apologies for the delay in updating the diffs - 
seems like I haven't even had an hour to sit down at my dev machine for the 
past couple weeks, much less actually do any coding :) I don't have commit 
access, so if you could do that, that would be much appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

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

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Neal via Phabricator via lldb-commits]

nealsid marked an inline comment as done.
nealsid added inline comments.



Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154
+TEST(FormatEntity, LookupAllEntriesInTree) {
+  for (const auto  : lookupStrings) {
+Entry e;

teemperor wrote:
> teemperor wrote:
> > You could just use `llvm::StringRef` here (it's shorter and more 
> > descriptive than the `auto`).
> I actually meant `llvm::StringRef` as the whole type (StringRef references 
> are not really useful as StringRef is already a lightweight String reference)
Done - kept it const since StringRef appears to depend on that for some of its 
methods.  I was looking through StringRef (and the use of optionals for 
something else) and as far as I understand, which could be entirely wrong, it's 
meant to provide a single interface over unowned character data whether it 
comes from a std::string or char*, and thought using string_view would be good, 
as well - is moving to C++17 on the roadmap? I know it's one of those things 
that everyone is in favor of and wants to do but nobody has time for :)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

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

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Neal via Phabricator via lldb-commits]

nealsid updated this revision to Diff 337334.
nealsid marked an inline comment as done.
nealsid added a comment.

Update type in for loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

Files:
  lldb/include/lldb/Core/FormatEntity.h
  lldb/source/Core/FormatEntity.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FormatEntityTest.cpp

Index: lldb/unittests/Core/FormatEntityTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FormatEntityTest.cpp
@@ -0,0 +1,159 @@
+//===-- FormatEntityTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Utility/Status.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+using Definition = FormatEntity::Entry::Definition;
+using Entry = FormatEntity::Entry;
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
+  Definition d("foo", "string");
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, "string");
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::EscapeCode);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 33UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+  Definition parent("parent", FormatEntity::Entry::Type::Invalid, 1, );
+  EXPECT_EQ(parent.name, "parent");
+  EXPECT_EQ(parent.string, nullptr);
+  EXPECT_EQ(parent.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.num_children, 1UL);
+  EXPECT_EQ(parent.children, );
+  EXPECT_FALSE(parent.keep_separator);
+
+  EXPECT_EQ(parent.children[0].name, "foo");
+  EXPECT_EQ(parent.children[0].string, nullptr);
+  EXPECT_EQ(parent.children[0].type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.children[0].data, 33UL);
+  EXPECT_EQ(parent.children[0].num_children, 0UL);
+  EXPECT_EQ(parent.children[0].children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+constexpr llvm::StringRef lookupStrings[] = {
+"${addr.load}",
+"${addr.file}",
+"${ansi.fg.black}",
+"${ansi.fg.red}",
+"${ansi.fg.green}",
+"${ansi.fg.yellow}",
+"${ansi.fg.blue}",
+"${ansi.fg.purple}",
+"${ansi.fg.cyan}",
+"${ansi.fg.white}",
+"${ansi.bg.black}",
+"${ansi.bg.red}",
+"${ansi.bg.green}",
+"${ansi.bg.yellow}",
+"${ansi.bg.blue}",
+"${ansi.bg.purple}",
+"${ansi.bg.cyan}",
+"${ansi.bg.white}",
+"${file.basename}",
+"${file.dirname}",
+"${file.fullpath}",
+"${frame.index}",
+"${frame.pc}",
+"${frame.fp}",
+"${frame.sp}",
+"${frame.flags}",
+"${frame.no-debug}",
+"${frame.reg.*}",
+"${frame.is-artificial}",
+"${function.id}",
+"${function.name}",
+"${function.name-without-args}",
+"${function.name-with-args}",
+"${function.mangled-name}",
+"${function.addr-offset}",
+"${function.concrete-only-addr-offset-no-padding}",
+"${function.line-offset}",
+"${function.pc-offset}",
+"${function.initial-function}",
+"${function.changed}",
+"${function.is-optimized}",
+"${line.file.basename}",
+"${line.file.dirname}",
+"${line.file.fullpath}",
+"${line.number}",
+"${line.column}",
+"${line.start-addr}",
+"${line.end-addr}",
+"${module.file.basename}",
+"${module.file.dirname}",
+"${module.file.fullpath}",
+"${process.id}",
+"${process.name}",
+"${process.file.basename}",
+"${process.file.dirname}",
+"${process.file.fullpath}",
+"${script.frame}",
+"${script.process}",
+"${script.target}",
+"${script.thread}",
+"${script.var}",
+"${script.svar}",
+"${script.thread}",
+

[Lldb-commits] [PATCH] D100443: [lldb-vscode] Reduce chattiness of progress events

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/tools/lldb-vscode/ProgressEvent.cpp:38-78
+const char *ProgressEvent::GetEventName() const {
+  if (m_event_type == progressStart)
+return "progressStart";
+  else if (m_event_type == progressEnd)
+return "progressEnd";
+  else if (m_event_type == progressUpdate)
+return "progressUpdate";

all of this is just moved from the existing code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100443

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

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Neal via Phabricator via lldb-commits]

nealsid updated this revision to Diff 337331.
nealsid added a comment.

Fixed comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

Files:
  lldb/include/lldb/Core/FormatEntity.h
  lldb/source/Core/FormatEntity.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FormatEntityTest.cpp

Index: lldb/unittests/Core/FormatEntityTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FormatEntityTest.cpp
@@ -0,0 +1,159 @@
+//===-- FormatEntityTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Utility/Status.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+using Definition = FormatEntity::Entry::Definition;
+using Entry = FormatEntity::Entry;
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
+  Definition d("foo", "string");
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, "string");
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::EscapeCode);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 33UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+  Definition parent("parent", FormatEntity::Entry::Type::Invalid, 1, );
+  EXPECT_EQ(parent.name, "parent");
+  EXPECT_EQ(parent.string, nullptr);
+  EXPECT_EQ(parent.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.num_children, 1UL);
+  EXPECT_EQ(parent.children, );
+  EXPECT_FALSE(parent.keep_separator);
+
+  EXPECT_EQ(parent.children[0].name, "foo");
+  EXPECT_EQ(parent.children[0].string, nullptr);
+  EXPECT_EQ(parent.children[0].type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.children[0].data, 33UL);
+  EXPECT_EQ(parent.children[0].num_children, 0UL);
+  EXPECT_EQ(parent.children[0].children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+constexpr llvm::StringRef lookupStrings[] = {
+"${addr.load}",
+"${addr.file}",
+"${ansi.fg.black}",
+"${ansi.fg.red}",
+"${ansi.fg.green}",
+"${ansi.fg.yellow}",
+"${ansi.fg.blue}",
+"${ansi.fg.purple}",
+"${ansi.fg.cyan}",
+"${ansi.fg.white}",
+"${ansi.bg.black}",
+"${ansi.bg.red}",
+"${ansi.bg.green}",
+"${ansi.bg.yellow}",
+"${ansi.bg.blue}",
+"${ansi.bg.purple}",
+"${ansi.bg.cyan}",
+"${ansi.bg.white}",
+"${file.basename}",
+"${file.dirname}",
+"${file.fullpath}",
+"${frame.index}",
+"${frame.pc}",
+"${frame.fp}",
+"${frame.sp}",
+"${frame.flags}",
+"${frame.no-debug}",
+"${frame.reg.*}",
+"${frame.is-artificial}",
+"${function.id}",
+"${function.name}",
+"${function.name-without-args}",
+"${function.name-with-args}",
+"${function.mangled-name}",
+"${function.addr-offset}",
+"${function.concrete-only-addr-offset-no-padding}",
+"${function.line-offset}",
+"${function.pc-offset}",
+"${function.initial-function}",
+"${function.changed}",
+"${function.is-optimized}",
+"${line.file.basename}",
+"${line.file.dirname}",
+"${line.file.fullpath}",
+"${line.number}",
+"${line.column}",
+"${line.start-addr}",
+"${line.end-addr}",
+"${module.file.basename}",
+"${module.file.dirname}",
+"${module.file.fullpath}",
+"${process.id}",
+"${process.name}",
+"${process.file.basename}",
+"${process.file.dirname}",
+"${process.file.fullpath}",
+"${script.frame}",
+"${script.process}",
+"${script.target}",
+"${script.thread}",
+"${script.var}",
+"${script.svar}",
+"${script.thread}",
+"${svar.dummy-svar-to-test-wildcard}",
+"${thread.id}",
+

[Lldb-commits] [PATCH] D100443: [lldb-vscode] Reduce chattiness of progress events

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: clayborg, aadsm.
Herald added a subscriber: mgorny.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Progress events internally have a completed count and a total count, which can 
mean that for a job with 2 total counts, then there will be 2 events 
fired. Sending all these events to the IDE can break it. For example, debugging 
a huge binary resulted in around 50 million messages, which rendered the IDE 
useless, as it was spending all of its resources simply parsing messages and 
updating the UI.

A way to fix this is to send unique percentage updates, which are at most 100 
per job, which is not much. I was able to debug that big target and confirm 
that only unique percentage notifications are sent. I can't write a test for 
this because the current test is flaky. I'll figure out later how to make the 
test reliable, but fixing this will unblock us from deploy a new version of 
lldb-vscode.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100443

Files:
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/ProgressEvent.cpp
  lldb/tools/lldb-vscode/ProgressEvent.h
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -374,7 +374,8 @@
 const char *message = lldb::SBDebugger::GetProgressFromEvent(
 event, progress_id, completed, total, is_debugger_specific);
 if (message)
-  g_vsc.SendProgressEvent(progress_id, message, completed, total);
+  g_vsc.SendProgressEvent(
+  ProgressEvent(progress_id, message, completed, total));
   }
 }
   }
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -49,6 +49,7 @@
 #include "ExceptionBreakpoint.h"
 #include "FunctionBreakpoint.h"
 #include "IOStream.h"
+#include "ProgressEvent.h"
 #include "RunInTerminal.h"
 #include "SourceBreakpoint.h"
 #include "SourceReference.h"
@@ -113,6 +114,7 @@
   uint32_t reverse_request_seq;
   std::map request_handlers;
   bool waiting_for_run_in_terminal;
+  ProgressEventFilterQueue progress_event_queue;
   // Keep track of the last stop thread index IDs as threads won't go away
   // unless we send a "thread" event to indicate the thread exited.
   llvm::DenseSet thread_ids;
@@ -136,8 +138,7 @@
 
   void SendOutput(OutputType o, const llvm::StringRef output);
 
-  void SendProgressEvent(uint64_t progress_id, const char *message,
- uint64_t completed, uint64_t total);
+  void SendProgressEvent(const ProgressEvent );
 
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -40,8 +40,10 @@
{"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
{"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
   focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
-  stop_at_entry(false), is_attach(false),
-  reverse_request_seq(0), waiting_for_run_in_terminal(false) {
+  stop_at_entry(false), is_attach(false), reverse_request_seq(0),
+  waiting_for_run_in_terminal(false),
+  progress_event_queue(
+  [&](const ProgressEvent ) { SendJSON(event.ToJSON()); }) {
   const char *log_file_path = getenv("LLDBVSCODE_LOG");
 #if defined(_WIN32)
   // Windows opens stdout and stdin in text mode which converts \n to 13,10
@@ -320,51 +322,8 @@
 //   };
 // }
 
-void VSCode::SendProgressEvent(uint64_t progress_id, const char *message,
-   uint64_t completed, uint64_t total) {
-  enum ProgressEventType {
-progressInvalid,
-progressStart,
-progressUpdate,
-progressEnd
-  };
-  const char *event_name = nullptr;
-  ProgressEventType event_type = progressInvalid;
-  if (completed == 0) {
-event_type = progressStart;
-event_name = "progressStart";
-  } else if (completed == total) {
-event_type = progressEnd;
-event_name = "progressEnd";
-  } else if (completed < total) {
-event_type = progressUpdate;
-event_name = "progressUpdate";
-  }
-  if (event_type == progressInvalid)
-return;
-
-  llvm::json::Object event(CreateEventObject(event_name));
-  llvm::json::Object body;
-  std::string progress_id_str;
-  llvm::raw_string_ostream progress_id_strm(progress_id_str);
-  progress_id_strm << progress_id;
-  progress_id_strm.flush();
-  

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Neal via Phabricator via lldb-commits]

nealsid updated this revision to Diff 337329.
nealsid added a comment.

CR feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

Files:
  lldb/include/lldb/Core/FormatEntity.h
  lldb/source/Core/FormatEntity.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FormatEntityTest.cpp

Index: lldb/unittests/Core/FormatEntityTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FormatEntityTest.cpp
@@ -0,0 +1,159 @@
+//===-- FormatEntityTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Utility/Status.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+using Definition = FormatEntity::Entry::Definition;
+using Entry = FormatEntity::Entry;
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
+  Definition d("foo", "string");
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, "string");
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::EscapeCode);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 33UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+  Definition parent("parent", FormatEntity::Entry::Type::Invalid, 1, );
+  EXPECT_EQ(parent.name, "parent");
+  EXPECT_EQ(parent.string, nullptr);
+  EXPECT_EQ(parent.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.num_children, 1UL);
+  EXPECT_EQ(parent.children, );
+  EXPECT_FALSE(parent.keep_separator);
+
+  EXPECT_EQ(parent.children[0].name, "foo");
+  EXPECT_EQ(parent.children[0].string, nullptr);
+  EXPECT_EQ(parent.children[0].type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.children[0].data, 33UL);
+  EXPECT_EQ(parent.children[0].num_children, 0UL);
+  EXPECT_EQ(parent.children[0].children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+constexpr llvm::StringRef lookupStrings[] = {
+"${addr.load}",
+"${addr.file}",
+"${ansi.fg.black}",
+"${ansi.fg.red}",
+"${ansi.fg.green}",
+"${ansi.fg.yellow}",
+"${ansi.fg.blue}",
+"${ansi.fg.purple}",
+"${ansi.fg.cyan}",
+"${ansi.fg.white}",
+"${ansi.bg.black}",
+"${ansi.bg.red}",
+"${ansi.bg.green}",
+"${ansi.bg.yellow}",
+"${ansi.bg.blue}",
+"${ansi.bg.purple}",
+"${ansi.bg.cyan}",
+"${ansi.bg.white}",
+"${file.basename}",
+"${file.dirname}",
+"${file.fullpath}",
+"${frame.index}",
+"${frame.pc}",
+"${frame.fp}",
+"${frame.sp}",
+"${frame.flags}",
+"${frame.no-debug}",
+"${frame.reg.*}",
+"${frame.is-artificial}",
+"${function.id}",
+"${function.name}",
+"${function.name-without-args}",
+"${function.name-with-args}",
+"${function.mangled-name}",
+"${function.addr-offset}",
+"${function.concrete-only-addr-offset-no-padding}",
+"${function.line-offset}",
+"${function.pc-offset}",
+"${function.initial-function}",
+"${function.changed}",
+"${function.is-optimized}",
+"${line.file.basename}",
+"${line.file.dirname}",
+"${line.file.fullpath}",
+"${line.number}",
+"${line.column}",
+"${line.start-addr}",
+"${line.end-addr}",
+"${module.file.basename}",
+"${module.file.dirname}",
+"${module.file.fullpath}",
+"${process.id}",
+"${process.name}",
+"${process.file.basename}",
+"${process.file.dirname}",
+"${process.file.fullpath}",
+"${script.frame}",
+"${script.process}",
+"${script.target}",
+"${script.thread}",
+"${script.var}",
+"${script.svar}",
+"${script.thread}",
+"${svar.dummy-svar-to-test-wildcard}",
+"${thread.id}",
+"${thread.protocol_id}",
+ 

[Lldb-commits] [lldb] accb095 - [lldb] Disable TestClangModulesUpdate.py because it's flaky

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-13T20:37:17-07:00
New Revision: accb0955129d94636280cbd713c905b2189e30d4

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

LOG: [lldb] Disable TestClangModulesUpdate.py because it's flaky

The test is failing intermittently on GreenDragon.

rdar://76540904

Added: 


Modified: 
lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py

Removed: 




diff  --git a/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py 
b/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
index a24acc249de9e..593b696706783 100644
--- a/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
+++ b/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
@@ -14,6 +14,7 @@ class TestClangModuleUpdate(TestBase):
 
 @skipIf(debug_info=no_match(["gmodules"]))
 @skipIfReproducer # VFS is a snapshot.
+@skipIfDarwin # rdar://76540904
 def test_expr(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
 f.write("""



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

[Lldb-commits] [lldb] e825eff - [lldb] Build debugserver 2-way fat on AS

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-13T19:55:13-07:00
New Revision: e825effe9ba98c81c9c1799f58e3a7a43e887b38

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

LOG: [lldb] Build debugserver 2-way fat on AS

When compiling for arm, build debugserver 2 way fat with an arm64 and
arm64e slice. You can only debug arm64e processes using an arm64e
debugserver.

Added: 


Modified: 
lldb/tools/debugserver/source/CMakeLists.txt

Removed: 




diff  --git a/lldb/tools/debugserver/source/CMakeLists.txt 
b/lldb/tools/debugserver/source/CMakeLists.txt
index 2e78896cf3c0..b2bf01d7774d 100644
--- a/lldb/tools/debugserver/source/CMakeLists.txt
+++ b/lldb/tools/debugserver/source/CMakeLists.txt
@@ -83,6 +83,30 @@ if (CXX_SUPPORTS_NO_EXTENDED_OFFSETOF)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-extended-offsetof")
 endif ()
 
+# When compiling for arm, build debugserver 2 way fat with an arm64 and arm64e
+# slice. You can only debug arm64e processes using an arm64e debugserver.
+include(CheckCSourceCompiles)
+check_c_source_compiles(
+"
+#include 
+#if TARGET_CPU_ARM
+#if TARGET_OS_OSX
+#warning Building for macOS
+#else
+#error Not building for macOS
+#endif
+#else
+#error Not building for ARM
+#endif
+int main() { return 0; }
+"
+BUILDING_FOR_ARM_OSX
+)
+
+if (BUILDING_FOR_ARM_OSX)
+  set(CMAKE_OSX_ARCHITECTURES "arm64;arm64e")
+endif ()
+
 check_library_exists(compression compression_encode_buffer "" 
HAVE_LIBCOMPRESSION)
 
 find_library(SECURITY_LIBRARY Security)



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

[Lldb-commits] [lldb] 479b672 - [lldb] Pretend host architecture is arm64 on AS

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-13T18:57:23-07:00
New Revision: 479b672ff9a9230dee37fad97413a88bc0ab362b

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

LOG: [lldb] Pretend host architecture is arm64 on AS

The arm64e architecture is a preview. On Apple Silicon, pretend the host
architecture is arm64.

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 98607a1549ea6..72924515d8817 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -243,6 +243,12 @@ static void ParseOSVersion(llvm::VersionTuple , 
NSString *Key) {
 len = sizeof(is_64_bit_capable);
 ::sysctlbyname("hw.cpu64bit_capable", _64_bit_capable, , NULL, 0);
 
+if (cputype == CPU_TYPE_ARM64 && cpusubtype == CPU_SUBTYPE_ARM64E) {
+  // The arm64e architecture is a preview. Pretend the host architecture
+  // is arm64.
+  cpusubtype = CPU_SUBTYPE_ARM64_ALL;
+}
+
 if (is_64_bit_capable) {
   if (cputype & CPU_ARCH_ABI64) {
 // We have a 64 bit kernel on a 64 bit system



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

[Lldb-commits] [PATCH] D100418: [lldb] [MainLoop] Support multiple callbacks per signal

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Support registering multiple callbacks for a single signal.  This is
necessary to support multiple co-existing native process instances, with
separate SIGCHLD handlers.

Since std::function does not support equality comparison,
RegisterSignal() now accepts an optional argument to define identifier
for the callback.  If multiple handlers for the same signal are expeted
to be registered, unique identifiers are used to distinguish between
them.

The system signal handler is registered on first request, additional
callback are added on subsequent requests.  The system signal handler
is removed when last callback is unregistered.


https://reviews.llvm.org/D100418

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/source/Host/common/MainLoop.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -301,7 +301,7 @@
 
   Status status;
   m_sigchld_handle = mainloop.RegisterSignal(
-  SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
+  SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status, this);
   assert(m_sigchld_handle && status.Success());
 
   for (const auto  : tids) {
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -296,19 +296,24 @@
 
 // We shall block the signal, then install the signal handler. The signal will
 // be unblocked in the Run() function to check for signal delivery.
-MainLoop::SignalHandleUP
-MainLoop::RegisterSignal(int signo, const Callback , Status ) {
+MainLoop::SignalHandleUP MainLoop::RegisterSignal(int signo,
+  const Callback ,
+  Status ,
+  void *callback_id) {
 #ifdef SIGNAL_POLLING_UNSUPPORTED
   error.SetErrorString("Signal polling is not supported on this platform.");
   return nullptr;
 #else
-  if (m_signals.find(signo) != m_signals.end()) {
-error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
-return nullptr;
+  auto signal_it = m_signals.find(signo);
+  if (signal_it != m_signals.end()) {
+assert(signal_it->second.callbacks.find(callback_id) ==
+   signal_it->second.callbacks.end());
+signal_it->second.callbacks[callback_id] = callback;
+return SignalHandleUP(new SignalHandle(*this, signo, callback_id));
   }
 
   SignalInfo info;
-  info.callback = callback;
+  info.callbacks[callback_id] = callback;
   struct sigaction new_action;
   new_action.sa_sigaction = 
   new_action.sa_flags = SA_SIGINFO;
@@ -340,7 +345,7 @@
   info.was_blocked = sigismember(_set, signo);
   m_signals.insert({signo, info});
 
-  return SignalHandleUP(new SignalHandle(*this, signo));
+  return SignalHandleUP(new SignalHandle(*this, signo, callback_id));
 #endif
 }
 
@@ -350,13 +355,21 @@
   assert(erased);
 }
 
-void MainLoop::UnregisterSignal(int signo) {
+void MainLoop::UnregisterSignal(int signo, void *callback_id) {
 #if SIGNAL_POLLING_UNSUPPORTED
   Status("Signal polling is not supported on this platform.");
 #else
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
+  auto cb_it = it->second.callbacks.find(callback_id);
+  assert(cb_it != it->second.callbacks.end());
+  it->second.callbacks.erase(cb_it);
+
+  // Do not remove the signal handler unless all callbacks have been erased.
+  if (!it->second.callbacks.empty())
+return;
+
   sigaction(signo, >second.old_action, nullptr);
 
   sigset_t set;
@@ -398,8 +411,15 @@
 
 void MainLoop::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
-  if (it != m_signals.end())
-it->second.callback(*this); // Do the work
+  if (it != m_signals.end()) {
+// The callback may actually register/unregister signal handlers,
+// so we need to create a copy first.
+llvm::SmallVector callbacks_to_run;
+for (auto  : it->second.callbacks)
+  callbacks_to_run.push_back(x.second);
+for (auto  : callbacks_to_run)
+  x(*this); // Do the work
+  }
 }
 
 void MainLoop::ProcessReadObject(IOObject::WaitableHandle handle) {
Index: lldb/include/lldb/Host/MainLoop.h
===
--- lldb/include/lldb/Host/MainLoop.h
+++ lldb/include/lldb/Host/MainLoop.h
@@ -56,7 +56,7 @@
   // handler. However, since the callback is not invoked synchronously, you
   // cannot use this mechanism to handle SIGSEGV and the like.
   SignalHandleUP RegisterSignal(int signo, const Callback ,
-

[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg

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

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

Rebase.


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

https://reviews.llvm.org/D100261

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

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -231,7 +231,6 @@
 
   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
@@ -2058,16 +2058,6 @@
 GDBRemoteCommunicationServerLLGS::Handle_H(StringExtractorGDBRemote ) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
-  // Fail if we don't have a current process.
-  if (!m_current_process ||
-  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-LLDB_LOGF(
-log,
-"GDBRemoteCommunicationServerLLGS::%s failed, no process available",
-__FUNCTION__);
-return SendErrorResponse(0x15);
-  }
-
   // Parse out which variant of $H is requested.
   packet.SetFilePos(strlen("H"));
   if (packet.GetBytesLeft() < 1) {
@@ -2079,14 +2069,14 @@
   }
 
   const char h_variant = packet.GetChar();
-  lldb::pid_t default_pid;
+  NativeProcessProtocol *default_process;
   switch (h_variant) {
   case 'g':
-default_pid = m_current_process->GetID();
+default_process = m_current_process;
 break;
 
   case 'c':
-default_pid = m_continue_process->GetID();
+default_process = m_continue_process;
 break;
 
   default:
@@ -2099,16 +2089,32 @@
   }
 
   // Parse out the thread number.
-  llvm::Expected tid_ret =
-  ReadTid(packet, /*allow_all=*/true, default_pid);
-  if (!tid_ret)
-return SendErrorResponse(tid_ret.takeError());
+  auto pid_tid = packet.GetPidTid(default_process ? default_process->GetID()
+  : LLDB_INVALID_PROCESS_ID);
+  if (!pid_tid)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
+  lldb::pid_t pid = pid_tid->first;
+  lldb::tid_t tid = pid_tid->second;
+
+  if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+  if (pid == LLDB_INVALID_PROCESS_ID)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "No current process and no PID provided"));
+
+  // Check the process ID and find respective process instance.
+  auto new_process_it = m_debugged_processes.find(pid);
+  if (new_process_it == m_debugged_processes.end())
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(),
+llvm::formatv("No process with PID {0} debugged", pid)));
 
-  lldb::tid_t tid = tid_ret.get();
   // Ensure we have the given thread when not specifying -1 (all threads) or 0
   // (any thread).
   if (tid != LLDB_INVALID_THREAD_ID && tid != 0) {
-NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
+NativeThreadProtocol *thread = new_process_it->second->GetThreadByID(tid);
 if (!thread) {
   LLDB_LOGF(log,
 "GDBRemoteCommunicationServerLLGS::%s failed, tid %" PRIu64
@@ -2118,13 +2124,15 @@
 }
   }
 
-  // Now switch the given thread type.
+  // Now switch the given process and thread type.
   switch (h_variant) {
   case 'g':
+m_current_process = new_process_it->second.get();
 SetCurrentThreadID(tid);
 break;
 
   case 'c':
+m_continue_process = new_process_it->second.get();
 SetContinueThreadID(tid);
 break;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason to client

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

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

Rebase. Store `mainloop` arg of the original process, and pass it to the 
children to make them independent of the 'main' process.


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

https://reviews.llvm.org/D100208

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h

Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -85,6 +85,12 @@
 
   void SetStoppedByTrace();
 
+  void SetStoppedByFork(lldb::pid_t child_pid);
+
+  void SetStoppedByVFork(lldb::pid_t child_pid);
+
+  void SetStoppedByVForkDone();
+
   void SetStoppedWithNoReason();
 
   void SetStoppedByProcessorTrace(llvm::StringRef description);
Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -394,6 +394,28 @@
   m_stop_info.details.signal.signo = SIGTRAP;
 }
 
+void NativeThreadLinux::SetStoppedByFork(lldb::pid_t child_pid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_pid;
+}
+
+void NativeThreadLinux::SetStoppedByVFork(lldb::pid_t child_pid) {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_pid;
+}
+
+void NativeThreadLinux::SetStoppedByVForkDone() {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonVForkDone;
+}
+
 void NativeThreadLinux::SetStoppedWithNoReason() {
   SetStopped();
 
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -49,6 +49,8 @@
 llvm::Expected>
 Attach(lldb::pid_t pid, NativeDelegate _delegate,
MainLoop ) const override;
+
+Extension GetSupportedExtensions() const override;
   };
 
   // NativeProcessProtocol Interface
@@ -136,6 +138,7 @@
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
+  MainLoop& m_main_loop;
 
   LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector> m_mem_region_cache;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -281,6 +281,11 @@
   pid, -1, native_delegate, Info.GetArchitecture(), mainloop, *tids_or));
 }
 
+NativeProcessLinux::Extension
+NativeProcessLinux::Factory::GetSupportedExtensions() const {
+  return Extension::fork | Extension::vfork;
+}
+
 // Public Instance Methods
 
 NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
@@ -288,7 +293,7 @@
const ArchSpec , MainLoop ,
llvm::ArrayRef<::pid_t> tids)
 : NativeProcessELF(pid, terminal_fd, delegate), m_arch(arch),
-  m_intel_pt_manager(pid) {
+  m_main_loop(mainloop), m_intel_pt_manager(pid) {
   if (m_terminal_fd != -1) {
 Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
 assert(status.Success());
@@ -647,7 +652,12 @@
   }
 
   case (SIGTRAP | (PTRACE_EVENT_VFORK_DONE << 8)): {
-ResumeThread(thread, thread.GetState(), LLDB_INVALID_SIGNAL_NUMBER);
+if ((m_enabled_extensions & Extension::vfork) == Extension::vfork) {
+  thread.SetStoppedByVForkDone();
+  StopRunningThreads(thread.GetID());
+}
+else
+  ResumeThread(thread, thread.GetState(), LLDB_INVALID_SIGNAL_NUMBER);
 break;
   }
 
@@ -912,16 +922,28 @@
 LLVM_FALLTHROUGH;
   case PTRACE_EVENT_FORK:
   case PTRACE_EVENT_VFORK: {
-MainLoop unused_loop;
-NativeProcessLinux child_process{static_cast<::pid_t>(child_pid),
- m_terminal_fd,
- m_delegate,
- m_arch,
- unused_loop,
- {static_cast<::pid_t>(child_pid)}};
-child_process.Detach();
-ResumeThread(*parent_thread, parent_thread->GetState(),
- LLDB_INVALID_SIGNAL_NUMBER);
+bool is_vfork = clone_info->event == PTRACE_EVENT_VFORK;
+NativeProcessLinux *child_process = new 

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

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

Now in a version that actually compiles.


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

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,9 @@
   MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocess,
+   void(NativeProcessProtocol *parent_process,
+std::unique_ptr _process));
 };
 
 // NB: This class doesn't use the override keyword to avoid
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@
 return eServerPacketType_C;
 
   case 'D':
-if (packet_size == 1)
-  return eServerPacketType_D;
-break;
+return eServerPacketType_D;
 
   case 'g':
 return eServerPacketType_g;
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
@@ -78,6 +78,10 @@
 
   void DidExec(NativeProcessProtocol *process) override;
 
+  void
+  NewSubprocess(NativeProcessProtocol *parent_process,
+std::unique_ptr _process) override;
+
   Status InitializeConnection(std::unique_ptr connection);
 
 protected:
@@ -89,7 +93,8 @@
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
-  std::unique_ptr m_debugged_process_up;
+  std::unordered_map>
+  m_debugged_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
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
@@ -243,14 +243,14 @@
 
   {
 std::lock_guard guard(m_debugged_process_mutex);
-assert(!m_debugged_process_up && "lldb-server creating debugged "
- "process but one already exists");
+assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+   "process but one already exists");
 auto process_or =
 m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
 if (!process_or)
   return Status(process_or.takeError());
-m_debugged_process_up = std::move(*process_or);
-m_continue_process = m_current_process = m_debugged_process_up.get();
+m_continue_process = m_current_process = process_or->get();
+m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -265,10 +265,10 @@
 LLDB_LOG(log,
  "pid = {0}: setting up stdout/stderr redirection via $O "
  "gdb-remote commands",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
 // Setup stdout/stderr mapping from inferior to $O
-auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
 if (terminal_fd >= 0) {
   LLDB_LOGF(log,
 "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -287,12 +287,12 @@
 LLDB_LOG(log,
  "pid = {0} skipping stdout/stderr redirection via $O: inferior "
  "will communicate over client-provided file descriptors",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
  m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
   return Status();
 }
@@ -304,12 +304,11 @@
 
   // Before we try to attach, make sure we aren't already monitoring something
   

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

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

Unset `m_current_process` and `m_continue_process` if we're about to detach it.


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

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,9 @@
   MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocess,
+   void(NativeProcessProtocol *parent_process,
+std::unique_ptr _process));
 };
 
 // NB: This class doesn't use the override keyword to avoid
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@
 return eServerPacketType_C;
 
   case 'D':
-if (packet_size == 1)
-  return eServerPacketType_D;
-break;
+return eServerPacketType_D;
 
   case 'g':
 return eServerPacketType_g;
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
@@ -78,6 +78,10 @@
 
   void DidExec(NativeProcessProtocol *process) override;
 
+  void
+  NewSubprocess(NativeProcessProtocol *parent_process,
+std::unique_ptr _process) override;
+
   Status InitializeConnection(std::unique_ptr connection);
 
 protected:
@@ -89,7 +93,8 @@
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
-  std::unique_ptr m_debugged_process_up;
+  std::unordered_map>
+  m_debugged_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
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
@@ -243,14 +243,14 @@
 
   {
 std::lock_guard guard(m_debugged_process_mutex);
-assert(!m_debugged_process_up && "lldb-server creating debugged "
- "process but one already exists");
+assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+   "process but one already exists");
 auto process_or =
 m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
 if (!process_or)
   return Status(process_or.takeError());
-m_debugged_process_up = std::move(*process_or);
-m_continue_process = m_current_process = m_debugged_process_up.get();
+m_continue_process = m_current_process = process_or->get();
+m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -265,10 +265,10 @@
 LLDB_LOG(log,
  "pid = {0}: setting up stdout/stderr redirection via $O "
  "gdb-remote commands",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
 // Setup stdout/stderr mapping from inferior to $O
-auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
 if (terminal_fd >= 0) {
   LLDB_LOGF(log,
 "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -287,12 +287,12 @@
 LLDB_LOG(log,
  "pid = {0} skipping stdout/stderr redirection via $O: inferior "
  "will communicate over client-provided file descriptors",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
  m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
   return Status();
 }
@@ -304,12 +304,11 @@
 
   // Before we try to attach, make sure we 

[Lldb-commits] [PATCH] D100384: [lldb/Plugins] Update ScriptedProcess Process Plugin

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 337226.
mib added a comment.

Address @JDevlieghere feedbacks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100384

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -11,7 +11,7 @@
 from lldbsuite.test import lldbtest
 
 
-class PlatformProcessCrashInfoTestCase(TestBase):
+class ScriptedProcesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -43,3 +43,55 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
+def test_launch_scripted_process_sbapi(self):
+"""Test that we can launch an lldb scripted process using the SBAPI,
+check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("my_scripted_process.MyScriptedProcess")
+
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
+
+def test_launch_scripted_process_cli(self):
+"""Test that we can launch an lldb scripted process from the command
+line, check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+error = lldb.SBError()
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2972,7 +2972,7 @@
   // If we're not already connected to the process, and if we have a platform
   // that can launch a process for debugging, go ahead and do that here.
   if (state != eStateConnected && platform_sp &&
-  platform_sp->CanDebugProcess()) {
+  platform_sp->CanDebugProcess() && !launch_info.IsScriptedProcess()) {
 LLDB_LOGF(log, "Target::%s asking the platform to debug the process",
   __FUNCTION__);
 
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -0,0 +1,139 @@
+//===-- ScriptedProcess.h - -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Sorry for the delay!

From my playing around it look like if you do all the same steps as you show in 
your example, except that you leave out the call to "Persist" and instead 
directly use the SBValue from the expression directly, then everything works 
correctly.

If that's true then the original persistent variable we made for the expression 
result was working correctly, and something about Persist messed it up.  That's 
not 100% surprising, since we're "re-persisting" an already persistent 
variable...

If persisting already persistent variables is indeed the only failure case, 
then I wonder if it wouldn't be more straightforward to just see if the 
ValueObject is already a persistent variable and have Persist just return the 
incoming variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D100338#2686410 , @augusto2112 
wrote:

> Thanks for the input @jasonmolenda. FWIW I also think it's worth implementing 
> this correctly.
>
> Could you help me figure out which callers I should change to 
> `force_live_memory` though?



> Maybe force live memory only for the functions in 
> `DynamicLoaderDarwinKernel.cpp` and `DynamicLoaderDarwin.cpp`?

Taking a quick look at the dynamic loader plugins, DynamicLoaderDarwin is 
calling ReadMemory in DynamicLoaderDarwin::GetThreadLocalData and I'm pretty 
sure the thread load storage address it is reading will be in a writable 
section, so our reimagined Target::ReadMemory will read from memory anyway.

DynamicLoaderDarwinKernel is calling Target::ReadMemory in 
ReadKextSummaryHeader and ReadKextSummaries -- it needs to read live memory, but

  (lldb) ima loo -vs gLoadedKextSummaries
  1 symbols match 'gLoadedKextSummaries' in /tmp/dl/mach.development.t8002:
  Address: mach.development.t8002[0x808c66b8] 
(mach.development.t8002.__DATA.__common + 394936)
  (lldb) tar mod dump sect
0x0011 zero-fill[0x80866000-0x808c8294)  rw-  
0x 0x 0x0001 mach.development.t8002.__DATA.__common

they're reading from a writable Section so we would do the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

@jasonmolenda actually, most of them are passing `prefer_file_cache = false`, 
unfortunately. But they might be doing this because there's they weren't sure 
if the memory was writable, so this was the safer option. I want to change all 
the ones we deem safe to `force_live_memory = false`, which is why I ask which 
files you think this change would be safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I should say, my suggestion of changing Target::ReadMemory is just an opinion, 
please do feel free to disagree!

I think if you look at the callers, I suspect all (or nearly all) of them are 
passing prefer_file_cache == true right now, which is the 
force_live_memory==false default case.  Anyone passing prefer_file_cache==false 
would want to pass force_live_memory==true, and it may be that they are doing 
this incorrect (you were originally looking at an instance where that was true 
in swift)

An example where we WANT prefer_file_cache==false aka force_live_memory==true 
is in CommandObjectMemory.cpp.  When people use the memory read command, we 
want to show them the actual memory contents [*], we don't want to optimize by 
getting data from a file.

[X] we still replace breakpoint instructions with the real instruction when 
reading instructions, iirc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

Thanks for the input @jasonmolenda. FWIW I also think it's worth implementing 
this correctly.

Could you help me figure out which callers I should change to 
`force_live_memory` though?

Here's a list of everywhere ReadMemory is called:

  SBTarget.cpp
  ReadInstructions
  ReadMemory
  
  CommandObjectMemory.cpp
  DoExecute
  
  Address.cpp
  ReadBytes
  
  Disassembler.cpp
  ParseInstructions
  
  Value.cpp
  GetValueAsData
  
  ValueObject.cpp
  GetPointeeData
  
  IRMemoryMap.cpp
  ReadMemory
  
  DynamicLoaderDarwinKernel.cpp
  ReadKextSummaries
  ReadKextSummaryHeader
  
  DynamicLoaderDarwin.cpp
  GetThreadLocalData
  
  EmulateInstructionMIPS.cpp
  SetInstruction
  
  UnwindAssemblyInstEmulation.cpp
  GetNonCallSiteUnwindPlanFromAssembly
  
  UnwindAssembly-x86.cpp
  AugmentUnwindPlanFromCallSite
  FirstNonPrologueInsn
  GetFastUnwindPlan
  GetNonCallSiteUnwindPlanFromAssembly
  
  Target.cpp  
  ReadCStringFromMemory
  ReadScalarIntegerFromMemory

Maybe force live memory only for the functions in 
`DynamicLoaderDarwinKernel.cpp` and `DynamicLoaderDarwin.cpp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

Re: [Lldb-commits] [lldb] 8a5af9e - [debugserver] Fix unintialized member variable

[Jonas Devlieghere via lldb-commits]

Thanks Shafik, addressed in ae8a5c68523c1d23dec721e28f89084d6561522a

On Tue, Apr 13, 2021 at 9:56 AM Shafik Yaghmour  wrote:

> I might be missing something here but I think
>
> m_launch_flavor
>
> Is also uninitialized.
>
> It looks like using in class member initialization would be a better fix
> for any case where the constructor is just a member initialization list
> with an empty body.
>
> > On Apr 13, 2021, at 9:47 AM, Jonas Devlieghere via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> >
> > Author: Jonas Devlieghere
> > Date: 2021-04-13T09:46:59-07:00
> > New Revision: 8a5af9e28443ce8290388439f9e36cf2727d7761
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/8a5af9e28443ce8290388439f9e36cf2727d7761
> > DIFF:
> https://github.com/llvm/llvm-project/commit/8a5af9e28443ce8290388439f9e36cf2727d7761.diff
> >
> > LOG: [debugserver] Fix unintialized member variable
> >
> > Caught by ubsan (__ubsan_handle_load_invalid_value_abort) when running
> > the unit tests.
> >
> > Added:
> >
> >
> > Modified:
> >lldb/tools/debugserver/source/RNBContext.h
> >
> > Removed:
> >
> >
> >
> >
> 
> > diff  --git a/lldb/tools/debugserver/source/RNBContext.h
> b/lldb/tools/debugserver/source/RNBContext.h
> > index 0b46151e47857..03cd7f350e63b 100644
> > --- a/lldb/tools/debugserver/source/RNBContext.h
> > +++ b/lldb/tools/debugserver/source/RNBContext.h
> > @@ -46,7 +46,8 @@ class RNBContext {
> >   RNBContext()
> >   : m_pid(INVALID_NUB_PROCESS), m_pid_stop_count(0),
> > m_events(0, all_event_bits), m_pid_pthread(), m_launch_status(),
> > -m_arg_vec(), m_env_vec(), m_detach_on_error(false) {}
> > +m_arg_vec(), m_env_vec(), m_detach_on_error(false),
> > +m_unmask_signals(false) {}
> >
> >   virtual ~RNBContext();
> >
> > @@ -148,11 +149,11 @@ class RNBContext {
> >   std::string m_working_directory;
> >   std::string m_process_event;
> >   bool m_detach_on_error;
> > +  bool m_unmask_signals;
> >
> >   void StartProcessStatusThread();
> >   void StopProcessStatusThread();
> >   static void *ThreadFunctionProcessStatus(void *arg);
> > -  bool m_unmask_signals;
> >
> > private:
> >   RNBContext(const RNBContext ) = delete;
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ae8a5c6 - [debugserver] Use class member initialization for RNBContext

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-13T10:39:27-07:00
New Revision: ae8a5c68523c1d23dec721e28f89084d6561522a

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

LOG: [debugserver] Use class member initialization for RNBContext

Address Shafik Yaghmour's post commit code review feedback.

Added: 


Modified: 
lldb/tools/debugserver/source/RNBContext.h

Removed: 




diff  --git a/lldb/tools/debugserver/source/RNBContext.h 
b/lldb/tools/debugserver/source/RNBContext.h
index 03cd7f350e63..562adadd874d 100644
--- a/lldb/tools/debugserver/source/RNBContext.h
+++ b/lldb/tools/debugserver/source/RNBContext.h
@@ -43,12 +43,7 @@ class RNBContext {
 all_event_bits = sticky_event_bits | normal_event_bits
   } event_t;
   // Constructors and Destructors
-  RNBContext()
-  : m_pid(INVALID_NUB_PROCESS), m_pid_stop_count(0),
-m_events(0, all_event_bits), m_pid_pthread(), m_launch_status(),
-m_arg_vec(), m_env_vec(), m_detach_on_error(false),
-m_unmask_signals(false) {}
-
+  RNBContext() = default;
   virtual ~RNBContext();
 
   nub_process_t ProcessID() const { return m_pid; }
@@ -132,24 +127,26 @@ class RNBContext {
 
 protected:
   // Classes that inherit from RNBContext can see and modify these
-  nub_process_t m_pid;
+  nub_process_t m_pid = INVALID_NUB_PROCESS;
   std::string m_stdin;
   std::string m_stdout;
   std::string m_stderr;
   std::string m_working_dir;
-  nub_size_t m_pid_stop_count;
-  PThreadEvent m_events; // Threaded events that we can wait for
+  nub_size_t m_pid_stop_count = 0;
+  /// Threaded events that we can wait for.
+  PThreadEvent m_events{0, all_event_bits};
   pthread_t m_pid_pthread;
-  nub_launch_flavor_t m_launch_flavor; // How to launch our inferior process
-  DNBError
-  m_launch_status; // This holds the status from the last launch attempt.
+  /// How to launch our inferior process.
+  nub_launch_flavor_t m_launch_flavor = eLaunchFlavorDefault;
+  /// This holds the status from the last launch attempt.
+  DNBError m_launch_status;
   std::vector m_arg_vec;
-  std::vector
-  m_env_vec; // This will be unparsed - entries FOO=value
+  /// This will be unparsed entries FOO=value
+  std::vector m_env_vec;
   std::string m_working_directory;
   std::string m_process_event;
-  bool m_detach_on_error;
-  bool m_unmask_signals;
+  bool m_detach_on_error = false;
+  bool m_unmask_signals = false;
 
   void StartProcessStatusThread();
   void StopProcessStatusThread();



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

[Lldb-commits] [PATCH] D100338: Add a setting that enables memory to be read from the file cache instead of process when the section LLDB is reading from is read-only

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

> Let's think about what the default behavior of Target::ReadMemory should be. 
> The majority use case is that we prefer the file cache if it is a read-only 
> section. It's a performance bug in almost every case to behave differently, I 
> maintain. There are times when we want to show actual raw memory to people. 
> Some naughty programs that manage to modify themselves, we need to see that 
> real naughtiness, not show what the file had and pretend that is what is in 
> memory.



> I think Target::ReadMemory should have a force_live_memory argument, not a 
> prefer_file_cache. It could be the final argument with a default value of 
> force_live_memory==false. Almost everyone should let Target::ReadMemory check 
> if this address is in a RO-section, and let it get the data from the file 
> cache if so. If it's not in a RO section, or any section, read from live 
> memory. And have the force_live_memory arg to override this.



> This is a larger set of changes though, and I don't mean to sign anyone up 
> for more than they intended. But, just thinking of this from a clean slate, I 
> think that's the better design. Adrian, what do you think?

Let's try to do the right thing here. I think your suggestions make a lot of 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100338

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

Re: [Lldb-commits] [lldb] 8a5af9e - [debugserver] Fix unintialized member variable

[Shafik Yaghmour via lldb-commits]

I might be missing something here but I think 

m_launch_flavor

Is also uninitialized. 

It looks like using in class member initialization would be a better fix for 
any case where the constructor is just a member initialization list with an 
empty body.

> On Apr 13, 2021, at 9:47 AM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> 
> Author: Jonas Devlieghere
> Date: 2021-04-13T09:46:59-07:00
> New Revision: 8a5af9e28443ce8290388439f9e36cf2727d7761
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/8a5af9e28443ce8290388439f9e36cf2727d7761
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8a5af9e28443ce8290388439f9e36cf2727d7761.diff
> 
> LOG: [debugserver] Fix unintialized member variable
> 
> Caught by ubsan (__ubsan_handle_load_invalid_value_abort) when running
> the unit tests.
> 
> Added: 
> 
> 
> Modified: 
>lldb/tools/debugserver/source/RNBContext.h
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/tools/debugserver/source/RNBContext.h 
> b/lldb/tools/debugserver/source/RNBContext.h
> index 0b46151e47857..03cd7f350e63b 100644
> --- a/lldb/tools/debugserver/source/RNBContext.h
> +++ b/lldb/tools/debugserver/source/RNBContext.h
> @@ -46,7 +46,8 @@ class RNBContext {
>   RNBContext()
>   : m_pid(INVALID_NUB_PROCESS), m_pid_stop_count(0),
> m_events(0, all_event_bits), m_pid_pthread(), m_launch_status(),
> -m_arg_vec(), m_env_vec(), m_detach_on_error(false) {}
> +m_arg_vec(), m_env_vec(), m_detach_on_error(false),
> +m_unmask_signals(false) {}
> 
>   virtual ~RNBContext();
> 
> @@ -148,11 +149,11 @@ class RNBContext {
>   std::string m_working_directory;
>   std::string m_process_event;
>   bool m_detach_on_error;
> +  bool m_unmask_signals;
> 
>   void StartProcessStatusThread();
>   void StopProcessStatusThread();
>   static void *ThreadFunctionProcessStatus(void *arg);
> -  bool m_unmask_signals;
> 
> private:
>   RNBContext(const RNBContext ) = delete;
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] [PATCH] D100256: [gdb-remote server] Abstract away getting current process

[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 rGf1812a284f28: [lldb] [gdb-remote server] Abstract away 
getting current process (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D100256?vs=337188=337194#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100256

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

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -86,6 +86,8 @@
   const NativeProcessProtocol::Factory _process_factory;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
+  NativeProcessProtocol *m_current_process;
+  NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
   std::unique_ptr m_debugged_process_up;
 
@@ -254,7 +256,7 @@
   // In any case, the function assumes that exactly one inferior is being
   // debugged and rejects pid values that do no match that inferior.
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
-  bool allow_all = false);
+  bool allow_all, lldb::pid_t default_pid);
 
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
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
@@ -70,6 +70,7 @@
 : GDBRemoteCommunicationServerCommon("gdb-remote.server",
  "gdb-remote.server.rx_packet"),
   m_mainloop(mainloop), m_process_factory(process_factory),
+  m_current_process(nullptr), m_continue_process(nullptr),
   m_stdio_communication("process.stdio") {
   RegisterPacketHandlers();
 }
@@ -249,6 +250,7 @@
 if (!process_or)
   return Status(process_or.takeError());
 m_debugged_process_up = std::move(*process_or);
+m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  m_continue_process = m_current_process = m_debugged_process_up.get();
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -735,15 +738,15 @@
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
   // Ensure we have a debugged process.
-  if (!m_debugged_process_up ||
-  (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
+  if (!m_current_process ||
+  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
 return SendErrorResponse(50);
 
   LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
-   m_debugged_process_up->GetID(), tid);
+   m_current_process->GetID(), tid);
 
   // Ensure we can get info on the given thread.
-  NativeThreadProtocol *thread = m_debugged_process_up->GetThreadByID(tid);
+  NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
   if (!thread)
 return SendErrorResponse(51);
 
@@ -766,7 +769,7 @@
   LLDB_LOG(
   log,
   "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
-  m_debugged_process_up->GetID(), tid, signum, int(tid_stop_info.reason),
+  m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
   tid_stop_info.details.exception.type);
 
   // Print the signal number.
@@ -804,9 +807,9 @@
 
 uint32_t thread_index = 0;
 NativeThreadProtocol *listed_thread;
-for (listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index);
+for (listed_thread = m_current_process->GetThreadAtIndex(thread_index);
  listed_thread; ++thread_index,
-listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index)) {
+listed_thread = m_current_process->GetThreadAtIndex(thread_index)) {
   if (thread_index > 0)
 response.PutChar(',');
   response.Printf("%" PRIx64, listed_thread->GetID());
@@ -821,7 +824,7 @@
 if (thread_index > 1) {
   const bool threads_with_valid_stop_info_only = true;
   llvm::Expected threads_info = GetJSONThreadsInfo(
-   

[Lldb-commits] [lldb] f1812a2 - [lldb] [gdb-remote server] Abstract away getting current process

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T18:53:32+02:00
New Revision: f1812a284f28e5b142e63d5deeb340be3ca5d4b3

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

LOG: [lldb] [gdb-remote server] Abstract away getting current process

Introduce new m_current_process and m_continue_process variables that
keep the pointers to currently selected process.  At this moment, this
is equivalent to m_debugged_process_up but it lays foundations for
the future multiprocess support.

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index e9034cb7414a..eba5deccc2d4 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -70,6 +70,7 @@ 
GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
 : GDBRemoteCommunicationServerCommon("gdb-remote.server",
  "gdb-remote.server.rx_packet"),
   m_mainloop(mainloop), m_process_factory(process_factory),
+  m_current_process(nullptr), m_continue_process(nullptr),
   m_stdio_communication("process.stdio") {
   RegisterPacketHandlers();
 }
@@ -249,6 +250,7 @@ Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
 if (!process_or)
   return Status(process_or.takeError());
 m_debugged_process_up = std::move(*process_or);
+m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -318,6 +320,7 @@ Status 
GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  m_continue_process = m_current_process = m_debugged_process_up.get();
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -735,15 +738,15 @@ 
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
   // Ensure we have a debugged process.
-  if (!m_debugged_process_up ||
-  (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
+  if (!m_current_process ||
+  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
 return SendErrorResponse(50);
 
   LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
-   m_debugged_process_up->GetID(), tid);
+   m_current_process->GetID(), tid);
 
   // Ensure we can get info on the given thread.
-  NativeThreadProtocol *thread = m_debugged_process_up->GetThreadByID(tid);
+  NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
   if (!thread)
 return SendErrorResponse(51);
 
@@ -766,7 +769,7 @@ 
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
   LLDB_LOG(
   log,
   "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
-  m_debugged_process_up->GetID(), tid, signum, int(tid_stop_info.reason),
+  m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
   tid_stop_info.details.exception.type);
 
   // Print the signal number.
@@ -804,9 +807,9 @@ 
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
 
 uint32_t thread_index = 0;
 NativeThreadProtocol *listed_thread;
-for (listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index);
+for (listed_thread = m_current_process->GetThreadAtIndex(thread_index);
  listed_thread; ++thread_index,
-listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index)) 
{
+listed_thread = m_current_process->GetThreadAtIndex(thread_index)) {
   if (thread_index > 0)
 response.PutChar(',');
   response.Printf("%" PRIx64, listed_thread->GetID());
@@ -821,7 +824,7 @@ 
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
 if (thread_index > 1) {
   const bool threads_with_valid_stop_info_only = true;
   llvm::Expected threads_info = GetJSONThreadsInfo(
-  *m_debugged_process_up, threads_with_valid_stop_info_only);
+  *m_current_process, threads_with_valid_stop_info_only);
   if (threads_info) {
 response.PutCString("jstopinfo:");
 StreamString unescaped_response;
@@ -831,7 +834,7 @@ 
GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
   } else {
  

[Lldb-commits] [lldb] 8a5af9e - [debugserver] Fix unintialized member variable

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-13T09:46:59-07:00
New Revision: 8a5af9e28443ce8290388439f9e36cf2727d7761

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

LOG: [debugserver] Fix unintialized member variable

Caught by ubsan (__ubsan_handle_load_invalid_value_abort) when running
the unit tests.

Added: 


Modified: 
lldb/tools/debugserver/source/RNBContext.h

Removed: 




diff  --git a/lldb/tools/debugserver/source/RNBContext.h 
b/lldb/tools/debugserver/source/RNBContext.h
index 0b46151e47857..03cd7f350e63b 100644
--- a/lldb/tools/debugserver/source/RNBContext.h
+++ b/lldb/tools/debugserver/source/RNBContext.h
@@ -46,7 +46,8 @@ class RNBContext {
   RNBContext()
   : m_pid(INVALID_NUB_PROCESS), m_pid_stop_count(0),
 m_events(0, all_event_bits), m_pid_pthread(), m_launch_status(),
-m_arg_vec(), m_env_vec(), m_detach_on_error(false) {}
+m_arg_vec(), m_env_vec(), m_detach_on_error(false),
+m_unmask_signals(false) {}
 
   virtual ~RNBContext();
 
@@ -148,11 +149,11 @@ class RNBContext {
   std::string m_working_directory;
   std::string m_process_event;
   bool m_detach_on_error;
+  bool m_unmask_signals;
 
   void StartProcessStatusThread();
   void StopProcessStatusThread();
   static void *ThreadFunctionProcessStatus(void *arg);
-  bool m_unmask_signals;
 
 private:
   RNBContext(const RNBContext ) = delete;



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

[Lldb-commits] [PATCH] D100384: [lldb/Plugins] Update ScriptedProcess Process Plugin

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:93
+
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+

You can move this into the `if` on line 98, or even just inline it. 



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:100-103
+LLDB_LOGF(
+log,
+"ScriptedProcess::%s failed to listen for m_async_broadcaster events",
+__FUNCTION__);





Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:138
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  LLDB_LOGF(log, "ScriptedProcess::%s ()", __FUNCTION__);

I see this newline between `log` and `LLDB_LOGF` across this patch and while I 
don't mind (it's consistent) I'm curious why it's here. The next line is 
immediately using `log` and it other places in this patch where a variable is 
used directly after there is (usually) no newline.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:205
+  bool is_running = false;
+  do {
+switch (event_type) {

Do we need this loop at all? It looks like nothing is ever setting `is_running` 
to true, so effectively this runs just the once? The two nested loops make it 
pretty hard to follow what the control flow is here. 



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:236
+}
+  } while (is_running); // do while
+} else {





Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:316-345
+  bool resume = false;
+  Status status;
+
+  // FIXME: Fetch data from thread.
+  const StateType thread_resume_state = eStateRunning;
+
+  LLDB_LOGF(log, "ScriptedProcess::%s thread_resume_state = %s", __FUNCTION__,





Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.h:128
+  lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
+  Broadcaster m_async_broadcaster;
+  lldb::ListenerSP m_async_listener_sp;

Can we add a Doxygen comment (group) that explains what these things are and 
what they're used for? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100384

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

[Lldb-commits] [PATCH] D100256: [gdb-remote server] Abstract away getting current process

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

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

Rebase and make linter happier. Now let's re-test and push.


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

https://reviews.llvm.org/D100256

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

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -86,6 +86,8 @@
   const NativeProcessProtocol::Factory _process_factory;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
+  NativeProcessProtocol *m_current_process;
+  NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
   std::unique_ptr m_debugged_process_up;
 
@@ -254,7 +256,7 @@
   // In any case, the function assumes that exactly one inferior is being
   // debugged and rejects pid values that do no match that inferior.
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
-  bool allow_all = false);
+  bool allow_all, lldb::pid_t default_pid);
 
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
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
@@ -70,6 +70,7 @@
 : GDBRemoteCommunicationServerCommon("gdb-remote.server",
  "gdb-remote.server.rx_packet"),
   m_mainloop(mainloop), m_process_factory(process_factory),
+  m_current_process(nullptr), m_continue_process(nullptr),
   m_stdio_communication("process.stdio") {
   RegisterPacketHandlers();
 }
@@ -249,6 +250,7 @@
 if (!process_or)
   return Status(process_or.takeError());
 m_debugged_process_up = std::move(*process_or);
+m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  m_continue_process = m_current_process = m_debugged_process_up.get();
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -735,15 +738,15 @@
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
   // Ensure we have a debugged process.
-  if (!m_debugged_process_up ||
-  (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
+  if (!m_current_process ||
+  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
 return SendErrorResponse(50);
 
   LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
-   m_debugged_process_up->GetID(), tid);
+   m_current_process->GetID(), tid);
 
   // Ensure we can get info on the given thread.
-  NativeThreadProtocol *thread = m_debugged_process_up->GetThreadByID(tid);
+  NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
   if (!thread)
 return SendErrorResponse(51);
 
@@ -766,7 +769,7 @@
   LLDB_LOG(
   log,
   "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
-  m_debugged_process_up->GetID(), tid, signum, int(tid_stop_info.reason),
+  m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
   tid_stop_info.details.exception.type);
 
   // Print the signal number.
@@ -804,9 +807,9 @@
 
 uint32_t thread_index = 0;
 NativeThreadProtocol *listed_thread;
-for (listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index);
+for (listed_thread = m_current_process->GetThreadAtIndex(thread_index);
  listed_thread; ++thread_index,
-listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index)) {
+listed_thread = m_current_process->GetThreadAtIndex(thread_index)) {
   if (thread_index > 0)
 response.PutChar(',');
   response.Printf("%" PRIx64, listed_thread->GetID());
@@ -821,7 +824,7 @@
 if (thread_index > 1) {
   const bool threads_with_valid_stop_info_only = true;
   llvm::Expected threads_info = GetJSONThreadsInfo(
-  *m_debugged_process_up, threads_with_valid_stop_info_only);
+  *m_current_process, threads_with_valid_stop_info_only);
   if (threads_info) {
 response.PutCString("jstopinfo:");
 StreamString unescaped_response;
@@ -831,7 +834,7 @@

[Lldb-commits] [PATCH] D100384: [lldb/Plugins] Update ScriptedProcess Process Plugin

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 337184.
mib added a comment.

Dead-code removal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100384

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -11,7 +11,7 @@
 from lldbsuite.test import lldbtest
 
 
-class PlatformProcessCrashInfoTestCase(TestBase):
+class ScriptedProcesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -43,3 +43,55 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
+def test_launch_scripted_process_sbapi(self):
+"""Test that we can launch an lldb scripted process using the SBAPI,
+check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("my_scripted_process.MyScriptedProcess")
+
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
+
+def test_launch_scripted_process_cli(self):
+"""Test that we can launch an lldb scripted process from the command
+line, check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+error = lldb.SBError()
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2972,7 +2972,7 @@
   // If we're not already connected to the process, and if we have a platform
   // that can launch a process for debugging, go ahead and do that here.
   if (state != eStateConnected && platform_sp &&
-  platform_sp->CanDebugProcess()) {
+  platform_sp->CanDebugProcess() && !launch_info.IsScriptedProcess()) {
 LLDB_LOGF(log, "Target::%s asking the platform to debug the process",
   __FUNCTION__);
 
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -0,0 +1,136 @@
+//===-- ScriptedProcess.h - -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny updated this revision to Diff 337164.
mgorny edited the summary of this revision.
mgorny added a comment.

Updated to keep a single list of all attached processes.


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

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,9 @@
   MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocess,
+   void(NativeProcessProtocol *parent_process,
+std::unique_ptr _process));
 };
 
 // NB: This class doesn't use the override keyword to avoid
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@
 return eServerPacketType_C;
 
   case 'D':
-if (packet_size == 1)
-  return eServerPacketType_D;
-break;
+return eServerPacketType_D;
 
   case 'g':
 return eServerPacketType_g;
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
@@ -78,6 +78,10 @@
 
   void DidExec(NativeProcessProtocol *process) override;
 
+  void
+  NewSubprocess(NativeProcessProtocol *parent_process,
+std::unique_ptr _process) override;
+
   Status InitializeConnection(std::unique_ptr connection);
 
 protected:
@@ -89,7 +93,8 @@
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
-  std::unique_ptr m_debugged_process_up;
+  std::unordered_map>
+  m_debugged_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
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
@@ -243,14 +243,14 @@
 
   {
 std::lock_guard guard(m_debugged_process_mutex);
-assert(!m_debugged_process_up && "lldb-server creating debugged "
- "process but one already exists");
+assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+   "process but one already exists");
 auto process_or =
 m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
 if (!process_or)
   return Status(process_or.takeError());
-m_debugged_process_up = std::move(*process_or);
-m_continue_process = m_current_process = m_debugged_process_up.get();
+m_continue_process = m_current_process = process_or->get();
+m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -265,10 +265,10 @@
 LLDB_LOG(log,
  "pid = {0}: setting up stdout/stderr redirection via $O "
  "gdb-remote commands",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
 // Setup stdout/stderr mapping from inferior to $O
-auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
 if (terminal_fd >= 0) {
   LLDB_LOGF(log,
 "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -287,12 +287,12 @@
 LLDB_LOG(log,
  "pid = {0} skipping stdout/stderr redirection via $O: inferior "
  "will communicate over client-provided file descriptors",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
  m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
   return Status();
 }
@@ -304,12 +304,11 @@
 
   // Before we try to 

[Lldb-commits] [PATCH] D100256: [gdb-remote server] Abstract away getting current process

[Pavel Labath via Phabricator via lldb-commits]

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

Looks about right.


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

https://reviews.llvm.org/D100256

___
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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D99944#2684280 , @jasonmolenda 
wrote:

> Hi Omair, sorry for the delay in looking at this.  It seems like we have two 
> overlapping patches here, @justincohen  patch in 
> https://reviews.llvm.org/D98886 and this one.
>
> I'm not convinced that we'll see a target where we have noncontiguous bits 
> used for addressing (the best reason for using a mask), but Justin points out 
> that Linux ARM's NT_ARM_PAC_MASK (gettable by ptrace PTRACE_GETREGSET) is 
> expressed as a mask (and googling around, it looks like you can have a 
> different mask for code pointers and data pointers, so Process would really 
> need to store both, but it may be that for crashpad only the code mask needs 
> to be tracked in Process).
>
> lldb needs to allow for users to set the number of bits used for addressing 
> in terms of a number -- 52 in your example -- but given that Linux wants to 
> give us a mask via the ptrace call, I think we should preserve and use that 
> mask.  Darwin's API also provide the value in terms of the number of bits 
> used for addressing, and as a user setting I think that's the most natural 
> way of handling it and we should allow it.  But internal to Process, we 
> should convert this to a CodeAddress mask and down in the ABI's where we 
> support AArch v8.3 ptrauth, we use bit 55 to clear/set the non-addressing 
> high bits.
>
> Omair, Justin, what do you think here?  I don't think it's especially hard to 
> accept this in terms of # of bits OR a mask, and we should use the more 
> general internal rep in lldb.  Another alternative would be "the mask should 
> be converted to the # of bits in addressing and stored in Process in those 
> terms".
>
> (Honestly the idea of an address mask makes me nervous all over - if the rule 
> is "take bit 55 and set all non-addressing bits to 0 or 1 depending on that", 
> then we're implicitly saying "the mask only masks off high bits".  If it ever 
> masked off bit 0, for instance, on a code address, then we're going to set 
> that to 0 or 1 when we set/clear non-addressing bits?  Obviously not.  So 
> it's not REALLY a mask, is it, it's just the # of bits used for addressing in 
> another more flexible format that you can't actually flex or it all breaks. :)

Frankly I am also not too sure about keeping single mask for both code and data 
masks. For now Linux kernel ensures both are the same but they can differ in 
future. For user-space Linux apps top-byte is used for tags and bit 55 only 
gives us a hint on whether we mask the top byte only or also need to mask off 
bits (54:VA_Bits). For unwinder code mask is relevant but we would also like to 
use the mask for clearing top-bits off watchpoint addresses in-case of a tagged 
data pointer and if we have a separate data_mask we ll essentially need to 
store that information separately.

On linux masks are readable through code_mask and data_mask registers. We dont 
really need to keep this information in process as @labath pointed out but that 
is only fine for user-space apps. In case we intend to debug linux kernel or 
any bare-metal app utilizing the top byte(s) which i am not aware of any user 
of LLDB right now using it to debug the Linux kernel but I dont wanna introduce 
anything here which makes LLDB user-app specific so thats why one of the option 
is to send the information over gdb-remote protocol qHostInfo but that also 
restricts LLDB to lldb-server only.

In any case we ll need atleast one mask for sure and I dont mind @justincohen 
writing that patch and I ll rebase this patch on top of it once its merged.


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] D97786: LLDB: Use path relative to binary, not relative to debugger CWD, for finding .dwo files.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D97786#2685814 , @cmtice wrote:

> Pavel, your last comment lost me completely.  How can I test the code I added 
> to lldb if I don't run lldb?  I am a complete newbie at writing test cases so 
> there's probably something basic I'm missing?  How would I observe the value 
> of the variable without running lldb?  Also, if the program doesn't contain 
> 'main', won't the compiler complain?  Is there an existing test that does 
> what you're suggesting that I could look at?

Sorry, I meant running the inferior -- you do definitely need to run lldb :)

And we can avoid the need for the main function by pointing lldb directly at 
the object file. As long as you don't try to run the file, lldb (mostly) does 
not care whether it's dealing with a full binary or just an object file. A lot 
of the .s tests in this directory use that approach, but most of them don't use 
split dwarf, so I don't know of a file you could copy verbatim. However, you 
could try to look at `dwarf5-split.s`, which does something pretty similar with 
a dwp file.

I'd imagine the test could look something like:

  ## As before
  # 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
  
  ## No linking
  
  # RUN: cd ../..
  
  # RUN: %lldb %t.o -o "target variable x" -b 2>&1 | FileCheck %s
  
  # CHECK: x = 10
  
  ## Debug info for "int x = 10;"


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] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

@jingham @teemperor ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D100256: [gdb-remote server] Abstract away getting current process

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

mgorny updated this revision to Diff 337154.
mgorny retitled this revision from "[lldb] [gdb-remote server] Abstract away 
getting current process" to "[gdb-remote server] Abstract away getting current 
process".
mgorny edited the summary of this revision.
mgorny added a comment.

Switched to using two pointers instead of a function. Made `ReadTid()` accept a 
default PID instead of playing with bools. Corrected `Hc` PID to apply only to 
c/C/s/vCont packets, similarly to how GDB does it.

@labath, I've left the `GetID()` asserts in place because I've just did a big 
search/replace, and replacing them would be non-trivial and I can't really 
spend much more time on this.


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

https://reviews.llvm.org/D100256

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

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -86,6 +86,8 @@
   const NativeProcessProtocol::Factory _process_factory;
   lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
   lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
+  NativeProcessProtocol *m_current_process;
+  NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
   std::unique_ptr m_debugged_process_up;
 
@@ -254,7 +256,8 @@
   // In any case, the function assumes that exactly one inferior is being
   // debugged and rejects pid values that do no match that inferior.
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
-  bool allow_all = false);
+  bool allow_all,
+  lldb::pid_t default_pid);
 
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
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
@@ -70,6 +70,7 @@
 : GDBRemoteCommunicationServerCommon("gdb-remote.server",
  "gdb-remote.server.rx_packet"),
   m_mainloop(mainloop), m_process_factory(process_factory),
+  m_current_process(nullptr), m_continue_process(nullptr),
   m_stdio_communication("process.stdio") {
   RegisterPacketHandlers();
 }
@@ -249,6 +250,7 @@
 if (!process_or)
   return Status(process_or.takeError());
 m_debugged_process_up = std::move(*process_or);
+m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  m_continue_process = m_current_process = m_debugged_process_up.get();
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -735,15 +738,15 @@
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
   // Ensure we have a debugged process.
-  if (!m_debugged_process_up ||
-  (m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
+  if (!m_current_process ||
+  (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
 return SendErrorResponse(50);
 
   LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
-   m_debugged_process_up->GetID(), tid);
+   m_current_process->GetID(), tid);
 
   // Ensure we can get info on the given thread.
-  NativeThreadProtocol *thread = m_debugged_process_up->GetThreadByID(tid);
+  NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
   if (!thread)
 return SendErrorResponse(51);
 
@@ -766,7 +769,7 @@
   LLDB_LOG(
   log,
   "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
-  m_debugged_process_up->GetID(), tid, signum, int(tid_stop_info.reason),
+  m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
   tid_stop_info.details.exception.type);
 
   // Print the signal number.
@@ -804,9 +807,9 @@
 
 uint32_t thread_index = 0;
 NativeThreadProtocol *listed_thread;
-for (listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index);
+for (listed_thread = m_current_process->GetThreadAtIndex(thread_index);
  listed_thread; ++thread_index,
-listed_thread = m_debugged_process_up->GetThreadAtIndex(thread_index)) {
+listed_thread = 

[Lldb-commits] [lldb] 872b1da - [lldb/test] s/add_no_ack_remote_stream/do_handshake

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-13T17:10:32+02:00
New Revision: 872b1da6ad276515ccfb1481009d23b129c72cac

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

LOG: [lldb/test] s/add_no_ack_remote_stream/do_handshake

These two functions are doing the same thing, only one of them is
sending the packets immediately and the other "queues" them to be sent
later. The first one is better as in case of errors, the backtrace will
point straight to the place that caused them.

Modify the first method to avoid duplication, and ten standardize on it.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachOrWait.py
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
lldb/test/API/tools/lldb-server/TestGdbRemoteCompletion.py
lldb/test/API/tools/lldb-server/TestGdbRemoteHostInfo.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/commandline/TestGdbRemoteConnection.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 5964b24203757..5451877a7bdd4 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -521,8 +521,9 @@ def prep_debug_monitor_and_inferior(
 server = self.connect_to_debug_monitor(attach_pid=attach_pid)
 self.assertIsNotNone(server)
 
+self.do_handshake()
+
 # Build the expected protocol stream
-self.add_no_ack_remote_stream()
 if inferior_env:
 for name, value in inferior_env.items():
 self.add_set_environment_packets(name, value)
@@ -531,60 +532,13 @@ def prep_debug_monitor_and_inferior(
 
 return {"inferior": inferior, "server": server}
 
-def expect_socket_recv(
-self,
-sock,
-expected_content_regex
-):
-response = ""
-timeout_time = time.time() + self.DEFAULT_TIMEOUT
-
-while not expected_content_regex.match(
-response) and time.time() < timeout_time:
-can_read, _, _ = select.select([sock], [], [], 
self.DEFAULT_TIMEOUT)
-if can_read and sock in can_read:
-recv_bytes = sock.recv(4096)
-if recv_bytes:
-response += seven.bitcast_to_string(recv_bytes)
-
-self.assertTrue(expected_content_regex.match(response))
-
-def expect_socket_send(self, sock, content):
-request_bytes_remaining = content
-timeout_time = time.time() + self.DEFAULT_TIMEOUT
-
-while len(request_bytes_remaining) > 0 and time.time() < timeout_time:
-_, can_write, _ = select.select([], [sock], [], 
self.DEFAULT_TIMEOUT)
-if can_write and sock in can_write:
-written_byte_count = 
sock.send(request_bytes_remaining.encode())
-request_bytes_remaining = request_bytes_remaining[
-written_byte_count:]
-self.assertEqual(len(request_bytes_remaining), 0)
-
-def do_handshake(self, stub_socket):
-# Write the ack.
-self.expect_socket_send(stub_socket, "+")
-
-# Send the start no ack mode packet.
-NO_ACK_MODE_REQUEST = "$QStartNoAckMode#b0"
-bytes_sent = stub_socket.send(NO_ACK_MODE_REQUEST.encode())
-self.assertEqual(bytes_sent, len(NO_ACK_MODE_REQUEST))
-
-# Receive the ack and "OK"
-self.expect_socket_recv(stub_socket, re.compile(
-r"^\+\$OK#[0-9a-fA-F]{2}$"))
-
-# Send the final ack.
-self.expect_socket_send(stub_socket, "+")
-
-def add_no_ack_remote_stream(self):
-self.test_sequence.add_log_lines(
-["read packet: +",
- "read packet: $QStartNoAckMode#b0",
- "send packet: +",
- "send packet: $OK#9a",
- "read packet: +"],
-True)
+def do_handshake(self):
+server = self._server
+server.send_ack()
+server.send_packet(b"QStartNoAckMode")
+self.assertEqual(server.get_normal_packet(), b"+")
+self.assertEqual(server.get_normal_packet(), b"OK")
+server.send_ack()
 
 def add_verified_launch_packets(self, launch_args):
 self.test_sequence.add_log_lines(

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 

[Lldb-commits] [PATCH] D100256: [lldb] [gdb-remote server] Abstract away getting current process

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D100256#2685807 , @mgorny wrote:

> In D100256#2685632 , @labath wrote:
>
>> Does this actually need to be a function? It seems like this could just be a 
>> pointer variable (or two), pointing into our pool of processes. I'd consider 
>> making a small struct to group the current process and the current thread 
>> within that process.
>
> I suppose a pointer might work for the proposed design. However, I'm 
> wondering why we have the `GetID()` check here — can we have an invalid value 
> of `m_debugged_process_up` then?

I think someone was just being overly cautious. I'm pretty sure the check is 
redundant...


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

https://reviews.llvm.org/D100256

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

[Lldb-commits] [lldb] 29a4d78 - [lldb][AArch64] Only run MTE memory region test if we have MTE

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-04-13T15:40:38+01:00
New Revision: 29a4d7813c75c1fbbd7afef33d32dde92598d1ad

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

LOG: [lldb][AArch64] Only run MTE memory region test if we have MTE

This test is flakey because it tries to read the proc/smaps
file of the first lldb-server process it finds. This process
can finish before we finish doing that.

http://lab.llvm.org:8011/#/builders/96/builds/6634/steps/6/logs/stdio

For now limit this to MTE targets which basically means
QEMU via lldb-dotest, which doesn't have this issue.

I'll fix the race condition shortly.

Added: 


Modified: 

lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py

Removed: 




diff  --git 
a/lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
 
b/lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
index ff8e01cb28c8c..3855a5aa5d73a 100644
--- 
a/lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
+++ 
b/lldb/test/API/linux/aarch64/mte_memory_region/TestAArch64LinuxMTEMemoryRegion.py
@@ -20,6 +20,8 @@ class AArch64LinuxMTEMemoryRegionTestCase(TestBase):
 @skipIf(archs=no_match(["aarch64"]))
 @skipUnlessPlatform(["linux"])
 def test_mte_regions(self):
+if not self.isAArch64MTE():
+self.skipTest('Target must support MTE.')
 if not self.hasLinuxVmFlags():
 self.skipTest('/proc/{pid}/smaps VmFlags must be present')
 



___
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.

[Caroline Tice via Phabricator via lldb-commits]

cmtice added a comment.

Pavel, your last comment lost me completely.  How can I test the code I added 
to lldb if I don't run lldb?  I am a complete newbie at writing test cases so 
there's probably something basic I'm missing?  How would I observe the value of 
the variable without running lldb?  Also, if the program doesn't contain 
'main', won't the compiler complain?  Is there an existing test that does what 
you're suggesting that I could look at?


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] [PATCH] D100256: [lldb] [gdb-remote server] Abstract away getting current process

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

mgorny added a comment.

In D100256#2685632 , @labath wrote:

> Does this actually need to be a function? It seems like this could just be a 
> pointer variable (or two), pointing into our pool of processes. I'd consider 
> making a small struct to group the current process and the current thread 
> within that process.

I suppose a pointer might work for the proposed design. However, I'm wondering 
why we have the `GetID()` check here — can we have an invalid value of 
`m_debugged_process_up` then?


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

https://reviews.llvm.org/D100256

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

[Lldb-commits] [lldb] d7ce89c - [lldb] Remove self-skipping code from lldb-server tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-13T16:23:43+02:00
New Revision: d7ce89c769d2a518efef5cc4eda152ded33b7ddf

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

LOG: [lldb] Remove self-skipping code from lldb-server tests

We already do category based skipping in checkDebugServerSupport in
dotest.py.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index a28d0634061ba..5964b24203757 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -227,8 +227,6 @@ def _init_llgs_test(self):
 self.debug_monitor_exe = re.sub(r' \(deleted\)$', '', exe)
 else:
 self.debug_monitor_exe = get_lldb_server_exe()
-if not self.debug_monitor_exe:
-self.skipTest("lldb-server exe not found")
 
 self.debug_monitor_extra_args = ["gdbserver"]
 self.setUpServerLogging(is_llgs=True)
@@ -237,8 +235,6 @@ def _init_llgs_test(self):
 
 def _init_debugserver_test(self):
 self.debug_monitor_exe = get_debugserver_exe()
-if not self.debug_monitor_exe:
-self.skipTest("debugserver exe not found")
 self.setUpServerLogging(is_llgs=False)
 self.reverse_connect = True
 



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

[Lldb-commits] [lldb] 14b9f32 - [lldb] Remote @debugserver_test from TestAppleSimulatorOSType

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-13T16:09:50+02:00
New Revision: 14b9f320fef9888ab63ff0e791060437a0e1dcc8

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

LOG: [lldb] Remote @debugserver_test from TestAppleSimulatorOSType

The annotation is now (since the introduction of @apple_simulator_test)
redundant, and the test could theoretically run on lldb-server too (if
it supported darwin hosts).

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py 
b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
index f44955ac838a6..467e6cdc69766 100644
--- a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
+++ b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
@@ -131,21 +131,18 @@ def check_simulator_ostype(self, sdk, platform_name, 
arch=platform.machine()):
 
 
 @apple_simulator_test('iphone')
-@debugserver_test
 @skipIfRemote
 def test_simulator_ostype_ios(self):
 self.check_simulator_ostype(sdk='iphonesimulator',
 platform_name='ios')
 
 @apple_simulator_test('appletv')
-@debugserver_test
 @skipIfRemote
 def test_simulator_ostype_tvos(self):
 self.check_simulator_ostype(sdk='appletvsimulator',
 platform_name='tvos')
 
 @apple_simulator_test('watch')
-@debugserver_test
 @skipIfRemote
 def test_simulator_ostype_watchos(self):
 self.check_simulator_ostype(sdk='watchsimulator',



___
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.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

One more thing I forgot to mention: In the all-paths-are-relative scenario, 
lldb will need some help with finding the source files. I believe that 
currently these will still be resolved relative to CWD, and that's something 
you may not want (?) Unlike dwos, this can be worked around currently by 
manually setting the target.source-map setting. It's not for this patch, but I 
think this is something you may want  to happen automatically as well (?)




Comment at: lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s:11
+
+# 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

Could you make the test case not involve running (so that it can work on 
non-linux systems too)? You should be able to simply observe the value of a 
global variable (target variable x) or something like that. Also, that means 
the executable does not need to contain the main function (and everything that 
goes with it), which would significantly reduce the size of the test input.


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] [PATCH] D100384: [lldb/Plugins] Update ScriptedProcess Process Plugin

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, jingham, jasonmolenda.
mib added a project: LLDB.
Herald added a subscriber: mgorny.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch is an update of D95713  since the 
commit have been reverted.

It fixes the hangs, crashes and other asynchronous issues by adopting a 
`Listener-Broadcaster` model for Scripted Processes.

rdar://65508855

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100384

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -11,7 +11,7 @@
 from lldbsuite.test import lldbtest
 
 
-class PlatformProcessCrashInfoTestCase(TestBase):
+class ScriptedProcesTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
@@ -43,3 +43,55 @@
 self.expect('script dir(ScriptedProcess)',
 substrs=["launch"])
 
+def test_launch_scripted_process_sbapi(self):
+"""Test that we can launch an lldb scripted process using the SBAPI,
+check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetProcessPluginName("ScriptedProcess")
+launch_info.SetScriptedProcessClassName("my_scripted_process.MyScriptedProcess")
+
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
+
+def test_launch_scripted_process_cli(self):
+"""Test that we can launch an lldb scripted process from the command
+line, check its process ID and read string from memory."""
+self.build()
+target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target, VALID_TARGET)
+
+scripted_process_example_relpath = ['..','..','..','..','examples','python','scripted_process','my_scripted_process.py']
+self.runCmd("command script import " + os.path.join(self.getSourceDir(),
+*scripted_process_example_relpath))
+
+process = target.GetProcess()
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertEqual(process.GetProcessID(), 42)
+
+error = lldb.SBError()
+hello_world = "Hello, world!"
+memory_read = process.ReadCStringFromMemory(0x500,
+len(hello_world) + 1, # NULL byte
+error)
+
+self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
+self.assertEqual(hello_world, memory_read)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2972,7 +2972,7 @@
   // If we're not already connected to the process, and if we have a platform
   // that can launch a process for debugging, go ahead and do that here.
   if (state != eStateConnected && platform_sp &&
-  platform_sp->CanDebugProcess()) {
+  platform_sp->CanDebugProcess() && !launch_info.IsScriptedProcess()) {
 LLDB_LOGF(log, "Target::%s asking the platform to debug the process",
   __FUNCTION__);
 
Index: 

[Lldb-commits] [PATCH] D100256: [lldb] [gdb-remote server] Abstract away getting current process

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Does this actually need to be a function? It seems like this could just be a 
pointer variable (or two), pointing into our pool of processes. I'd consider 
making a small struct to group the current process and the current thread 
within that process.


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

https://reviews.llvm.org/D100256

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny added a comment.

@labath, if we're going to remove `m_debugged_process_up`, then I suppose it 
makes sense to merge D100256  first. Could 
you review that one, please?


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

https://reviews.llvm.org/D100191

___
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

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

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

Rebase + 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 );
+
   // 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,8 @@
 
   // 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) {
Index: 

[Lldb-commits] [lldb] f152472 - [lldb] Require x86 for various NativePDB, Breakpad and Minidump tests

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-04-13T12:51:48Z
New Revision: f152472af576e77f9b8736dafaffd31fce7b13b0

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

LOG: [lldb] Require x86 for various NativePDB, Breakpad and Minidump tests

These tests fail if you build without the x86 llvm backend.
Either because they use an x86 triple or try to backtrace which
requires some x86 knowledge to see all frames.

Reviewed By: labath

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

Added: 


Modified: 
lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
lldb/test/Shell/Minidump/disassemble-no-module.yaml
lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp
lldb/test/Shell/SymbolFile/symbol-binding.test

Removed: 




diff  --git 
a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py 
b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 76fd8746aa2a1..39e96a65f7389 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -124,6 +124,7 @@ def test_thread_info_in_minidump(self):
 stop_description = thread.GetStopDescription(256)
 self.assertIn("SIGSEGV", stop_description)
 
+@skipIfLLVMTargetMissing("X86")
 def test_stack_info_in_minidump(self):
 """Test that we can see a trivial stack in a breakpad-generated 
Minidump."""
 # target create linux-x86_64 -c linux-x86_64.dmp
@@ -362,6 +363,7 @@ def do_change_pid_in_minidump(self, core, newcore, offset, 
oldpid, newpid):
 newpid += "\n"
 f.write(newpid.encode('utf-8'))
 
+@skipIfLLVMTargetMissing("X86")
 def test_deeper_stack_in_minidump_with_same_pid_running(self):
 """Test that we read the information from the core correctly even if we
 have a running process with the same PID"""
@@ -373,6 +375,7 @@ def 
test_deeper_stack_in_minidump_with_same_pid_running(self):
str(os.getpid()))
 self.do_test_deeper_stack("linux-x86_64_not_crashed", new_core, 
os.getpid())
 
+@skipIfLLVMTargetMissing("X86")
 def test_two_cores_same_pid(self):
 """Test that we handle the situation if we have two core files with 
the same PID """
 new_core = self.getBuildArtifact("linux-x86_64_not_crashed-pid.dmp")
@@ -385,6 +388,7 @@ def test_two_cores_same_pid(self):
   new_core, self._linux_x86_64_pid)
 self.test_stack_info_in_minidump()
 
+@skipIfLLVMTargetMissing("X86")
 def test_local_variables_in_minidump(self):
 """Test that we can examine local variables in a Minidump."""
 # Launch with the Minidump, and inspect a local variable.

diff  --git a/lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test 
b/lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
index dfa98c5665611..e3f1e33e0ff0f 100644
--- a/lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
+++ b/lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
@@ -1,3 +1,5 @@
+// REQUIRES: x86
+
 // RUN: cd %p/Inputs
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 \
 // RUN:   %lldb -c sigsegv.dmp -s sigsegv.lldbinit | FileCheck %s

diff  --git a/lldb/test/Shell/Minidump/disassemble-no-module.yaml 
b/lldb/test/Shell/Minidump/disassemble-no-module.yaml
index 64cd1145f317c..00c12c8b9ab93 100644
--- a/lldb/test/Shell/Minidump/disassemble-no-module.yaml
+++ b/lldb/test/Shell/Minidump/disassemble-no-module.yaml
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # RUN: yaml2obj %s -o %t
 # RUN: %lldb -c %t -o bt -o disassemble 2>&1 | FileCheck %s
 

diff  --git a/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test 
b/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
index 1c1dabec59447..0bcebaa00540d 100644
--- a/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
+++ b/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # RUN: yaml2obj %S/Inputs/unwind-via-stack-win.yaml -o %t
 # RUN: %lldb -c %t \
 # RUN:   -o "target symbols add %S/Inputs/unwind-via-raSearch.syms" \

diff  --git 
a/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml 
b/lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
index 8309c9e105e00..6075c33bb88ab 100644
--- 

[Lldb-commits] [PATCH] D100194: [lldb] Require x86 for various NativePDB, Breakpad and Minidump tests

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf152472af576: [lldb] Require x86 for various NativePDB, 
Breakpad and Minidump tests (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100194

Files:
  lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
  lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
  lldb/test/Shell/Minidump/disassemble-no-module.yaml
  lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
  lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
  lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
  lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
  lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp
  lldb/test/Shell/SymbolFile/symbol-binding.test

Index: lldb/test/Shell/SymbolFile/symbol-binding.test
===
--- lldb/test/Shell/SymbolFile/symbol-binding.test
+++ lldb/test/Shell/SymbolFile/symbol-binding.test
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # Some targets do not have the .size directive.
 # RUN: %clang -target x86_64-unknown-unknown-elf %S/Inputs/symbol-binding.s -c -o %t.o
 # RUN: %lldb %t.o -s %s -o quit | FileCheck %s
Index: lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/s_constant.cpp
@@ -1,5 +1,5 @@
 // clang-format off
-// REQUIRES: lld
+// REQUIRES: lld, x86
 
 // Test that we can display S_CONSTANT records.
 
Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
@@ -1,5 +1,5 @@
 // clang-format off
-// REQUIRES: lld
+// REQUIRES: lld, x86
 
 // RUN: %clang_cl --target=i386-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
Index: lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
@@ -1,5 +1,5 @@
 // clang-format off
-// REQUIRES: lld
+// REQUIRES: lld, x86
 
 // Test that we can show disassembly and source.
 // RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
Index: lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
===
--- lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
+++ lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # RUN: yaml2obj --docnum=1 %s -o %t.dmp
 # RUN: yaml2obj --docnum=2 %s -o %T/unwind-via-stack-win-no-memory-info.exe
 # RUN: %lldb -c %t.dmp %T/unwind-via-stack-win-no-memory-info.exe \
Index: lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
+++ lldb/test/Shell/SymbolFile/Breakpad/unwind-via-raSearch.test
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # RUN: yaml2obj %S/Inputs/unwind-via-stack-win.yaml -o %t
 # RUN: %lldb -c %t \
 # RUN:   -o "target symbols add %S/Inputs/unwind-via-raSearch.syms" \
Index: lldb/test/Shell/Minidump/disassemble-no-module.yaml
===
--- lldb/test/Shell/Minidump/disassemble-no-module.yaml
+++ lldb/test/Shell/Minidump/disassemble-no-module.yaml
@@ -1,3 +1,5 @@
+# REQUIRES: x86
+
 # RUN: yaml2obj %s -o %t
 # RUN: %lldb -c %t -o bt -o disassemble 2>&1 | FileCheck %s
 
Index: lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
===
--- lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
+++ lldb/test/Shell/Minidump/Windows/Sigsegv/sigsegv.test
@@ -1,3 +1,5 @@
+// REQUIRES: x86
+
 // RUN: cd %p/Inputs
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 \
 // RUN:   %lldb -c sigsegv.dmp -s sigsegv.lldbinit | FileCheck %s
Index: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
===
--- lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -124,6 +124,7 @@
 stop_description = thread.GetStopDescription(256)
 self.assertIn("SIGSEGV", stop_description)
 
+@skipIfLLVMTargetMissing("X86")
 def test_stack_info_in_minidump(self):
 

[Lldb-commits] [lldb] c8d18cb - Reland "[lldb] [Process] Watch for fork/vfork notifications" for Linux

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T14:38:31+02:00
New Revision: c8d18cba4e2f63f2135749ef87d09e7f4f405712

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

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

Big thanks to Pavel Labath for figuring out my mistake.

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

Added: 
lldb/include/lldb/Host/linux/Host.h

Modified: 
lldb/source/Host/linux/Host.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/test/Shell/Subprocess/clone-follow-parent-wp.test

Removed: 




diff  --git a/lldb/include/lldb/Host/linux/Host.h 
b/lldb/include/lldb/Host/linux/Host.h
new file mode 100644
index 0..409a9fc9b3220
--- /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 520a00df35f6d..334634a357f57 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 
Environment(environ); }
 Status Host::ShellExpandArguments(ProcessLaunchInfo _info) {
   return Status("unimplemented");
 }
+
+llvm::Optional lldb_private::getPIDForTID(lldb::pid_t tid) {
+  ::pid_t tracerpid, tgid = LLDB_INVALID_PROCESS_ID;
+  ProcessInstanceInfo process_info;
+  ProcessState state;
+
+  if (!GetStatusInfo(tid, process_info, state, tracerpid, tgid) ||
+  tgid == LLDB_INVALID_PROCESS_ID)
+return llvm::None;
+  return tgid;
+}

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5a876ce5befec..40ba5e2e604b2 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -30,6 +30,7 @@
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
+#include "lldb/Host/linux/Host.h"
 #include "lldb/Host/linux/Ptrace.h"
 #include "lldb/Host/linux/Uio.h"
 #include "lldb/Host/posix/ProcessLauncherPosixFork.h"
@@ -383,14 +384,22 @@ Status 
NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t pid) {
   ptrace_opts |= PTRACE_O_TRACEEXIT;
 
   // Have the tracer trace threads which spawn in the inferior process.
-  // TODO: if we want to support tracing the 

[Lldb-commits] [PATCH] D100194: [lldb] Require x86 for various NativePDB, Breakpad and Minidump tests

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml:13
 # CHECK: frame #2: 0x000b1085 unwind-via-stack-win-no-memory-info.exe`main + 5
 # CHECK: frame #3: 0x77278494 kernel32.dll
 # CHECK-NOT: frame

DavidSpickett wrote:
> @labath In this case, without the x86 backend we do get the first 3 frames 
> just not the kernel32.dll frame.
> 
> ```
> (lldb) target symbols add 
> ../llvm-project/lldb/test/Shell/SymbolFile/Breakpad/Inputs/unwind-via-raSearch.syms
> symbol file 
> '/home/david.spickett/llvm-project/lldb/test/Shell/SymbolFile/Breakpad/Inputs/unwind-via-raSearch.syms'
>  has been added to '/home/david.spickett/build-llvm-aarch64/foo/foo.exe'
> (lldb) thread backtrace
> * thread #1
>   * frame #0: 0x000b1092 foo.exe`many_pointer_args + 2
> frame #1: 0x000b1079 foo.exe`call_many + 105
> frame #2: 0x000b1085 foo.exe`main + 
> ```
> 
> So my guess is that the last frame requires x86 specific knowledge to work 
> out. If you think that's not the case or would prefer I investigate first I 
> can remove it from this patch.
Nah, that's fine. I'm pretty sure that is what's happening, and overall, I 
would actually say lldb should flat out refuse to open binaries/core files/etc. 
for which it does not have an appropriate back end


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100194

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

[Lldb-commits] [lldb] 7da3b44 - Reland "[lldb] [Process] Watch for fork/vfork notifications" for NetBSD

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T14:35:44+02:00
New Revision: 7da3b44d67f81e4cff3ac4f72888e667bd9e6adb

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

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

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

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

Modified: 
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp 
b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
index 57f0eb3cceb65..1f357e3e96d79 100644
--- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -256,6 +256,24 @@ void NativeProcessNetBSD::MonitorSIGTRAP(lldb::pid_t pid) {
 SetState(StateType::eStateStopped, true);
 return;
   }
+  case TRAP_CHLD: {
+ptrace_state_t pst;
+Status error = PtraceWrapper(PT_GET_PROCESS_STATE, pid, , sizeof(pst));
+if (error.Fail()) {
+  SetState(StateType::eStateInvalid);
+  return;
+}
+
+if (pst.pe_report_event == PTRACE_VFORK_DONE) {
+  Status error =
+  PtraceWrapper(PT_CONTINUE, pid, reinterpret_cast(1), 0);
+  if (error.Fail())
+SetState(StateType::eStateInvalid);
+  return;
+} else
+  MonitorClone(pst.pe_other_pid);
+return;
+  }
   case TRAP_LWP: {
 ptrace_state_t pst;
 Status error = PtraceWrapper(PT_GET_PROCESS_STATE, pid, , sizeof(pst));
@@ -510,7 +528,7 @@ Status NativeProcessNetBSD::Detach() {
   if (GetID() == LLDB_INVALID_PROCESS_ID)
 return error;
 
-  return PtraceWrapper(PT_DETACH, GetID());
+  return PtraceWrapper(PT_DETACH, GetID(), reinterpret_cast(1));
 }
 
 Status NativeProcessNetBSD::Signal(int signo) {
@@ -738,17 +756,17 @@ Status NativeProcessNetBSD::GetFileLoadAddress(const 
llvm::StringRef _name,
 
 void NativeProcessNetBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-  // 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.
+return;
 
   if (wait_pid == -1) {
 Status error(errno, eErrorTypePOSIX);
 LLDB_LOG(log, "waitpid ({0}, , _) failed: {1}", GetID(), error);
+return;
   }
 
   WaitStatus wait_status = WaitStatus::Decode(status);
@@ -936,8 +954,9 @@ Status NativeProcessNetBSD::SetupTrace() {
   PtraceWrapper(PT_GET_EVENT_MASK, GetID(), , sizeof(events));
   if (status.Fail())
 return status;
-  // TODO: PTRACE_FORK | PTRACE_VFORK | PTRACE_POSIX_SPAWN?
-  events.pe_set_event |= PTRACE_LWP_CREATE | PTRACE_LWP_EXIT;
+  // TODO: PTRACE_POSIX_SPAWN?
+  events.pe_set_event |= PTRACE_LWP_CREATE | PTRACE_LWP_EXIT | PTRACE_FORK |
+ PTRACE_VFORK | PTRACE_VFORK_DONE;
   status = PtraceWrapper(PT_SET_EVENT_MASK, GetID(), , sizeof(events));
   if (status.Fail())
 return status;
@@ -974,3 +993,39 @@ Status NativeProcessNetBSD::ReinitializeThreads() {
 
   return error;
 }
+
+void NativeProcessNetBSD::MonitorClone(::pid_t child_pid) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
+  LLDB_LOG(log, "clone, child_pid={0}", child_pid);
+
+  int status;
+  ::pid_t wait_pid =
+  llvm::sys::RetryAfterSignal(-1, ::waitpid, child_pid, , 0);
+  if (wait_pid != child_pid) {
+LLDB_LOG(log,
+ "waiting for pid {0} failed. Assuming the pid has "
+ "disappeared in the meantime",
+ child_pid);
+return;
+  }
+  if (WIFEXITED(status)) {
+LLDB_LOG(log,
+ "waiting for pid {0} returned an 'exited' event. Not "
+ "tracking it.",
+ child_pid);
+return;
+  }
+
+  MainLoop unused_loop;
+  NativeProcessNetBSD child_process{static_cast<::pid_t>(child_pid),
+m_terminal_fd, m_delegate, m_arch,
+unused_loop};
+  child_process.Detach();
+  Status pt_error =
+  PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast(1), 0);
+  if (pt_error.Fail()) {
+LLDB_LOG_ERROR(log, std::move(pt_error.ToError()),
+   "unable to resume parent process {1}: {0}", GetID());
+SetState(StateType::eStateInvalid);
+  }
+}

diff  --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h 
b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
index 3d59a4f72e94e..3384078a0d6d0 100644
--- 

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

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

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



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:904-905
+  NativeThreadLinux _thread = AddThread(child_pid, /*resume*/ true);
+  // Resume the newly created thread.
+  ResumeThread(child_thread, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
+  ThreadWasCreated(child_thread);

labath wrote:
> This is resuming the thread a second time. If the thread manages to stop 
> between the resume inside AddThread, and here, we will miss the breakpoint 
> (and subsequently crash).
> 
> Deleting these two lines should fix things.
Hmm, I wonder why it ended up being added here. Probably my mistake, sorry, and 
thanks for debugging it. I'm going to test and reland it shortly!


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] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D98822#2685256 , @mgorny wrote:

> In D98822#2685246 , @labath wrote:
>
>> I've reverted this (121cff78a8032a73aa4fb820625dc1ecae8e3997 
>> ) 
>> because it made the linux bot super-flaky. I think it's likely that this 
>> affects _only_ linux (and the BSD code is fine -- though we don't have a bot 
>> to verify that), but I didn't want to experiment with partial reverts while 
>> the bots are wonky. However, it may be interesting the re-land the linux 
>> support in a separate commit, once we figure out the problem -- my money is 
>> on the nondeterminism of thread creation.
>
> How about recommitting without the `clone()` change, i.e. assuming that 
> `clone` always means thread, and seeing if that causes trouble still?

I think I have it figured out -- see the inlined comment. It seems this was one 
of those (rare) cases where enabling logging actually makes a race _more_ 
likely...




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:904-905
+  NativeThreadLinux _thread = AddThread(child_pid, /*resume*/ true);
+  // Resume the newly created thread.
+  ResumeThread(child_thread, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
+  ThreadWasCreated(child_thread);

This is resuming the thread a second time. If the thread manages to stop 
between the resume inside AddThread, and here, we will miss the breakpoint (and 
subsequently crash).

Deleting these two lines should fix things.


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] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1036
+std::unique_ptr _process) {
+  // Apparently the process has been dealt with by another delegate.
+  if (!child_process)

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > You no longer have to worry about that...
> > Don't I? The process plugin puts the new instance in an `std::unique_ptr`, 
> > and passes that to all delegates. Only one delegate can take the pointer 
> > over. While I don't think we really have a case for multiple delegates 
> > doing `NewSubprocess()`, I suppose we should check rather than crash. Or 
> > maybe just put an `assert` for it.
> I deleted the multi-delegate thingy in c9cf394f796e1 ;)
Ah, cool, I'll get to rebasing shortly.


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1036
+std::unique_ptr _process) {
+  // Apparently the process has been dealt with by another delegate.
+  if (!child_process)

mgorny wrote:
> labath wrote:
> > You no longer have to worry about that...
> Don't I? The process plugin puts the new instance in an `std::unique_ptr`, 
> and passes that to all delegates. Only one delegate can take the pointer 
> over. While I don't think we really have a case for multiple delegates doing 
> `NewSubprocess()`, I suppose we should check rather than crash. Or maybe just 
> put an `assert` for it.
I deleted the multi-delegate thingy in c9cf394f796e1 ;)


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [lldb] 63d7564 - Reland "[lldb] [Process] Watch for fork/vfork notifications" for FreeBSD

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T13:19:42+02:00
New Revision: 63d75641054afb0fe43928e9430a2bb92f09e121

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

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

The original commit was reverted because of the problems it introduced
on Linux.  However, FreeBSD should not be affected, so restore that part
and we will address Linux separately.

While at it, remove the dbreg hack as the underlying issue has been
fixed in the FreeBSD kernel and the problem is unlikely to happen
in real life use anyway.

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

Added: 
lldb/test/Shell/Subprocess/Inputs/fork.cpp
lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
lldb/test/Shell/Subprocess/fork-follow-parent.test
lldb/test/Shell/Subprocess/lit.local.cfg
lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
lldb/test/Shell/Subprocess/vfork-follow-parent.test

Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 5961ff4439db..67a7b737c7d6 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -247,6 +247,19 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) 
{
 return;
   }
 
+  if (info.pl_flags & PL_FLAG_FORKED) {
+MonitorClone(info.pl_child_pid);
+return;
+  }
+
+  if (info.pl_flags & PL_FLAG_VFORK_DONE) {
+Status error =
+PtraceWrapper(PT_CONTINUE, pid, reinterpret_cast(1), 0);
+if (error.Fail())
+  SetState(StateType::eStateInvalid);
+return;
+  }
+
   if (info.pl_lwpid > 0) {
 for (const auto  : m_threads) {
   if (t->GetID() == static_cast(info.pl_lwpid))
@@ -705,17 +718,17 @@ NativeProcessFreeBSD::GetFileLoadAddress(const 
llvm::StringRef _name,
 
 void NativeProcessFreeBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-  // Process all pending waitpid notifications.
   int status;
   ::pid_t wait_pid =
   llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), , WNOHANG);
 
   if (wait_pid == 0)
-return; // We are done.
+return;
 
   if (wait_pid == -1) {
 Status error(errno, eErrorTypePOSIX);
 LLDB_LOG(log, "waitpid ({0}, , _) failed: {1}", GetID(), error);
+return;
   }
 
   WaitStatus wait_status = WaitStatus::Decode(status);
@@ -885,7 +898,7 @@ Status NativeProcessFreeBSD::SetupTrace() {
   PtraceWrapper(PT_GET_EVENT_MASK, GetID(), , sizeof(events));
   if (status.Fail())
 return status;
-  events |= PTRACE_LWP;
+  events |= PTRACE_LWP | PTRACE_FORK | PTRACE_VFORK;
   status = PtraceWrapper(PT_SET_EVENT_MASK, GetID(), , sizeof(events));
   if (status.Fail())
 return status;
@@ -919,3 +932,39 @@ Status NativeProcessFreeBSD::ReinitializeThreads() {
 bool NativeProcessFreeBSD::SupportHardwareSingleStepping() const {
   return !m_arch.IsMIPS();
 }
+
+void NativeProcessFreeBSD::MonitorClone(::pid_t child_pid) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
+  LLDB_LOG(log, "fork, child_pid={0}", child_pid);
+
+  int status;
+  ::pid_t wait_pid =
+  llvm::sys::RetryAfterSignal(-1, ::waitpid, child_pid, , 0);
+  if (wait_pid != child_pid) {
+LLDB_LOG(log,
+ "waiting for pid {0} failed. Assuming the pid has "
+ "disappeared in the meantime",
+ child_pid);
+return;
+  }
+  if (WIFEXITED(status)) {
+LLDB_LOG(log,
+ "waiting for pid {0} returned an 'exited' event. Not "
+ "tracking it.",
+ child_pid);
+return;
+  }
+
+  MainLoop unused_loop;
+  NativeProcessFreeBSD child_process{static_cast<::pid_t>(child_pid),
+ m_terminal_fd, m_delegate, m_arch,
+ unused_loop};
+  child_process.Detach();
+  Status pt_error =
+  PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast(1), 0);
+  if (pt_error.Fail()) {
+LLDB_LOG_ERROR(log, pt_error.ToError(),
+   "unable to resume parent process {1}: {0}", GetID());
+SetState(StateType::eStateInvalid);
+  }
+}

diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
index ceffc370ca33..3ed5f743deba 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -113,6 +113,7 @@ class NativeProcessFreeBSD : public NativeProcessELF,
   

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny added a comment.

In D100191#2685039 , @labath wrote:

> I like this a lot more than the previous version. The thing I'd like to know 
> is, whether we can replace `m_additional_processes` with something like 
> `m_debugged_processes` (i.e., just have a single list of all processes we are 
> debugging. We could replace all occurrences of `m_debugged_process_up` with 
> `m_current_process`, which just points inside that list. Is there any reason 
> for the "privileged" position of the original process? The way I see it, most 
> of the operations would just work even if we treated them as equals. The only 
> thing which might not work is resuming (or TBE, reporting the stops) of the 
> subprocesses, but if we wanted to be safe, we could prevent that in some 
> other way (preventing the changing of process through Hc; returning 
> "crippled" process instances for subprocess, which return errors on resume 
> operations; ...).
>
> WDYT?

I've tried to limit changes to the minimum but sure, I can try doing that. I 
suppose it's going to make following children cleaner.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1036
+std::unique_ptr _process) {
+  // Apparently the process has been dealt with by another delegate.
+  if (!child_process)

labath wrote:
> You no longer have to worry about that...
Don't I? The process plugin puts the new instance in an `std::unique_ptr`, and 
passes that to all delegates. Only one delegate can take the pointer over. 
While I don't think we really have a case for multiple delegates doing 
`NewSubprocess()`, I suppose we should check rather than crash. Or maybe just 
put an `assert` for it.


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [lldb] aab81c2 - [lldb] [gdb-remote server] Refactor handling qSupported

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T12:12:25+02:00
New Revision: aab81c2f40d2098f9014473a1e7c8fb7b074360b

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

LOG: [lldb] [gdb-remote server] Refactor handling qSupported

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.

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

Added: 


Modified: 

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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index c933364cff9c..a0d88b3ab988 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -831,26 +831,10 @@ 
GDBRemoteCommunicationServerCommon::Handle_qPlatform_chmod(
 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::SmallVector client_features;
+  packet.GetStringRef().split(client_features, ';');
+  return SendPacketNoLock(llvm::join(HandleFeatures(client_features), ";"));
 }
 
 GDBRemoteCommunication::PacketResult
@@ -1312,3 +1296,18 @@ 
GDBRemoteCommunicationServerCommon::GetModuleInfo(llvm::StringRef module_path,
 
   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 server and llgs.
+  return {
+  llvm::formatv("PacketSize={0}", max_packet_size),
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",
+  };
+}

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
index 0f933c09cbd4..51d9fdd68037 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -145,6 +145,11 @@ class GDBRemoteCommunicationServerCommon : public 
GDBRemoteCommunicationServer {
   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(llvm::ArrayRef client_features);
+
 private:
   ModuleSpec GetModuleInfo(llvm::StringRef module_path, llvm::StringRef 
triple);
 };

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 2f615866a2f4..e9034cb7414a 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3534,3 +3534,16 @@ 

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

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rGaab81c2f40d2: [lldb] [gdb-remote server] Refactor handling 
qSupported (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D100140?vs=336977=337086#toc

Repository:
  rG LLVM Github Monorepo

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

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(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,10 @@
 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::SmallVector client_features;
+  packet.GetStringRef().split(client_features, ';');
+  return SendPacketNoLock(llvm::join(HandleFeatures(client_features), ";"));
 }
 
 GDBRemoteCommunication::PacketResult
@@ -1312,3 +1296,18 @@
 
   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 server and llgs.
+  return {
+  llvm::formatv("PacketSize={0}", max_packet_size),
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+ 

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

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

mgorny added a comment.

In D98822#2685246 , @labath wrote:

> I've reverted this (121cff78a8032a73aa4fb820625dc1ecae8e3997 
> ) 
> because it made the linux bot super-flaky. I think it's likely that this 
> affects _only_ linux (and the BSD code is fine -- though we don't have a bot 
> to verify that), but I didn't want to experiment with partial reverts while 
> the bots are wonky. However, it may be interesting the re-land the linux 
> support in a separate commit, once we figure out the problem -- my money is 
> on the nondeterminism of thread creation.

How about recommitting without the `clone()` change, i.e. assuming that `clone` 
always means thread, and seeing if that causes trouble still?


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] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I don't really understand what's going on, but in the one case I was able to 
catch this "in the act" the inferior process had one thread which was stopped 
one byte after the breakpoint location (there's no valid instruction there). 
This leads me to believe that the `FixupBreakpointPCAsNeeded` logic did not 
fire for some reason.


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] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1310-1311
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",

mgorny wrote:
> labath wrote:
> > I think that these two should also be llgs-specific.
> I actually left them here because the respective 
> `Handle_QThreadSuffixSupported` and `Handle_QListThreadsInStopReply` methods 
> are also in common.
Fair enough. I'll see what I can do about that once this patch lands.


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] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I've reverted this (121cff78a8032a73aa4fb820625dc1ecae8e3997 
) because 
it made the linux bot super-flaky. I think it's likely that this affects _only_ 
linux (and the BSD code is fine -- though we don't have a bot to verify that), 
but I didn't want to experiment with partial reverts while the bots are wonky. 
However, it may be interesting the re-land the linux support in a separate 
commit, once we figure out the problem -- my money is on the nondeterminism of 
thread creation.


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] D100153: [lldb] [Process] Introduce protocol extension support API

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

mgorny added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

labath wrote:
> Are you sure that returning success is the best "default" behavior? Maybe the 
> base implementation should always return an error (as it does not implement 
> any extensions)? Or return success, only if one enables an empty set of 
> extensions?
I'm not sure whether we need error handling here at all.

The current impl doesn't do anything but setting an instance var here. The 
original impl controlled events to `PTRACE_SETOPTIONS` but in the end I've 
decided to enable tracing fork/vfork unconditionally, and just using extensions 
to decide whether to handle it locally or defer to client.

I suppose it might make sense to review other patches first and get back to 
this one once we're sure what we need.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3592-3595
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");

labath wrote:
> ... or actually, I am wondering if this should not be a hard error/assertion. 
> In the current set up, I think it would be a programmer error if the factory 
> reports an extension as supported, but then the instance fails to enable it...
Technically, the original idea was that this would fail if `ptrace()` failed to 
set options. But as I said, this is no longer the case, so I'm not even sure if 
we need any error reporting here.


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] D100140: [lldb] [gdb-remote server] Refactor handling qSupported

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

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1310-1311
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",

labath wrote:
> I think that these two should also be llgs-specific.
I actually left them here because the respective 
`Handle_QThreadSuffixSupported` and `Handle_QListThreadsInStopReply` methods 
are also in common.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h:151
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+

labath wrote:
> this `const` is useless
Yeah, I probably meant to do `const ...&` but I guess it doesn't matter much 
for `ArrayRef`.


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] [lldb] ff31af4 - [lldb] [gdb-remote client] Refactor handling qSupported

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-13T11:20:11+02:00
New Revision: ff31af4f55af65d173cedcd2f09063c81c8f4f12

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

LOG: [lldb] [gdb-remote client] Refactor handling qSupported

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().

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index c032fc20b5f3..ef6441289fc5 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -345,6 +345,9 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   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 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   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 {
-supported_compressions.push_back(current_compression);
-current_compression = end_of_compressions;
-  }
+m_qSupported_response = response.GetStringRef().str();
+
+llvm::SmallVector 

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

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

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:375
+
+for (auto x : server_features) {
+  if (x == "qXfer:auxv:read+")

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > not a big deal, but this probably shouldn't be auto.
> > Could you explain a bit? I thought `auto` is convenient here since the 
> > actual type is visible three lines higher.
> Well.. my reasoning was something like this:
> - llvm guidelines say that auto should be used (only?) when the type is 
> obvious from context
> - "context" is not exactly specified, but I would take that to mean the 
> enclosing expression/statement (not the statement two lines up). The use of 
> `cast` in the example supports this.
> - the actual type is not that complicated (llvm::StringRef), so it e.g. won't 
> cause the line to be split in two
> - given the above, and the generally cautious approach to auto in llvm, its 
> better to err on the side of spelling out the type
> 
> But like I said, it's not a big deal...
Yeah, this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

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

[Lldb-commits] [lldb] 121cff7 - Revert "[lldb] [Process] Watch for fork/vfork notifications" and associated followups

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-13T11:03:06+02:00
New Revision: 121cff78a8032a73aa4fb820625dc1ecae8e3997

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

LOG: Revert "[lldb] [Process] Watch for fork/vfork notifications" and 
associated followups

This commit has caused the following tests to be flaky:
TestThreadSpecificBpPlusCondition.py
TestExitDuringExpression.py

The exact cause is not known yet, but since both tests deal with
threads, my guess is it has something to do with the tracking of
creation of new threads (which the commit touches upon).

This reverts the following commits:
d01bff8cbdc98fb8751f7bf10af19b47ae5c445d,
ba62ebc48e8c424ce3a78ba01acda679d536dd47,
e761b6b4c58d4f7ae1073d925d7cb321d68ee93a,
a345419ee03095c8cdfbe1c2728467c4da8fa0a4.

Added: 


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: 
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/lit.local.cfg
lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
lldb/test/Shell/Subprocess/vfork-follow-parent.test



diff  --git a/lldb/include/lldb/Host/linux/Host.h 
b/lldb/include/lldb/Host/linux/Host.h
deleted file mode 100644
index 409a9fc9b322..
--- a/lldb/include/lldb/Host/linux/Host.h
+++ /dev/null
@@ -1,22 +0,0 @@
-//===-- 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 334634a357f5..520a00df35f6 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -27,7 +27,6 @@
 #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"
 
@@ -54,8 +53,7 @@ class ProcessLaunchInfo;
 }
 
 static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo ,
-  ProcessState , ::pid_t ,
-  ::pid_t ) {
+  ProcessState , ::pid_t ) {
   Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST);
 
   auto BufferOrError = getProcFile(Pid, "status");
@@ -109,9 +107,6 @@ 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;
@@ -209,7 +204,6 @@ 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();
 
@@ -220,7 +214,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, 

[Lldb-commits] [PATCH] D100195: [lldb] Require x86 for unwind no-return test

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1f3187ca8a3: [lldb] Require x86 for unwind no-return test 
(authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100195

Files:
  
lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py


Index: 
lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
===
--- 
lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
+++ 
lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
@@ -15,6 +15,7 @@
 NO_DEBUG_INFO_TESTCASE = True
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfLLVMTargetMissing("X86")
 def test(self):
 target = self.dbg.CreateTarget("test.out")
 process = target.LoadCore("test.core")


Index: lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
===
--- lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
+++ lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
@@ -15,6 +15,7 @@
 NO_DEBUG_INFO_TESTCASE = True
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfLLVMTargetMissing("X86")
 def test(self):
 target = self.dbg.CreateTarget("test.out")
 process = target.LoadCore("test.core")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] a1f3187 - [lldb] Require x86 for unwind no-return test

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-04-13T08:51:04Z
New Revision: a1f3187ca8a39a62f4a430da823c01aa7874b0d3

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

LOG: [lldb] Require x86 for unwind no-return test

The core file used is built for i386 so we
need the x86 backend to be able to load it.

Reviewed By: labath

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

Added: 


Modified: 

lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
 
b/lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
index 552fdef4a471..d242cb75fd8d 100644
--- 
a/lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
+++ 
b/lldb/test/API/functionalities/unwind/noreturn/module-end/TestNoReturnModuleEnd.py
@@ -15,6 +15,7 @@ class TestNoreturnModuleEnd(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfLLVMTargetMissing("X86")
 def test(self):
 target = self.dbg.CreateTarget("test.out")
 process = target.LoadCore("test.core")



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

[Lldb-commits] [PATCH] D100192: [lldb][Arm/AArch64] Add basic disassemble tests for Arm/AArch64

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG96c82166b6e3: [lldb][Arm/AArch64] Add basic disassemble 
tests for Arm/AArch64 (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100192

Files:
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -32,6 +32,12 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
+elif arch == "aarch64":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"aarch64")
+raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
+elif arch == "arm":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", "arm")
+raw_bytes = bytearray([0x63, 0x30, 0xa0, 0xe3])
 else:
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64")
 raw_bytes = bytearray([0x48, 0x89, 0xe5])
@@ -52,6 +58,12 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
+elif arch == "aarch64":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+elif arch == "arm":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "r3, #99")
 else:
 self.assertEqual(inst.GetMnemonic(target), "movq")
 self.assertEqual(inst.GetOperands(target),


Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -32,6 +32,12 @@
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
+elif arch == "aarch64":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", "aarch64")
+raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
+elif arch == "arm":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", "arm")
+raw_bytes = bytearray([0x63, 0x30, 0xa0, 0xe3])
 else:
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64")
 raw_bytes = bytearray([0x48, 0x89, 0xe5])
@@ -52,6 +58,12 @@
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
+elif arch == "aarch64":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+elif arch == "arm":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "r3, #99")
 else:
 self.assertEqual(inst.GetMnemonic(target), "movq")
 self.assertEqual(inst.GetOperands(target),
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 96c8216 - [lldb][Arm/AArch64] Add basic disassemble tests for Arm/AArch64

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-04-13T08:49:48Z
New Revision: 96c82166b6e38fd0c138876fb21d2a61af3cfcac

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

LOG: [lldb][Arm/AArch64] Add basic disassemble tests for Arm/AArch64

Previously the test would fail if you built on Arm/AArch64
but did not have the x86 llvm backend enabled.

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

Removed: 




diff  --git 
a/lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py 
b/lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
index 6be7d011df76..d6e0b32f133e 100644
--- a/lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ b/lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -32,6 +32,12 @@ def test_disassemble_raw_data(self):
 elif re.match("powerpc64le", arch):
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"powerpc64le")
 raw_bytes = bytearray([0x00, 0x00, 0x80, 0x38])
+elif arch == "aarch64":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", 
"aarch64")
+raw_bytes = bytearray([0x60, 0x0c, 0x80, 0x52])
+elif arch == "arm":
+target = self.dbg.CreateTargetWithFileAndTargetTriple("", "arm")
+raw_bytes = bytearray([0x63, 0x30, 0xa0, 0xe3])
 else:
 target = self.dbg.CreateTargetWithFileAndTargetTriple("", "x86_64")
 raw_bytes = bytearray([0x48, 0x89, 0xe5])
@@ -52,6 +58,12 @@ def test_disassemble_raw_data(self):
 elif re.match("powerpc64le", arch):
 self.assertEqual(inst.GetMnemonic(target), "li")
 self.assertEqual(inst.GetOperands(target), "4, 0")
+elif arch == "aarch64":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+elif arch == "arm":
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetOperands(target), "r3, #99")
 else:
 self.assertEqual(inst.GetMnemonic(target), "movq")
 self.assertEqual(inst.GetOperands(target),



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

[Lldb-commits] [PATCH] D100193: [lldb] Require x86 backend for a bunch of DWARF tests

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a64d80a959b: [lldb] Require x86 backend for a bunch of 
DWARF tests (authored by DavidSpickett).

Changed prior to commit:
  https://reviews.llvm.org/D100193?vs=336783=337068#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100193

Files:
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value-bitfields.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_decl_file-DW_AT_specification-crosscu.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_declaration-with-children.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_location-DW_AT_const_value.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_loclists_base.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_low_pc-addrx.s
  lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-smaller-than-struct.s
  lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_const_value.s
  
lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
  lldb/test/Shell/SymbolFile/DWARF/DW_TAG_variable-invalid_location.s
  
lldb/test/Shell/SymbolFile/DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/B.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/module.modulemap
  lldb/test/Shell/SymbolFile/DWARF/Inputs/debug-line-basic.script
  lldb/test/Shell/SymbolFile/DWARF/Inputs/debug-types-basic.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/debug_loc-aslr.yaml
  
lldb/test/Shell/SymbolFile/DWARF/Inputs/dir-separator-no-comp-dir-relative-name.lldbinit
  lldb/test/Shell/SymbolFile/DWARF/Inputs/dir-separator-posix.lldbinit
  lldb/test/Shell/SymbolFile/DWARF/Inputs/dir-separator-windows.lldbinit
  lldb/test/Shell/SymbolFile/DWARF/Inputs/find-variable-file-2.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
  lldb/test/Shell/SymbolFile/DWARF/apple-index-is-used.cpp
  lldb/test/Shell/SymbolFile/DWARF/array-sizes.s
  lldb/test/Shell/SymbolFile/DWARF/childless-compile-unit.s
  lldb/test/Shell/SymbolFile/DWARF/compilercontext.ll
  lldb/test/Shell/SymbolFile/DWARF/debug-line-basic.s
  lldb/test/Shell/SymbolFile/DWARF/debug-names-compressed.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-address-ranges.s
  lldb/test/Shell/SymbolFile/DWARF/debug-types-basic.test
  lldb/test/Shell/SymbolFile/DWARF/debug-types-dwarf5.s
  lldb/test/Shell/SymbolFile/DWARF/debug-types-dwo-cross-reference.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-line-tables.s
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test
  lldb/test/Shell/SymbolFile/DWARF/debug-types-signature-loop.s
  lldb/test/Shell/SymbolFile/DWARF/debug_aranges-empty-section.s
  lldb/test/Shell/SymbolFile/DWARF/debug_line-relative_path.s
  lldb/test/Shell/SymbolFile/DWARF/debug_line-tombstone.s
  lldb/test/Shell/SymbolFile/DWARF/debug_loc-aslr.s
  lldb/test/Shell/SymbolFile/DWARF/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/debug_loc_and_loclists.s
  lldb/test/Shell/SymbolFile/DWARF/debug_loclists-dwo.s
  lldb/test/Shell/SymbolFile/DWARF/debug_ranges-missing-section.s
  lldb/test/Shell/SymbolFile/DWARF/debug_ranges.s
  lldb/test/Shell/SymbolFile/DWARF/debug_ranges_and_rnglists.test
  lldb/test/Shell/SymbolFile/DWARF/debug_rnglists-dwo.s
  lldb/test/Shell/SymbolFile/DWARF/debug_rnglists.s
  lldb/test/Shell/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s
  lldb/test/Shell/SymbolFile/DWARF/dir-separator-no-comp-dir.s
  lldb/test/Shell/SymbolFile/DWARF/dir-separator-posix.s
  lldb/test/Shell/SymbolFile/DWARF/dir-separator-windows.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-atomic.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-implicit-const.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-index-is-used.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-line-strp.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-partial-index.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-split.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5_locations.s
  lldb/test/Shell/SymbolFile/DWARF/dwarf5_tu_index_abbrev_offset.s
  lldb/test/Shell/SymbolFile/DWARF/dwo-type-in-main-file.s
  lldb/test/Shell/SymbolFile/DWARF/dwp-debug-types.s
  lldb/test/Shell/SymbolFile/DWARF/dwp-separate-debug-file.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwp.s
  lldb/test/Shell/SymbolFile/DWARF/find-basic-function.cpp
  lldb/test/Shell/SymbolFile/DWARF/find-basic-namespace.cpp
  

[Lldb-commits] [lldb] 8a64d80 - [lldb] Require x86 backend for a bunch of DWARF tests

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-04-13T08:47:41Z
New Revision: 8a64d80a959bf2844df33f9112e456f33de7b468

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

LOG: [lldb] Require x86 backend for a bunch of DWARF tests

By moving them into a folder with a local lit config
requiring x86. All these tests use x86 target triples.

There are two tests that require target-x86_64 because
they run program files (instead of just needing the backend).
Those are moved to the x86 folder also but their REQUIRES are
unchanged.

Reviewed By: JDevlieghere

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value-bitfields.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s

lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_data_bit_offset-DW_OP_stack_value.s

lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_decl_file-DW_AT_specification-crosscu.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_declaration-with-children.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_location-DW_AT_const_value.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_low_pc-addrx.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_piece-smaller-than-struct.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_piece-struct.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_GNU_call_site-DW_AT_low_pc.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_basic_type_DW_ATE_UTF_nonC.ll
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-DW_AT_const_value.s

lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s
lldb/test/Shell/SymbolFile/DWARF/x86/DW_TAG_variable-invalid_location.s

lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/ModuleOwnership/A.h
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/ModuleOwnership/B.h
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/ModuleOwnership/module.modulemap
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/debug-line-basic.script
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/debug-types-basic.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/debug_loc-aslr.yaml

lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/dir-separator-no-comp-dir-relative-name.lldbinit
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/dir-separator-posix.lldbinit
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/dir-separator-windows.lldbinit
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-2.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/subprogram_ranges.s
lldb/test/Shell/SymbolFile/DWARF/x86/apple-index-is-used.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/array-sizes.s
lldb/test/Shell/SymbolFile/DWARF/x86/childless-compile-unit.s
lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
lldb/test/Shell/SymbolFile/DWARF/x86/debug-line-basic.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-names-compressed.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-address-ranges.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-basic.test
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwarf5.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-dwo-cross-reference.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-line-tables.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-missing-signature.test
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_aranges-empty-section.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_line-relative_path.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_line-tombstone.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc-aslr.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges-missing-section.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_ranges_and_rnglists.test
lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists.s

lldb/test/Shell/SymbolFile/DWARF/x86/dir-separator-no-comp-dir-relative-name.s
lldb/test/Shell/SymbolFile/DWARF/x86/dir-separator-no-comp-dir.s
lldb/test/Shell/SymbolFile/DWARF/x86/dir-separator-posix.s
lldb/test/Shell/SymbolFile/DWARF/x86/dir-separator-windows.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-atomic.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-debug_line-file-index.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-debug_line.s
lldb/test/Shell/SymbolFile/DWARF/x86/dwarf5-implicit-const.s

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D100146#2685026 , @mgorny wrote:

> In D100146#2684942 , @labath wrote:
>
>> looks great, just fix the build errors :)
>
> Yeah, I'm trying to see if I can reproduce them when building with Clang.

I believe StringRef is not implicitly convertible to std::string these days, 
but all the conversions are in conditionally compiled code. Maybe you don't 
have any of it enabled?

My guess is it would be sufficient to change `avail_name` to a StringRef (and 
fix the errors caused by that).




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:375
+
+for (auto x : server_features) {
+  if (x == "qXfer:auxv:read+")

mgorny wrote:
> labath wrote:
> > not a big deal, but this probably shouldn't be auto.
> Could you explain a bit? I thought `auto` is convenient here since the actual 
> type is visible three lines higher.
Well.. my reasoning was something like this:
- llvm guidelines say that auto should be used (only?) when the type is obvious 
from context
- "context" is not exactly specified, but I would take that to mean the 
enclosing expression/statement (not the statement two lines up). The use of 
`cast` in the example supports this.
- the actual type is not that complicated (llvm::StringRef), so it e.g. won't 
cause the line to be split in two
- given the above, and the generally cautious approach to auto in llvm, its 
better to err on the side of spelling out the type

But like I said, it's not a big deal...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like this a lot more than the previous version. The thing I'd like to know 
is, whether we can replace `m_additional_processes` with something like 
`m_debugged_processes` (i.e., just have a single list of all processes we are 
debugging. We could replace all occurrences of `m_debugged_process_up` with 
`m_current_process`, which just points inside that list. Is there any reason 
for the "privileged" position of the original process? The way I see it, most 
of the operations would just work even if we treated them as equals. The only 
thing which might not work is resuming (or TBE, reporting the stops) of the 
subprocesses, but if we wanted to be safe, we could prevent that in some other 
way (preventing the changing of process through Hc; returning "crippled" 
process instances for subprocess, which return errors on resume operations; 
...).

WDYT?




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1036
+std::unique_ptr _process) {
+  // Apparently the process has been dealt with by another delegate.
+  if (!child_process)

You no longer have to worry about that...


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [lldb] c9cf394 - [lldb] Replace NativeProcess delegate list with a single delegate

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-13T09:49:38+02:00
New Revision: c9cf394f796e181b9ee3bc9b69d84fbbba2fe45c

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

LOG: [lldb] Replace NativeProcess delegate list with a single delegate

In all this time, we've never used more than one delegate. The logic to
support multiple delegates is therefore untested, and becomes
particularly unwieldy once we need to support multiple processes.

Just remove it.

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h 
b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 36b22d72b1962..72ac05509936d 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -222,36 +222,6 @@ class NativeProcessProtocol {
 virtual void DidExec(NativeProcessProtocol *process) = 0;
   };
 
-  /// Register a native delegate.
-  ///
-  /// Clients can register nofication callbacks by passing in a
-  /// NativeDelegate impl and passing it into this function.
-  ///
-  /// Note: it is required that the lifetime of the
-  /// native_delegate outlive the NativeProcessProtocol.
-  ///
-  /// \param[in] native_delegate
-  /// A NativeDelegate impl to be called when certain events
-  /// happen within the NativeProcessProtocol or related threads.
-  ///
-  /// \return
-  /// true if the delegate was registered successfully;
-  /// false if the delegate was already registered.
-  ///
-  /// \see NativeProcessProtocol::NativeDelegate.
-  bool RegisterNativeDelegate(NativeDelegate _delegate);
-
-  /// Unregister a native delegate previously registered.
-  ///
-  /// \param[in] native_delegate
-  /// A NativeDelegate impl previously registered with this process.
-  ///
-  /// \return Returns \b true if the NativeDelegate was
-  /// successfully removed from the process, \b false otherwise.
-  ///
-  /// \see NativeProcessProtocol::NativeDelegate
-  bool UnregisterNativeDelegate(NativeDelegate _delegate);
-
   virtual Status GetLoadedModuleFileSpec(const char *module_path,
  FileSpec _spec) = 0;
 
@@ -377,8 +347,7 @@ class NativeProcessProtocol {
 
   llvm::Optional m_exit_status;
 
-  std::recursive_mutex m_delegates_mutex;
-  std::vector m_delegates;
+  NativeDelegate _delegate;
   NativeWatchpointList m_watchpoint_list;
   HardwareBreakpointMap m_hw_breakpoints_map;
   int m_terminal_fd;

diff  --git a/lldb/source/Host/common/NativeProcessProtocol.cpp 
b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 070fda664678a..d15bb21e8e6db 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -25,10 +25,8 @@ using namespace lldb_private;
 
 NativeProcessProtocol::NativeProcessProtocol(lldb::pid_t pid, int terminal_fd,
  NativeDelegate )
-: m_pid(pid), m_terminal_fd(terminal_fd) {
-  bool registered = RegisterNativeDelegate(delegate);
-  assert(registered);
-  (void)registered;
+: m_pid(pid), m_delegate(delegate), m_terminal_fd(terminal_fd) {
+  delegate.InitializeDelegate(this);
 }
 
 lldb_private::Status NativeProcessProtocol::Interrupt() {
@@ -295,64 +293,21 @@ Status 
NativeProcessProtocol::RemoveHardwareBreakpoint(lldb::addr_t addr) {
   return error;
 }
 
-bool NativeProcessProtocol::RegisterNativeDelegate(
-NativeDelegate _delegate) {
-  std::lock_guard guard(m_delegates_mutex);
-  if (llvm::is_contained(m_delegates, _delegate))
-return false;
-
-  m_delegates.push_back(_delegate);
-  native_delegate.InitializeDelegate(this);
-  return true;
-}
-
-bool NativeProcessProtocol::UnregisterNativeDelegate(
-NativeDelegate _delegate) {
-  std::lock_guard guard(m_delegates_mutex);
-
-  const auto initial_size = m_delegates.size();
-  m_delegates.erase(
-  remove(m_delegates.begin(), m_delegates.end(), _delegate),
-  m_delegates.end());
-
-  // We removed the delegate if the count of delegates shrank after removing
-  // all copies of the given native_delegate from the vector.
-  return m_delegates.size() < initial_size;
-}
-
 void NativeProcessProtocol::SynchronouslyNotifyProcessStateChanged(
 lldb::StateType state) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
 
-  std::lock_guard guard(m_delegates_mutex);
-  for (auto native_delegate : m_delegates)
-

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

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

mgorny added a comment.

In D100146#2684942 , @labath wrote:

> looks great, just fix the build errors :)

Yeah, I'm trying to see if I can reproduce them when building with Clang.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:375
+
+for (auto x : server_features) {
+  if (x == "qXfer:auxv:read+")

labath wrote:
> not a big deal, but this probably shouldn't be auto.
Could you explain a bit? I thought `auto` is convenient here since the actual 
type is visible three lines higher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like that (particularly the dependency graph :P). Let's just figure out the 
error handling..

In D100153#2678223 , @mgorny wrote:

> 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.

Maybe we could start by feature-ifying one of the existing fields currently 
controlled by ifdefs. Say, `QPassSignals+`. It does not have a client 
component, so it won't cover everything, but I think it will give us something.




Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

Are you sure that returning success is the best "default" behavior? Maybe the 
base implementation should always return an error (as it does not implement any 
extensions)? Or return success, only if one enables an empty set of extensions?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3592-3595
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");

... or actually, I am wondering if this should not be a hard error/assertion. 
In the current set up, I think it would be a programmer error if the factory 
reports an extension as supported, but then the instance fails to enable it...


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] D100195: [lldb] Require x86 for unwind no-return test

[Pavel Labath via Phabricator via lldb-commits]

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

In D100195#2679590 , @DavidSpickett 
wrote:

> (apologies for the review spam, you're the author of the test in this case)

np

> lldb in fact crashes when you try to load the core file. Would be nice to 
> give a good failure message if we can but skipping this test makes sense 
> anyway.

Agree completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100195

___
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

[Pavel Labath via Phabricator via lldb-commits]

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

cool




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:835
+  // Parse client-indicated features.
+  llvm::StringRef view = packet.GetStringRef();
+  llvm::SmallVector client_features;

remove variable used only once?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1310-1311
+  "QStartNoAckMode+",
+  "QThreadSuffixSupported+",
+  "QListThreadsInStopReply+",
+  "qEcho+",

I think that these two should also be llgs-specific.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h:151
+  virtual std::vector
+  HandleFeatures(const llvm::ArrayRef client_features);
+

this `const` is useless


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] D100146: [lldb] [gdb-remote client] Refactor handling qSupported

[Pavel Labath via Phabricator via lldb-commits]

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

looks great, just fix the build errors :)




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:375
+
+for (auto x : server_features) {
+  if (x == "qXfer:auxv:read+")

not a big deal, but this probably shouldn't be auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100146

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