[Lldb-commits] [PATCH] D116255: [lldb] [Process/FreeBSDKernel] Support finding all processes

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for doing this.

Interesting solution to the problem. I'm not sure how long will the diff files 
remain applicable, but I would've accepted this even without them so I think 
all is fine.




Comment at: 
lldb/test/API/functionalities/postmortem/FreeBSDKernel/TestFreeBSDKernelVMCore.py:60-61
+self.assertEqual(
+[thread.GetFrameAtIndex(i).addr.GetLoadAddress(target)
+ for i in range(thread.GetNumFrames())],
+bt_expected)

If you want to be even more fancy :)


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

https://reviews.llvm.org/D116255

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

[Lldb-commits] [lldb] ca271f4 - [lldb-server/linux] Fix waitpid for multithreaded forks

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-01-03T14:27:52+01:00
New Revision: ca271f4ef5a2a4bf115ac11ada70bbd7c737d77d

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

LOG: [lldb-server/linux] Fix waitpid for multithreaded forks

The lldb-server code is currently set up in a way that each
NativeProcess instance does its own waitpid handling. This works fine
for BSDs, where the code can do a waitpid(process_id), and get
information for all threads in that process.

The situation is trickier on linux, because waitpid(pid) will only
return information for the main thread of the process (one whose tid ==
pid). For this reason the linux code does a waitpid(-1), to get
information for all threads. This was fine while we were supporting just
a single process, but becomes a problem when we have multiple processes
as they end up stealing each others events.

There are two possible solutions to this problem:
- call waitpid(-1) centrally, and then dispatch the events to the
  appropriate process
- have each process call waitpid(tid) for all the threads it manages

This patch implements the second approach. Besides fitting better into
the existing design, it also has the added benefit of ensuring
predictable ordering for thread/process creation events (which come in
pairs -- one for the parent and one for the child). The first approach
OTOH, would make this ordering even more complicated since we would
have to keep the half-threads hanging in mid-air until we find the
process we should attach them to.

The downside to this approach is an increased number of syscalls (one
waitpid for each thread), but I think we're pretty far from optimizing
things like this, and so the cleanliness of the design is worth it.

The included test reproduces the circumstances which should demonstrate
the bug (which manifests as a hung test), but I have not been able to
get it to fail. The only place I've seen this failure modes are very
rare hangs in the thread sanitizer tests (tsan forks an addr2line
process to produce its error messages).

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 8f5496d9f4e5a..4a77e791343ca 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -426,22 +426,24 @@ Status 
NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t pid) {
 }
 
 // Handles all waitpid events from the inferior process.
