[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td:13
+EnumValues<"OptionEnumValues(g_abi_enums)">,
+Desc<"A mapping of ABI override to use for specific modules. The module 
name is matched by its file name with extension. These versions are checked in 
sequence: exact, lowercase, exact with '.debug' suffix stripped, lowercase with 
'.debug' suffix stripped.">;
 }

alvinhochun wrote:
> DavidSpickett wrote:
> > What are the values accepted for this?
> It's the same enum as the setting `plugin.object-file.pe-coff.abi` added in 
> D127048 - `default`, `msvc` and `gnu`.
Which makes sense but the user isn't always going to see both descriptions so 
I'd repeat that information here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127234

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

[Lldb-commits] [PATCH] D128201: [lldb][windows] Fix crash on getting nested exception

[David Spickett via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128201

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

[Lldb-commits] [PATCH] D128226: [lldb] Remove an outdated comment. NFC.

[David Spickett via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128226

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

[Lldb-commits] [PATCH] D128221: [LLDB] Add Arm64 CodeView to LLDB regnum mapping

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.

Are there public docs for these numbers?


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

https://reviews.llvm.org/D128221

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

[Lldb-commits] [PATCH] D128253: [lldb] [MainLoop] Support "pending callbacks", to be called once

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

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

Support adding a "pending callback" to the main loop, that will be
called once after all the pending events are processed.  This can be
e.g. to defer destroying the process instance until its exit is fully
processed, as suggested in D127500 .

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128253

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/source/Host/common/MainLoop.cpp
  lldb/unittests/Host/MainLoopTest.cpp

Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -98,6 +98,56 @@
   ASSERT_EQ(1u, callback_count);
 }
 
+TEST_F(MainLoopTest, PendingCallback) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  socketpair[1],
+  [&](MainLoopBase &loop) {
+// Both callbacks should be called before the loop terminates.
+loop.AddPendingCallback(make_callback());
+loop.AddPendingCallback(make_callback());
+loop.RequestTermination();
+  },
+  error);
+  ASSERT_TRUE(error.Success());
+  ASSERT_TRUE(handle);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(2u, callback_count);
+}
+
+TEST_F(MainLoopTest, PendingCallbackCalledOnlyOnce) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  socketpair[1],
+  [&](MainLoopBase &loop) {
+// Add one pending callback on the first iteration.
+if (callback_count == 0) {
+  loop.AddPendingCallback([&](MainLoopBase &loop) {
+callback_count++;
+  });
+}
+// Terminate the loop on second iteration.
+if (callback_count++ >= 1)
+  loop.RequestTermination();
+  },
+  error);
+  ASSERT_TRUE(error.Success());
+  ASSERT_TRUE(handle);
+  ASSERT_TRUE(loop.Run().Success());
+  // 2 iterations of read callback + 1 call of pending callback.
+  ASSERT_EQ(3u, callback_count);
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -347,6 +347,10 @@
 #endif
 }
 
+void MainLoop::AddPendingCallback(const Callback &callback) {
+  m_pending_callbacks.push_back(callback);
+}
+
 void MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
   bool erased = m_read_fds.erase(handle);
   UNUSED_IF_ASSERT_DISABLED(erased);
@@ -401,6 +405,10 @@
   return error;
 
 impl.ProcessEvents();
+
+for (const Callback &callback : m_pending_callbacks)
+  callback(*this);
+m_pending_callbacks.clear();
   }
   return Status();
 }
Index: lldb/include/lldb/Host/MainLoopBase.h
===
--- lldb/include/lldb/Host/MainLoopBase.h
+++ lldb/include/lldb/Host/MainLoopBase.h
@@ -46,6 +46,13 @@
 llvm_unreachable("Not implemented");
   }
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  virtual void AddPendingCallback(const Callback &callback) {
+llvm_unreachable("Not implemented");
+  }
+
   // Waits for registered events and invoke the proper callbacks. Returns when
   // all callbacks deregister themselves or when someone requests termination.
   virtual Status Run() { llvm_unreachable("Not implemented"); }
Index: lldb/include/lldb/Host/MainLoop.h
===
--- lldb/include/lldb/Host/MainLoop.h
+++ lldb/include/lldb/Host/MainLoop.h
@@ -14,6 +14,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include 
 #include 
+#include 
 
 #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
 #define SIGNAL_POLLING_UNSUPPORTED 1
@@ -59,6 +60,11 @@
   SignalHandleUP RegisterSignal(int signo, const Callback &callback,
 Status &error);
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  void AddPendingCallback(const Callback &callback) override;
+
   Status Run() override;
 
   // This should only be performed from a callback. Do not attempt to terminate
@@ -104,6 +110,7 @@
 
   llvm::DenseMap m_read_fds;
   llvm::DenseMap m_signals;
+  std::vector m_pending_callbacks;
 #

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

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

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

Updated to delay removing the process (and therefore stopping lldb-server) 
until the last process exits.


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

https://reviews.llvm.org/D127500

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -262,3 +262,37 @@
 "send packet: $Eff#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_kill_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+child_pid = ret["child_pid"]
+self.reset_test_sequence()
+
+exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+self.test_sequence.add_log_lines([
+# kill all processes
+"read packet: $k#00",
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertEqual(set([ret["pid1"], ret["pid2"]]),
+ set([parent_pid, child_pid]))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -992,13 +992,24 @@
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
-
-  // We are ready to exit the debug monitor.
-  m_exit_now = true;
-  m_mainloop.RequestTermination();
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m_continue_process = nullptr;
+
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([&, pid](MainLoopBase &loop) {
+m_debugged_processes.erase(pid);
+if (m_debugged_processes.empty()) {
+  // Close the pipe to the inferior terminal i/o if we launched it and set
+  // one up.
+  MaybeCloseInferiorTerminalConnection();
+
+  // We are ready to exit the debug monitor.
+  m_exit_now = true;
+  loop.RequestTermination();
+}
+  });
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1161,12 +1172,19 @@
 }
 
 void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
+  Log *log = GetLog(LLDBLog::Process);
+
   // Don't forward if not connected (e.g. when attaching).
   if (!m_stdio_communication.IsConnected())
 return;
 
+  // If we're not debugging multiple processes, this should be called
+  // once.
+  if (!bool(m_extensions_supported &
+NativeProcessProtocol::Extension::multiprocess))
+assert(!m_stdio_handle_up);
+
   Status error;
-  lldbassert(!m_stdio_handle_up);
   m_stdio_handle_up = m_mainloop.RegisterReadObject(
   m_stdio_communication.GetConnection()->GetReadObject(),
   [this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1174,15 +1192,27 @@
   if (!m_stdio_handle_up) {
 // Not much we can do about the failure. Log it and continue without
 // forwarding.
-if (Log *log = GetLog(LLDBLog::Process))
-  LLDB_LOGF(log,
-"GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio "
-"forwarding: %s",
-__FUNCTION__, error.AsCString());
+LLDB_LOG(log, "Failed to set up stdio forwarding: {0}", error);
   }
 }
 
 void GDBRemoteCommunicationServerLLGS::StopSTDIOForwarding() {
+  Log *log = GetLog(LLDBLog::Process);
+
+  if (bool(m_extensions_supported &
+   NativeProcessProtocol::Extension::multiprocess)) {
+// Leave stdio forwarding open if there's at least one running process
+// still.
+for (auto &x : m_debugged_processes) {
+  if (x.second->IsRunning()

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: mgorny, emrekultursay.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

New glibc versions (since 2.34 or including this
https://github.com/bminor/glibc/commit/ed3ce71f5c64c5f07cbde0ef03554ea8950d8f2c
patch) trigger the rendezvous breakpoint after they have already added
some modules to the list. This did not play well with our dynamic
loader plugin which was doing a diff of the the reported modules in the
before (RT_ADD) and after (RT_CONSISTENT) states. Specifically, it
caused us to miss some of the modules.

While I think the old behavior makes more sense, I don't think that lldb
is doing the right thing either, as the documentation states that we
should not be expecting a consistent view in the RT_ADD (and RT_DELETE)
states.

Therefore, this patch changes the lldb algorithm to compare the module
list against the previous consistent snapshot. This fixes the previous
issue, and I believe it is more correct in general. It also reduces the
number of times we are fetching the module info, which should speed up
the debugging of processes with many shared libraries.

The change in RefreshModules ensures we don't broadcast the loaded
notification for the dynamic loader (ld.so) module more than once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128264

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -433,27 +433,31 @@
 for (; I != E; ++I) {
   ModuleSP module_sp =
   LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
-  if (module_sp.get()) {
-if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
-&m_process->GetTarget()) == m_interpreter_base &&
-module_sp != m_interpreter_module.lock()) {
-  if (m_interpreter_module.lock() == nullptr) {
-m_interpreter_module = module_sp;
-  } else {
-// If this is a duplicate instance of ld.so, unload it.  We may end
-// up with it if we load it via a different path than before
-// (symlink vs real path).
-// TODO: remove this once we either fix library matching or avoid
-// loading the interpreter when setting the rendezvous breakpoint.
-UnloadSections(module_sp);
-loaded_modules.Remove(module_sp);
-continue;
-  }
+  if (!module_sp.get())
+continue;
+
+  if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
+  &m_process->GetTarget()) == m_interpreter_base) {
+ModuleSP interpreter_sp = m_interpreter_module.lock();
+if (m_interpreter_module.lock() == nullptr) {
+  m_interpreter_module = module_sp;
+} else if (module_sp == m_interpreter_module.lock()) {
+  // Module already loaded.
+  continue;
+} else {
+  // If this is a duplicate instance of ld.so, unload it.  We may end
+  // up with it if we load it via a different path than before
+  // (symlink vs real path).
+  // TODO: remove this once we either fix library matching or avoid
+  // loading the interpreter when setting the rendezvous breakpoint.
+  UnloadSections(module_sp);
+  loaded_modules.Remove(module_sp);
+  continue;
 }
-
-loaded_modules.AppendIfNeeded(module_sp);
-new_modules.Append(module_sp);
   }
+
+  loaded_modules.AppendIfNeeded(module_sp);
+  new_modules.Append(module_sp);
 }
 m_process->GetTarget().ModulesDidLoad(new_modules);
   }
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -210,14 +210,7 @@
 
   case eAdd:
   case eDelete:
-// Some versions of the android dynamic linker might send two
-// notifications with state == eAdd back to back. Ignore them until we
-// get an eConsistent notification.
-if (!(m_previous.state == eConsistent ||
-  (m_previous.state == eAdd && m_current.state == eDelete)))
-  return eNoAction;
-
-return eTakeSnapshot;
+return eNoAction;
   }
 
   return eNoAction;
@@ -229,9 +222,9 @@
   if (action == eNoAction)
 return false;
 
+  m_added_soentries.clear();
+  m_removed_soentries.clear();
   if (action == eTakeSnapshot) {
-m_added_soentries.clear();
-m_removed_soentries.cle

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via Phabricator via lldb-commits]

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

@emrekultursay: Mainly added you in case you want to try this out on android.




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:425
 // and all DT_NEEDED entries on *BSD.
 if (m_initial_modules_added) {
   I = m_rendezvous.loaded_begin();

@mgorny: I suspect this workaround may no longer be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(This was breaking several shared library tests, most notably TestLoadUnload)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [lldb] 6a85b9d - Support expressions in the context of a reference

[Pavel Labath via lldb-commits]

Author: Emre Kultursay
Date: 2022-06-21T14:30:07+02:00
New Revision: 6a85b9d16387ddd7c356336c8a77ba6b88f9a610

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

LOG: Support expressions in the context of a reference

...type variable by dereferencing the variable before
evaluating the expression.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Expression/UserExpression.cpp
lldb/test/API/commands/expression/context-object/TestContextObject.py
lldb/test/API/commands/expression/context-object/main.cpp

Removed: 




diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index fd26cbd0a6ccc..f821603f03e56 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,21 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);
+if (!error.Success()) {
+  LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
+"a reference type that can't be dereferenced, can't run "
+"expressions.");
+  error.SetErrorString(
+  "passed context object of an reference type cannot be deferenced");
+  return lldb::eExpressionSetupError;
+}
+
+ctx_obj = deref_ctx_sp.get();
+  }
+
   lldb_private::ExecutionPolicy execution_policy = 
options.GetExecutionPolicy();
   lldb::LanguageType language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()

diff  --git 
a/lldb/test/API/commands/expression/context-object/TestContextObject.py 
b/lldb/test/API/commands/expression/context-object/TestContextObject.py
index a53cbac306503..45f7a003837b9 100644
--- a/lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ b/lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -16,34 +16,34 @@ def test_context_object(self):
 frame = thread.GetFrameAtIndex(0)
 
 #
-# Test C++ struct variable
+# Test C++ struct variable and reference-to-struct variable
 #
-
-obj_val = frame.FindVariable("cpp_struct")
-self.assertTrue(obj_val.IsValid())
-
-# Test an empty expression evaluation
-value = obj_val.EvaluateExpression("")
-self.assertFalse(value.IsValid())
-self.assertFalse(value.GetError().Success())
-
-# Test retrieveing of a field (not a local with the same name)
-value = obj_val.EvaluateExpression("field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test functions evaluation
-value = obj_val.EvaluateExpression("function()")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test that we retrieve the right global
-value = obj_val.EvaluateExpression("global.field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
+for obj in "cpp_struct", "cpp_struct_ref":
+obj_val = frame.FindVariable(obj)
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+

[Lldb-commits] [PATCH] D128126: Support expressions in the context of a reference

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a85b9d16387: Support expressions in the context of a 
reference (authored by emrekultursay, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D128126?vs=438454&id=438659#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128126

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/test/API/commands/expression/context-object/TestContextObject.py
  lldb/test/API/commands/expression/context-object/main.cpp

Index: lldb/test/API/commands/expression/context-object/main.cpp
===
--- lldb/test/API/commands/expression/context-object/main.cpp
+++ lldb/test/API/commands/expression/context-object/main.cpp
@@ -31,6 +31,9 @@
   cpp_namespace::CppStruct cpp_struct = cpp_namespace::GetCppStruct();
   cpp_struct.function();
 
+  cpp_namespace::CppStruct &cpp_struct_ref = cpp_struct;
+  cpp_struct_ref.function();
+
   int field = ;
 
   cpp_namespace::CppUnion cpp_union;
Index: lldb/test/API/commands/expression/context-object/TestContextObject.py
===
--- lldb/test/API/commands/expression/context-object/TestContextObject.py
+++ lldb/test/API/commands/expression/context-object/TestContextObject.py
@@ -16,34 +16,34 @@
 frame = thread.GetFrameAtIndex(0)
 
 #
-# Test C++ struct variable
+# Test C++ struct variable and reference-to-struct variable
 #
-
-obj_val = frame.FindVariable("cpp_struct")
-self.assertTrue(obj_val.IsValid())
-
-# Test an empty expression evaluation
-value = obj_val.EvaluateExpression("")
-self.assertFalse(value.IsValid())
-self.assertFalse(value.GetError().Success())
-
-# Test retrieveing of a field (not a local with the same name)
-value = obj_val.EvaluateExpression("field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test functions evaluation
-value = obj_val.EvaluateExpression("function()")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
-
-# Test that we retrieve the right global
-value = obj_val.EvaluateExpression("global.field")
-self.assertTrue(value.IsValid())
-self.assertSuccess(value.GetError())
-self.assertEqual(value.GetValueAsSigned(), )
+for obj in "cpp_struct", "cpp_struct_ref":
+obj_val = frame.FindVariable(obj)
+self.assertTrue(obj_val.IsValid())
+
+# Test an empty expression evaluation
+value = obj_val.EvaluateExpression("")
+self.assertFalse(value.IsValid())
+self.assertFalse(value.GetError().Success())
+
+# Test retrieveing of a field (not a local with the same name)
+value = obj_val.EvaluateExpression("field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test functions evaluation
+value = obj_val.EvaluateExpression("function()")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
+
+# Test that we retrieve the right global
+value = obj_val.EvaluateExpression("global.field")
+self.assertTrue(value.IsValid())
+self.assertSuccess(value.GetError())
+self.assertEqual(value.GetValueAsSigned(), )
 
 #
 # Test C++ union variable
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -145,8 +145,9 @@
   Log *log(GetLog(LLDBLog::Expressions | LLDBLog::Step));
 
   if (ctx_obj) {
-static unsigned const ctx_type_mask =
-lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
+static unsigned const ctx_type_mask = lldb::TypeFlags::eTypeIsClass |
+  lldb::TypeFlags::eTypeIsStructUnion |
+  lldb::TypeFlags::eTypeIsReference;
 if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
   LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
 "an invalid type, can't run expressions.");
@@ -155,6 +156,21 @@
 }
   }
 
+  if (ctx_obj && ctx_obj->GetTypeInfo() & lldb::TypeFlags::eTypeIsReference) {
+Status error;
+lldb::ValueObjectSP deref_ctx_sp = ctx_obj->Dereference(error);

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, DavidSpickett, omjavaid, alvinhochun.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

25c8a061c5739677d2fc0af29a8cc9520207b923 
 / D127048 
 added an option
for setting the ABI to GNU.

When an object file is loaded, there's only minimal verification
done for the architecture spec set for it, if the object file only
provides one.

However, for i386 object files, the PECOFF object file plugin
provides two architectures, i386-pc-windows and i686-pc-windows.
This picks a totally different codepath in
TargetList::CreateTargetInternal, where it's treated as a fat
binary. This goes through more verifications to see if the
architectures provided by the object file matches what the
platform plugin supports.

The PlatformWindows() constructor explicitly adds the
"i386-pc-windows" and "i686-pc-windows" architectures (even when
running on other architectures), which allows this "fat binary
verification" to succeed for the i386 object files that provide
two architectures.

However, after this commit, if the object file is advertised with
the different environment (either when built in a mingw environment,
or if that setting is set), the fat binary validation won't accept
the file any longer.

Update ArchSpec::IsEqualTo with more logic for the Windows use
cases; mismatching vendors is not an issue (they don't have any
practical effect on Windows), and GNU and MSVC environments are
compatible to the point that PlatformWindows can handle object
files for both environments/ABIs.

As a separate path forward, one could also consider to stop returning
two architecture specs from ObjectFilePECOFF::GetModuleSpecifications
for i386 files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,54 @@
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,22 +979,29 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
-const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
-const bool lhs_vendor_specified = TripleVendorWasSpecified();
-// Both architectures had the vendor specified, so if they aren't equal
-// then we return false
-if (rhs_vendor_specified && lhs_vendor_specified)
-  return false;
-
-

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D125509#3586928 , @zequanwu wrote:

>> The main thing I am wondering is if it wouldn't be better to (instead of 
>> essentially duplicating the DWARFExpression interface in the 
>> DWARFExpressionList class) somehow split the process of finding the 
>> appropriate expression (for the given PC value or whatever), and the actual 
>> evaluation. This would be particularly nice if the arguments can be split in 
>> such a way that those that are used for locating the expression are not 
>> repeated in the "evaluate" call. Have you considered this?
>
> Then we need to duplicate the code that gets expression data and register 
> kind before every call to `DWARFExpression::Evaluate`, making it less clean.

Ok, fair enough.

I noticed one issue (described inline) but other than that, I think this should 
be ok.




Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:29-31
+  : m_module_wp(), m_dwarf_cu(dwarf_cu) {
+if (module_sp)
+  m_module_wp = module_sp;

I believe this is equivalent.



Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:103
+const DWARFExpression &rhs) const {
+  return true;
+}

`false` would be more correct here (`(a(loc.Expr.data(), loc.Expr.size());
-  return DataExtractor(buffer_sp, byte_order, addr_size);
-}
-
-bool DWARFExpression::DumpLocations(Stream *s, lldb::DescriptionLevel level,
-addr_t load_function_start, addr_t addr,
-ABI *abi) {
-  if (!IsLocationList()) {
-DumpLocation(s, m_data, level, abi);
-return true;
-  }
-  bool dump_all = addr == LLDB_INVALID_ADDRESS;
-  llvm::ListSeparator separator;
-  auto callback = [&](llvm::DWARFLocationExpression loc) -> bool {
-if (loc.Range &&
-(dump_all || (loc.Range->LowPC <= addr && addr < loc.Range->HighPC))) {
-  uint32_t addr_size = m_data.GetAddressByteSize();
-  DataExtractor data = ToDataExtractor(loc, m_data.GetByteOrder(),
-   m_data.GetAddressByteSize());
-  s->AsRawOstream() << separator;
-  s->PutCString("[");
-  s->AsRawOstream() << llvm::format_hex(loc.Range->LowPC,
-2 + 2 * addr_size);
-  s->PutCString(", ");
-  s->AsRawOstream() << llvm::format_hex(loc.Range->HighPC,
-2 + 2 * addr_size);
-  s->PutCString(") -> ");
-  DumpLocation(s, data, level, abi);
-  return dump_all;
-}
-return true;
-  };
-  if (!GetLocationExpressions(load_function_start, callback))
-return false;
-  return true;
-}
-
-bool DWARFExpression::GetLocationExpressions(
-addr_t load_function_start,
-llvm::function_ref callback) const {
-  if (load_function_start == LLDB_INVALID_ADDRESS)
-return false;
-
-  Log *log = GetLog(LLDBLog::Expressions);
-
+bool DWARFExpression::ParseDWARFLocationList(
+const DWARFUnit *dwarf_cu, const DataExtractor &data,

zequanwu wrote:
> labath wrote:
> > Could this go into the list class?
> This function calls `ReadAddressFromDebugAddrSection` that is called multiple 
> places in `DWARFExpression`. If we move it to `DWARFExpressionList`, we need 
> to duplicate the `ReadAddressFromDebugAddrSection`.
Ok. If that's the only problem, then I think we can fix that by moving 
`ReadAddressFromDebugAddrSection` to the DWARFUnit class (if it does not 
contain something like that already). IIUC, this is equivalent to 
`getAddrOffsetSectionItem` in llvm DWARFUnit, so I'd give it a similar name.



Comment at: lldb/source/Expression/DWARFExpressionList.cpp:109
+return false;
+  return expr->MatchesOperand(frame, operand);
+}

Could we make `MatchesOperand` const (and ditch the Mutable calls above)?



Comment at: lldb/source/Expression/DWARFExpressionList.cpp:179
+  if (m_exprs.GetSize() == 1) {
+expr = m_exprs.Back()->data;
+  } else {

I don't think this is correct.

We should be able to distinguish between a location list that is always valid 
(i.e., one which is not a location list at all), and a location list which 
happens to contain a single valid range.

That could probably be achieved by making the range of the always-valid entry 
be (0, UINT_MAX), but maybe it would be better to have an explicit 
fallback/default entry for this (one that is used when no ranged entry 
applies). This is how the default location entries in DWARF5 are supposed to 
work, although I am not sure if any compiler actually makes use of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> However, after this commit, if the object file is advertised with the 
> different environment (either when built in a mingw environment, or if that 
> setting is set), the fat binary validation won't accept the file any longer.

Is "this commit" referring to 
https://reviews.llvm.org/rG25c8a061c5739677d2fc0af29a8cc9520207b923 or to the 
change we're reviewing here?

I'm struggling to think through what this is actually fixing. Is the issue that 
one half of a fat binary can have a different ABI? Wouldn't both sides of the 
fat binary be loaded as the ABI chosen in the setting, or is that exactly what 
you're fixing.




Comment at: lldb/source/Utility/ArchSpec.cpp:986
+
+  if (lhs_triple_vendor != rhs_triple_vendor) {
+// On Windows, the vendor field doesn't have any practical effect, but

You could use `isOSWindows` if you want. Maybe use an intermediate bool to 
clear up the if statement.
```
bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows();
if ((lhs_triple_vendor != rhs_triple_vendor) && 
(exact_match || !both_windows)) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

Yes, this looks right to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

[Pavel Labath via Phabricator via lldb-commits]

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

Ok, looks good. Some extra suggestions inline.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:490
+response.GetString());
+  else
+return SendPacketNoLock(response.GetString());

no else after return



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1592-1602
   Status error = m_continue_process->Resume(actions);
   if (error.Fail()) {
 LLDB_LOG(log, "c failed for process {0}: {1}", m_continue_process->GetID(),
  error);
 return SendErrorResponse(GDBRemoteServerError::eErrorResume);
   }
 

Could we have a helper function for this, and make all the continue-like 
actions go through that?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1753
+// notifications.
+for (auto it = m_stop_notification_queue.begin();
+ it != m_stop_notification_queue.end();) {

`llvm::erase_if(m_stop_notification_queue, []{ return !begins_with_W_or_X;});`



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3264
   // Notify we attached by sending a stop packet.
-  return SendStopReasonForState(m_current_process->GetState());
+  return SendStopReasonForState(m_current_process->GetState(), false);
 }

add `/*force_synchronous=*/` here and elsewhere.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3749
+return SendErrorResponse(Status("No pending notification to ack"));
+  m_stop_notification_queue.pop_front();
+  if (!m_stop_notification_queue.empty())

mgorny wrote:
> labath wrote:
> > Is this correct? I would expect to this to first read (`front()`) the 
> > packet before popping it.
> Yes, it is. I suppose it could be a bit misleading. The first message is sent 
> as asynchronous notification but we keep it on the queue (and technically, 
> the protocol assumes we may resend the asynchronous notification if we think 
> client may have not gotten it). `vStopped` means client has processed it and 
> is ready for the next one, so we pop the one that's been sent already and 
> send the next one, wait for the next ACK and so on.
Yeah, it is confusing. Having a comment (with the explanation you just made) 
would help a lot. However, if you don't see any use case for the resending, we 
could also delete it, and use the standard queue pattern. Up to you...



Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1443
+m = self.expect_gdbremote_sequence()
+del m["O_content"]
+threads = [m]

mgorny wrote:
> labath wrote:
> > Is this necessary? (I get that it's not useful, but it seems weird...)
> It's convenient because it lets us compare the matches from `threads` and 
> `threads_verify`.
Ok, I suppose I can live with that.



Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1525
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:new"])
+self.test_sequence.add_log_lines(

mgorny wrote:
> labath wrote:
> > Are you sure this is not racy (like, can the inferior terminate before we 
> > get a chance to interrupt it)?
> Well, if I'm reading the code right, the new thread waits 60 seconds, so yes, 
> technically it's racy ;-). Do you have a better idea? Some new argument that 
> puts the process in an infinite sleep loop?
That's ok, a 60s wait is fine. I forgot there was a default sleep action -- I 
though the process would quit immediately...


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

https://reviews.llvm.org/D125575

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3598967 , @DavidSpickett 
wrote:

>> However, after this commit, if the object file is advertised with the 
>> different environment (either when built in a mingw environment, or if that 
>> setting is set), the fat binary validation won't accept the file any longer.
>
> Is "this commit" referring to 
> https://reviews.llvm.org/rG25c8a061c5739677d2fc0af29a8cc9520207b923 or to the 
> change we're reviewing here?

Sorry, I meant 25c8a061c5739677d2fc0af29a8cc9520207b923 
.

> I'm struggling to think through what this is actually fixing. Is the issue 
> that one half of a fat binary can have a different ABI? Wouldn't both sides 
> of the fat binary be loaded as the ABI chosen in the setting, or is that 
> exactly what you're fixing.

The issue is that in the case of a fat binary, we run a loop to check and pick 
an architecture out of it: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/TargetList.cpp#L173-L184
 This is checked against the architectures added here: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp#L127-L131

(In the case of a non-fat binary, i.e. all other architectures than i386/i686 
on PECOFF, we hit this case here, where we don't validate the arch spec at all: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/TargetList.cpp#L160-L164)

Now since the addition of the new setting in 
25c8a061c5739677d2fc0af29a8cc9520207b923 
, 
`ObjectFile::GetModuleSpecifications` can return `i386-pc-windows-gnu` and 
`i686-pc-windows-gnu`, while `PlatformWindows` provides a list containing 
`i386-pc-windows` (which is normalized to `i386-pc-windows-msvc`) and 
`i686-pc-windows` (`...-msvc`). Due to the differing environments, these are 
deemed incompatible, and LLDB flat out refuses to load the file, with the error 
message `error: no matching platforms found for this file`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Now since the addition of the new setting in 
> 25c8a061c5739677d2fc0af29a8cc9520207b923 
> , 
> ObjectFile::GetModuleSpecifications can return i386-pc-windows-gnu and 
> i686-pc-windows-gnu, while PlatformWindows provides a list containing 
> i386-pc-windows (which is normalized to i386-pc-windows-msvc) and 
> i686-pc-windows (...-msvc). Due to the differing environments, these are 
> deemed incompatible, and LLDB flat out refuses to load the file, with the 
> error message error: no matching platforms found for this file.

Cool, now I understand. Can you add something like that as a comment in the 
test file? Without that context the test seems not to be testing much at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [lldb] 1004d6e - [lldb] Skip Recognizer/assert.test on linux

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-06-21T16:51:02+02:00
New Revision: 1004d6e7e2eb24edc01d7e330c538ce5cb590001

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

LOG: [lldb] Skip Recognizer/assert.test on linux

-> PR56144

Added: 


Modified: 
lldb/test/Shell/Recognizer/assert.test

Removed: 




diff  --git a/lldb/test/Shell/Recognizer/assert.test 
b/lldb/test/Shell/Recognizer/assert.test
index 359598cc8ba63..c24c37b5cb1bc 100644
--- a/lldb/test/Shell/Recognizer/assert.test
+++ b/lldb/test/Shell/Recognizer/assert.test
@@ -1,6 +1,10 @@
 # XFAIL: target-arm && linux-gnu
 # XFAIL: system-freebsd
 # XFAIL: system-netbsd
+#
+# llvm.org/pr56144
+# UNSUPPORTED: system-linux
+#
 # UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s



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

[Lldb-commits] [PATCH] D127667: [lldb] [llgs] Implement the vKill packet

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

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

Rebase and update for changes in D127500 .


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

https://reviews.llvm.org/D127667

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -296,3 +296,60 @@
 ret = self.expect_gdbremote_sequence()
 self.assertEqual(set([ret["pid1"], ret["pid2"]]),
  set([parent_pid, child_pid]))
+
+def vkill_test(self, kill_parent=False, kill_child=False):
+assert kill_parent or kill_child
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+if kill_parent:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(parent_pid),
+"send packet: $OK#00",
+], True)
+if kill_child:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(child_pid),
+"send packet: $OK#00",
+], True)
+self.test_sequence.add_log_lines([
+# check child PID/TID
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: ${}#00".format("Eff" if kill_child else "OK"),
+# check parent PID/TID
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: ${}#00".format("Eff" if kill_parent else "OK"),
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_vkill_child(self):
+self.vkill_test(kill_child=True)
+
+@add_test_categories(["fork"])
+def test_vkill_parent(self):
+self.vkill_test(kill_parent=True)
+
+@add_test_categories(["fork"])
+def test_vkill_both(self):
+self.vkill_test(kill_parent=True, kill_child=True)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -367,6 +367,8 @@
 return eServerPacketType_vCont;
   if (PACKET_MATCHES("vCont?"))
 return eServerPacketType_vCont_actions;
+  if (PACKET_STARTS_WITH("vKill;"))
+return eServerPacketType_vKill;
   if (PACKET_STARTS_WITH("vRun;"))
 return eServerPacketType_vRun;
 }
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
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/MainLoop.h"
@@ -95,6 +96,7 @@
   std::recursive_mutex m_debugged_process_mutex;
   std::unordered_map>
   m_debugged_processes;
+  std::unordered_set m_vkilled_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
@@ -121,6 +123,8 @@
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vKill(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_qProcessInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qC(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1967
+
+  if (multiprocess) {
+for (auto &pid_ptr : m_debugged_processes)

without multiprocess extensions, we should never have more than one process, 
right? Could we just unconditionally use the multiprocess loop here (perhaps 
with an `assert(m_debugged_processes.size() == 1 || multiprocess)`) ?


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

https://reviews.llvm.org/D128152

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

[Lldb-commits] [PATCH] D127193: [lldb] [llgs] Fix signo sent with fork/vfork/vforkdone events

[Pavel Labath via Phabricator via lldb-commits]

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

cool


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

https://reviews.llvm.org/D127193

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

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 438733.
mstorsjo added a comment.

Added a comment to the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,61 @@
+## Check that i386 executables can be opened in both ABI modes.
+## I386 executables are special in the sense that the PECOFF object
+## file plugin returns two architectures for them, i386 and i686, which
+## causes more elaborate comparisons to be run against the Platform plugin's
+## architecture list. Check that this is accepted despite potential mismatches
+## in the environment part of the triple.
+
+# RUN: yaml2obj %s -o %t
+
+## Default ABI is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+# CHECK-LABEL: image list --triple --basename
+# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]]
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4480
+  ImageBase:   268435456
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine: IMAGE_FILE_MACHINE_I386
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.data
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  8192
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+  - Name:.debug_info
+Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  16384
+VirtualSize: 64
+SectionData: DEADBEEFBAADF00D
+symbols: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,22 +979,29 @@
 
   const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor();
   const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor();
-  if (lhs_triple_vendor != rhs_triple_vendor) {
-const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
-const bool lhs_vendor_specified = TripleVendorWasSpecified();
-// Both architectures had the vendor specified, so if they aren't equal
-// then we return false
-if (rhs_vendor_specified && lhs_vendor_specified)
-  return false;
-
-// Only fail if both vendor types are not unknown
-if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
-rhs_triple_vendor != llvm::Triple::UnknownVendor)
-  return false;
-  }
 
   const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
   const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+  if (lhs_triple_vendor != rhs_triple_vendor) {
+// On Windows, the vendor field doesn't have any practical effect, but
+// it is often set to either "pc" or "w64".
+if (exact_match || lhs_triple_os != llvm::Triple::Win32 ||
+rhs_triple_os != llvm::Triple::Win32) {
+  const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
+  const bool lhs_vendor_specified = TripleVendorWasSpecified();
+  // Both architectures had the vendor specified, so if they aren't equal
+  // then we return false
+  if (rhs_vendor_specified && lhs_vendor_specified)
+return false;
+
+  // Only fail if both vendor types are not unknown
+  if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
+  rhs_triple_vendor != llvm::Triple::UnknownVendor)
+return false;
+}
+  }
+
   const llvm::Triple::EnvironmentType lhs_triple_env =
   lhs_triple.getEnvironment();
   const llvm::Trip

[Lldb-commits] [PATCH] D127234: [lldb] Add setting to override PE/COFF ABI by module name

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun updated this revision to Diff 438734.
alvinhochun added a comment.

Added test and improved setting description as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127234

Files:
  lldb/include/lldb/Interpreter/OptionValueDictionary.h
  lldb/source/Interpreter/OptionValueDictionary.cpp
  lldb/source/Interpreter/Property.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
  lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml

Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
===
--- lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi.yaml
@@ -1,4 +1,8 @@
 # RUN: yaml2obj %s -o %t
+# RUN: yaml2obj %s -o %t.debug
+# RUN: mkdir -p %t.dir
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE
+# RUN: yaml2obj %s -o %t.dir/UPPER_CASE.debug
 
 ## Default ABI is msvc:
 # RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
@@ -10,6 +14,57 @@
 # RUN:   -f %t -o "image list --triple --basename" -o exit | \
 # RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
 
+## Default ABI is msvc, module override is gnu:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is gnu, module override is msvc:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s
+
+## Default ABI is msvc, module override is gnu (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=gnu" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp.debug %s
+
+## Default ABI is gnu, module override is msvc (with .debug suffix):
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi %basename_t.tmp=msvc" \
+# RUN:   -f %t.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp.debug %s
+
+## Check that case-sensitive match is chosen before lower-case:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE %s
+
+## Check that lower-case match with .debug suffix is chosen before case-sensitive match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that case-sensitive match without .debug suffix is chosen before lower-case match without .debug suffix:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi UPPER_CASE.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
+## Check that lower-case match without .debug suffix works:
+# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+# RUN:   -O "settings append plugin.object-file.pe-coff.module-abi upper_case.debug=gnu" \
+# RUN:   -f %t.dir/UPPER_CASE.debug -o "image list --triple --basename" -o exit | \
+# RUN:   FileCheck -DABI=gnu -DFILENAME=UPPER_CASE.debug %s
+
 # CHECK-LABEL: image list --triple --basename
 # CHECK-NEXT: x86_64-pc-windows-[[ABI]] [[FILENAME]]
 
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFFProperties.td
@@ -6,4 +6,9 @@
 DefaultEnumValue<"llvm::Triple::UnknownEnvironm

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

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun updated this revision to Diff 438737.
alvinhochun added a comment.

Added Windows-specific test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127436

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Target/TargetList.cpp
  lldb/test/Shell/Commands/command-target-create-resolve-exe.test


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# CHECK-LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error
Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -121,6 +121,14 @@
   if (!user_exe_path.empty()) {
 ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
 FileSystem::Instance().Resolve(module_spec.GetFileSpec());
+
+// Try to resolve the exe based on PATH and/or platform-specific suffixes,
+// but only if using the host platform.
+if (platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+  FileSystem::Instance().ResolveExecutableLocation(
+  module_spec.GetFileSpec());
+
 // Resolve the executable in case we are given a path to a application
 // bundle like a .app bundle on MacOSX.
 Host::ResolveExecutableInBundle(module_spec.GetFileSpec());
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -299,12 +299,6 @@
 
   const char *file_path = command.GetArgumentAtIndex(0);
   LLDB_SCOPED_TIMERF("(lldb) target create '%s'", file_path);
-  FileSpec file_spec;
-
-  if (file_path) {
-file_spec.SetFile(file_path, FileSpec::Style::native);
-FileSystem::Instance().Resolve(file_spec);
-  }
 
   bool must_set_platform_path = false;
 
@@ -333,6 +327,18 @@
 
   PlatformSP platform_sp = target_sp->GetPlatform();
 
+  FileSpec file_spec;
+  if (file_path) {
+file_spec.SetFile(file_path, FileSpec::Style::native);
+FileSystem::Instance().Resolve(file_spec);
+
+// Try to resolve the exe based on PATH and/or platform-specific
+// suffixes, but only if using the host platform.
+if (platform_sp && platform_sp->IsHost() &&
+!FileSystem::Instance().Exists(file_spec))
+  FileSystem::Instance().ResolveExecutableLocation(file_spec);
+  }
+
   if (remote_file) {
 if (platform_sp) {
   // I have a remote file.. two possible cases


Index: lldb/test/Shell/Commands/command-target-create-resolve-exe.test
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,28 @@
+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testm

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

[David M. Lary via Phabricator via lldb-commits]

dmlary added a comment.

Sent this via email, but doesn't look like it showed up here:

> I'm less concerned about the getter and the property and more about the 
> underlying functionality. It doesn't look like it's being tested, and adding 
> it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, `Section.GetLog2Align()`?  Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128069

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

[Lldb-commits] [lldb] bc04d24 - [lldb] [llgs] Implement non-stop style stop notification packets

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T19:04:20+02:00
New Revision: bc04d240850bab340341eaf3601c5ca996667319

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

LOG: [lldb] [llgs] Implement non-stop style stop notification packets

Implement the support for %Stop asynchronous notification packet format
in LLGS.  This does not implement full support for non-stop mode for
threaded programs -- process plugins continue stopping all threads
on every event.  However, it will be used to implement asynchronous
events in multiprocess debugging.

The non-stop protocol is enabled using QNonStop packet.  When it is
enabled, the server uses notification protocol instead of regular stop
replies.  Since all threads are always stopped, notifications are always
generated for all active threads and copied into stop notification
queue.

If the queue was empty, the initial asynchronous %Stop notification
is sent to the client immediately.  The client needs to (eventually)
acknowledge the notification by sending the vStopped packet, in which
case it is popped from the queue and the stop reason for the next thread
is reported.  This continues until notification queue is empty again,
in which case an OK reply is sent.

Asychronous notifications are also used for vAttach results and program
exits.  The `?` packet uses a hybrid approach -- it returns the first
stop reason synchronously, and exposes the stop reasons for remaining
threads via vStopped queue.

The change includes a test case for a program generating a segfault
on 3 threads.  The server is expected to generate a stop notification
for the segfaulting thread, along with the notifications for the other
running threads (with "no stop reason").  This verifies that the stop
reasons are correctly reported for all threads, and that notification
queue works.

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

Added: 
lldb/test/API/tools/lldb-server/TestNonStop.py

Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index c1bf593c11dcb..2a63212e9d287 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -174,7 +174,10 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_QMemTags, // write memory tags
 
 eServerPacketType_qLLDBSaveCore,
-eServerPacketType_QSetIgnoredExceptions
+eServerPacketType_QSetIgnoredExceptions,
+eServerPacketType_QNonStop,
+eServerPacketType_vStopped,
+eServerPacketType_vCtrlC,
   };
 
   ServerPacketType GetServerPacketType() const;

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 cea963e456592..df1fd2cb71b6f 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
@@ -851,6 +851,7 @@ def add_qSupported_packets(self, client_features=[]):
 "memory-tagging",
 "qSaveCore",
 "native-signals",
+"QNonStop",
 ]
 
 def parse_qSupported_response(self, context):

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
index 12dafd15283bc..d1d265d8852b5 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -866,7 +866,7 @@ def _handle_output_packet_string(packet_contents):
 
 class Server(object):
 
-_GDB_REMOTE_PACKET_REGEX = re.compile(br'^\$([^\#]*)#[0-9a-fA-F]{2}')
+_GDB_REMOTE_PACKET_REGEX = re.compile(br'^[\$%]([^\#]*)#[0-9a-fA-F]{2}')
 
 class ChecksumMismatch(Exception):
 pass

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index d660a4d070421..e5461c1899ec8 100644
--- a/lldb/source/Plug

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc04d240850b: [lldb] [llgs] Implement non-stop style stop 
notification packets (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D125575?vs=438007&id=438747#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125575

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestNonStop.py

Index: lldb/test/API/tools/lldb-server/TestNonStop.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -0,0 +1,165 @@
+from lldbsuite.test.lldbtest import *
+
+import gdbremote_testcase
+
+
+class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_run(self):
+self.build()
+self.set_inferior_startup_launch()
+thread_num = 3
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault"] + thread_num * ["thread:new"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+segv_signo = lldbutil.get_signal_number('SIGSEGV')
+all_threads = set()
+all_segv_threads = []
+
+# we should get segfaults from all the threads
+for segv_no in range(thread_num):
+# first wait for the notification event
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^%Stop:(T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+  "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+  },
+ ], True)
+m = self.expect_gdbremote_sequence()
+del m["O_content"]
+threads = [m]
+
+# then we may get events for the remaining threads
+# (but note that not all threads may have been started yet)
+while True:
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vStopped#00",
+ {"direction": "send",
+  "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+  "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+  },
+ ], True)
+m = self.expect_gdbremote_sequence()
+if m["packet"] == "OK":
+break
+del m["O_content"]
+threads.append(m)
+
+segv_threads = []
+other_threads = []
+for t in threads:
+signo = int(t["signo"], 16)
+if signo == segv_signo:
+segv_threads.append(t["thread_id"])
+else:
+self.assertEqual(signo, 0)
+other_threads.append(t["thread_id"])
+
+# verify that exactly one thread segfaulted
+self.assertEqual(len(segv_threads), 1)
+# we should get only one segv from every thread
+self.assertNotIn(segv_threads[0], all_segv_threads)
+all_segv_threads.extend(segv_threads)
+# segv_threads + other_threads should always be a superset
+# of all_threads, i.e. we should get states for all threads
+# already started
+self.assertFalse(
+all_threads.difference(other_threads + segv_threads))
+all_threads.update(other_threads + segv_threads)
+
+# verify that `?` returns the same result
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $?#00",
+ ], True)
+threads_verify = []
+while True:
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+  "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+  "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+ 

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

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

mgorny planned changes to this revision.
mgorny added a comment.

This is now pending rebase due to the nonstop patch landing.


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

https://reviews.llvm.org/D127500

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

[Lldb-commits] [PATCH] D128253: [lldb] [MainLoop] Support "pending callbacks", to be called once

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b04eb23ae1a: [lldb] [MainLoop] Support "pending 
callbacks", to be called once (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128253

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/include/lldb/Host/MainLoopBase.h
  lldb/source/Host/common/MainLoop.cpp
  lldb/unittests/Host/MainLoopTest.cpp

Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -98,6 +98,56 @@
   ASSERT_EQ(1u, callback_count);
 }
 
+TEST_F(MainLoopTest, PendingCallback) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  socketpair[1],
+  [&](MainLoopBase &loop) {
+// Both callbacks should be called before the loop terminates.
+loop.AddPendingCallback(make_callback());
+loop.AddPendingCallback(make_callback());
+loop.RequestTermination();
+  },
+  error);
+  ASSERT_TRUE(error.Success());
+  ASSERT_TRUE(handle);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(2u, callback_count);
+}
+
+TEST_F(MainLoopTest, PendingCallbackCalledOnlyOnce) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  socketpair[1],
+  [&](MainLoopBase &loop) {
+// Add one pending callback on the first iteration.
+if (callback_count == 0) {
+  loop.AddPendingCallback([&](MainLoopBase &loop) {
+callback_count++;
+  });
+}
+// Terminate the loop on second iteration.
+if (callback_count++ >= 1)
+  loop.RequestTermination();
+  },
+  error);
+  ASSERT_TRUE(error.Success());
+  ASSERT_TRUE(handle);
+  ASSERT_TRUE(loop.Run().Success());
+  // 2 iterations of read callback + 1 call of pending callback.
+  ASSERT_EQ(3u, callback_count);
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -347,6 +347,10 @@
 #endif
 }
 