-void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, WaitStatus status) {
+void NativeProcessLinux::MonitorCallback(NativeThreadLinux &thread,
+ WaitStatus status) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   // Certain activities 
diff er based on whether the pid is the tid of the main
   // thread.
-  const bool is_main_thread = (pid == GetID());
+  const bool is_main_thread = (thread.GetID() == GetID());
 
   // Handle when the thread exits.
   if (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal) {
 LLDB_LOG(log,
  "got exit status({0}) , tid = {1} ({2} main thread), process "
  "state = {3}",
- status, pid, is_main_thread ? "is" : "is not", GetState());
+ status, thread.GetID(), is_main_thread ? "is" : "is not",
+ GetState());
 
 // This is a thread that exited.  Ensure we're not tracking it anymore.
-StopTrackingThread(pid);
+StopTrackingThread(thread);
 
 if (is_main_thread) {
   // The main thread exited.  We're done monitoring.  Report to delegate.
@@ -454,37 +456,15 @@ void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, 
WaitStatus status) {
   }
 
   siginfo_t info;
-  const auto info_err = GetSignalInfo(pid, &info);
-  auto thread_sp = GetThreadByID(pid);
-
-  if (!thread_sp) {
-// Normally, the only situation when we cannot find the thread is if we
-// have just received a new thread notification. This is indicated by
-// GetSignalInfo() returning si_code == SI_USER and si_pid == 0
-LLDB_LOG(log, "received notification about an unknown tid {0}.", pid);
-
-if (info_err.Fail()) {
-  LLDB_LOG(log,
-   "(tid {0}) GetSignalInfo failed ({1}). "
-   "Ingoring this notification.",
-   pid, info_err);
-  return;
-}
-
-LLDB_LOG(log, "tid {0}, si_code: {1}, si_pid: {2}", pid, info.si_code,
- info.si_pid);
-
-MonitorC

[Lldb-commits] [PATCH] D116372: [lldb-server/linux] Fix waitpid for multithreaded forks

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGca271f4ef5a2: [lldb-server/linux] Fix waitpid for 
multithreaded forks (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116372

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.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
@@ -6,6 +6,40 @@
 class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 mydir = TestBase.compute_mydir(__file__)
 
+@add_test_categories(["fork"])
+def test_fork_multithreaded(self):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=["thread:new"]*2 + ["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
+fork_regex = "[$]T.*;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"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid = int(ret["pid"], 16)
+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: $k#00",
+], True)
+self.expect_gdbremote_sequence()
+
 def fork_and_detach_test(self, variant):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=[variant])
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -164,7 +164,7 @@
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  void MonitorCallback(lldb::pid_t pid, WaitStatus status);
+  void MonitorCallback(NativeThreadLinux &thread, WaitStatus status);
 
   void WaitForCloneNotification(::pid_t pid);
 
@@ -180,7 +180,7 @@
 
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
-  bool StopTrackingThread(lldb::tid_t thread_id);
+  void StopTrackingThread(NativeThreadLinux &thread);
 
   /// Create a new thread.
   ///
@@ -243,20 +243,9 @@
   /// Manages Intel PT process and thread traces.
   IntelPTManager m_intel_pt_manager;
 
-  struct CloneInfo {
-int event;
-lldb::tid_t parent_tid;
-  };
-
-  // Map of child processes that have been signaled once, and we are
-  // waiting for the second signal.
-  llvm::DenseMap> m_pending_pid_map;
-
-  // Handle a clone()-like event.  If received by parent, clone_info contains
-  // additional info.  Returns true if the event is handled, or false if it
-  // is pending second notification.
-  bool MonitorClone(lldb::pid_t child_pid,
-llvm::Optional clone_info);
+  // Handle a clone()-like event.
+  bool MonitorClone(NativeThreadLinux &parent, lldb::pid_t child_pid,
+int event);
 };
 
 } // namespace process_linux
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -426,22 +426,24 @@
 }
 
 // Handles all waitpid events from the inferior process.