+void MainLoop::AddPendingCallback(const Callback &callback) {
+  m_pending_callbacks.push_back(callback);
+}
+
 void MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
   bool erased = m_read_fds.erase(handle);
   UNUSED_IF_ASSERT_DISABLED(erased);
@@ -401,6 +405,10 @@
   return error;
 
 impl.ProcessEvents();
+
+for (const Callback &callback : m_pending_callbacks)
+  callback(*this);
+m_pending_callbacks.clear();
   }
   return Status();
 }
Index: lldb/include/lldb/Host/MainLoopBase.h
===
--- lldb/include/lldb/Host/MainLoopBase.h
+++ lldb/include/lldb/Host/MainLoopBase.h
@@ -46,6 +46,13 @@
 llvm_unreachable("Not implemented");
   }
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  virtual void AddPendingCallback(const Callback &callback) {
+llvm_unreachable("Not implemented");
+  }
+
   // Waits for registered events and invoke the proper callbacks. Returns when
   // all callbacks deregister themselves or when someone requests termination.
   virtual Status Run() { llvm_unreachable("Not implemented"); }
Index: lldb/include/lldb/Host/MainLoop.h
===
--- lldb/include/lldb/Host/MainLoop.h
+++ lldb/include/lldb/Host/MainLoop.h
@@ -14,6 +14,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include 
 #include 
+#include 
 
 #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
 #define SIGNAL_POLLING_UNSUPPORTED 1
@@ -59,6 +60,11 @@
   SignalHandleUP RegisterSignal(int signo, const Callback &callback,
 Status &error);
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  void AddPendingCallback(const Callback &callback) override;
+
   Status Run() override;
 
   // This should only be performed from a callback. Do not attempt to terminate
@@ -104,6 +110,7 @@
 
   llvm::DenseMap m_read_fds;
   llvm::DenseMap m_signals;
+  std::vector m_pending_callbacks;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis

[Lldb-commits] [lldb] d6b3de7 - [lldb] [llgs] Fix signo sent with fork/vfork/vforkdone events

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T19:47:30+02:00
New Revision: d6b3de72566f874f198b2ddcecbec53e95c623fe

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

LOG: [lldb] [llgs] Fix signo sent with fork/vfork/vforkdone events

Fix ThreadStopInfo struct to include the signal number for all events.
Since signo was not included in the details for fork, vfork
and vforkdone stops, the code incidentally referenced the wrong union
member, resulting in wrong signo being sent.

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

Added: 


Modified: 
lldb/include/lldb/Host/Debug.h
lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Debug.h b/lldb/include/lldb/Host/Debug.h
index 622b7200edbc6..56a6efd1b4d6c 100644
--- a/lldb/include/lldb/Host/Debug.h
+++ b/lldb/include/lldb/Host/Debug.h
@@ -130,12 +130,8 @@ class ResumeActionList {
 
 struct ThreadStopInfo {
   lldb::StopReason reason;
+  uint32_t signo;
   union {
-// eStopReasonSignal
-struct {
-  uint32_t signo;
-} signal;
-
 // eStopReasonException
 struct {
   uint64_t type;

diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
index a75668f3b5c7f..8e6399bcf9c79 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
@@ -81,7 +81,7 @@ void NativeThreadFreeBSD::SetStoppedBySignal(uint32_t signo,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonSignal;
-  m_stop_info.details.signal.signo = signo;
+  m_stop_info.signo = signo;
 
   m_stop_description.clear();
   if (info) {
@@ -100,19 +100,19 @@ void NativeThreadFreeBSD::SetStoppedBySignal(uint32_t 
signo,
 void NativeThreadFreeBSD::SetStoppedByBreakpoint() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonBreakpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByTrace() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonTrace;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByExec() {
   SetStopped();
   m_stop_info.reason = StopReason::eStopReasonExec;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByWatchpoint(uint32_t wp_index) {
@@ -127,7 +127,7 @@ void NativeThreadFreeBSD::SetStoppedByWatchpoint(uint32_t 
wp_index) {
   SetStopped();
   m_stop_description = ostr.str();
   m_stop_info.reason = StopReason::eStopReasonWatchpoint;
-  m_stop_info.details.signal.signo = SIGTRAP;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedByFork(lldb::pid_t child_pid,
@@ -135,6 +135,7 @@ void NativeThreadFreeBSD::SetStoppedByFork(lldb::pid_t 
child_pid,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -144,6 +145,7 @@ void NativeThreadFreeBSD::SetStoppedByVFork(lldb::pid_t 
child_pid,
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.signo = SIGTRAP;
   m_stop_info.details.fork.child_pid = child_pid;
   m_stop_info.details.fork.child_tid = child_tid;
 }
@@ -152,13 +154,14 @@ void NativeThreadFreeBSD::SetStoppedByVForkDone() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonVForkDone;
+  m_stop_info.signo = SIGTRAP;
 }
 
 void NativeThreadFreeBSD::SetStoppedWithNoReason() {
   SetStopped();
 
   m_stop_info.reason = StopReason::eStopReasonNone;
-  m_stop_info.details.signal.signo = 0;
+  m_stop_info.signo = 0;
 }
 
 void NativeThreadFreeBSD::SetStopped() {

diff  --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index 47cc6ec19829a..9572f617e60d3 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -48,19 +48,19 @@ void LogThreadStopInfo(Log &log, const ThreadStopInfo 
&stop_info,
 return;
   case eStopReasonTrace:
 log.Printf("%s: %s trace, stopping signal 0x%" PRIx32

[Lldb-commits] [lldb] 5b04eb2 - [lldb] [MainLoop] Support "pending callbacks", to be called once

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T19:47:30+02:00
New Revision: 5b04eb23ae1a4db074b7ddc2e0c136d008fb5bc7

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

LOG: [lldb] [MainLoop] Support "pending callbacks", to be called once

Support adding a "pending callback" to the main loop, that will be
called once after all the pending events are processed.  This can be
e.g. to defer destroying the process instance until its exit is fully
processed, as suggested in D127500.

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

Added: 


Modified: 
lldb/include/lldb/Host/MainLoop.h
lldb/include/lldb/Host/MainLoopBase.h
lldb/source/Host/common/MainLoop.cpp
lldb/unittests/Host/MainLoopTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/MainLoop.h 
b/lldb/include/lldb/Host/MainLoop.h
index 94499f5834633..f04ac7359cc18 100644
--- a/lldb/include/lldb/Host/MainLoop.h
+++ b/lldb/include/lldb/Host/MainLoop.h
@@ -14,6 +14,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include 
 #include 
+#include 
 
 #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
 #define SIGNAL_POLLING_UNSUPPORTED 1
@@ -59,6 +60,11 @@ class MainLoop : public MainLoopBase {
   SignalHandleUP RegisterSignal(int signo, const Callback &callback,
 Status &error);
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  void AddPendingCallback(const Callback &callback) override;
+
   Status Run() override;
 
   // This should only be performed from a callback. Do not attempt to terminate
@@ -104,6 +110,7 @@ class MainLoop : public MainLoopBase {
 
   llvm::DenseMap m_read_fds;
   llvm::DenseMap m_signals;
+  std::vector m_pending_callbacks;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif

diff  --git a/lldb/include/lldb/Host/MainLoopBase.h 
b/lldb/include/lldb/Host/MainLoopBase.h
index 67857b26ac70b..2a2899341a51b 100644
--- a/lldb/include/lldb/Host/MainLoopBase.h
+++ b/lldb/include/lldb/Host/MainLoopBase.h
@@ -46,6 +46,13 @@ class MainLoopBase {
 llvm_unreachable("Not implemented");
   }
 
+  // Add a pending callback that will be executed once after all the pending
+  // events are processed. The callback will be executed even if termination
+  // was requested.
+  virtual void AddPendingCallback(const Callback &callback) {
+llvm_unreachable("Not implemented");
+  }
+
   // Waits for registered events and invoke the proper callbacks. Returns when
   // all callbacks deregister themselves or when someone requests termination.
   virtual Status Run() { llvm_unreachable("Not implemented"); }

diff  --git a/lldb/source/Host/common/MainLoop.cpp 
b/lldb/source/Host/common/MainLoop.cpp
index d36587ce2346e..8e384c9266b6b 100644
--- a/lldb/source/Host/common/MainLoop.cpp
+++ b/lldb/source/Host/common/MainLoop.cpp
@@ -347,6 +347,10 @@ MainLoop::RegisterSignal(int signo, const Callback 
&callback, Status &error) {
 #endif
 }
 
+void MainLoop::AddPendingCallback(const Callback &callback) {
+  m_pending_callbacks.push_back(callback);
+}
+
 void MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
   bool erased = m_read_fds.erase(handle);
   UNUSED_IF_ASSERT_DISABLED(erased);
@@ -401,6 +405,10 @@ Status MainLoop::Run() {
   return error;
 
 impl.ProcessEvents();
+
+for (const Callback &callback : m_pending_callbacks)
+  callback(*this);
+m_pending_callbacks.clear();
   }
   return Status();
 }

diff  --git a/lldb/unittests/Host/MainLoopTest.cpp 
b/lldb/unittests/Host/MainLoopTest.cpp
index 890f6eb66197e..fc79c0e38a392 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -98,6 +98,56 @@ TEST_F(MainLoopTest, TerminatesImmediately) {
   ASSERT_EQ(1u, callback_count);
 }
 
+TEST_F(MainLoopTest, PendingCallback) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  socketpair[1],
+  [&](MainLoopBase &loop) {
+// Both callbacks should be called before the loop terminates.
+loop.AddPendingCallback(make_callback());
+loop.AddPendingCallback(make_callback());
+loop.RequestTermination();
+  },
+  error);
+  ASSERT_TRUE(error.Success());
+  ASSERT_TRUE(handle);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(2u, callback_count);
+}
+
+TEST_F(MainLoopTest, PendingCallbackCalledOnlyOnce) {
+  char X = 'X';
+  size_t len = sizeof(X);
+  ASSERT_TRUE(socketpair[0]->Write(&X, len).Success());
+
+  MainLoop loop;
+  Status error;
+  auto handle = loop.RegisterReadObject(
+  s

[Lldb-commits] [lldb] 313d9c1 - [lldb] [llgs] Refactor fork/vfork tests, verify state

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T19:47:30+02:00
New Revision: 313d9c1519b7f93b3dcb9ae1d535adf7b0da821d

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

LOG: [lldb] [llgs] Refactor fork/vfork tests, verify state

Refactor the fork and vfork tests to reuse the code better, avoid
unnecessary regexps and avoid unnecessary conversions between
hex-strings and integers.

Verify the server state after detaching.  In particular, verify that
the detached process' PID/TID pair is no longer valid,
and that the correct process remains running.

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

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 4d3ab1efeac7..c0f5629f97fc 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -5,6 +5,12 @@
 
 class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_capture = {1: "parent_pid", 2: "parent_tid",
+3: "child_pid", 4: "child_tid"}
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+
 @add_test_categories(["fork"])
 def test_fork_multithreaded(self):
 self.build()
@@ -15,26 +21,19 @@ def test_fork_multithreaded(self):
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T05.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": fork_regex,
- "capture": {1: "pid", 2: "tid"}},
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-pid = int(ret["pid"], 16)
+child_pid = ret["child_pid"]
 self.reset_test_sequence()
 
 # detach the forked child
 self.test_sequence.add_log_lines([
-"read packet: $D;{:x}#00".format(pid),
-{"direction": "send", "regex": r"[$]OK#.*"},
-], True)
-ret = self.expect_gdbremote_sequence()
-self.reset_test_sequence()
-
-# resume the parent
-self.test_sequence.add_log_lines([
+"read packet: $D;{}#00".format(child_pid),
+"send packet: $OK#00",
 "read packet: $k#00",
 ], True)
 self.expect_gdbremote_sequence()
@@ -49,45 +48,59 @@ def fork_and_detach_test(self, variant):
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T05.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": fork_regex,
- "capture": {1: "pid", 2: "tid"}},
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-pid = int(ret["pid"], 16)
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
 self.reset_test_sequence()
 
 # detach the forked child
 self.test_sequence.add_log_lines([
-"read packet: $D;{:x}#00".format(pid),
-{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $D;{}#00".format(child_pid),
+"send packet: $OK#00",
+# verify that the current process is correct
+"read packet: $qC#00",
+"send packet: $QC{}#00".format(parent_tid),
+# verify that the correct processes are detached/available
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
 ], True)
-ret = self.expect_gdbremote_sequence()
+self.expect_gdbremote_sequence()
 self.reset_test_sequence()
+return parent_pid, parent_tid
 
 @add_test_categories(["fork"])
 def test_fork(self):
-self.fork_and_detach_test("fork")
+parent_pid, _ = self.fork_and_detach_test("fork")
 
 # resume the parent
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": r"[$]W00;process:[0-9a-f]+#

[Lldb-commits] [PATCH] D127193: [lldb] [llgs] Fix signo sent with fork/vfork/vforkdone events

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd6b3de72566f: [lldb] [llgs] Fix signo sent with 
fork/vfork/vforkdone events (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D127193?vs=438410&id=438755#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127193

Files:
  lldb/include/lldb/Host/Debug.h
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -2704,7 +2704,7 @@
 std::ostringstream ostrm;
 // Output the T packet with the thread
 ostrm << 'T';
-int signum = tid_stop_info.details.signal.signo;
+int signum = tid_stop_info.signo;
 DNBLogThreadedIf(
 LOG_RNB_PROC, "%8d %s got signal signo = %u, exc_type = %u",
 (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
@@ -5450,9 +5450,9 @@
   break;
 
 case eStopTypeSignal:
-  if (tid_stop_info.details.signal.signo != 0) {
+  if (tid_stop_info.signo != 0) {
 thread_dict_sp->AddIntegerItem("signal",
-   tid_stop_info.details.signal.signo);
+   tid_stop_info.signo);
 reason_value = "signal";
   }
   break;
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -15,7 +15,7 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
+fork_regex = "[$]T05.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
 {"direction": "send", "regex": fork_regex,
@@ -49,7 +49,7 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
+fork_regex = "[$]T05.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
 {"direction": "send", "regex": fork_regex,
@@ -85,7 +85,7 @@
 # resume the parent
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+{"direction": "send", "regex": r"[$]T05.*vforkdone.*"},
 "read packet: $c#00",
 {"direction": "send", "regex": r"[$]W00;process:[0-9a-f]+#.*"},
 ], True)
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
@@ -728,7 +728,7 @@
   return llvm::make_error(
   "failed to get stop reason", llvm::inconvertibleErrorCode());
 
-const int signum = tid_stop_info.details.signal.signo;
+const int signum = tid_stop_info.signo;
 if (log) {
   LLDB_LOGF(log,
 "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
@@ -804,7 +804,7 @@
 
   // Output the T packet with the thread
   response.PutChar('T');
-  int signum = tid_stop_info.details.signal.signo;
+  int signum = tid_stop_info.signo;
   LLDB_LOG(
   log,
   "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -253,13 +253,12 @@
 
   ThreadStopInfo stop_info;
   stop_info.reason = reason;
-
   // No signal support on Windows but required to provide a 'valid' signum.
+  stop_info.signo = SIGTRAP;
+
   if (reason == StopReason::eStopReasonException) {
 stop_info.details.exception.type = 0;
 stop_info.details.exception.data_count = 0;
-  } else {
-stop_info.details.signal.signo = SIGTRAP;
   }
 
   thread.SetStopReason(stop_info, des

[Lldb-commits] [PATCH] D127290: [lldb] [llgs] Refactor fork/vfork tests, verify state

[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 rG313d9c1519b7: [lldb] [llgs] Refactor fork/vfork tests, 
verify state (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D127290?vs=435160&id=438756#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127290

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -5,6 +5,12 @@
 
 class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_capture = {1: "parent_pid", 2: "parent_tid",
+3: "child_pid", 4: "child_tid"}
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+
 @add_test_categories(["fork"])
 def test_fork_multithreaded(self):
 self.build()
@@ -15,26 +21,19 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T05.*;fork:p([0-9a-f]+)[.]([0-9a-f]+).*"
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": fork_regex,
- "capture": {1: "pid", 2: "tid"}},
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-pid = int(ret["pid"], 16)
+child_pid = ret["child_pid"]
 self.reset_test_sequence()
 
 # detach the forked child
 self.test_sequence.add_log_lines([
-"read packet: $D;{:x}#00".format(pid),
-{"direction": "send", "regex": r"[$]OK#.*"},
-], True)
-ret = self.expect_gdbremote_sequence()
-self.reset_test_sequence()
-
-# resume the parent
-self.test_sequence.add_log_lines([
+"read packet: $D;{}#00".format(child_pid),
+"send packet: $OK#00",
 "read packet: $k#00",
 ], True)
 self.expect_gdbremote_sequence()
@@ -49,45 +48,59 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-fork_regex = "[$]T05.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": fork_regex,
- "capture": {1: "pid", 2: "tid"}},
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
 ], True)
 ret = self.expect_gdbremote_sequence()
-pid = int(ret["pid"], 16)
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
 self.reset_test_sequence()
 
 # detach the forked child
 self.test_sequence.add_log_lines([
-"read packet: $D;{:x}#00".format(pid),
-{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $D;{}#00".format(child_pid),
+"send packet: $OK#00",
+# verify that the current process is correct
+"read packet: $qC#00",
+"send packet: $QC{}#00".format(parent_tid),
+# verify that the correct processes are detached/available
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
 ], True)
-ret = self.expect_gdbremote_sequence()
+self.expect_gdbremote_sequence()
 self.reset_test_sequence()
+return parent_pid, parent_tid
 
 @add_test_categories(["fork"])
 def test_fork(self):
-self.fork_and_detach_test("fork")
+parent_pid, _ = self.fork_and_detach_test("fork")
 
 # resume the parent
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": r"[$]W00;process:[0-9a-f]+#.*"},
+"send packet: $W00;process:{}#00".format(parent_pid),
 ], True)
 self.expect_gdbremote_sequence()
 
 @add_test_categories(["fork"])
 def test_vfork(self):
-self.fork_and_detach_test("vfork")
+parent_pid, parent_tid = self.fork_and_detach_test("vfork")
 
 # resume the parent
 self.test_sequence.add_log_lines([
 "read packet: $c#00",
-{"direction": "send", "regex": r"[$]T05.*vforkdone.*"},
+{"direction":

[Lldb-commits] [lldb] 13eb5b3 - [lldb] [llgs] Add a test for detach-all packet

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T19:47:31+02:00
New Revision: 13eb5b3455fbeb959ab36974e17ba03222d6a4d0

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

LOG: [lldb] [llgs] Add a test for detach-all packet

Add a test verifying that plain 'D' packet correctly detaches all
processes.

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 2a62e7a1fd0e..1cddb5b29c59 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3375,6 +3375,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vRun(
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
+  Log *log = GetLog(LLDBLog::Process);
   StopSTDIOForwarding();
 
   lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
@@ -3398,6 +3399,9 @@ 
GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
   for (auto it = m_debugged_processes.begin();
it != m_debugged_processes.end();) {
 if (pid == LLDB_INVALID_PROCESS_ID || pid == it->first) {
+  LLDB_LOGF(log,
+"GDBRemoteCommunicationServerLLGS::%s detaching %" PRId64,
+__FUNCTION__, it->first);
   if (llvm::Error e = it->second->Detach().ToError())
 detach_error = llvm::joinErrors(std::move(detach_error), std::move(e));
   else {

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index c0f5629f97fc..bbd5912edf7d 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -222,3 +222,43 @@ def test_detach_current(self):
 "send packet: $E44#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_detach_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+# double-check our PIDs
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+# detach all processes
+"read packet: $D#00",
+"send packet: $OK#00",
+# verify that both PIDs are invalid now
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $Eff#00",
+], True)
+self.expect_gdbremote_sequence()



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

[Lldb-commits] [PATCH] D127291: [lldb] [llgs] Add a test for detach-all packet

[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 rG13eb5b3455fb: [lldb] [llgs] Add a test for detach-all packet 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127291

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


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -222,3 +222,43 @@
 "send packet: $E44#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_detach_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+# double-check our PIDs
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+# detach all processes
+"read packet: $D#00",
+"send packet: $OK#00",
+# verify that both PIDs are invalid now
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $Eff#00",
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: $Eff#00",
+], True)
+self.expect_gdbremote_sequence()
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
@@ -3375,6 +3375,7 @@
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) {
+  Log *log = GetLog(LLDBLog::Process);
   StopSTDIOForwarding();
 
   lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
@@ -3398,6 +3399,9 @@
   for (auto it = m_debugged_processes.begin();
it != m_debugged_processes.end();) {
 if (pid == LLDB_INVALID_PROCESS_ID || pid == it->first) {
+  LLDB_LOGF(log,
+"GDBRemoteCommunicationServerLLGS::%s detaching %" PRId64,
+__FUNCTION__, it->first);
   if (llvm::Error e = it->second->Detach().ToError())
 detach_error = llvm::joinErrors(std::move(detach_error), std::move(e));
   else {


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -222,3 +222,43 @@
 "send packet: $E44#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_detach_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+# double-check our PIDs
+"read packet: $Hgp{

[Lldb-commits] [lldb] 80c04c6 - [lldb] [llgs] Attempt to fix LLGS tests on Windows

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-21T20:12:07+02:00
New Revision: 80c04c664a2a1900c5fdbf88c2c11e3f4f4ebf71

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

LOG: [lldb] [llgs] Attempt to fix LLGS tests on Windows

Sponsored by: The FreeBSD Foundation

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestNonStop.py 
b/lldb/test/API/tools/lldb-server/TestNonStop.py
index f2647995a778..2317d7061112 100644
--- a/lldb/test/API/tools/lldb-server/TestNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -1,3 +1,4 @@
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 import gdbremote_testcase
@@ -7,6 +8,7 @@ class 
LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIfWindows  # no SIGSEGV support
 def test_run(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -127,7 +129,7 @@ def test_vCtrlC(self):
  "read packet: $vCtrlC#00",
  "send packet: $OK#00",
  {"direction": "send",
-  "regex": r"^%Stop:T13",
+  "regex": r"^%Stop:T",
   },
  ], True)
 self.expect_gdbremote_sequence()
@@ -147,6 +149,7 @@ def test_exit(self):
  ], True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows  # no clue, the result makes zero sense
 def test_exit_query(self):
 self.build()
 self.set_inferior_startup_launch()



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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

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

This adds a command interpreter fuzzer to LLDB's fuzzing library. The input 
data from the fuzzer is used as input for the command interpreter. Input data 
for the fuzzer is guided by a dictionary of keywords used in LLDB, such as 
"breakpoint", "target" and others.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,53 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp - Fuzz LLDB's command interpreter
+//-===//
+//
+// 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 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+#include "utils/TempFile.h"
+
+using namespace lldb;
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+
+  // Create a breakpoint in the target program and then use the fuzzer
+  // generated input as input for the command interpreter
+  if (thisinterpreter.IsValid()) {
+thisinterpreter.HandleCommand("breakpoint set --name main", ro, false);
+thisinterpreter.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,24 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  ObjectYAML
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+lldbFuzzerUtils
+)
+
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && $ -dict=inputdictionary.txt -only_ascii=1
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(lldb-target-fuzzer)
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(utils)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D128069#3599456 , @dmlary wrote:

> Sent this via email, but doesn't look like it showed up here:
>
>> I'm less concerned about the getter and the property and more about the 
>> underlying functionality. It doesn't look like it's being tested, and adding 
>> it to the SB API allows to change that.
>
> Just to clarify, are you asking me to add a test for the underlying
> functionality, `Section.GetLog2Align()`?  Or is the goal to add the
> test at the SBSection level to ensure we detect if someone ever
> changes the underlying api?

We should test any APIs we add in a python test IMHO. Also testing that the 
".alignment" property works


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128069

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:3
+  Support
+  ObjectYAML
+  )

I assume we don't need this anymore if we're using the dummy target?



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:21
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && 
$ -dict=inputdictionary.txt 
-only_ascii=1
+USES_TERMINAL

Shouldn't we use an absolute path for the input dictionary? Something like 
`${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt`



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:44
+  if (thisinterpreter.IsValid()) {
+thisinterpreter.HandleCommand("breakpoint set --name main", ro, false);
+thisinterpreter.HandleCommand(str.c_str(), ro, false);

Why do we need a breakpoint?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [lldb] 1490f87 - Roll back Michał's changes to debugserver, not meant for there

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2022-06-21T12:57:42-07:00
New Revision: 1490f87154fb31222903a05f6d64c0508016f55c

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

LOG: Roll back Michał's changes to debugserver, not meant for there

Michał's change in https://reviews.llvm.org/D127193 did a search &
replace for a pattern that also appears in debugserver, but it
shouldn't be done there.

Added: 


Modified: 
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/RNBRemote.cpp 
b/lldb/tools/debugserver/source/RNBRemote.cpp
index 3043f03c47a59..ebb2125524e9c 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -2704,7 +2704,7 @@ rnb_err_t 
RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) {
 std::ostringstream ostrm;
 // Output the T packet with the thread
 ostrm << 'T';
-int signum = tid_stop_info.signo;
+int signum = tid_stop_info.details.signal.signo;
 DNBLogThreadedIf(
 LOG_RNB_PROC, "%8d %s got signal signo = %u, exc_type = %u",
 (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__,
@@ -5450,9 +5450,9 @@ RNBRemote::GetJSONThreadsInfo(bool 
threads_with_valid_stop_info_only) {
   break;
 
 case eStopTypeSignal:
-  if (tid_stop_info.signo != 0) {
+  if (tid_stop_info.details.signal.signo != 0) {
 thread_dict_sp->AddIntegerItem("signal",
-   tid_stop_info.signo);
+   tid_stop_info.details.signal.signo);
 reason_value = "signal";
   }
   break;



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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:3
+  Support
+  ObjectYAML
+  )

JDevlieghere wrote:
> I assume we don't need this anymore if we're using the dummy target?
Yes this isn't necessary anymore, I'll remove it and update the revision.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:21
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && 
$ -dict=inputdictionary.txt 
-only_ascii=1
+USES_TERMINAL

JDevlieghere wrote:
> Shouldn't we use an absolute path for the input dictionary? Something like 
> `${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt`
Yes an absolute path is better here, I'll add it and update the revision.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:44
+  if (thisinterpreter.IsValid()) {
+thisinterpreter.HandleCommand("breakpoint set --name main", ro, false);
+thisinterpreter.HandleCommand(str.c_str(), ro, false);

JDevlieghere wrote:
> Why do we need a breakpoint?
This was a leftover from when I ran this fuzzer with a non-dummy target, 
removing it doesn't seem to affect the fuzzer so I can take this line out and 
update the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova updated this revision to Diff 438810.
cassanova added a comment.

Removed ObjectYAML link component from CMakeLists file, changed fuzzer 
invocation to use a relative path for the dictionary file, removed line that 
sets a breakpoint in the fuzzer's LLDB process.


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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp

Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,51 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp - Fuzz LLDB's command interpreter
+//-===//
+//
+// 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 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+#include "utils/TempFile.h"
+
+using namespace lldb;
+using namespace lldb_fuzzer;
+using namespace llvm;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (thisinterpreter.IsValid()) {
+thisinterpreter.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,23 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+lldbFuzzerUtils
+)
+
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && $ -dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
 add_subdirectory(lldb-target-fuzzer)
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(utils)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/tools/lldb-fuzzer/CMakeLists.txt:1-2
 add_subdirectory(lldb-target-fuzzer)
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(utils)

nit: I'd sort these alphabetically



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:15
+liblldb
+lldbFuzzerUtils
+)

I don't think this used any longer. 



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:1-2
+//===-- lldb-commandinterpreter-fuzzer.cpp - Fuzz LLDB's command interpreter
+//-===//
+//

ASCII art needs fixing :-) 



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:17
+#include "lldb/API/SBTarget.h"
+#include "utils/TempFile.h"
+

Not used?


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:39
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+

Nit: the variable naming does really follow the lldb's style.


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:39
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+

mib wrote:
> Nit: the variable naming does really follow the lldb's style.
doesn't *


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 438817.
zequanwu marked 5 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Expression/DWARFExpressionList.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/Utility/RangeMap.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/DWARFExpressionList.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
@@ -23,6 +23,7 @@
   include_dirs = [ ".." ]
   sources = [
 "DWARFExpression.cpp",
+"DWARFExpressionList.cpp",
 "DiagnosticManager.cpp",
 "Expression.cpp",
 "ExpressionVariable.cpp",
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -19,29 +19,29 @@
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x0
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F0
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0001}, language = "", file = '1.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x2
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F1
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0001-0x0002)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0003, 0x0001): DW_OP_reg1 RDX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0001, 0x0002): DW_OP_reg1 RDX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0002}, language = "", file = '2.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x4
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F2
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0002-0x0003)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0005, 0x0001): DW_OP_reg2 RCX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0002, 0x0003): DW_OP_reg2 RCX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0003}, language = "", file = '3.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x6
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F3
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0003-0x0004)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0007, 0x0001): DW_OP_reg3 RBX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0003, 0x0004): DW_OP_reg3 RBX

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

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

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

Rebase and fix for nonstop.


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

https://reviews.llvm.org/D127500

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -262,3 +262,37 @@
 "send packet: $Eff#00",
 ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_kill_all(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+child_pid = ret["child_pid"]
+self.reset_test_sequence()
+
+exit_regex = "[$]X09;process:([0-9a-f]+)#.*"
+self.test_sequence.add_log_lines([
+# kill all processes
+"read packet: $k#00",
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid1"}},
+{"direction": "send", "regex": exit_regex,
+ "capture": {1: "pid2"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertEqual(set([ret["pid1"], ret["pid2"]]),
+ set([parent_pid, child_pid]))
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1041,17 +1041,26 @@
   __FUNCTION__, process->GetID());
   }
 
-  // Close the pipe to the inferior terminal i/o if we launched it and set one
-  // up.
-  MaybeCloseInferiorTerminalConnection();
-
-  // When running in non-stop mode, wait for the vStopped to clear
-  // the notification queue.
-  if (!m_non_stop) {
-// We are ready to exit the debug monitor.
-m_exit_now = true;
-m_mainloop.RequestTermination();
-  }
+  if (m_current_process == process)
+m_current_process = nullptr;
+  if (m_continue_process == process)
+m_continue_process = nullptr;
+
+  lldb::pid_t pid = process->GetID();
+  m_mainloop.AddPendingCallback([&, pid](MainLoopBase &loop) {
+m_debugged_processes.erase(pid);
+// When running in non-stop mode, wait for the vStopped to clear
+// the notification queue.
+if (m_debugged_processes.empty() && !m_non_stop) {
+  // Close the pipe to the inferior terminal i/o if we launched it and set
+  // one up.
+  MaybeCloseInferiorTerminalConnection();
+
+  // We are ready to exit the debug monitor.
+  m_exit_now = true;
+  loop.RequestTermination();
+}
+  });
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
@@ -1214,12 +1223,19 @@
 }
 
 void GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding() {
+  Log *log = GetLog(LLDBLog::Process);
+
   // Don't forward if not connected (e.g. when attaching).
   if (!m_stdio_communication.IsConnected())
 return;
 
+  // If we're not debugging multiple processes, this should be called
+  // once.
+  if (!bool(m_extensions_supported &
+NativeProcessProtocol::Extension::multiprocess))
+assert(!m_stdio_handle_up);
+
   Status error;
-  lldbassert(!m_stdio_handle_up);
   m_stdio_handle_up = m_mainloop.RegisterReadObject(
   m_stdio_communication.GetConnection()->GetReadObject(),
   [this](MainLoopBase &) { SendProcessOutput(); }, error);
@@ -1227,15 +1243,27 @@
   if (!m_stdio_handle_up) {
 // Not much we can do about the failure. Log it and continue without
 // forwarding.
-if (Log *log = GetLog(LLDBLog::Process))
-  LLDB_LOGF(log,
-"GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio "
-"forwarding: %s",
-__FUNCTION__, error.AsCString());
+LLDB_LOG(log, "Failed to set up stdio forwarding: {0}", error);
   }
 }
 
 void GDBRemoteCommunicationServerLLGS::StopSTDIOForwarding() {
+  Log *log = GetLog(LLDBLog::Process);
+
+  if (bool(m_extensions_supported &
+   NativeProcessProtocol::Extension::multiprocess

[Lldb-commits] [PATCH] D127667: [lldb] [llgs] Implement the vKill packet

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

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

Rebase.


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

https://reviews.llvm.org/D127667

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -296,3 +296,60 @@
 ret = self.expect_gdbremote_sequence()
 self.assertEqual(set([ret["pid1"], ret["pid2"]]),
  set([parent_pid, child_pid]))
+
+def vkill_test(self, kill_parent=False, kill_child=False):
+assert kill_parent or kill_child
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+if kill_parent:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(parent_pid),
+"send packet: $OK#00",
+], True)
+if kill_child:
+self.test_sequence.add_log_lines([
+# kill the process
+"read packet: $vKill;{}#00".format(child_pid),
+"send packet: $OK#00",
+], True)
+self.test_sequence.add_log_lines([
+# check child PID/TID
+"read packet: $Hgp{}.{}#00".format(child_pid, child_tid),
+"send packet: ${}#00".format("Eff" if kill_child else "OK"),
+# check parent PID/TID
+"read packet: $Hgp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: ${}#00".format("Eff" if kill_parent else "OK"),
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_vkill_child(self):
+self.vkill_test(kill_child=True)
+
+@add_test_categories(["fork"])
+def test_vkill_parent(self):
+self.vkill_test(kill_parent=True)
+
+@add_test_categories(["fork"])
+def test_vkill_both(self):
+self.vkill_test(kill_parent=True, kill_child=True)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -372,6 +372,8 @@
 return eServerPacketType_vCont;
   if (PACKET_MATCHES("vCont?"))
 return eServerPacketType_vCont_actions;
+  if (PACKET_STARTS_WITH("vKill;"))
+return eServerPacketType_vKill;
   if (PACKET_STARTS_WITH("vRun;"))
 return eServerPacketType_vRun;
   if (PACKET_MATCHES("vStopped"))
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
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/MainLoop.h"
@@ -95,6 +96,7 @@
   std::recursive_mutex m_debugged_process_mutex;
   std::unordered_map>
   m_debugged_processes;
+  std::unordered_set m_vkilled_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
@@ -129,6 +131,8 @@
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vKill(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_qProcessInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qC(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remo

[Lldb-commits] [PATCH] D127862: [lldb] [llgs] Support resuming one process with PID!=current via vCont

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

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

Rebase.


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

https://reviews.llvm.org/D127862

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -354,7 +354,7 @@
 def test_vkill_both(self):
 self.vkill_test(kill_parent=True, kill_child=True)
 
-def resume_one_test(self, run_order):
+def resume_one_test(self, run_order, use_vCont=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
 self.add_qSupported_packets(["multiprocess+",
@@ -395,11 +395,19 @@
 else:
 assert False, "unexpected x={}".format(x)
 
+if use_vCont:
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $vCont;c:p{}.{}#00".format(*pidtid),
+], True)
+else:
+self.test_sequence.add_log_lines([
+# continue the selected process
+"read packet: $Hcp{}.{}#00".format(*pidtid),
+"send packet: $OK#00",
+"read packet: $c#00",
+], True)
 self.test_sequence.add_log_lines([
-# continue the selected process
-"read packet: $Hcp{}.{}#00".format(*pidtid),
-"send packet: $OK#00",
-"read packet: $c#00",
 {"direction": "send", "regex": expect},
 ], True)
 # if at least one process remained, check both PIDs
@@ -431,3 +439,117 @@
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
+
+@add_test_categories(["fork"])
+def test_vCont_parent(self):
+self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_child(self):
+self.resume_one_test(run_order=["child", "child"], use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_parent_then_child(self):
+self.resume_one_test(run_order=["parent", "parent", "child", "child"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_child_then_parent(self):
+self.resume_one_test(run_order=["child", "child", "parent", "parent"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_interspersed(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ use_vCont=True)
+
+@add_test_categories(["fork"])
+def test_vCont_two_processes(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid = ret["parent_pid"]
+parent_tid = ret["parent_tid"]
+child_pid = ret["child_pid"]
+child_tid = ret["child_tid"]
+self.reset_test_sequence()
+
+self.test_sequence.add_log_lines([
+# try to resume both processes
+"read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+parent_pid, parent_tid, child_pid, child_tid),
+"send packet: $E03#00",
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_vCont_all_processes_explicit(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+

[Lldb-commits] [PATCH] D128170: [lldb] [llgs] Implement the 'T' packet

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

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

Rebase.


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

https://reviews.llvm.org/D128170

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -866,3 +866,76 @@
  "send packet: $QCp{:x}.{:x}#00".format(*pidtid),
  ], True)
 self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_T(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(len(pidtids), 4)
+max_pid = max(pid for pid, tid in pidtids)
+max_tid = max(tid for pid, tid in pidtids)
+bad_pidtids = (
+(max_pid, max_tid + 1, "E02"),
+(max_pid + 1, max_tid, "E01"),
+(max_pid + 1, max_tid + 1, "E01"),
+)
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+[
+ # test explicit PID+TID
+ "read packet: $Tp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ # test implicit PID via Hg
+ "read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $T{:x}#00".format(max_tid + 1),
+ "send packet: $E02#00",
+ "read packet: $T{:x}#00".format(pidtid[1]),
+ "send packet: $OK#00",
+ ], True)
+for pid, tid, expected in bad_pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Tp{:x}.{:x}#00".format(pid, tid),
+ "send packet: ${}#00".format(expected),
+ ], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -243,6 +243,8 @@
 
   PacketResult Handle_QMemTags(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_T(StringExtractorGDBRemote &packet);
+
   void SetCurrentThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetCurrentThreadID() const;
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
@@ -107,6 +107,8 @@
 &GDBRemoteCommunicationServerLLGS::Handle_P);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qC,
 &GDBRemoteCommunicationServerLLGS::Handle_qC);
+  RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_T,
+&GDBRemoteCommunicationServerLLGS::Handle_T);
   RegisterMemberFunctionHandler(
 

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt:15
+liblldb
+lldbFuzzerUtils
+)

JDevlieghere wrote:
> I don't think this used any longer. 
Ok, I can remove this library.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:17
+#include "lldb/API/SBTarget.h"
+#include "utils/TempFile.h"
+

JDevlieghere wrote:
> Not used?
Not in this file, but removing that include causes the compiler to not 
recognize the llvm and lldb_fuzzer namespaces. That's also fine in this file 
because neither of those namespaces are used but I still found it strange.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:39
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+

mib wrote:
> mib wrote:
> > Nit: the variable naming does really follow the lldb's style.
> doesn't *
Oh shoot, I didn't notice. Would `interpreter` or `ci` be a better variable 
name?


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:39
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+

cassanova wrote:
> mib wrote:
> > mib wrote:
> > > Nit: the variable naming does really follow the lldb's style.
> > doesn't *
> Oh shoot, I didn't notice. Would `interpreter` or `ci` be a better variable 
> name?
I was more annoyed by the fact that the variable started with `this`. It's a 
reserved keyword in C++ and that can it make error prone. However, `ci` is a 
great candidate for this :)


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: lldb/source/Expression/DWARFExpressionList.cpp:179
+  if (m_exprs.GetSize() == 1) {
+expr = m_exprs.Back()->data;
+  } else {

labath wrote:
> I don't think this is correct.
> 
> We should be able to distinguish between a location list that is always valid 
> (i.e., one which is not a location list at all), and a location list which 
> happens to contain a single valid range.
> 
> That could probably be achieved by making the range of the always-valid entry 
> be (0, UINT_MAX), but maybe it would be better to have an explicit 
> fallback/default entry for this (one that is used when no ranged entry 
> applies). This is how the default location entries in DWARF5 are supposed to 
> work, although I am not sure if any compiler actually makes use of them.
> That could probably be achieved by making the range of the always-valid entry 
> be (0, UINT_MAX)
This looks better to me as it saves one default entry and there is no 
difference in terms of using DWARFExpressionList APIs. 
Added `IsAlwaysValidSingleExpr` to check if it is an always-valid entry and 
uses it to distinguish between the two kinds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 438831.
JDevlieghere edited the summary of this revision.

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

https://reviews.llvm.org/D127986

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Log.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -339,14 +339,19 @@
   Emit(message);
 }
 
-StreamLogHandler::StreamLogHandler(int fd, bool should_close, bool unbuffered)
-: m_stream(fd, should_close, unbuffered) {}
+StreamLogHandler::StreamLogHandler(int fd, bool should_close,
+   size_t buffer_size)
+: m_stream(fd, should_close, false) {
+  if (buffer_size > 0)
+m_stream.SetBufferSize(buffer_size);
+}
 
-void StreamLogHandler::Emit(llvm::StringRef message) {
-  m_stream << message;
+StreamLogHandler::~StreamLogHandler() {
   m_stream.flush();
 }
 
+void StreamLogHandler::Emit(llvm::StringRef message) { m_stream << message; }
+
 CallbackLogHandler::CallbackLogHandler(lldb::LogOutputCallback callback,
void *baton)
 : m_callback(callback), m_baton(baton) {}
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1409,7 +1409,7 @@
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- llvm::raw_ostream &error_stream) {
+ size_t buffer_size, llvm::raw_ostream &error_stream) {
   const bool should_close = true;
 
   std::shared_ptr log_handler_sp;
@@ -1420,7 +1420,7 @@
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
 log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close);
+GetOutputFile().GetDescriptor(), !should_close, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1441,7 +1441,7 @@
   }
 
   log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close);
+  (*file)->GetDescriptor(), should_close, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -428,9 +428,11 @@
 Desc<"Clears the current command history.">;
 }
 
-let Command = "log" in {
+let Command = "log enable" in {
   def log_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
 Desc<"Set the destination file to log to.">;
+  def log_buffer_size : Option<"buffer", "b">, Group<1>, Arg<"UnsignedInteger">,
+Desc<"Set the log to be buffered, using the specified buffer size.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
 Desc<"Enable thread safe logging to avoid interweaved log lines.">;
   def log_verbose : Option<"verbose", "v">, Group<1>,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/FileSpec.h"
@@ -21,7 +22,7 @@
 using namespace lldb;
 using namespace lldb_private;
 
-#define LLDB_OPTIONS_log
+#define LLDB_OPTIONS_log_enable
 #include "CommandOptions.inc"
 
 /// Common completion logic for log enable/disable.
@@ -89,6 +90,10 @@
 log_file.SetFile(option_arg, FileSpec::Style::native);
 FileSystem::Instance().Resolve(log_file);
 break;
+  case 'b':
+error =
+buffer_size.SetValueFromString(option_arg, eVarSetOperationAssig

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D128026#3595457 , @labath wrote:

> I feel I should note that the llvm streams already have a buffered mode, and 
> given the way things are implemented now (our log class writes the entire log 
> message in one call, and the raw_ostream flushing the entire buffer whenever 
> a message does not fit), I don't think this class has any advantages over 
> using stream buffering (that's a very long-winded way of saying you're not 
> going to get partial messages in either of the solutions). The only two 
> differences I see are:
>
> - one of them specifies the size, the other specifies the number of messages
> - different behavior in case of races (in non-thread-safe mode). I'll write 
> more on that elsewhere.

As I said in the other patch, I (personally) don't really care all that much 
about buffering except for the circular buffer. I've implemented an 
alterantive, stream based buffering approach in 
https://reviews.llvm.org/D127986. I'll put up a separate patch for the circular 
buffer.


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

https://reviews.llvm.org/D128026

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

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Recent revisions of the Objective-C runtime changed
objc_debug_class_getNameRaw() in a way that no longer triggers lazy
names to be instantiated. This has the unintended side-effect of making
generic bridged Swift classes, such as _SwiftDeferredNSDictionary
to become invisible to the Objective-C runtime.

This patch detects this situation and forces the names to be
instantiated by calling class_getName() and discarding the result before
calling objc_debug_class_getNameRaw() again.

Many thanks to Mike Ash for outlining the solution and Adrian for
authoring the downstream patch.

rdar://95245318


https://reviews.llvm.org/D128306

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -205,7 +205,7 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
@@ -239,6 +239,7 @@
 void free(void *ptr);
 size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
 const char* objc_debug_class_getNameRaw(Class cls);
+const char* class_getName(Class cls);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
@@ -278,7 +279,11 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr) {
+   class_getName(isa); // Realize name of lazy classes.
+   name_ptr = objc_debug_class_getNameRaw(isa);
+}
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -205,7 +205,7 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
@@ -239,6 +239,7 @@
 void free(void *ptr);
 size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
 const char* objc_debug_class_getNameRaw(Class cls);
+const char* class_getName(Class cls);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
@@ -278,7 +279,11 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr) {
+   class_getName(isa); // Realize name of lazy classes.
+   name_ptr = objc_debug_class_getNameRaw(isa);
+}
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova updated this revision to Diff 438839.
cassanova added a comment.

Sorted subdirectories alphabetically in top-level CMakeLists file.

Removed lldbfuzzer link library in command interpreter CMakeLists file.

Fixed ASCII art in command interpreter source file, renamed `thisinterpreter` 
to `ci`, removed unused include in command interpreter source file.


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

https://reviews.llvm.org/D128292

Files:
  lldb/tools/lldb-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
  lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
  
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp


Index: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp - Fuzz LLDB's command interpreter 
-===//
+//
+// 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 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBTarget.h"
+
+using namespace lldb;
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) {
+  SBDebugger::Initialize();
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  // Convert the data into a null-terminated string
+  std::string str((char *)data, size);
+
+  // Create a debugger and a dummy target
+  SBDebugger debugger = SBDebugger::Create(false);
+  SBTarget target = debugger.GetDummyTarget();
+
+  // Create a command interpreter for the current debugger
+  // A return object is needed to run the command interpreter
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter ci = debugger.GetCommandInterpreter();
+
+  // Use the fuzzer generated input as input for the command interpreter
+  if (ci.IsValid()) {
+ci.HandleCommand(str.c_str(), ro, false);
+  }
+
+  debugger.DeleteTarget(target);
+  SBDebugger::Destroy(debugger);
+  SBModule::GarbageCollectAllocatedModules();
+
+  return 0;
+}
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/inputdictionary.txt
@@ -0,0 +1,4 @@
+kw1="breakpoint set"
+kw2="target"
+kw3="run"
+kw4="frame info"
Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/CMakeLists.txt
@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_llvm_fuzzer(lldb-commandinterpreter-fuzzer
+  EXCLUDE_FROM_ALL
+  lldb-commandinterpreter-fuzzer.cpp
+  )
+
+if(TARGET lldb-commandinterpreter-fuzzer)
+  target_include_directories(lldb-commandinterpreter-fuzzer PRIVATE ..)
+  target_link_libraries(lldb-commandinterpreter-fuzzer
+PRIVATE
+liblldb
+)
+
+  add_custom_target(fuzz-lldb-commandinterpreter
+COMMENT "Running the LLDB command interpreter fuzzer..."
+COMMAND cd ${CMAKE_CURRENT_SOURCE_DIR} && 
$ 
-dict=${CMAKE_CURRENT_SOURCE_DIR}/inputdictionary.txt  -only_ascii=1
+USES_TERMINAL
+)
+endif()
Index: lldb/tools/lldb-fuzzer/CMakeLists.txt
===
--- lldb/tools/lldb-fuzzer/CMakeLists.txt
+++ lldb/tools/lldb-fuzzer/CMakeLists.txt
@@ -1,2 +1,3 @@
+add_subdirectory(lldb-commandinterpreter-fuzzer)
 add_subdirectory(lldb-target-fuzzer)
 add_subdirectory(utils)


Index: lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
===
--- /dev/null
+++ lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp
@@ -0,0 +1,47 @@
+//===-- lldb-commandinterpreter-fuzzer.cpp - Fuzz LLDB's command interpreter -===//
+//
+// 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 
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandInterpreterRunOptions.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/S

[Lldb-commits] [PATCH] D128292: [lldb/Fuzzer] Add command interpreter fuzzer for LLDB

[Chelsea Cassanova via Phabricator via lldb-commits]

cassanova added inline comments.



Comment at: 
lldb/tools/lldb-fuzzer/lldb-commandinterpreter-fuzzer/lldb-commandinterpreter-fuzzer.cpp:39
+  SBCommandReturnObject ro = SBCommandReturnObject();
+  SBCommandInterpreter thisinterpreter = debugger.GetCommandInterpreter();
+

mib wrote:
> cassanova wrote:
> > mib wrote:
> > > mib wrote:
> > > > Nit: the variable naming does really follow the lldb's style.
> > > doesn't *
> > Oh shoot, I didn't notice. Would `interpreter` or `ci` be a better variable 
> > name?
> I was more annoyed by the fact that the variable started with `this`. It's a 
> reserved keyword in C++ and that can it make error prone. However, `ci` is a 
> great candidate for this :)
Ah, good point about that. I renamed it to `ci`.


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

https://reviews.llvm.org/D128292

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good!


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> Mainly added you in case you want to try this out on android.

Thanks. I tested this on a few Android API levels, and it looks good to me.




Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:441
+  &m_process->GetTarget()) == m_interpreter_base) {
+ModuleSP interpreter_sp = m_interpreter_module.lock();
+if (m_interpreter_module.lock() == nullptr) {

Use this variable below, instead of calling lock() again?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

This is an interesting take that makes more sense than specifying a size in 
bytes and a "m_circular" since that kind of thing could cut the first message. 
Saving entire messages as they are logged might make more sense for our 
circular log messages as we would ensure we have full message lines, though it 
does come with a "copy each message" cost




Comment at: lldb/source/Utility/Log.cpp:361
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+

maybe assert size > 0 here to make sure we don't crash in Emit() or set m_size 
to 1 if size is zero?



Comment at: lldb/source/Utility/Log.cpp:366
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size)

If we are going to buffer, I worry about the performance with saving an array 
or strings that must be copied each time. Though I do like that we are saving 
messages as entire buffers.

If we go the circular buffer route we can either have one fixed size buffer 
that we copy into and then try the keep the current insert index, or copy the 
strings like you are already doing.


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

https://reviews.llvm.org/D128026

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

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Dave Lee via Phabricator via lldb-commits]

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

thanks Jim




Comment at: lldb/source/Commands/CommandObjectProcess.cpp:600
+  m_options.m_run_to_bkpt_args, target, result, &run_to_bkpt_ids, 
+  BreakpointName::Permissions::disablePerm);
+  if (!result.Succeeded()) {

from a quick read, it looks like this step will silently ignore bkpts it can't 
disable.

then a couple lines down the user might be told the breakpoints don't actually 
exist, which may not be true?



Comment at: lldb/source/Commands/CommandObjectProcess.cpp:636-637
+  if (bp_sp->IsEnabled()) {
+if (loc_id == LLDB_INVALID_BREAK_ID)
+  any_enabled = true;
+else {

this logic looks off, why is `any_enabled` set to true when the loc_id is 
`LLDB_INVALID_BREAK_ID`?



Comment at: 
lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py:2
+"""
+Test the "process continue -t" option.
+"""

s/-t/-b/



Comment at: 
lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py:16
+
+mydir = TestBase.compute_mydir(__file__)
+

if you edit this file before committing, you can remove this line. `mydir` is 
now automatically computed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

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

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

[Adrian Prantl via Phabricator via lldb-commits]

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

Thanks!


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

https://reviews.llvm.org/D128306

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

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add a setting to specify the preferred dynamic class info extractor function.


https://reviews.llvm.org/D128312

Files:
  lldb/include/lldb/Target/Target.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -54,6 +54,10 @@
 EnumValues<"OptionEnumValues(g_import_std_module_value_types)">,
 Desc<"Import the 'std' C++ module to improve expression parsing involving "
  " C++ standard library types.">;
+  def DynamicClassInfoHelper: Property<"dynamic-class-info-helper", "Enum">,
+DefaultEnumValue<"eDynamicClassInfoHelperAuto">,
+EnumValues<"OptionEnumValues(g_dynamic_class_info_helper_value_types)">,
+Desc<"Set the way we parse the dynamic class info from the Objective-C runtime.">;
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
 DefaultTrue,
 Desc<"Automatically apply fix-it hints to expressions.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3801,6 +3801,21 @@
 },
 };
 
+static constexpr OptionEnumValueElement
+g_dynamic_class_info_helper_value_types[] = {
+{
+eDynamicClassInfoHelperAuto,
+"auto",
+"Automatically determine the best helper function.",
+},
+{eDynamicClassInfoHelperRealizedClassesStruct, "RealizedClassesStruct",
+ "Prefer parsing the realized classes struct."},
+{eDynamicClassInfoHelperCopyRealizedClassList, "CopyRealizedClassList",
+ "Prefer using the CopyRealizedClassList API."},
+{eDynamicClassInfoHelperGetRealizedClassList, "GetRealizedClassList",
+ "Prefer using the GetRealizedClassList API."},
+};
+
 static constexpr OptionEnumValueElement g_hex_immediate_style_values[] = {
 {
 Disassembler::eHexStyleC,
@@ -4299,6 +4314,13 @@
   nullptr, idx, g_target_properties[idx].default_uint_value);
 }
 
+DynamicClassInfoHelper TargetProperties::GetDynamicClassInfoHelper() const {
+  const uint32_t idx = ePropertyDynamicClassInfoHelper;
+  return (DynamicClassInfoHelper)
+  m_collection_sp->GetPropertyAtIndexAsEnumeration(
+  nullptr, idx, g_target_properties[idx].default_uint_value);
+}
+
 bool TargetProperties::GetEnableAutoApplyFixIts() const {
   const uint32_t idx = ePropertyAutoApplyFixIts;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -341,7 +341,7 @@
 /// must use gdb_objc_realized_classes. Otherwise, we prefer
 /// objc_getRealizedClassList_trylock and objc_copyRealizedClassList
 /// respectively, depending on availability.
-Helper ComputeHelper() const;
+Helper ComputeHelper(ExecutionContext &exe_ctx) const;
 
 UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
  Helper helper);
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1724,7 +1724,8 @@
 }
 
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::Helper
-AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
+AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper(
+ExecutionContext &exe_ctx) const {
   if (!m_runtime.m_has_objc_copyRealizedClassList &&
   !m_runtime.m_has_objc_getRealizedClassList_trylock)
 return DynamicClassInfoExtractor::gdb_objc_realized_classes;
@@ -1732,10 +1733,20 @@
   if (Process *process = m_runtime.GetProcess()) {
 if (DynamicLoader *loader = process->GetDynamicLoader()) {
   if (loader->IsFullyInitialized()) {
-if (m_runtime.m_has_objc_getRealizedClassList_trylock)
-  return DynamicClassInfoExtractor::objc_getRealizedClassList_trylock;
-if (m_runtime.m_has_objc_copyRealizedClassList)
-  return DynamicClassInfoExtractor::objc_copyRealizedClassList;
+switch (exe_ctx.GetTargetRef().GetDyna

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

With D127986  I don't think we need this 
anymore.


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

https://reviews.llvm.org/D128026

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

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Target/TargetProperties.td:57
  " C++ standard library types.">;
+  def DynamicClassInfoHelper: Property<"dynamic-class-info-helper", "Enum">,
+DefaultEnumValue<"eDynamicClassInfoHelperAuto">,

Should this have "objc" in the name?



Comment at: lldb/source/Target/TargetProperties.td:60
+EnumValues<"OptionEnumValues(g_dynamic_class_info_helper_value_types)">,
+Desc<"Set the way we parse the dynamic class info from the Objective-C 
runtime.">;
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,

Configure how LLDB parses dynamic Objective-C class metadata. By default LLDB 
will choose the most appropriate method for the target OS.


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

https://reviews.llvm.org/D128312

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

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

[Adrian Prantl via Phabricator via lldb-commits]

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

Patch looks fine. Wording suggestion for the user-visible parts inside.


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

https://reviews.llvm.org/D128312

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

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 438876.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Adress Adrian's code review feedback


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

https://reviews.llvm.org/D128312

Files:
  lldb/include/lldb/Target/Target.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -54,6 +54,10 @@
 EnumValues<"OptionEnumValues(g_import_std_module_value_types)">,
 Desc<"Import the 'std' C++ module to improve expression parsing involving "
  " C++ standard library types.">;
+  def DynamicClassInfoHelper: Property<"objc-dynamic-class-extractor", "Enum">,
+DefaultEnumValue<"eDynamicClassInfoHelperAuto">,
+EnumValues<"OptionEnumValues(g_dynamic_class_info_helper_value_types)">,
+Desc<"Configure how LLDB parses dynamic Objective-C class metadata. By default LLDB will choose the most appropriate method for the target OS.">;
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
 DefaultTrue,
 Desc<"Automatically apply fix-it hints to expressions.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3801,6 +3801,22 @@
 },
 };
 
+static constexpr OptionEnumValueElement
+g_dynamic_class_info_helper_value_types[] = {
+{
+eDynamicClassInfoHelperAuto,
+"auto",
+"Automatically determine the most appropriate method for the "
+"target OS.",
+},
+{eDynamicClassInfoHelperRealizedClassesStruct, "RealizedClassesStruct",
+ "Prefer using the realized classes struct."},
+{eDynamicClassInfoHelperCopyRealizedClassList, "CopyRealizedClassList",
+ "Prefer using the CopyRealizedClassList API."},
+{eDynamicClassInfoHelperGetRealizedClassList, "GetRealizedClassList",
+ "Prefer using the GetRealizedClassList API."},
+};
+
 static constexpr OptionEnumValueElement g_hex_immediate_style_values[] = {
 {
 Disassembler::eHexStyleC,
@@ -4299,6 +4315,13 @@
   nullptr, idx, g_target_properties[idx].default_uint_value);
 }
 
+DynamicClassInfoHelper TargetProperties::GetDynamicClassInfoHelper() const {
+  const uint32_t idx = ePropertyDynamicClassInfoHelper;
+  return (DynamicClassInfoHelper)
+  m_collection_sp->GetPropertyAtIndexAsEnumeration(
+  nullptr, idx, g_target_properties[idx].default_uint_value);
+}
+
 bool TargetProperties::GetEnableAutoApplyFixIts() const {
   const uint32_t idx = ePropertyAutoApplyFixIts;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -341,7 +341,7 @@
 /// must use gdb_objc_realized_classes. Otherwise, we prefer
 /// objc_getRealizedClassList_trylock and objc_copyRealizedClassList
 /// respectively, depending on availability.
-Helper ComputeHelper() const;
+Helper ComputeHelper(ExecutionContext &exe_ctx) const;
 
 UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
  Helper helper);
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1724,7 +1724,8 @@
 }
 
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::Helper
-AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
+AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper(
+ExecutionContext &exe_ctx) const {
   if (!m_runtime.m_has_objc_copyRealizedClassList &&
   !m_runtime.m_has_objc_getRealizedClassList_trylock)
 return DynamicClassInfoExtractor::gdb_objc_realized_classes;
@@ -1732,10 +1733,20 @@
   if (Process *process = m_runtime.GetProcess()) {
 if (DynamicLoader *loader = process->GetDynamicLoader()) {
   if (loader->IsFullyInitialized()) {
-if (m_runtime.m_has_objc_getRealizedClassList_trylock)
-  return DynamicClassInfoExtractor::objc_getRealizedClassList_trylock;
-if (m_runtime.m_has_objc_copyRealizedClassList)
-  return 

[Lldb-commits] [PATCH] D126513: Add -b (--continue-to-breakpoint) option to the "process continue" command

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 438879.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126513

Files:
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/commands/process/continue_to_bkpt/Makefile
  lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
  lldb/test/API/commands/process/continue_to_bkpt/main.c

Index: lldb/test/API/commands/process/continue_to_bkpt/main.c
===
--- /dev/null
+++ lldb/test/API/commands/process/continue_to_bkpt/main.c
@@ -0,0 +1,18 @@
+#include 
+
+int main (int argc, char const *argv[])
+{
+  int pass_me = argc + 10; // Stop here to get started.
+  printf("This is the zeroth stop\n");
+  printf("This is the first stop\n");
+  printf("This is the second stop\n");
+  printf("This is the third stop\n");
+  printf("This is the fourth stop\n");
+  printf("This is the fifth stop\n");
+  printf("This is the sixth stop\n");
+  printf("This is the seventh stop\n");
+  printf("This is the eighth stop\n");
+  printf("This is the nineth stop\n");
+
+  return 0;
+}
Index: lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
===
--- /dev/null
+++ lldb/test/API/commands/process/continue_to_bkpt/TestContinueToBkpts.py
@@ -0,0 +1,132 @@
+"""
+Test the "process continue -b" option.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestContinueToBkpts(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_continue_to_breakpoints(self):
+"""Test that the continue to breakpoints feature works correctly."""
+self.build()
+self.do_test_continue_to_breakpoint()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.main_source_spec = lldb.SBFileSpec("main.c")
+
+def continue_and_check(self, stop_list, bkpt_to_hit, loc_to_hit = 0):
+"""Build up a command that will run a continue -b commands using the breakpoints on stop_list, and
+   ensure that we hit bkpt_to_hit.
+   If loc_to_hit is not 0, also verify that we hit that location."""
+command = "process continue"
+for elem in stop_list:
+command += " -b {0}".format(elem)
+self.expect(command)
+self.assertEqual(self.thread.stop_reason, lldb.eStopReasonBreakpoint, "Hit a breakpoint")
+self.assertEqual(self.thread.GetStopReasonDataAtIndex(0), bkpt_to_hit, "Hit the right breakpoint")
+if loc_to_hit != 0:
+self.assertEqual(self.thread.GetStopReasonDataAtIndex(1), loc_to_hit, "Hit the right location")
+for bkpt_id in self.bkpt_list:
+bkpt = self.target.FindBreakpointByID(bkpt_id)
+self.assertTrue(bkpt.IsValid(), "Breakpoint id's round trip")
+if bkpt.MatchesName("disabled"):
+self.assertFalse(bkpt.IsEnabled(), "Disabled breakpoints stay disabled: {0}".format(bkpt.GetID()))
+else:
+self.assertTrue(bkpt.IsEnabled(), "Enabled breakpoints stay enabled: {0}".format(bkpt.GetID()))
+# Also do our multiple location one:
+bkpt = self.target.FindBreakpointByID(self.multiple_loc_id)
+self.assertTrue(bkpt.IsValid(), "Breakpoint with locations round trip")
+for i in range(1,3):
+loc = bkpt.FindLocationByID(i)
+self.assertTrue(loc.IsValid(), "Locations round trip")
+if i == 2:
+self.assertTrue(loc.IsEnabled(), "Locations that were enabled stay enabled")
+else:
+self.assertFalse(loc.IsEnabled(), "Locations that were disabled stay disabled")
+
+def do_test_continue_to_breakpoint(self):
+"""Test the continue to breakpoint feature."""
+(self.target, process, self.thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   "Stop here to get started", self.main_source_spec)
+
+# Now set up all our breakpoints:
+bkpt_pattern = "This is the {0} stop"
+bkpt_elements = ["zeroth", "first", "second", "third", "fourth", "fifth", "sixth", "seventh", "eighth", "nineth"]
+disabled_bkpts = ["first", "eigth"]
+bkpts_for_MyBKPT = ["first", "sixth", "nineth"]
+self.bkpt_list = []
+for elem in bkpt_elements:
+bkpt = self.target.BreakpointCreateBySourceRegex(bkpt_pattern.format(elem), self.main_source_spec)
+self.assertGreater(bkpt.GetNumLocations(), 0, "Found a bkpt match")
+self.bkpt_list.append(bkpt.GetID())
+bkpt.AddName(elem)
+

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[walter erquinigo via Phabricator via lldb-commits]

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

In order to provide simple scripting support on top of instruction traces, a 
simple solution is to enhance the `dump instructions` command and allow 
printing in json and directly to a file. The format is verbose and not space 
efficient, but it's not supposed to be used for really large traces, in which 
case the TraceCursor API is the way to go.

- add a -j option for printing the dump in json
- add a -J option for pretty printing the json output
- add a -F option for specifying an output file
- add a -a option for dumping all the instructions available starting at the 
initial point configured with the other flags
- add tests for all cases
- refactored the instruction dumper and abstracted the actual "printing" logic. 
There are two writer implementations: CLI and JSON. This made the dumper itself 
much more readable and maintanable

sample output:

  (lldb) thread trace dump instructions  -t -a --id 100 -J
  [
{
  "id": 100,
  "tsc": "43591204528448966"
  "loadAddress": "0x407a91",
  "module": "a.out",
  "symbol": "void std::deque>::_M_push_back_aux(Foo&&)",
  "mnemonic": "movq",
  "source": "/usr/include/c++/8/bits/deque.tcc",
  "line": 492,
  "column": 30
},
...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128316

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceTSC.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -17,7 +17,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -58,7 +58,10 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
+
+self.expect("thread trace dump instructions --tsc -c 1 --pretty-json",
+patterns=['''"tsc": "\d+"'''])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -73,6 +76,9 @@
 self.expect("thread trace dump instructions --tsc -c 1",
 patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
+self.expect("thread trace dump instructions --tsc -c 1 --json",
+patterns=['''"tsc":null'''])
+
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testPSBPeriod(self):
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -26,11 +26,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionF

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

[Med Ismail Bennani via Phabricator via lldb-commits]

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

Hi @mgorny ! `TestNonStop.py` is also failing on the macOS bots: 
https://green.lab.llvm.org/green/job/lldb-cmake/44743/

Let me know if you need help reproducing the issue, otherwise skip the test on 
Darwin systems `@skipIfDarwin`

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125575

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

[Lldb-commits] [PATCH] D128316: [trace] Add an option to dump instructions in json and to a file

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 438884.
wallace added a comment.

add a test for -a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128316

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceTSC.py

Index: lldb/test/API/commands/trace/TestTraceTSC.py
===
--- lldb/test/API/commands/trace/TestTraceTSC.py
+++ lldb/test/API/commands/trace/TestTraceTSC.py
@@ -17,7 +17,7 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -58,7 +58,10 @@
 
 self.expect("n")
 self.expect("thread trace dump instructions --tsc -c 1",
-patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x00400511movl"])
+patterns=["0: \[tsc=\d+\] 0x00400511movl"])
+
+self.expect("thread trace dump instructions --tsc -c 1 --pretty-json",
+patterns=['''"tsc": "\d+"'''])
 
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
@@ -73,6 +76,9 @@
 self.expect("thread trace dump instructions --tsc -c 1",
 patterns=["0: \[tsc=unavailable\] 0x00400511movl"])
 
+self.expect("thread trace dump instructions --tsc -c 1 --json",
+patterns=['''"tsc":null'''])
+
 @testSBAPIAndCommands
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testPSBPeriod(self):
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -13,11 +13,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -26,11 +26,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 2 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+  substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
"m.out`foo() + 65 at multi_thread.cpp:12:21",
-   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+   "19520: [tsc=40450075477657246] 0x00400ba7jg 0x400bb3"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+  substrs=["67910: [tsc=40450075477799536] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
 @testSBAPIAndCommands
@@ -39,11 +39,11 @@
 trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
 self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 self.expect("thread trace dump instructions 3 -t",
-  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing 

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

@labath let me know if this is what you had in mind


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [lldb] c08f61b - [lldb] Instantiate lazily named classes on macOS Ventura.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-21T18:51:38-07:00
New Revision: c08f61b45e3b93c25bc0405a489a382a54a5d941

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

LOG: [lldb] Instantiate lazily named classes on macOS Ventura.

Recent revisions of the Objective-C runtime changed
objc_debug_class_getNameRaw() in a way that no longer triggers lazy
names to be instantiated. This has the unintended side-effect of making
generic bridged Swift classes, such as _SwiftDeferredNSDictionary
to become invisible to the Objective-C runtime.

This patch detects this situation and forces the names to be
instantiated by calling class_getName() and discarding the result before
calling objc_debug_class_getNameRaw() again.

Many thanks to Mike Ash for outlining the solution and Adrian for
authoring the downstream patch.

rdar://95245318

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

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 4194004fe61c..5c00da8b40ad 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -205,7 +205,7 @@ __lldb_apple_objc_v2_get_dynamic_class_info2(void 
*gdb_objc_realized_classes_ptr
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
@@ -239,6 +239,7 @@ extern "C" {
 void free(void *ptr);
 size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
 const char* objc_debug_class_getNameRaw(Class cls);
+const char* class_getName(Class cls);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
@@ -278,7 +279,11 @@ __lldb_apple_objc_v2_get_dynamic_class_info3(void 
*gdb_objc_realized_classes_ptr
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr) {
+   class_getName(isa); // Realize name of lazy classes.
+   name_ptr = objc_debug_class_getNameRaw(isa);
+}
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;



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

[Lldb-commits] [lldb] c866f85 - [lldb] Add a setting to specify the preferred dynamic class info extractor o

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-21T18:51:39-07:00
New Revision: c866f8544c929578a49e0b222f2171da71c9f415

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

LOG: [lldb] Add a setting to specify the preferred dynamic class info extractor 
o

Add a setting to configure how LLDB parses dynamic Objective-C class
metadata. By default LLDB will choose the most appropriate method for
the target OS.

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

Added: 


Modified: 
lldb/include/lldb/Target/Target.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 53fe4831b82fa..93ea8504f8bac 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -71,6 +71,13 @@ enum ImportStdModule {
   eImportStdModuleTrue,
 };
 
+enum DynamicClassInfoHelper {
+  eDynamicClassInfoHelperAuto,
+  eDynamicClassInfoHelperRealizedClassesStruct,
+  eDynamicClassInfoHelperCopyRealizedClassList,
+  eDynamicClassInfoHelperGetRealizedClassList,
+};
+
 class TargetExperimentalProperties : public Properties {
 public:
   TargetExperimentalProperties();
@@ -152,6 +159,8 @@ class TargetProperties : public Properties {
 
   ImportStdModule GetImportStdModule() const;
 
+  DynamicClassInfoHelper GetDynamicClassInfoHelper() const;
+
   bool GetEnableAutoApplyFixIts() const;
 
   uint64_t GetNumberOfRetriesWithFixits() const;

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 5c00da8b40ade..4d21f1f3765f3 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1724,7 +1724,8 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::GetClassInfoArgs(Helper helper) {
 }
 
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::Helper
-AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
+AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper(
+ExecutionContext &exe_ctx) const {
   if (!m_runtime.m_has_objc_copyRealizedClassList &&
   !m_runtime.m_has_objc_getRealizedClassList_trylock)
 return DynamicClassInfoExtractor::gdb_objc_realized_classes;
@@ -1732,10 +1733,20 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
   if (Process *process = m_runtime.GetProcess()) {
 if (DynamicLoader *loader = process->GetDynamicLoader()) {
   if (loader->IsFullyInitialized()) {
-if (m_runtime.m_has_objc_getRealizedClassList_trylock)
-  return DynamicClassInfoExtractor::objc_getRealizedClassList_trylock;
-if (m_runtime.m_has_objc_copyRealizedClassList)
-  return DynamicClassInfoExtractor::objc_copyRealizedClassList;
+switch (exe_ctx.GetTargetRef().GetDynamicClassInfoHelper()) {
+case eDynamicClassInfoHelperAuto:
+  [[clang::fallthrough]];
+case eDynamicClassInfoHelperGetRealizedClassList:
+  if (m_runtime.m_has_objc_getRealizedClassList_trylock)
+return 
DynamicClassInfoExtractor::objc_getRealizedClassList_trylock;
+  [[clang::fallthrough]];
+case eDynamicClassInfoHelperCopyRealizedClassList:
+  if (m_runtime.m_has_objc_copyRealizedClassList)
+return DynamicClassInfoExtractor::objc_copyRealizedClassList;
+  [[clang::fallthrough]];
+case eDynamicClassInfoHelperRealizedClassesStruct:
+  return DynamicClassInfoExtractor::gdb_objc_realized_classes;
+}
   }
 }
   }
@@ -1872,7 +1883,7 @@ 
AppleObjCRuntimeV2::DynamicClassInfoExtractor::UpdateISAToDescriptorMap(
   Status err;
 
   // Compute which helper we're going to use for this update.
-  const DynamicClassInfoExtractor::Helper helper = ComputeHelper();
+  const DynamicClassInfoExtractor::Helper helper = ComputeHelper(exe_ctx);
 
   // Read the total number of classes from the hash table
   const uint32_t num_classes =

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
index 0a2c8ff29275a..1a8e9d4aa339c 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -341,7 +341,7 @@ class AppleObjCRuntimeV2 : public AppleObjCRuntime {
 /// 

[Lldb-commits] [PATCH] D128306: [lldb] Instantiate lazily named classes on macOS Ventura.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc08f61b45e3b: [lldb] Instantiate lazily named classes on 
macOS Ventura. (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128306

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -205,7 +205,7 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
@@ -239,6 +239,7 @@
 void free(void *ptr);
 size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
 const char* objc_debug_class_getNameRaw(Class cls);
+const char* class_getName(Class cls);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
@@ -278,7 +279,11 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr) {
+   class_getName(isa); // Realize name of lazy classes.
+   name_ptr = objc_debug_class_getNameRaw(isa);
+}
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -205,7 +205,7 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
@@ -239,6 +239,7 @@
 void free(void *ptr);
 size_t objc_getRealizedClassList_trylock(Class *buffer, size_t len);
 const char* objc_debug_class_getNameRaw(Class cls);
+const char* class_getName(Class cls);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
@@ -278,7 +279,11 @@
 {
 Class isa = realized_class_list[i];
 const char *name_ptr = objc_debug_class_getNameRaw(isa);
-if (name_ptr == NULL)
+if (!name_ptr) {
+   class_getName(isa); // Realize name of lazy classes.
+   name_ptr = objc_debug_class_getNameRaw(isa);
+}
+if (!name_ptr)
 continue;
 const char *s = name_ptr;
 uint32_t h = 5381;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128312: [lldb] Add a setting to specify the preferred dynamic class info extractor function

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc866f8544c92: [lldb] Add a setting to specify the preferred 
dynamic class info extractor o (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128312

Files:
  lldb/include/lldb/Target/Target.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -54,6 +54,10 @@
 EnumValues<"OptionEnumValues(g_import_std_module_value_types)">,
 Desc<"Import the 'std' C++ module to improve expression parsing involving "
  " C++ standard library types.">;
+  def DynamicClassInfoHelper: Property<"objc-dynamic-class-extractor", "Enum">,
+DefaultEnumValue<"eDynamicClassInfoHelperAuto">,
+EnumValues<"OptionEnumValues(g_dynamic_class_info_helper_value_types)">,
+Desc<"Configure how LLDB parses dynamic Objective-C class metadata. By default LLDB will choose the most appropriate method for the target OS.">;
   def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
 DefaultTrue,
 Desc<"Automatically apply fix-it hints to expressions.">;
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3801,6 +3801,22 @@
 },
 };
 
+static constexpr OptionEnumValueElement
+g_dynamic_class_info_helper_value_types[] = {
+{
+eDynamicClassInfoHelperAuto,
+"auto",
+"Automatically determine the most appropriate method for the "
+"target OS.",
+},
+{eDynamicClassInfoHelperRealizedClassesStruct, "RealizedClassesStruct",
+ "Prefer using the realized classes struct."},
+{eDynamicClassInfoHelperCopyRealizedClassList, "CopyRealizedClassList",
+ "Prefer using the CopyRealizedClassList API."},
+{eDynamicClassInfoHelperGetRealizedClassList, "GetRealizedClassList",
+ "Prefer using the GetRealizedClassList API."},
+};
+
 static constexpr OptionEnumValueElement g_hex_immediate_style_values[] = {
 {
 Disassembler::eHexStyleC,
@@ -4299,6 +4315,13 @@
   nullptr, idx, g_target_properties[idx].default_uint_value);
 }
 
+DynamicClassInfoHelper TargetProperties::GetDynamicClassInfoHelper() const {
+  const uint32_t idx = ePropertyDynamicClassInfoHelper;
+  return (DynamicClassInfoHelper)
+  m_collection_sp->GetPropertyAtIndexAsEnumeration(
+  nullptr, idx, g_target_properties[idx].default_uint_value);
+}
+
 bool TargetProperties::GetEnableAutoApplyFixIts() const {
   const uint32_t idx = ePropertyAutoApplyFixIts;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -341,7 +341,7 @@
 /// must use gdb_objc_realized_classes. Otherwise, we prefer
 /// objc_getRealizedClassList_trylock and objc_copyRealizedClassList
 /// respectively, depending on availability.
-Helper ComputeHelper() const;
+Helper ComputeHelper(ExecutionContext &exe_ctx) const;
 
 UtilityFunction *GetClassInfoUtilityFunction(ExecutionContext &exe_ctx,
  Helper helper);
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1724,7 +1724,8 @@
 }
 
 AppleObjCRuntimeV2::DynamicClassInfoExtractor::Helper
-AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper() const {
+AppleObjCRuntimeV2::DynamicClassInfoExtractor::ComputeHelper(
+ExecutionContext &exe_ctx) const {
   if (!m_runtime.m_has_objc_copyRealizedClassList &&
   !m_runtime.m_has_objc_getRealizedClassList_trylock)
 return DynamicClassInfoExtractor::gdb_objc_realized_classes;
@@ -1732,10 +1733,20 @@
   if (Process *process = m_runtime.GetProcess()) {
 if (DynamicLoader *loader = process->GetDynamicLoader()) {
   if (loader->IsFullyInitialized()) {
-if (m_runtime.m_has_objc_getRealizedClassList_trylock)
-  return DynamicClassInfoExtrac

[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets

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

mgorny added a comment.

In D125575#3600640 , @mib wrote:

> Hi @mgorny ! `TestNonStop.py` is also failing on the macOS bots: 
> https://green.lab.llvm.org/green/job/lldb-cmake/44743/
>
> Let me know if you need help reproducing the issue, otherwise skip the test 
> on Darwin systems `@skipIfDarwin`

Sorry about that. Actually, I need to mark them as LLGS-specific. Thanks for 
reporting!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125575

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

[Lldb-commits] [lldb] 5e9aed1 - [lldb] [test] Mark TestNonStop as LLGS-specific

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-22T05:36:30+02:00
New Revision: 5e9aed1be5a5608d97c2ec3ac87698a8257b0ea7

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

LOG: [lldb] [test] Mark TestNonStop as LLGS-specific

Thanks for Med Ismail Bennani for reporting the debugserver failures.

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestNonStop.py 
b/lldb/test/API/tools/lldb-server/TestNonStop.py
index 2317d7061112..ebe66366b173 100644
--- a/lldb/test/API/tools/lldb-server/TestNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -9,6 +9,7 @@ class 
LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @skipIfWindows  # no SIGSEGV support
+@add_test_categories(["llgs"])
 def test_run(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -116,6 +117,7 @@ def test_run(self):
 # finally, verify that all threads have started
 self.assertEqual(len(all_threads), thread_num + 1)
 
+@add_test_categories(["llgs"])
 def test_vCtrlC(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -134,6 +136,7 @@ def test_vCtrlC(self):
  ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["llgs"])
 def test_exit(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -150,6 +153,7 @@ def test_exit(self):
 self.expect_gdbremote_sequence()
 
 @skipIfWindows  # no clue, the result makes zero sense
+@add_test_categories(["llgs"])
 def test_exit_query(self):
 self.build()
 self.set_inferior_startup_launch()



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

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

[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/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1967
+
+  if (multiprocess) {
+for (auto &pid_ptr : m_debugged_processes)

labath wrote:
> without multiprocess extensions, we should never have more than one process, 
> right? Could we just unconditionally use the multiprocess loop here (perhaps 
> with an `assert(m_debugged_processes.size() == 1 || multiprocess)`) ?
Yes, that makes sense. We enable fork & vfork only if multiprocess is 
supported, so that shouldn't happen unless we deal with broken Process plugin.


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

https://reviews.llvm.org/D128152

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [PATCH] D128152: [lldb] [llgs] Support multiprocess in qfThreadInfo

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

mgorny updated this revision to Diff 438914.
mgorny marked an inline comment as done.
mgorny added a comment.

Simplify to use a single loop for both multiprocess and non-multiprocess modes. 
Replace the static function with `include_pid` with a class method, to prepare 
for adding a new helper to print PID+TID.


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

https://reviews.llvm.org/D128152

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -554,6 +554,66 @@
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_threadinfo(self):
+self.build()
+self.prep_debug_monitor_and_inferior(
+inferior_args=["fork",
+   "thread:new",
+   "trap",
+   ])
+self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("fork-events+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+pidtids = [
+(ret["parent_pid"], ret["parent_tid"]),
+(ret["child_pid"], ret["child_tid"]),
+]
+prev_pidtids = set(self.parse_threadinfo_packets(ret))
+self.assertEqual(prev_pidtids,
+ frozenset((int(pid, 16), int(tid, 16))
+   for pid, tid in pidtids))
+self.reset_test_sequence()
+
+for pidtid in pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ {"direction": "send",
+  "regex": "^[$]T05thread:p{}.{}.*".format(*pidtid),
+  },
+ ], True)
+self.add_threadinfo_collection_packets()
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+new_pidtids = set(self.parse_threadinfo_packets(ret))
+added_pidtid = new_pidtids - prev_pidtids
+prev_pidtids = new_pidtids
+
+# verify that we've got exactly one new thread, and that
+# the PID matches
+self.assertEqual(len(added_pidtid), 1)
+self.assertEqual(added_pidtid.pop()[0], int(pidtid[0], 16))
+
+for pidtid in new_pidtids:
+self.test_sequence.add_log_lines(
+["read packet: $Hgp{:x}.{:x}#00".format(*pidtid),
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_memory_read_write(self):
 self.build()
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
@@ -159,6 +159,9 @@
 
   PacketResult Handle_qRegisterInfo(StringExtractorGDBRemote &packet);
 
+  void AddProcessThreads(StreamGDBRemote &response,
+ NativeProcessProtocol &process, bool &had_any);
+
   PacketResult Handle_qfThreadInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1997,38 +1997,46 @@
   return SendPacketNoLock(response.GetString());
 }
 
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo(
-StringExtractorGDBRemote &packet) {
+void GDBRemoteCommunicationServerLLGS::AddProcessThreads(
+StreamGDBRemote &response, NativeProcessProtocol &process, bool &had_any) {
   Log *log = GetLog(LLDBLog::Thread);
 
-

[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Add an OSLog log handler on macOS. These log messages end up in Console.app and 
will be part of a sysdiagnose.


https://reviews.llvm.org/D128321

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -32,6 +32,12 @@
 #include 
 #endif
 
+#if defined(__APPLE__)
+#include 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+#endif
+
 using namespace lldb_private;
 
 llvm::ManagedStatic Log::g_channel_map;
@@ -389,3 +395,15 @@
   }
   stream.flush();
 }
+
+#if defined(__APPLE__)
+OSLogLogHandler::OSLogLogHandler() {
+  std::call_once(g_os_log_once, []() {
+g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+  });
+}
+
+void OSLogLogHandler::Emit(llvm::StringRef message) {
+  os_log_with_type(g_os_log, OS_LOG_TYPE_DEFAULT, "%{public}s", 
message.data());
+}
+#endif
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -96,6 +96,14 @@
   size_t m_total_count = 0;
 };
 
+#if defined(__APPLE__)
+class OSLogLogHandler : public LogHandler {
+public:
+  OSLogLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+#endif
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -32,6 +32,12 @@
 #include 
 #endif
 
+#if defined(__APPLE__)
+#include 
+static os_log_t g_os_log;
+static std::once_flag g_os_log_once;
+#endif
+
 using namespace lldb_private;
 
 llvm::ManagedStatic Log::g_channel_map;
@@ -389,3 +395,15 @@
   }
   stream.flush();
 }
+
+#if defined(__APPLE__)
+OSLogLogHandler::OSLogLogHandler() {
+  std::call_once(g_os_log_once, []() {
+g_os_log = os_log_create("com.apple.dt.lldb", "lldb");
+  });
+}
+
+void OSLogLogHandler::Emit(llvm::StringRef message) {
+  os_log_with_type(g_os_log, OS_LOG_TYPE_DEFAULT, "%{public}s", message.data());
+}
+#endif
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -96,6 +96,14 @@
   size_t m_total_count = 0;
 };
 
+#if defined(__APPLE__)
+class OSLogLogHandler : public LogHandler {
+public:
+  OSLogLogHandler();
+  void Emit(llvm::StringRef message) override;
+};
+#endif
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128323: [lldb] Add support for specifying a log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, kastiglione, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch adds a new flag to `log enable` allowing the user to specify a log 
handler. This makes it possible to enable logging to a circular buffer (D127937 
) or with OSLog (D128321 
) as well as the default stream handler. As 
is the patch is more of an RFC/POC. It needs some cleaning up as well as a test 
case, which I'll add if everyone is happy with the direction.


https://reviews.llvm.org/D128323

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, eLogHandlerStream, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1406,10 +1406,30 @@
debugger_id, once);
 }
 
+static std::shared_ptr
+CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close,
+ size_t buffer_size) {
+
+  switch (log_handler_kind) {
+  case eLogHandlerStream:
+return std::make_shared(fd, should_close, buffer_size);
+  case eLogHandlerRotating:
+return std::make_shared(buffer_size);
+#if defined(__APPLE__)
+  case eLogHandlerOSLog:
+return std::make_shared();
+#endif
+  case eLogHandlerCallback:
+return {};
+  }
+  return {};
+}
+
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- size_t buffer_size, llvm::raw_ostream &error_stream) {
+ size_t buffer_size, LogHandlerKind log_handler_kind,
+ llvm::raw_ostream &error_stream) {
   const bool should_close = true;
 
   std::shared_ptr log_handler_sp;
@@ -1419,8 +1439,9 @@
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = std::make_shared(
-GetOutputFile().GetDescriptor(), !should_close, buffer_size);
+log_handler_sp =
+CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(),
+ !should_close, buffer_size);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1440,8 +1461,9 @@
 return false;
   }
 
-  log_handler_sp = std::make_shared(
-  (*file)->GetDescriptor(), should_close, buffer_size);
+  log_handler_sp =
+  CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(),
+   !should_close, buffer_size);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -433,6 +433,8 @@
 Desc<"Set the destination file to log to.">;
   def log_buffer_size : Option<"buffer", "b">, Group<1>, Arg<"UnsignedInteger">,
 Desc<"Set the log to be buffered, using the specified buffer size.">;
+  def log_handler : Option<"handler", "h">, Group<1>,
+EnumArg<"Value", "LogHandlerType()">, Desc<"Use a custom handler.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
 Desc<"Enable thread safe logging to avoid interweaved log lines.">;
   def log_verbose : Option<"verbose", "v">, Group<1>,
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionValueEnumeration.h"
 #include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/Args.h"
@@ -22,6 +23,30 @@
 using namespace lldb;
 using namespace lldb_private;
 
+static constexpr OptionEnumValueElement g_log_handler_type[] = {
+{
+eLogHand

[Lldb-commits] [PATCH] D128324: [lldb] [llgs] Introduce a AppendThreadIDToResponse() helper

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

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

Introduce a helper function to append GDB Remote Serial Protocol "thread
IDs", with optional PID in multiprocess mode.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128324

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
@@ -273,6 +273,9 @@
   // in non-stop mode, no response otherwise.
   PacketResult SendContinueSuccessResponse();
 
+  void AppendThreadIDToResponse(StreamString &response, lldb::pid_t pid,
+lldb::tid_t tid);
+
 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
@@ -826,10 +826,8 @@
 
   // Include the (pid and) tid.
   response.PutCString("thread:");
-  if (bool(m_extensions_supported &
-   NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", process.GetID());
-  response.Format("{0:x-};", thread.GetID());
+  AppendThreadIDToResponse(response, process.GetID(), thread.GetID());
+  response.PutChar(';');
 
   // Include the thread name if there is one.
   const std::string thread_name = thread.GetName();
@@ -1448,9 +1446,8 @@
 
   StreamString response;
   response.PutCString("QC");
-  if (bool(m_extensions_supported & 
NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", m_current_process->GetID());
-  response.Format("{0:x-}", thread->GetID());
+  AppendThreadIDToResponse(response, m_current_process->GetID(),
+   thread->GetID());
 
   return SendPacketNoLock(response.GetString());
 }
@@ -2019,10 +2016,7 @@
 LLDB_LOG(log, "iterated thread {0} (tid={1})", thread_index,
  thread->GetID());
 response.PutChar(had_any ? ',' : 'm');
-if (bool(m_extensions_supported &
- NativeProcessProtocol::Extension::multiprocess))
-  response.Format("p{0:x-}.", pid);
-response.Format("{0:x-}", thread->GetID());
+AppendThreadIDToResponse(response, pid, thread->GetID());
 had_any = true;
   }
 }
@@ -4164,6 +4158,14 @@
   return m_non_stop ? SendOKResponse() : PacketResult::Success;
 }
 
+void GDBRemoteCommunicationServerLLGS::AppendThreadIDToResponse(
+StreamString &response, lldb::pid_t pid, lldb::tid_t tid) {
+  if (bool(m_extensions_supported &
+   NativeProcessProtocol::Extension::multiprocess))
+response.Format("p{0:x-}.", pid);
+  response.Format("{0:x-}", tid);
+}
+
 std::string
 lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
bool reverse_connect) {


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
@@ -273,6 +273,9 @@
   // in non-stop mode, no response otherwise.
   PacketResult SendContinueSuccessResponse();
 
+  void AppendThreadIDToResponse(StreamString &response, lldb::pid_t pid,
+lldb::tid_t tid);
+
 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
@@ -826,10 +826,8 @@
 
   // Include the (pid and) tid.
   response.PutCString("thread:");
-  if (bool(m_extensions_supported &
-   NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}.", process.GetID());
-  response.Format("{0:x-};", thread.GetID());
+  AppendThreadIDToResponse(response, process.GetID(), thread.GetID());
+  response.PutChar(';');
 
   // Include the thread name if there is one.
   const std::string thread_name = thread.GetName();
@@ -1448,9 +1446,8 @@
 
   StreamString response;
   response.PutCString("QC");
-  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::multiprocess))
-response.Format("p{0:x-}