-void NativeProcessLinux::MonitorCallback(lldb::pid_t pid, WaitStatus status) {
+void NativeProcessLinux::MonitorCallback(NativeThreadLinux &thread,
+ WaitStatus status) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   // Certain activities differ based on whether the pid is the tid of the main
   // thread.
-  const bool is_main_thread = (pid == GetID());
+  const bool is_main_thread = (thread.GetID() == GetID());
 
   // Handle when the thread exits.
   if (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal) {
 LLDB_LOG(log,
  "got exit status({0}) , tid = {1} ({2} main thread), process "
  "state = {3}",
- status, pid, is_main_thread ? "is" : "is not", GetState());
+ status, thr

[Lldb-commits] [lldb] 862fffd - [lldb/qemu] Set qemu's "ld prefix" based on the platform sysroot

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-01-03T14:48:13+01:00
New Revision: 862fffd8231c8c44a8ea8071041eac8919aed346

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

LOG: [lldb/qemu] Set qemu's "ld prefix" based on the platform sysroot

Both serve the same purpose (finding shared libraries) and allow one to
launch a dynamically linked executable by just specifying the platform
sysroot.

Added: 


Modified: 
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/test/API/qemu/TestQemuLaunch.py

Removed: 




diff  --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp 
b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index 84e10042a97c..dd7546d8fa15 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -191,6 +191,8 @@ lldb::ProcessSP 
PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
   launch_info.SetArguments(args, true);
 
   Environment emulator_env = Host::GetEnvironment();
+  if (ConstString sysroot = GetSDKRootDirectory())
+emulator_env["QEMU_LD_PREFIX"] = sysroot.GetStringRef().str();
   for (const auto &KV : GetGlobalProperties().GetEmulatorEnvVars())
 emulator_env[KV.first()] = KV.second;
   launch_info.GetEnvironment() = ComputeLaunchEnvironment(

diff  --git a/lldb/test/API/qemu/TestQemuLaunch.py 
b/lldb/test/API/qemu/TestQemuLaunch.py
index e27d7a70fa0b..afa158339b6e 100644
--- a/lldb/test/API/qemu/TestQemuLaunch.py
+++ b/lldb/test/API/qemu/TestQemuLaunch.py
@@ -249,3 +249,11 @@ def test_arg0(self):
 
 self.assertEqual(state["program"], self.getBuildArtifact())
 self.assertEqual(state["0"], "ARG0")
+
+def test_sysroot(self):
+sysroot = self.getBuildArtifact("sysroot")
+self.runCmd("platform select qemu-user --sysroot %s" % sysroot)
+state = self._run_and_get_state()
+self.assertEqual(state["environ"]["QEMU_LD_PREFIX"], sysroot)
+self.assertIn("QEMU_LD_PREFIX",
+state["environ"]["QEMU_UNSET_ENV"].split(","))



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

[Lldb-commits] [lldb] a8ae682 - [lldb] Delete GDBRemoteCommunicationReplayServer

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-01-03T16:13:57+01:00
New Revision: a8ae6828a98dcd5ea083eb07be8ad6db77b688a2

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

LOG: [lldb] Delete GDBRemoteCommunicationReplayServer

This survived the reproducer deletion.

Added: 


Modified: 
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Removed: 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h



diff  --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
index f594f43b3f136..425839c883a44 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
@@ -14,7 +14,6 @@
 
 #include "Plugins/Process/Utility/GDBRemoteSignals.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
-#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h"
 #include "lldb/Target/Platform.h"
 
 namespace lldb_private {
@@ -155,7 +154,6 @@ class PlatformRemoteGDBServer : public Platform, private 
UserIDResolver {
 
 protected:
   process_gdb_remote::GDBRemoteCommunicationClient m_gdb_client;
-  process_gdb_remote::GDBRemoteCommunicationReplayServer m_gdb_replay_server;
   std::string m_platform_description; // After we connect we can get a more
   // complete description of what we are
   // connected to

diff  --git a/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt 
b/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
index 448d032b381f1..d578033e1c414 100644
--- a/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
@@ -20,7 +20,6 @@ add_lldb_library(lldbPluginProcessGDBRemote PLUGIN
   GDBRemoteCommunication.cpp
   GDBRemoteCommunicationClient.cpp
   GDBRemoteCommunicationHistory.cpp
-  GDBRemoteCommunicationReplayServer.cpp
   GDBRemoteCommunicationServer.cpp
   GDBRemoteCommunicationServerCommon.cpp
   GDBRemoteCommunicationServerLLGS.cpp

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
deleted file mode 100644
index c91d7cb5ac30f..0
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
+++ /dev/null
@@ -1,314 +0,0 @@
-//===-- GDBRemoteCommunicationReplayServer.cpp 
===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include 
-
-#include "lldb/Host/Config.h"
-#include "llvm/ADT/ScopeExit.h"
-
-#include "GDBRemoteCommunicationReplayServer.h"
-#include "ProcessGDBRemoteLog.h"
-
-// C Includes
-// C++ Includes
-#include 
-
-// Project includes
-#include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Utility/ConstString.h"
-#include "lldb/Utility/Event.h"
-#include "lldb/Utility/FileSpec.h"
-#include "lldb/Utility/StreamString.h"
-#include "lldb/Utility/StringExtractorGDBRemote.h"
-
-using namespace llvm;
-using namespace lldb;
-using namespace lldb_private;
-using namespace lldb_private::process_gdb_remote;
-
-/// Check if the given expected packet matches the actual packet.
-static bool unexpected(llvm::StringRef expected, llvm::StringRef actual) {
-  // The 'expected' string contains the raw data, including the leading $ and
-  // trailing checksum. The 'actual' string contains only the packet's content.
-  if (expected.contains(actual))
-return false;
-  // Contains a PID which might be 
diff erent.
-  if (expected.contains("vAttach"))
-return false;
-  // Contains a ascii-hex-path.
-  if (expected.contains("QSetSTD"))
-return false;
-  // Contains environment values.
-  if (expected.contains("QEnvironment"))
-return false;
-
-  return true;
-}
-
-/// Check if we should reply to the given packet.
-static bool skip(llvm::StringRef data) {
-  assert(!data.empty() && "Empty packet?");
-
-  // We've already acknowledge the '+' packet so we're done here.
-  if (data == "+")
-return true;
-
-  /// Don't 't reply to ^C. We need this because of stop reply packets, which
-  /// are only returned when the targ

[Lldb-commits] [PATCH] D116539: [lldb/platform-gdb] Clear cached protocol state upon disconnection

[Pavel Labath via Phabricator via lldb-commits]

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

Previously we would persist the flags indicating whether the remote side
supports a particular feature across reconnects, which is obviously not
a good idea.

I implement the clearing by nuking (its the only way to be sure :) the
entire GDBRemoteCommunication object in the disconnect operation and
creating a new one upon connection. This allows us to maintain a nice
invariant that the GDBRemoteCommunication object (which is now a
pointer) exists only if it is connected. The downside to that is that a
lot of functions now needs to check the validity of the pointer instead
of blindly accessing the object.

The process communication does not suffer from the same issue because we
always destroy the entire Process object for a relaunch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116539

Files:
  lldb/packages/Python/lldbsuite/test/lldbgdbclient.py
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -107,6 +107,25 @@
 "vFile:close:5",
 ])
 
+self.runCmd("platform disconnect")
+
+# For a new onnection, we should attempt vFile:size once again.
+server2 = MockGDBServer(self.server_socket_class())
+server2.responder = Responder()
+server2.start()
+self.addTearDownHook(lambda:server2.stop())
+self.runCmd("platform connect " + server2.get_connect_url())
+self.match("platform get-size /other/file.txt",
+   [r"File size of /other/file\.txt \(remote\): 66051"])
+
+self.assertPacketLogContains([
+"vFile:size:2f6f746865722f66696c652e747874",
+"vFile:open:2f6f746865722f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+],
+log=server2.responder.packetLog)
+
 @skipIfWindows
 def test_file_permissions(self):
 """Test 'platform get-permissions'"""
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
@@ -153,7 +153,8 @@
   GetPendingGdbServerList(std::vector &connection_urls);
 
 protected:
-  process_gdb_remote::GDBRemoteCommunicationClient m_gdb_client;
+  std::unique_ptr
+  m_gdb_client_up;
   std::string m_platform_description; // After we connect we can get a more
   // complete description of what we are
   // connected to
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -96,7 +96,8 @@
 
   const auto module_path = module_file_spec.GetPath(false);
 
-  if (!m_gdb_client.GetModuleInfo(module_file_spec, arch, module_spec)) {
+  if (!m_gdb_client_up ||
+  !m_gdb_client_up->GetModuleInfo(module_file_spec, arch, module_spec)) {
 LLDB_LOGF(
 log,
 "PlatformRemoteGDBServer::%s - failed to get module info for %s:%s",
@@ -127,11 +128,7 @@
 
 /// Default Constructor
 PlatformRemoteGDBServer::PlatformRemoteGDBServer()
-: Platform(false), // This is a remote platform
-  m_gdb_client() {
-  m_gdb_client.SetPacketTimeout(
-  process_gdb_remote::ProcessGDBRemote::GetPacketTimeout());
-}
+: Platform(/*is_host=*/false) {}
 
 /// Destructor.
 ///
@@ -147,29 +144,36 @@
 }
 
 bool PlatformRemoteGDBServer::GetRemoteOSVersion() {
-  m_os_version = m_gdb_client.GetOSVersion();
+  if (m_gdb_client_up)
+m_os_version = m_gdb_client_up->GetOSVersion();
   return !m_os_version.empty();
 }
 
 llvm::Optional PlatformRemoteGDBServer::GetRemoteOSBuildString() {
-  return m_gdb_client.GetOSBuildString();
+  if (!m_gdb_client_up)
+return llvm::None;
+  return m_gdb_client_up->GetOSBuildString();
 }
 
 llvm::Optional
 PlatformRemoteGDBServer::GetRemoteOSKernelDescription() {
-  return m_gdb_client.GetOSKernelDescription();
+  if (!m_gdb_client_up)
+return llvm::None;
+  return m_gdb_client_up->GetOSKernelDescription();
 }
 
 // Remote P

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Kadir Cetinkaya via Phabricator via lldb-commits]

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

thanks, lgtm!




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:995
   /// \param NumCandidates the number of overload candidates
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,

why not just drop the method? it is not pure, and no-op in base too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

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

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

sammccall added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:995
   /// \param NumCandidates the number of overload candidates
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,

kadircet wrote:
> why not just drop the method? it is not pure, and no-op in base too.
That's a reasonable question, but it's not my code so IDK if it's a style thing.
(I was tempted to remove it but it seems gratuitous for this kind of change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

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

[Lldb-commits] [lldb] 92417ea - [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via lldb-commits]

Author: Sam McCall
Date: 2022-01-03T20:14:59+01:00
New Revision: 92417eaf3329dc823c905ec6a608b83ac62b4f7c

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

LOG: [CodeCompletion] Signature help for braced constructor calls

Implementation is based on the "expected type" as used for
designated-initializers in braced init lists. This means it can deduce the type
in some cases where it's not written:

  void foo(Widget);
  foo({ /*help here*/ });

Only basic constructor calls are in scope of this patch, excluded are:
 - aggregate initialization (no help is offered for aggregates)
 - initializer_list initialization (no help is offered for these constructors)

Fixes https://github.com/clangd/clangd/issues/306

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseInit.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/ctor-signature.cpp
clang/tools/libclang/CIndexCodeCompletion.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 774cdea218d00..edde19f96202f 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -555,7 +555,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
}},
   {"signatureHelpProvider",
llvm::json::Object{
-   {"triggerCharacters", {"(", ",", ")", "<", ">"}},
+   {"triggerCharacters", {"(", ")", "{", "}", "<", ">", ","}},
}},
   {"declarationProvider", true},
   {"definitionProvider", true},

diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index bdfa1df194537..53d8f0d6cdeb7 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -921,7 +921,8 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 assert(!OpenParLoc.isInvalid());
 SourceManager &SrcMgr = S.getSourceManager();
 OpenParLoc = SrcMgr.getFileLoc(OpenParLoc);
@@ -961,8 +962,9 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
 paramIndexForArg(Candidate, SigHelp.activeParameter);
   }
 
-  const auto *CCS = Candidate.CreateSignatureString(
-  CurrentArg, S, *Allocator, CCTUInfo, true);
+  const auto *CCS =
+  Candidate.CreateSignatureString(CurrentArg, S, *Allocator, CCTUInfo,
+  /*IncludeBriefComment=*/true, 
Braced);
   assert(CCS && "Expected the CodeCompletionString to be non-null");
   ScoredSignatures.push_back(processOverloadCandidate(
   Candidate, *CCS,
@@ -1163,7 +1165,8 @@ class ParamNameCollector final : public 
CodeCompleteConsumer {
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 assert(CurrentArg <= (unsigned)std::numeric_limits::max() &&
"too many arguments");
 

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test 
b/clang-tools-extra/clangd/test/initialize-params.test
index 72823f3a0683d..2affc8b2466dd 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -107,10 +107,12 @@
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
-# CHECK-NEXT:  ",",
 # CHECK-NEXT:  ")",
+# CHECK-NEXT:  "{",
+# CHE

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall 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 rG92417eaf3329: [CodeCompletion] Signature help for braced 
constructor calls (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D116317?vs=396362&id=397111#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -995,7 +995,8 @@
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 // At the moment we don't filter out any overloaded candidates.
   }
 
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -656,14 +656,15 @@
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates,
-   SourceLocation OpenParLoc) override {
+   SourceLocation OpenParLoc,
+   bool Braced) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
-CodeCompletionString *StoredCompletion
-  = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+CodeCompletionString *StoredCompletion =
+Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
 getCodeCompletionTUInfo(),
-includeBriefComments());
-
+includeBriefComments(), Braced);
+
 CXCompletionResult R;
 R.CursorKind = CXCursor_OverloadCandidate;
 R.CompletionString = StoredCompletion;
Index: clang/test/CodeCompletion/ctor-signature.cpp
===
--- clang/test/CodeCompletion/ctor-signature.cpp
+++ clang/test/CodeCompletion/ctor-signature.cpp
@@ -15,3 +15,40 @@
   // CHECK-CC2: OVERLOAD: Foo(<#const Foo &#>)
   // CHECK-CC2: OVERLOAD: Foo(<#Foo &&#>
 }
+
+namespace std {
+template  struct initializer_list {};
+} // namespace std
+
+struct Bar {
+  // CHECK-BRACED: OVERLOAD: Bar{<#int#>}
+  Bar(int);
+  // CHECK-BRACED: OVERLOAD: Bar{<#double#>, double}
+  Bar(double, double);
+  // FIXME: no support for init-list constructors yet.
+  // CHECK-BRACED-NOT: OVERLOAD: {{.*}}char
+  Bar(std::initializer_list C);
+  // CHECK-BRACED: OVERLOAD: Bar{<#const Bar &#>}
+  // CHECK-BRACED: OVERLOAD: Bar{<#T *Pointer#>}
+  template  Bar(T *Pointer);
+};
+
+auto b1 = Bar{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:15 %s | FileCheck -check-prefix=CHECK-BRACED %s
+Bar b2{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s | FileCheck -check-prefix=CHECK-BRACED %s
+static int consumeBar(Bar) { return 0; }
+int b3 = consumeBar({});
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:41:22 %s | FileCheck -check-prefix=CHECK-BRACED %s
+
+struct Aggregate {
+  // FIXME: no support for aggregates yet.
+  // CHECK-AGGREGATE-NOT: OVERLOAD: Aggregate{<#const Aggregate &#>}
+  // CHECK-AGGREGATE-NOT: OVERLOAD: {{.*}}first
+  int first;
+  int second;
+};
+
+Aggregate a{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:52:13 %s | FileCheck -check-prefix=CHECK-AGGREGATE %s
+
Index: clang/lib/Sema/SemaCo

[Lldb-commits] [PATCH] D116419: [lldb] Display MachO seg, sec of memory region

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

I see now this isn't valid as written. The address range and rwx permissions 
printed by `memory region` are for the segment not the section. I think it 
would be helpful to print the section too, but as this change is written now, 
it could appear that all the info is for the section, when it's really for the 
segment. Maybe a better change would be to print a second line with an address 
range for the section, and the section's name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116419

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

[Lldb-commits] [lldb] 67c937f - [lldb] Use std::move in StringList (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-01-03T15:36:19-08:00
New Revision: 67c937f846b18e3113e126c37c69a222c0e99c1c

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

LOG: [lldb] Use std::move in StringList (NFC)

Added: 


Modified: 
lldb/source/Utility/StringList.cpp

Removed: 




diff  --git a/lldb/source/Utility/StringList.cpp 
b/lldb/source/Utility/StringList.cpp
index baff34ae3a5ef..f78681c05a3d7 100644
--- a/lldb/source/Utility/StringList.cpp
+++ b/lldb/source/Utility/StringList.cpp
@@ -42,7 +42,9 @@ void StringList::AppendString(const char *str) {
 
 void StringList::AppendString(const std::string &s) { m_strings.push_back(s); }
 
-void StringList::AppendString(std::string &&s) { m_strings.push_back(s); }
+void StringList::AppendString(std::string &&s) {
+  m_strings.push_back(std::move(s));
+}
 
 void StringList::AppendString(const char *str, size_t str_len) {
   if (str)
@@ -133,9 +135,9 @@ void StringList::InsertStringAtIndex(size_t idx, const 
std::string &str) {
 
 void StringList::InsertStringAtIndex(size_t idx, std::string &&str) {
   if (idx < m_strings.size())
-m_strings.insert(m_strings.begin() + idx, str);
+m_strings.insert(m_strings.begin() + idx, std::move(str));
   else
-m_strings.push_back(str);
+m_strings.push_back(std::move(str));
 }
 
 void StringList::DeleteStringAtIndex(size_t idx) {



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

[Lldb-commits] [PATCH] D116211: With firmware debug sess, if gdb stub knows the UUID/address of binary, try to load it

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D116211#3211319 , @labath wrote:

> What happened to the test case?

I didn't think of a way of constructing one very easily with what we have r.n. 
:/  The closest would be a gdb_remote_client test, but we need a binary that 
the gdb server can reply with the UUID of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116211

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