[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[Alexey Merzlyakov via lldb-commits]

@@ -760,6 +772,61 @@ def test_riscv64_regs(self):
 
 self.expect("register read --all")
 
+@skipIfLLVMTargetMissing("RISCV")

AlexeyMerzlyakov wrote:

Just to be on the same page:
Should I move `test_riscv64_regs_gpr`/`test_riscv64_regs_gpr_fpr` tests 
containing register value checks to separate place (like 
`lldb/test/API/linux/aarch64/sme_core_file/TestAArch64LinuxSMECoreFile.py` or 
better to `lldb/test/API/linux/riscv64/fpr_core_file/TestRV64FPRCoreFile.py`). 
And will keep main tests for RISC-V cores (`test_riscv64_gpr` and 
`test_riscv64_gpr_fpr`) in `TestLinuxCore.py`, as they are using `do_test()` 
infrastructure from this file? Or you mean something else?

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[Alexey Merzlyakov via lldb-commits]

https://github.com/AlexeyMerzlyakov edited 
https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[Alexey Merzlyakov via lldb-commits]

https://github.com/AlexeyMerzlyakov edited 
https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[Alexey Merzlyakov via lldb-commits]

https://github.com/AlexeyMerzlyakov edited 
https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[Alexey Merzlyakov via lldb-commits]

https://github.com/AlexeyMerzlyakov edited 
https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -150,12 +153,17 @@ static void 
client_handle(GDBRemoteCommunicationServerPlatform &platform,
   printf("Disconnected.\n");
 }
 
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
 static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
-  std::lock_guard guard(gdbserver_portmap_mutex);
-  gdbserver_portmap.FreePortForProcess(pid);
+  if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+  g_terminate = true;

labath wrote:

This kind of casual handling of atomics is making me very nervous. Instead 
`g_single_pid`, I think it be better to just pass a different callback (or a 
different argument to this callback). 

And could we get rid of the g_terminate and g_main_loop checks by waiting for 
the child platform process to be reaped before exiting?

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -667,7 +669,22 @@ ConnectionStatus
 ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+  int64_t fd = -1;
+  if (!s.getAsInteger(0, fd)) {
+// Assume we own fd.
+std::unique_ptr tcp_socket =
+std::make_unique((NativeSocket)fd, true, false);
+m_io_sp = std::move(tcp_socket);
+m_uri = s.str();
+return eConnectionStatusSuccess;
+  }
+  if (error_ptr)
+*error_ptr = Status::FromErrorStringWithFormat(
+"invalid file descriptor: \"%s\"", s.str().c_str());

labath wrote:

```suggestion
*error_ptr = Status::FromErrorStringWithFormatv(
"invalid file descriptor: \"{0}\"", s);
```

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -160,66 +95,58 @@ 
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
 default;
 
 Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
-  if (!port) {
-llvm::Expected available_port = 
m_port_map.GetNextAvailablePort();
-if (available_port)
-  port = *available_port;
-else
-  return Status(available_port.takeError());
-  }
-
-  // Spawn a new thread to accept the port that gets bound after binding to
-  // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+  std::ostringstream url;
+  if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+  // Just check that GDBServer exists. GDBServer must be launched after
+  // accepting the connection.
+  FileSpec debugserver_file_spec;
+  if (!GetDebugserverPath(nullptr, debugserver_file_spec))
+return Status::FromErrorString("unable to locate debugserver");
+  return Status();
+}
 
-  // ignore the hostname send from the remote end, just use the ip address that
-  // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+  } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+  return Status::FromErrorString("protocol must be tcp");
+  }
 
   // Spawn a debugserver and try to get the port it listens to.
   ProcessLaunchInfo debugserver_launch_info;
-  if (hostname.empty())
-hostname = "127.0.0.1";
-
   Log *log = GetLog(LLDBLog::Platform);
-  LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+  LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(),
+   (int64_t)fd);

labath wrote:

```suggestion
  LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(),
   fd);
```

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -666,7 +756,23 @@ ConnectionStatus
 ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+  int64_t fd = -1;
+  if (!s.getAsInteger(0, fd)) {
+// Assume we own fd.
+std::unique_ptr tcp_socket =
+std::make_unique((NativeSocket)fd, true, false);
+m_io_sp = std::move(tcp_socket);
+m_uri = s.str();
+return eConnectionStatusSuccess;
+  }

labath wrote:

Reducing the number of ifdefs is great, but it also must be balanced with 
considerations like code readability and user confusion. I don't think this 
strikes the right balance as FD's are a thing on windows as well, and so this 
could be very surprising. Handles are windows-specific, but we are already 
using them in lldb as a generic concept of an object identifier. E.g. 
Socket::GetWaitableHandle returns the fd (`int`) on posix and the socket handle 
(`SOCKET`) on windows.

Here's a specific suggestion: We can sidestep this problem by not roundtripping 
the socket through a string URL. ConnectionFileDescriptor has a constructor 
which takes a `Socket` argument, so I think we could use that. If we also make 
`SharedSocket::GetNativeSocket` (`GetSocket` ?) return a Socket object, then we 
can also reduce the usage of raw NativeSocket values.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -879,20 +879,12 @@ lldb::thread_result_t 
GDBRemoteCommunication::ListenThread() {
   return {};
 }
 
-Status GDBRemoteCommunication::StartDebugserverProcess(
-const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+bool GDBRemoteCommunication::GetDebugserverPath(
+Platform *platform, FileSpec &debugserver_file_spec) {

labath wrote:

Minimizing changes is not what I'm concerned with. This change in particular 
could be easily split off into its own patch.
I don't think
```
FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
if (GetDebugserverPath(platform, debugserver_file_spec)) {
```
is inherently better than
```
launch_info.GetExecutableFile() = GetDebugserverPath(platform);
if (launch_info.GetExecutableFile()) {
```
or 
```
if (FileSpec debugserver_file_spec = GetDebugserverPath(platform)) {
  launch_info.GetExecutableFile() = debugserver_file_spec;
```

In fact, I'd say it's the opposite, because that pattern leaves it ambiguous as 
to what is the value of the by-ref argument in the failure case (is it 
unchanged, empty/invalid, or undefined)?

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Pavel Labath via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

I don't think this usage of rvalue references is very idiomatic. It saves an 
object copy (move), but:
- that should be very cheap anyway
- it increases the risk of improper handling and weird side-effects -- a 
function like `void use(Error E) {}` will always crash at the closing bracket 
(due to not checking the error), but `use(Error &&E){}` will never crash 
*inside* the function (because it doesn't own the error), and whether the 
program as a whole crashes depends on whether the caller does any other 
operation that would clear the error object.

So, I think it would be better to stick to value semantics. Other than that, 
this seems fine.

https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Pavel Labath via lldb-commits]

@@ -26,6 +26,36 @@ class raw_ostream;
 
 namespace lldb_private {
 
+class MachKernelError

labath wrote:

These aren't yet used in this PR right? Can we get rid of them?

https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

[Pavel Labath via lldb-commits]

@@ -96,16 +124,44 @@ Status Status::FromErrorStringWithFormat(const char 
*format, ...) {
   return Status(string);
 }
 
-llvm::Error Status::ToError() const {
-  if (Success())
-return llvm::Error::success();
-  if (m_type == ErrorType::eErrorTypePOSIX)
-return llvm::errorCodeToError(
-std::error_code(m_code, std::generic_category()));
-  return llvm::createStringError(AsCString());
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+   std::string msg) {
+  return Status(llvm::make_error(
+  std::error_code(result, expression_category()), msg));
 }
 
-Status::~Status() = default;
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error cloneError(llvm::Error &error) {
+  llvm::Error result = llvm::Error::success();
+  llvm::consumeError(std::move(result));
+  llvm::visitErrors(error, [&](const llvm::ErrorInfoBase &error) {
+if (error.isA())
+  result = llvm::make_error(error.convertToErrorCode());
+else if (error.isA())
+  result = llvm::make_error(error.convertToErrorCode());
+else if (error.isA())
+  result = llvm::make_error(error.convertToErrorCode(),
+ error.message());
+else
+  result =
+  llvm::createStringError(error.convertToErrorCode(), error.message());
+  });
+  return result;
+}
+
+Status Status::FromError(llvm::Error &&error) {
+  // Existing clients assume that the conversion to Status consumes
+  // and destroys the error. Use cloneError to convert all unnown
+  // errors to strings.
+  llvm::Error clone = cloneError(error);
+  llvm::consumeError(std::move(error));

labath wrote:

Interesting. I think we should look into this more closely as it may impact the 
overall design choice. I think it should be possible to store python exception 
objects for ~arbitrarily long (at least as long as the python interpreter 
exists, which I think we never destroy). And `PythonException` object seems to 
handle reference counting correctly -- as long as you don't copy it: it has a 
(implicit) copy constructor, that that just does a bytewise copy without 
incrementing the python reference count. I'm not sure how you implemented 
copying originally, but I think the solution could be as simple as adding some 
Py_INCREFs to its copy constructor.

https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix and speedup the `memory find` command (PR #104193)

[Pavel Labath via lldb-commits]

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/104193

>From ecd84ae96fd59eeb4a2bb47d80980d861dc042d1 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 14 Aug 2024 19:58:27 +0200
Subject: [PATCH 1/3] [lldb] Fix and speedup the `memory find` command

This patch fixes an issue where the `memory find` command would
effectively stop searching after encountering a memory read error (which
could happen due to unreadable memory), without giving any indication
that it has done so (it would just print it could not find the pattern).

To make matters worse, it would not terminate after encountering this
error, but rather proceed to slowly increment the address pointer, which
meant that searching a large region could take a very long time (and
give the appearance that lldb is actually searching for the thing).

The patch fixes this first problem (*) by detecting read errors and
skipping over (using GetMemoryRegionInfo) the unreadable parts of memory
and resuming the search after them. It also reads the memory in bulk (up
to 1MB), which speeds up the search significantly (up to 6x for live
processes, 18x for core files).

(*) The fix does not work on windows yet, because the ReadMemory API
does not return partial results (like it does for other systems). I'm
preparing a separate patch to deal with that.
---
 lldb/source/Target/Process.cpp| 69 ++-
 .../memory/holes/TestMemoryHoles.py   | 12 
 2 files changed, 47 insertions(+), 34 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ae64f6f261bad7..6c5c5162e24686 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -114,33 +114,6 @@ class ProcessOptionValueProperties
   }
 };
 
-class ProcessMemoryIterator {
-public:
-  ProcessMemoryIterator(Process &process, lldb::addr_t base)
-  : m_process(process), m_base_addr(base) {}
-
-  bool IsValid() { return m_is_valid; }
-
-  uint8_t operator[](lldb::addr_t offset) {
-if (!IsValid())
-  return 0;
-
-uint8_t retval = 0;
-Status error;
-if (0 == m_process.ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-  m_is_valid = false;
-  return 0;
-}
-
-return retval;
-  }
-
-private:
-  Process &m_process;
-  const lldb::addr_t m_base_addr;
-  bool m_is_valid = true;
-};
-
 static constexpr OptionEnumValueElement g_follow_fork_mode_values[] = {
 {
 eFollowParent,
@@ -3379,21 +3352,49 @@ lldb::addr_t Process::FindInMemory(lldb::addr_t low, 
lldb::addr_t high,
   if (region_size < size)
 return LLDB_INVALID_ADDRESS;
 
+  // See "Boyer-Moore string search algorithm".
   std::vector bad_char_heuristic(256, size);
-  ProcessMemoryIterator iterator(*this, low);
-
   for (size_t idx = 0; idx < size - 1; idx++) {
 decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx];
 bad_char_heuristic[bcu_idx] = size - idx - 1;
   }
-  for (size_t s = 0; s <= (region_size - size);) {
+
+  // Memory we're currently searching through.
+  llvm::SmallVector mem;
+  // Position of the memory buffer.
+  addr_t mem_pos = low;
+  // Maximum number of bytes read (and buffered). We need to read at least
+  // `size` bytes for a successful match.
+  const size_t max_read_size = std::max(size, 0x1);
+
+  for (addr_t cur_addr = low; cur_addr <= (high - size);) {
+if (cur_addr + size > mem_pos + mem.size()) {
+  // We need to read more data. We don't attempt to reuse the data we've
+  // already read (up to `size-1` bytes from `cur_addr` to
+  // `mem_pos+mem.size()`).  This is fine for patterns much smaller than
+  // max_read_size. For very
+  // long patterns we may need to do something more elaborate.
+  mem.resize_for_overwrite(max_read_size);
+  Status error;
+  mem.resize(ReadMemory(cur_addr, mem.data(),
+std::min(mem.size(), high - cur_addr), error));
+  mem_pos = cur_addr;
+  if (size > mem.size()) {
+// We didn't read enough data. Skip to the next memory region.
+MemoryRegionInfo info;
+error = GetMemoryRegionInfo(mem_pos + mem.size(), info);
+if (error.Fail())
+  break;
+cur_addr = info.GetRange().GetRangeEnd();
+continue;
+  }
+}
 int64_t j = size - 1;
-while (j >= 0 && buf[j] == iterator[s + j])
+while (j >= 0 && buf[j] == mem[cur_addr + j - mem_pos])
   j--;
 if (j < 0)
-  return low + s;
-else
-  s += bad_char_heuristic[iterator[s + size - 1]];
+  return cur_addr; // We have a match.
+cur_addr += bad_char_heuristic[mem[cur_addr + size - 1 - mem_pos]];
   }
 
   return LLDB_INVALID_ADDRESS;
diff --git a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py 
b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
index 6ecb25b2e4db28..bb19eceba6b546 100644
--- a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
+++ b/lldb/test/

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[David Spickett via lldb-commits]

@@ -760,6 +772,61 @@ def test_riscv64_regs(self):
 
 self.expect("register read --all")
 
+@skipIfLLVMTargetMissing("RISCV")

DavidSpickett wrote:

I'm saying that if risc-v grows a lot of strange register state, it will be 
better served by separate test cases.

For this PR, just consider renaming the test functions here so that they list 
what they are testing. So riscv64_gpr, as opposed to to riscv64_nofpr.

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[David Spickett via lldb-commits]

@@ -760,6 +772,61 @@ def test_riscv64_regs(self):
 
 self.expect("register read --all")
 
+@skipIfLLVMTargetMissing("RISCV")

DavidSpickett wrote:

As an example of what I mean about future extensions being "too weird" -

In the new Arm extension support I'm doing right now, I wrote a program file 
that can be used for live processes, and to generate a core file. This keeps 
all tests for that extension in one place. This helps because I have to use 
weird assembly to trigger these things, and I only have to write that once. And 
I don't have to put that in the standard corefile tests with some #ifdefs which 
get confusing.

With floating point, you can write normal C code to set values, which can be 
included easily in a program file that's quite generic.

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFC] Move few static helpers to the class Socket (PR #106640)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/106640

>From fe58bf1f08553b75f089692077abdff0ccaf96bb Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 30 Aug 2024 01:29:58 +0400
Subject: [PATCH 1/2] [lldb][NFC] Move few static helpers to the class Socket

---
 lldb/include/lldb/Host/Socket.h   |  4 
 lldb/source/Host/common/Socket.cpp| 24 +-
 lldb/source/Host/common/TCPSocket.cpp | 29 +--
 3 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 304a91bdf6741b..141566c00497b8 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -34,9 +34,11 @@ namespace lldb_private {
 #if defined(_WIN32)
 typedef SOCKET NativeSocket;
 typedef lldb::pipe_t shared_fd_t;
+typedef const char *set_socket_option_arg_type;
 #else
 typedef int NativeSocket;
 typedef NativeSocket shared_fd_t;
+typedef const void *set_socket_option_arg_type;
 #endif
 class Socket;
 class TCPSocket;
@@ -138,6 +140,8 @@ class Socket : public IOObject {
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
+  static int CloseSocket(NativeSocket sockfd);
+  static Status GetLastError();
   static void SetLastError(Status &error);
   static NativeSocket CreateSocket(const int domain, const int type,
const int protocol,
diff --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 1a506aa95b2465..62473a79e290e8 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -386,11 +386,7 @@ Status Socket::Close() {
   LLDB_LOGF(log, "%p Socket::Close (fd = %" PRIu64 ")",
 static_cast(this), static_cast(m_socket));
 
-#if defined(_WIN32)
-  bool success = closesocket(m_socket) == 0;
-#else
-  bool success = ::close(m_socket) == 0;
-#endif
+  bool success = CloseSocket(m_socket) == 0;
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
   if (!success) {
@@ -427,6 +423,24 @@ void Socket::SetLastError(Status &error) {
 #endif
 }
 
+Status Socket::GetLastError() {
+  std::error_code EC;
+#ifdef _WIN32
+  EC = llvm::mapWindowsError(WSAGetLastError());
+#else
+  EC = std::error_code(errno, std::generic_category());
+#endif
+  return EC;
+}
+
+int Socket::CloseSocket(NativeSocket sockfd) {
+#ifdef _WIN32
+  return ::closesocket(sockfd);
+#else
+  return ::close(sockfd);
+#endif
+}
+
 NativeSocket Socket::CreateSocket(const int domain, const int type,
   const int protocol,
   bool child_processes_inherit, Status &error) 
{
diff --git a/lldb/source/Host/common/TCPSocket.cpp 
b/lldb/source/Host/common/TCPSocket.cpp
index fc005814308d90..cba511b9f7f966 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -33,28 +33,9 @@
 #include 
 #endif
 
-#ifdef _WIN32
-#define CLOSE_SOCKET closesocket
-typedef const char *set_socket_option_arg_type;
-#else
-#include 
-#define CLOSE_SOCKET ::close
-typedef const void *set_socket_option_arg_type;
-#endif
-
 using namespace lldb;
 using namespace lldb_private;
 
-static Status GetLastSocketError() {
-  std::error_code EC;
-#ifdef _WIN32
-  EC = llvm::mapWindowsError(WSAGetLastError());
-#else
-  EC = std::error_code(errno, std::generic_category());
-#endif
-  return EC;
-}
-
 static const int kType = SOCK_STREAM;
 
 TCPSocket::TCPSocket(bool should_close, bool child_processes_inherit)
@@ -213,7 +194,7 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) 
{
 reinterpret_cast(&option_value);
 if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
  sizeof(option_value)) == -1) {
-  CLOSE_SOCKET(fd);
+  CloseSocket(fd);
   continue;
 }
 
@@ -229,8 +210,8 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) 
{
   err = ::listen(fd, backlog);
 
 if (err == -1) {
-  error = GetLastSocketError();
-  CLOSE_SOCKET(fd);
+  error = GetLastError();
+  CloseSocket(fd);
   continue;
 }
 
@@ -251,7 +232,7 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) 
{
 
 void TCPSocket::CloseListenSockets() {
   for (auto socket : m_listen_sockets)
-CLOSE_SOCKET(socket.first);
+CloseSocket(socket.first);
   m_listen_sockets.clear();
 }
 
@@ -280,7 +261,7 @@ llvm::Expected> 
TCPSocket::Accept(
 
   const lldb_private::SocketAddress &AddrIn = m_listen_sockets[fd];
   if (!AddrIn.IsAnyAddr() && AcceptAddr != AddrIn) {
-CLOSE_SOCKET(sock);
+CloseSocket(sock);
 LLDB_LOG(log, "rejecting incoming connection from {0} (expecting {1})",
  AcceptAddr.GetIPAddress(), AddrIn.GetIPAddress());
 return;

>From 492c5fe2d78fc13f57fc10b9780e3e0e63250046 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: W

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[David Spickett via lldb-commits]

@@ -760,6 +772,61 @@ def test_riscv64_regs(self):
 
 self.expect("register read --all")
 
+@skipIfLLVMTargetMissing("RISCV")

DavidSpickett wrote:

> For this PR, just consider renaming the test functions here so that they list 
> what they are testing. So riscv64_gpr, as opposed to to riscv64_nofpr.

Or if you really want to be super clear: `riscv_gpr_only`

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFC] Move few static helpers to the class Socket (PR #106640)

[Dmitry Vasilyev via lldb-commits]

@@ -34,9 +34,11 @@ namespace lldb_private {
 #if defined(_WIN32)
 typedef SOCKET NativeSocket;
 typedef lldb::pipe_t shared_fd_t;
+typedef const char *set_socket_option_arg_type;

slydiman wrote:

Updated. Fixed also a typo
`set`_socket_option_arg_type option_value_p =
  reinterpret_cast<`get`_socket_option_arg_type>(&option_value);

https://github.com/llvm/llvm-project/pull/106640
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][NFC] Move few static helpers to the class Socket (PR #106640)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.

cool. thanks.

https://github.com/llvm/llvm-project/pull/106640
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cc5c526 - [lldb] Fix and speedup the `memory find` command (#104193)

[via lldb-commits]

Author: Pavel Labath
Date: 2024-09-04T11:30:58+02:00
New Revision: cc5c526c80a4cacf7ed5b7fbe50072594ec1aeaf

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

LOG: [lldb] Fix and speedup the `memory find` command (#104193)

This patch fixes an issue where the `memory find` command would
effectively stop searching after encountering a memory read error (which
could happen due to unreadable memory), without giving any indication
that it has done so (it would just print it could not find the pattern).

To make matters worse, it would not terminate after encountering this
error, but rather proceed to slowly increment the address pointer, which
meant that searching a large region could take a very long time (and
give the appearance that lldb is actually searching for the thing).

The patch fixes this first problem by detecting read errors and
skipping over (using GetMemoryRegionInfo) the unreadable parts of memory
and resuming the search after them. It also reads the memory in bulk
(`max(sizeof(pattern))`), which speeds up the search significantly (up
to 6x for live processes, 18x for core files).

Added: 


Modified: 
lldb/source/Target/Process.cpp
lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ae64f6f261bad7..6c5c5162e24686 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -114,33 +114,6 @@ class ProcessOptionValueProperties
   }
 };
 
-class ProcessMemoryIterator {
-public:
-  ProcessMemoryIterator(Process &process, lldb::addr_t base)
-  : m_process(process), m_base_addr(base) {}
-
-  bool IsValid() { return m_is_valid; }
-
-  uint8_t operator[](lldb::addr_t offset) {
-if (!IsValid())
-  return 0;
-
-uint8_t retval = 0;
-Status error;
-if (0 == m_process.ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-  m_is_valid = false;
-  return 0;
-}
-
-return retval;
-  }
-
-private:
-  Process &m_process;
-  const lldb::addr_t m_base_addr;
-  bool m_is_valid = true;
-};
-
 static constexpr OptionEnumValueElement g_follow_fork_mode_values[] = {
 {
 eFollowParent,
@@ -3379,21 +3352,49 @@ lldb::addr_t Process::FindInMemory(lldb::addr_t low, 
lldb::addr_t high,
   if (region_size < size)
 return LLDB_INVALID_ADDRESS;
 
+  // See "Boyer-Moore string search algorithm".
   std::vector bad_char_heuristic(256, size);
-  ProcessMemoryIterator iterator(*this, low);
-
   for (size_t idx = 0; idx < size - 1; idx++) {
 decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx];
 bad_char_heuristic[bcu_idx] = size - idx - 1;
   }
-  for (size_t s = 0; s <= (region_size - size);) {
+
+  // Memory we're currently searching through.
+  llvm::SmallVector mem;
+  // Position of the memory buffer.
+  addr_t mem_pos = low;
+  // Maximum number of bytes read (and buffered). We need to read at least
+  // `size` bytes for a successful match.
+  const size_t max_read_size = std::max(size, 0x1);
+
+  for (addr_t cur_addr = low; cur_addr <= (high - size);) {
+if (cur_addr + size > mem_pos + mem.size()) {
+  // We need to read more data. We don't attempt to reuse the data we've
+  // already read (up to `size-1` bytes from `cur_addr` to
+  // `mem_pos+mem.size()`).  This is fine for patterns much smaller than
+  // max_read_size. For very
+  // long patterns we may need to do something more elaborate.
+  mem.resize_for_overwrite(max_read_size);
+  Status error;
+  mem.resize(ReadMemory(cur_addr, mem.data(),
+std::min(mem.size(), high - cur_addr), error));
+  mem_pos = cur_addr;
+  if (size > mem.size()) {
+// We didn't read enough data. Skip to the next memory region.
+MemoryRegionInfo info;
+error = GetMemoryRegionInfo(mem_pos + mem.size(), info);
+if (error.Fail())
+  break;
+cur_addr = info.GetRange().GetRangeEnd();
+continue;
+  }
+}
 int64_t j = size - 1;
-while (j >= 0 && buf[j] == iterator[s + j])
+while (j >= 0 && buf[j] == mem[cur_addr + j - mem_pos])
   j--;
 if (j < 0)
-  return low + s;
-else
-  s += bad_char_heuristic[iterator[s + size - 1]];
+  return cur_addr; // We have a match.
+cur_addr += bad_char_heuristic[mem[cur_addr + size - 1 - mem_pos]];
   }
 
   return LLDB_INVALID_ADDRESS;

diff  --git a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py 
b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
index 1c2c90d483ea3f..c61ae15b9dda70 100644
--- a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
+++ b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
@@ -4

[Lldb-commits] [lldb] [lldb] Fix and speedup the `memory find` command (PR #104193)

[Pavel Labath via lldb-commits]

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/104193
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0b2550f - [lldb][test] Disable TestMultipleDebuggers on Linux

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2024-09-04T09:33:35Z
New Revision: 0b2550f8ab77a53f560f6a7a1b401c4803a36d48

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

LOG: [lldb][test] Disable TestMultipleDebuggers on Linux

This used to timeout (https://github.com/llvm/llvm-project/issues/101162)
now it's aborting 
(https://github.com/llvm/llvm-project/pull/105765#issuecomment-2327645665)

Added: 


Modified: 
lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py

Removed: 




diff  --git a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py 
b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
index bee6e66aa7219f..1fd4806cd74f4d 100644
--- a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
+++ b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
@@ -13,6 +13,7 @@ class TestMultipleSimultaneousDebuggers(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 # Sometimes times out on Linux, see 
https://github.com/llvm/llvm-project/issues/101162.
+@skipIfLinux
 @skipIfNoSBHeaders
 @skipIfWindows
 @skipIfHostIncompatibleWithTarget



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

[Lldb-commits] [lldb] deeafea - [lldb][NFC] Move few static helpers to the class Socket (#106640)

[via lldb-commits]

Author: Dmitry Vasilyev
Date: 2024-09-04T13:38:55+04:00
New Revision: deeafeab815ddfe7b507e9f79fe8f992265a9f3b

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

LOG: [lldb][NFC] Move few static helpers to the class Socket (#106640)

Fixed a typo in Socket::SetOption().

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/common/TCPSocket.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 304a91bdf6741b..764a048976eb41 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -112,8 +112,17 @@ class Socket : public IOObject {
   static llvm::Expected>
   UdpConnect(llvm::StringRef host_and_port, bool child_processes_inherit);
 
-  int GetOption(int level, int option_name, int &option_value);
-  int SetOption(int level, int option_name, int option_value);
+  static int GetOption(NativeSocket sockfd, int level, int option_name,
+   int &option_value);
+  int GetOption(int level, int option_name, int &option_value) {
+return GetOption(m_socket, level, option_name, option_value);
+  };
+
+  static int SetOption(NativeSocket sockfd, int level, int option_name,
+   int option_value);
+  int SetOption(int level, int option_name, int option_value) {
+return SetOption(m_socket, level, option_name, option_value);
+  };
 
   NativeSocket GetNativeSocket() const { return m_socket; }
   SocketProtocol GetSocketProtocol() const { return m_protocol; }
@@ -138,6 +147,8 @@ class Socket : public IOObject {
 
   virtual size_t Send(const void *buf, const size_t num_bytes);
 
+  static int CloseSocket(NativeSocket sockfd);
+  static Status GetLastError();
   static void SetLastError(Status &error);
   static NativeSocket CreateSocket(const int domain, const int type,
const int protocol,

diff  --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 1a506aa95b2465..1a63571b94c6f1 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -386,11 +386,7 @@ Status Socket::Close() {
   LLDB_LOGF(log, "%p Socket::Close (fd = %" PRIu64 ")",
 static_cast(this), static_cast(m_socket));
 
-#if defined(_WIN32)
-  bool success = closesocket(m_socket) == 0;
-#else
-  bool success = ::close(m_socket) == 0;
-#endif
+  bool success = CloseSocket(m_socket) == 0;
   // A reference to a FD was passed in, set it to an invalid value
   m_socket = kInvalidSocketValue;
   if (!success) {
@@ -400,18 +396,20 @@ Status Socket::Close() {
   return error;
 }
 
-int Socket::GetOption(int level, int option_name, int &option_value) {
+int Socket::GetOption(NativeSocket sockfd, int level, int option_name,
+  int &option_value) {
   get_socket_option_arg_type option_value_p =
   reinterpret_cast(&option_value);
   socklen_t option_value_size = sizeof(int);
-  return ::getsockopt(m_socket, level, option_name, option_value_p,
+  return ::getsockopt(sockfd, level, option_name, option_value_p,
   &option_value_size);
 }
 
-int Socket::SetOption(int level, int option_name, int option_value) {
+int Socket::SetOption(NativeSocket sockfd, int level, int option_name,
+  int option_value) {
   set_socket_option_arg_type option_value_p =
-  reinterpret_cast(&option_value);
-  return ::setsockopt(m_socket, level, option_name, option_value_p,
+  reinterpret_cast(&option_value);
+  return ::setsockopt(sockfd, level, option_name, option_value_p,
   sizeof(option_value));
 }
 
@@ -427,6 +425,24 @@ void Socket::SetLastError(Status &error) {
 #endif
 }
 
+Status Socket::GetLastError() {
+  std::error_code EC;
+#ifdef _WIN32
+  EC = llvm::mapWindowsError(WSAGetLastError());
+#else
+  EC = std::error_code(errno, std::generic_category());
+#endif
+  return EC;
+}
+
+int Socket::CloseSocket(NativeSocket sockfd) {
+#ifdef _WIN32
+  return ::closesocket(sockfd);
+#else
+  return ::close(sockfd);
+#endif
+}
+
 NativeSocket Socket::CreateSocket(const int domain, const int type,
   const int protocol,
   bool child_processes_inherit, Status &error) 
{

diff  --git a/lldb/source/Host/common/TCPSocket.cpp 
b/lldb/source/Host/common/TCPSocket.cpp
index fc005814308d90..4f1518ef697ff0 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -33,28 +33,9 @@
 #include 
 #endif
 
-#ifdef _WIN32
-#define CLOSE_SOCKET closesocket
-typedef const char *set_socket_option_arg_type;
-#else
-#include 
-#define CLOSE_SOCKET ::close
-typedef const void *set_socket_option_arg_type;
-#

[Lldb-commits] [lldb] [lldb][NFC] Move few static helpers to the class Socket (PR #106640)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman closed 
https://github.com/llvm/llvm-project/pull/106640
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5a658ee - [lldb][test] Skip some lldb-server tests on Windows

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2024-09-04T09:43:18Z
New Revision: 5a658ee933065d0e4ef1a65d9a6ddfba2874ee98

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

LOG: [lldb][test] Skip some lldb-server tests on Windows

These are known to return errors occasionaly on our Windows on Arm
bot.

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
index ad84a40932c650..a2ac1fdb6270f4 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py
@@ -58,6 +58,7 @@ def test_launch_via_vRun(self):
 self.assertEqual(context["O_content"], b"arg1\r\narg2\r\narg3\r\n")
 
 @add_test_categories(["llgs"])
+@skipIfWindows  # Sometimes returns '$E1f'.
 def test_launch_via_vRun_no_args(self):
 self.build()
 server = self.connect_to_debug_monitor()

diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 93485cd32f5196..592037db502aaf 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -468,6 +468,7 @@ def test_Hg_fails_on_zero_pid(self):
 self.Hg_fails_on_pid(0)
 
 @add_test_categories(["llgs"])
+@skipIfWindows  # Sometimes returns '$E37'.
 def test_Hg_fails_on_minus_one_pid(self):
 self.build()
 self.set_inferior_startup_launch()

diff  --git a/lldb/test/API/tools/lldb-server/TestNonStop.py 
b/lldb/test/API/tools/lldb-server/TestNonStop.py
index 62bda48ee049be..841de508187977 100644
--- a/lldb/test/API/tools/lldb-server/TestNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -276,6 +276,7 @@ def test_multiple_vCont(self):
 self.expect_gdbremote_sequence()
 
 @add_test_categories(["llgs"])
+@skipIfWindows  # Sometimes results in '$E37' instead of expected '$OK'
 def test_vCont_then_stop(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] [lldb] d77ccae - [lldb] Fix 32 bit compile error

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2024-09-04T10:22:58Z
New Revision: d77ccae4a629ba11b5c28f97222a8834c5e5c183

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

LOG: [lldb] Fix 32 bit compile error

https://lab.llvm.org/buildbot/#/builders/18/builds/3247/steps/4/logs/stdio

In code added by https://github.com/llvm/llvm-project/issues/87471.

Added: 


Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6c5c5162e24686..97ce2c14458e93 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3377,7 +3377,8 @@ lldb::addr_t Process::FindInMemory(lldb::addr_t low, 
lldb::addr_t high,
   mem.resize_for_overwrite(max_read_size);
   Status error;
   mem.resize(ReadMemory(cur_addr, mem.data(),
-std::min(mem.size(), high - cur_addr), error));
+std::min(mem.size(), high - cur_addr),
+error));
   mem_pos = cur_addr;
   if (size > mem.size()) {
 // We didn't read enough data. Skip to the next memory region.



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

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

dlav-sc wrote:

> > Well, for now I assume that existing tests are enough. Meaning we have a 
> > list of tests that start passing
> > after these patches are applied.
> 
> How are you running the tests? We're working on getting the tests running in 
> a sane fashion, using user space qemu.

For now I just run Python API tests. They support remote debugging, so 
dotest.py has options like `--platform-name`, `--platform-url` and 
`--platform-working-dir` that I use to connect to qemu-system-riscv64 with 
lldb-server running on it.

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correctly reconstruct simplified names for type units (PR #107240)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/107240

We need to resolve the type signature to get a hold of the template argument 
dies.

The associated test case passes even without this patch, but it only does it by 
accident (because the subsequent code considers the types to be in an anonymous 
namespace and this not subject to uniqueing). This will change once my other 
patch starts resolving names correctly.

>From ef394926d3d56d8a8dc7fc06940ce01f8d9b512f Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 4 Sep 2024 15:12:20 +0200
Subject: [PATCH] [lldb] Correctly reconstruct simplified names for type units

We need to resolve the type signature to get a hold of the template
argument dies.

The associated test case passes even without this patch, but it only
does it by accident (because the subsequent code considers the types to
be in an anonymous namespace and this not subject to uniqueing). This
will change once my other patch starts resolving names correctly.
---
 .../Plugins/SymbolFile/DWARF/DWARFASTParser.h |  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  |  6 +++--
 .../SymbolFile/DWARF/DWARFASTParserClang.h|  4 ++--
 .../DWARF/x86/type-definition-search.cpp  | 23 +++
 4 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
index 636ff4b5cf11dc..971cbe47fb702d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
@@ -58,7 +58,7 @@ class DWARFASTParser {
   virtual void EnsureAllDIEsInDeclContextHaveBeenParsed(
   CompilerDeclContext decl_context) = 0;
 
-  virtual std::string GetDIEClassTemplateParams(const DWARFDIE &die) = 0;
+  virtual std::string GetDIEClassTemplateParams(DWARFDIE die) = 0;
 
   static std::optional
   ParseChildArrayInfo(const DWARFDIE &parent_die,
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3c58be26c266bb..4d688f9a1358b2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -818,8 +818,10 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext 
&sc,
  &attrs.decl, clang_type, resolve_state, payload);
 }
 
-std::string
-DWARFASTParserClang::GetDIEClassTemplateParams(const DWARFDIE &die) {
+std::string DWARFASTParserClang::GetDIEClassTemplateParams(DWARFDIE die) {
+  if (DWARFDIE signature_die = die.GetReferencedDIE(DW_AT_signature))
+die = signature_die;
+
   if (llvm::StringRef(die.GetName()).contains("<"))
 return {};
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index ae6720608121f5..2806fbbeb99d24 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -106,8 +106,8 @@ class DWARFASTParserClang : public 
lldb_private::plugin::dwarf::DWARFASTParser {
   /// \return A string, including surrounding '<>', of the template parameters.
   /// If the DIE's name already has '<>', returns an empty string because
   /// it's assumed that the caller is using the DIE name anyway.
-  std::string GetDIEClassTemplateParams(
-  const lldb_private::plugin::dwarf::DWARFDIE &die) override;
+  std::string
+  GetDIEClassTemplateParams(lldb_private::plugin::dwarf::DWARFDIE die) 
override;
 
   void MapDeclDIEToDefDIE(const lldb_private::plugin::dwarf::DWARFDIE 
&decl_die,
   const lldb_private::plugin::dwarf::DWARFDIE 
&def_die);
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp
index bce6ed36b0968e..5a40a6e0fbc270 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/type-definition-search.cpp
@@ -4,15 +4,20 @@
 
 // REQUIRES: lld
 
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g 
-gsimple-template-names -DFILE_A
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g 
-gsimple-template-names -DFILE_B
-// RUN: ld.lld %t-a.o %t-b.o -o %t
-// RUN: %lldb %t -o "target variable --ptr-depth 1 --show-types both_a both_b" 
-o exit | FileCheck %s
-
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g 
-fdebug-types-section -DFILE_A
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g 
-fdebug-types-section -DFILE_B
-// RUN: ld.lld %t-a.o %t-b.o -o %t
-// RUN: %lldb %t -o "target variable --ptr-depth 1 --show-types both_a both_b" 
-o exit | FileCheck %s
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-n-a.o -g 
-gsimple-template-names -DFILE_A
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-n-b.o -g 
-gsimple-template-names

[Lldb-commits] [lldb] [GDBRemote] Handle 'heap' memory region info type (PR #105883)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/105883

>From e91140e2c059df5f65ad907e9c93329c75e36dc4 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Fri, 23 Aug 2024 13:10:00 -0700
Subject: [PATCH 1/2] [GDBRemote] Handle 'heap' memory region info type

This should cause the memory region info "is stack" field to be set to "no".
---
 .../Process/gdb-remote/GDBRemoteCommunicationClient.cpp  | 2 ++
 .../gdb-remote/GDBRemoteCommunicationClientTest.cpp  | 9 +
 2 files changed, 11 insertions(+)

diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 0297fe363f69e1..ac04049548e437 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1638,6 +1638,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
   for (llvm::StringRef entry : llvm::split(value, ',')) {
 if (entry == "stack")
   region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
+if (entry == "heap")
+  region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
   }
 } else if (name == "error") {
   StringExtractorGDBRemote error_extractor(value);
diff --git 
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 18020c8e43fe06..ce5ab2cf508293 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -364,6 +364,15 @@ TEST_F(GDBRemoteCommunicationClientTest, 
GetMemoryRegionInfo) {
   EXPECT_TRUE(result.get().Success());
   EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
   EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
+
+  result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  HandlePacket(server, "qMemoryRegionInfo:a000",
+   "start:a000;size:2000;type:heap;");
+  EXPECT_TRUE(result.get().Success());
+  EXPECT_EQ(MemoryRegionInfo::eNo, region_info.IsStackMemory());
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {

>From 4f874c829643c12283afd87c52b762a897c93d11 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Wed, 4 Sep 2024 07:02:16 -0700
Subject: [PATCH 2/2] Fixup! address review comments

---
 .../Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index ac04049548e437..55d76ca8532d39 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1638,7 +1638,7 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
   for (llvm::StringRef entry : llvm::split(value, ',')) {
 if (entry == "stack")
   region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
-if (entry == "heap")
+else if (entry == "heap")
   region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
   }
 } else if (name == "error") {

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

[Lldb-commits] [lldb] 1b0a802 - [GDBRemote] Handle 'heap' memory region info type (#105883)

[via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2024-09-04T07:02:55-07:00
New Revision: 1b0a80249399dadfe0c3f682fff77bf9eb666535

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

LOG: [GDBRemote] Handle 'heap' memory region info type (#105883)

This should cause the memory region info "is stack" field to be set to
"no".

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 0297fe363f69e1..55d76ca8532d39 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1638,6 +1638,8 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo(
   for (llvm::StringRef entry : llvm::split(value, ',')) {
 if (entry == "stack")
   region_info.SetIsStackMemory(MemoryRegionInfo::eYes);
+else if (entry == "heap")
+  region_info.SetIsStackMemory(MemoryRegionInfo::eNo);
   }
 } else if (name == "error") {
   StringExtractorGDBRemote error_extractor(value);

diff  --git 
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 18020c8e43fe06..ce5ab2cf508293 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -364,6 +364,15 @@ TEST_F(GDBRemoteCommunicationClientTest, 
GetMemoryRegionInfo) {
   EXPECT_TRUE(result.get().Success());
   EXPECT_EQ(MemoryRegionInfo::eYes, region_info.GetMemoryTagged());
   EXPECT_EQ(MemoryRegionInfo::eYes, region_info.IsStackMemory());
+
+  result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  HandlePacket(server, "qMemoryRegionInfo:a000",
+   "start:a000;size:2000;type:heap;");
+  EXPECT_TRUE(result.get().Success());
+  EXPECT_EQ(MemoryRegionInfo::eNo, region_info.IsStackMemory());
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {



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

[Lldb-commits] [lldb] [GDBRemote] Handle 'heap' memory region info type (PR #105883)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/105883
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[David Spickett via lldb-commits]

@@ -728,7 +746,6 @@ def test_riscv64_regs(self):
 "fa2",
 "fa3",
 "fa4",
-"fa5",

DavidSpickett wrote:

I would replace this line with:
```
# fa5 is non-zero and checked in the list above.
```

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][RISCV] Support optionally disabled FPR for riscv64 (PR #104547)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Just one more thing then this is good to go.

Do you have commit access or will you need me to merge it for you?

https://github.com/llvm/llvm-project/pull/104547
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -0,0 +1,58 @@
+//===--- DirectToIndirectFCR.h - RISC-V specific pass 
-===//
+//
+// 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
+//
+//===--===//
+
+#pragma once
+
+#include "lldb/lldb-types.h"
+
+#include "llvm/IR/Instructions.h"
+#include "llvm/Pass.h"
+
+namespace lldb_private {
+
+class ExecutionContext;
+
+// During the lldb expression execution lldb wraps a user expression, jittes
+// fabricated code and then puts it into the stack memory. Thus, if user tried
+// to make a function call there will be a jump from a stack address to a code
+// sections's address. RISC-V Architecture doesn't have a large code model yet
+// and can make only a +-2GiB jumps, but in 64-bit architecture a distance
+// between stack addresses and code sections's addresses is longer. Therefore,
+// relocations resolver obtains an invalid address. To avoid such problem, this
+// pass should be used. It replaces function calls with appropriate function's
+// addresses explicitly. By doing so it removes relocations related to function
+// calls. This pass should be cosidered as temprorary solution until a large
+// code model will be approved.
+class DirectToIndirectFCR : public llvm::FunctionPass {

dlav-sc wrote:

addressed. Changed to `DirectCallReplacementPass`.

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 34b10e165d809bb133d37dfe934859800f21a100 
66d055aa303c8fa07c4db2023dc175bad48a15a0 --extensions cpp,h -- 
lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.cpp 
lldb/source/Plugins/Architecture/RISCV/ArchitectureRISCV.h 
lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.cpp 
lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.h 
lldb/test/API/riscv/expressions/main.cpp lldb/include/lldb/Core/Architecture.h 
lldb/source/Expression/IRExecutionUnit.cpp 
lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp 
lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.h 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp 
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp 
b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index fa36d764d3..02f9fae57c 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -190,8 +190,7 @@ static void LogInitInfo(Log *log, const Thread &thread, 
addr_t sp,
   std::stringstream ss;
   ss << "ABISysV_riscv::PrepareTrivialCall"
  << " (tid = 0x" << std::hex << thread.GetID() << ", sp = 0x" << sp
- << ", func_addr = 0x" << func_addr << ", return_addr = 0x"
- << return_addr;
+ << ", func_addr = 0x" << func_addr << ", return_addr = 0x" << return_addr;
 
   for (auto &&[idx, arg] : enumerate(args))
 ss << ", arg" << std::dec << idx << " = 0x" << std::hex << arg;
diff --git 
a/lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.cpp 
b/lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.cpp
index 74096ebb20..4b73d0cf5e 100644
--- a/lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.cpp
+++ b/lldb/source/Plugins/Architecture/RISCV/DirectCallReplacementPass.cpp
@@ -53,14 +53,15 @@ std::string getFunctionName(const llvm::CallInst *ci) {
 bool DirectCallReplacementPass::canBeReplaced(const llvm::CallInst *ci) {
   assert(ci);
   Log *log = GetLog(LLDBLog::Expressions);
-  
+
   auto *return_value_ty = ci->getType();
   if (!(return_value_ty->isIntegerTy() || return_value_ty->isVoidTy() ||
 return_value_ty->isPointerTy())) {
-LLDB_LOG(log, "DirectCallReplacementPass::{0}() function {1} has 
unsupported "
-   "return type ({2})\n", __FUNCTION__,
-   getFunctionName(ci),
-   GetValueTypeStr(return_value_ty));
+LLDB_LOG(log,
+ "DirectCallReplacementPass::{0}() function {1} has unsupported "
+ "return type ({2})\n",
+ __FUNCTION__, getFunctionName(ci),
+ GetValueTypeStr(return_value_ty));
 return false;
   }
 
@@ -70,10 +71,11 @@ bool DirectCallReplacementPass::canBeReplaced(const 
llvm::CallInst *ci) {
   });
 
   if (arg != ci->arg_end()) {
-LLDB_LOG(log, "DirectCallReplacementPass::{0}() argument {1} of {2} 
function "
-   "has unsupported type ({3})\n", __FUNCTION__,
-   (*arg)->getName(), getFunctionName(ci),
-   GetValueTypeStr((*arg)->getType()));
+LLDB_LOG(log,
+ "DirectCallReplacementPass::{0}() argument {1} of {2} function "
+ "has unsupported type ({3})\n",
+ __FUNCTION__, (*arg)->getName(), getFunctionName(ci),
+ GetValueTypeStr((*arg)->getType()));
 return false;
   }
   return true;
@@ -91,7 +93,7 @@ DirectCallReplacementPass::getFunctionArgsAsValues(const 
llvm::CallInst *ci) {
 std::optional
 DirectCallReplacementPass::getFunctionAddress(const llvm::CallInst *ci) const {
   Log *log = GetLog(LLDBLog::Expressions);
-  
+
   auto *target = m_exe_ctx.GetTargetPtr();
   const auto &lldb_module_sp = target->GetExecutableModule();
   const auto &symtab = lldb_module_sp->GetSymtab();
@@ -104,17 +106,21 @@ DirectCallReplacementPass::getFunctionAddress(const 
llvm::CallInst *ci) const {
   ConstString(name), lldb::SymbolType::eSymbolTypeCode,
   Symtab::Debug::eDebugNo, Symtab::Visibility::eVisibilityExtern);
   if (!symbol) {
-LLDB_LOG(log, "DirectCallReplacementPass::{0}() can't find {1} in 
symtab\n", __FUNCTION__, name);
+LLDB_LOG(log, "DirectCallReplacementPass::{0}() can't find {1} in 
symtab\n",
+ __FUNCTION__, name);
 return std::nullopt;
   }
 
   lldb::addr_t addr = symbol->GetLoadAddress(target);
-  LLDB_LOG(log, "DirectCallReplacementPass::{0}() found address ({1:x}) of 
symbol {2}\n", __FUNCTION__, addr,
- name);
+  LLDB_LOG(
+  log,
+  "DirectCallReplacementPass::{0}() found address ({1:x}) of symbol {2}\n",
+  __FUNCTION__, addr, name);
   return addr;
 }
 
-llvm::CallInst *Di

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -163,11 +166,83 @@ TotalArgsSizeInWords(bool is_rv64,
   return total_size;
 }
 
+static bool UpdateRegister(RegisterContext *reg_ctx,
+   const lldb::RegisterKind reg_kind,
+   const uint32_t reg_num, const addr_t value) {
+  Log *log = GetLog(LLDBLog::Expressions);
+
+  const RegisterInfo *reg_info = reg_ctx->GetRegisterInfo(reg_kind, reg_num);
+
+  LLDB_LOG(log, "Writing %s: 0x%" PRIx64, reg_info->name,
+   static_cast(value));
+  if (!reg_ctx->WriteRegisterFromUnsigned(reg_info, value)) {
+LLDB_LOG(log, "Writing %s: failed", reg_info->name);

dlav-sc wrote:

addressed. I've changed log messages.

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -0,0 +1,194 @@
+//===--- DirectToIndirectFCR.cpp - RISC-V specific pass 
---===//
+//
+// 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 "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Value.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+
+#include "Plugins/Architecture/RISCV/DirectToIndirectFCR.h"
+
+#include "lldb/Core/Architecture.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/Symtab.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include 
+
+using namespace llvm;
+using namespace lldb_private;
+
+namespace {
+std::string GetValueTypeStr(const llvm::Type *value_ty) {
+  assert(value_ty);
+  std::string str_type;
+  llvm::raw_string_ostream rso(str_type);
+  value_ty->print(rso);
+  return rso.str();
+}
+
+template  void LogMessage(const char *msg, Args &&...args) {
+  Log *log = GetLog(LLDBLog::Expressions);
+  LLDB_LOG(log, msg, std::forward(args)...);
+}

dlav-sc wrote:

addressed

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -0,0 +1,194 @@
+//===--- DirectToIndirectFCR.cpp - RISC-V specific pass 
---===//
+//
+// 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 "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Value.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+
+#include "Plugins/Architecture/RISCV/DirectToIndirectFCR.h"
+
+#include "lldb/Core/Architecture.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/Symtab.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+#include 
+
+using namespace llvm;
+using namespace lldb_private;
+
+namespace {
+std::string GetValueTypeStr(const llvm::Type *value_ty) {
+  assert(value_ty);
+  std::string str_type;
+  llvm::raw_string_ostream rso(str_type);
+  value_ty->print(rso);
+  return rso.str();
+}
+
+template  void LogMessage(const char *msg, Args &&...args) {
+  Log *log = GetLog(LLDBLog::Expressions);
+  LLDB_LOG(log, msg, std::forward(args)...);
+}
+} // namespace
+
+bool DirectToIndirectFCR::canBeReplaced(const llvm::CallInst *ci) {
+  assert(ci);
+  auto *return_value_ty = ci->getType();
+  if (!(return_value_ty->isIntegerTy() || return_value_ty->isVoidTy())) {
+LogMessage("DirectToIndirectFCR: function {0} has unsupported "
+   "return type ({1})\n",
+   ci->getCalledFunction()->getName(),
+   GetValueTypeStr(return_value_ty));
+return false;
+  }
+
+  const auto *arg = llvm::find_if_not(ci->args(), [](const auto &arg) {
+const auto *type = arg->getType();
+return type->isIntegerTy();
+  });
+
+  if (arg != ci->arg_end()) {
+LogMessage("DirectToIndirectFCR: argument {0} of {1} function "
+   "has unsupported type ({2})\n",
+   (*arg)->getName(), ci->getCalledFunction()->getName(),
+   GetValueTypeStr((*arg)->getType()));
+return false;
+  }
+  return true;
+}
+
+std::vector
+DirectToIndirectFCR::getFunctionArgsAsValues(const llvm::CallInst *ci) {
+  assert(ci);
+  std::vector args{};
+  llvm::transform(ci->args(), std::back_inserter(args),
+  [](const auto &arg) { return arg.get(); });
+  return args;
+}
+
+std::optional
+DirectToIndirectFCR::getFunctionAddress(const llvm::CallInst *ci) const {
+  auto *target = m_exe_ctx.GetTargetPtr();
+  const auto &lldb_module_sp = target->GetExecutableModule();
+  const auto &symtab = lldb_module_sp->GetSymtab();
+  const llvm::StringRef name = ci->getCalledFunction()->getName();
+
+  // eSymbolTypeCode: we try to find function
+  // eDebugNo: not a debug symbol
+  // eVisibilityExtern: function from extern module

dlav-sc wrote:

addressed. Changed this place

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -0,0 +1,58 @@
+//===--- DirectToIndirectFCR.h - RISC-V specific pass 
-===//
+//
+// 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
+//
+//===--===//
+
+#pragma once

dlav-sc wrote:

addressed

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

@@ -60,44 +60,45 @@ class DWARFDebugInfoEntry {
 return attrs;
   }
 
-  dw_offset_t
-  GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
-DWARFFormValue &formValue,
-dw_offset_t *end_attr_offset_ptr = nullptr,
-bool check_specification_or_abstract_origin = false) const;
+  dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
+DWARFFormValue &formValue,
+dw_offset_t *end_attr_offset_ptr = nullptr,
+bool check_elaborating_dies = false) const;
 
-  const char *GetAttributeValueAsString(
-  const DWARFUnit *cu, const dw_attr_t attr, const char *fail_value,
-  bool check_specification_or_abstract_origin = false) const;
+  const char *
+  GetAttributeValueAsString(const DWARFUnit *cu, const dw_attr_t attr,
+const char *fail_value,
+bool check_elaborating_dies = false) const;
 
-  uint64_t GetAttributeValueAsUnsigned(
-  const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
-  bool check_specification_or_abstract_origin = false) const;
+  uint64_t
+  GetAttributeValueAsUnsigned(const DWARFUnit *cu, const dw_attr_t attr,
+  uint64_t fail_value,
+  bool check_elaborating_dies = false) const;

Michael137 wrote:

Out of curiousity, when do we not want to recurse through the 
specifications/origin/signature?

https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[via lldb-commits]

@@ -0,0 +1,58 @@
+//===--- DirectToIndirectFCR.h - RISC-V specific pass 
-===//
+//
+// 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
+//
+//===--===//
+
+#pragma once
+
+#include "lldb/lldb-types.h"
+
+#include "llvm/IR/Instructions.h"
+#include "llvm/Pass.h"
+
+namespace lldb_private {
+
+class ExecutionContext;
+
+// During the lldb expression execution lldb wraps a user expression, jittes
+// fabricated code and then puts it into the stack memory. Thus, if user tried
+// to make a function call there will be a jump from a stack address to a code
+// sections's address. RISC-V Architecture doesn't have a large code model yet
+// and can make only a +-2GiB jumps, but in 64-bit architecture a distance
+// between stack addresses and code sections's addresses is longer. Therefore,
+// relocations resolver obtains an invalid address. To avoid such problem, this
+// pass should be used. It replaces function calls with appropriate function's
+// addresses explicitly. By doing so it removes relocations related to function
+// calls. This pass should be cosidered as temprorary solution until a large

dlav-sc wrote:

addressed. Tricky ones

https://github.com/llvm/llvm-project/pull/99336
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[Dhruv Srivastava via lldb-commits]

@@ -38,6 +38,12 @@ endif()
 include(LLDBConfig)
 include(AddLLDB)
 
+# This has been added to keep the AIX build isolated for now.
+# It will need to be modified later.
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  add_definitions("-D__AIX__")

DhruvSrivastavaX wrote:

Ok Thats helpful 😄 
I will give it go, trying to integrate it with the overall changes. 
Meanwhile, shall I remove `__AIX__` from this PR to keep that discussion 
separate? 

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[David Spickett via lldb-commits]

DavidSpickett wrote:

A couple of suggestions.

Perhaps you could reduce the PRs just to the files with the biggest differences 
on AIX, and include those changes in the PR. So that it is copying the file + 
making the AIX changes in one PR. Even if you have to add files that aren't 
used yet, we could wait to land the PR until we have reviewed the follow ups 
that do use them.

Maybe this is a good tactic for ptrace.h and Host.cpp, going by what you've 
said. Do one PR for each file so that everything is as clear as possible.

For the rest, a way to see whether one file for Linux and AIX would work would 
be to modify the existing file and add `#ifdef _AIX` everywhere a change is 
needed. Not as real code, but as a demonstration of exactly how awkward that 
would be to work with. You could post that as a draft PR and it would be very 
easy for us to see the additions, and recommend ways to refactor it if possible.

It doesn't even have to be buildable, as long as we get a rough idea of the 
changes.

(I do understand the instinct to split the initial copy-paste step into its own 
PR, I would have done the same, but in this case I think it makes it harder for 
us to assess the differences)

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[David Spickett via lldb-commits]

@@ -38,6 +38,12 @@ endif()
 include(LLDBConfig)
 include(AddLLDB)
 
+# This has been added to keep the AIX build isolated for now.
+# It will need to be modified later.
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  add_definitions("-D__AIX__")

DavidSpickett wrote:

Yes. If we need a cmake generated definition, add that in its own PR along with 
anything that uses it.

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

@@ -377,11 +378,6 @@ static void GetDeclContextImpl(DWARFDIE die,
   die = spec;
   continue;
 }
-// To find the name of a type in a type unit, we must follow the signature.
-if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {

Michael137 wrote:

Could you elaborate on why we don't need this anymore?

https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[David Spickett via lldb-commits]

@@ -38,6 +38,12 @@ endif()
 include(LLDBConfig)
 include(AddLLDB)
 
+# This has been added to keep the AIX build isolated for now.
+# It will need to be modified later.
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+  add_definitions("-D__AIX__")

DavidSpickett wrote:

Well, with *something* that uses it. I'm not asking you to look into the future 
and know all possible uses.

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

@@ -50,7 +50,8 @@ class ElaboratingDIEIterator
 m_worklist.pop_back();
 
 // And add back any items that elaborate it.
-for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
+for (dw_attr_t attr :
+ {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {

Michael137 wrote:

Just from cursory reading it seems like `ElaboratingDIEIterator` is only used 
for method DIEs currently? Which might be why no tests were affected here?

https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

https://github.com/Michael137 deleted 
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

[Michael Buch via lldb-commits]

@@ -50,7 +50,8 @@ class ElaboratingDIEIterator
 m_worklist.pop_back();
 
 // And add back any items that elaborate it.
-for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
+for (dw_attr_t attr :
+ {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {

Michael137 wrote:

Just from cursory reading it seems like `ElaboratingDIEIterator` is only useful 
for method DIEs currently? Which might be why no tests were affected here?

If you do need it as part of this patch then there's a couple of comments to 
fix up in `ElaboratingDIEIterator` that specifically talk about 
`DW_AT_specification`/`DW_AT_abstract_origin`

https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correctly reconstruct simplified names for type units (PR #107240)

[Michael Buch via lldb-commits]

https://github.com/Michael137 approved this pull request.

makes sense

https://github.com/llvm/llvm-project/pull/107240
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[Dhruv Srivastava via lldb-commits]

DhruvSrivastavaX wrote:

Ok sure. 

For readability and clarity, for the files having comparatively bigger changes, 
I can drop it as multiple commits in the same PR, so that the changes can be 
seen as cleanly as possible:
1. Commit 1 - copy paste the original
2. Commit 2 - Make AIX dependant changes

And for each of these bigger files, we can have respective PRs as per your 
suggestion.

And for the rest of the common files, another draft PR should be ok for the 
readability pov.
It can be used as reference for the changes that will be dropped in future. 

One question though? For the comparatively smaller changes across multiple 
files, is it okay to club the in one PR if they are related? 

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)

[Dhruv Srivastava via lldb-commits]

DhruvSrivastavaX wrote:

One more thing. For platform differentiating files/paths like HostInfoAIX 
future files like PlatformAIX, How do you suggest we add them?

To keep the code path more streamlined, shall we keep their code under Linux 
with #if _AIX, Or shall we use some form of inheritance to keep AIX as its own 
platform?

https://github.com/llvm/llvm-project/pull/106910
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

[via lldb-commits]

Da-Viper wrote:

> Can we modify this patch to support both formats? It should be easy to see if 
> "env" is a dictionary, and if it is, use your new code. If it is an array, 
> use the old parsing code. I would like to make sure this change doesn't break 
> anyone that has existing launch configurations.

Should the older one be marked as deprecated or we just support both and test 
both ?



https://github.com/llvm/llvm-project/pull/106919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/104238

>From 18b8a835a3182e5be5f0dd848977333d9b8a26fa Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 30 Aug 2024 01:08:24 +0400
Subject: [PATCH 1/2] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run lldb-server gdbserver 
--fd on all platforms.
Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../posix/ConnectionFileDescriptorPosix.cpp   |  21 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |  45 ++-
 .../gdb-remote/GDBRemoteCommunication.h   |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 287 +--
 .../GDBRemoteCommunicationServerPlatform.h|  82 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   2 +-
 .../TestPlatformLaunchGDBServer.py|  12 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |  98 -
 lldb/tools/lldb-server/Acceptor.h |  60 
 lldb/tools/lldb-server/CMakeLists.txt |   1 -
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  23 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 335 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 .../tools/lldb-server/tests/LLGSTest.cpp  |   4 -
 .../tools/lldb-server/tests/TestClient.cpp|   4 -
 .../tools/lldb-server/tests/TestClient.h  |   4 +
 20 files changed, 415 insertions(+), 721 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp
 delete mode 100644 lldb/tools/lldb-server/Acceptor.h
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 6a40f66be39b18..0c9672ea6aa753 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.c

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -150,12 +153,17 @@ static void 
client_handle(GDBRemoteCommunicationServerPlatform &platform,
   printf("Disconnected.\n");
 }
 
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
 static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
-  std::lock_guard guard(gdbserver_portmap_mutex);
-  gdbserver_portmap.FreePortForProcess(pid);
+  if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+  g_terminate = true;

slydiman wrote:

We need this callback only in case of non-server mode and tcp protocol.
Note this callback is called from the MonitorChildProcessThread. 
See the issue #101475. We must be very careful with this callback and we cannot 
change any arguments. 
I have removed g_terminate.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
+
   // the test suite makes many connections in parallel, let's not miss any.
   // The highest this should get reasonably is a function of the number
   // of target CPUs. For now, let's just use 100.
   const int backlog = 100;
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::unique_ptr sock_named;
+  std::unique_ptr sock_platform;
+  std::unique_ptr sock_gdb;
+  std::string hostname;
+  uint16_t platform_port = 0;
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
-  if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+  std::string socket_id;
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen platform: %s\n", error.AsCString());
+  return socket_error;
+}
+if (platform_port == 0)
+  platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+  socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+   /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+  printf("Failed to listen gdb: %s\n", error.AsCString());
+  return socket_error;
+}
+if (gdbserver_port == 0)
+  gdbserver_port = sock_gdb->GetLocalPortNumber();
+  } else {
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+  fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+  return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+  printf("Failed to listen: %s\n", error.AsCString());
+  return socket_error;
+}
+socket_id = hostname;
   }
+
   if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
   return 1;
 }
   }
 
-  GDBRemoteCommunicationServerPlatform platform(
-  acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+  lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+  GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
   if (port_offset > 0)
 platform.SetPortOffset(port_offset);
 
-  do {
-const bool children_inherit_accept_socket = true;
-  

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -879,20 +879,12 @@ lldb::thread_result_t 
GDBRemoteCommunication::ListenThread() {
   return {};
 }
 
-Status GDBRemoteCommunication::StartDebugserverProcess(
-const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+bool GDBRemoteCommunication::GetDebugserverPath(
+Platform *platform, FileSpec &debugserver_file_spec) {

slydiman wrote:

Updated.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -160,66 +95,58 @@ 
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
 default;
 
 Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
-  if (!port) {
-llvm::Expected available_port = 
m_port_map.GetNextAvailablePort();
-if (available_port)
-  port = *available_port;
-else
-  return Status(available_port.takeError());
-  }
-
-  // Spawn a new thread to accept the port that gets bound after binding to
-  // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+  std::ostringstream url;
+  if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+  // Just check that GDBServer exists. GDBServer must be launched after
+  // accepting the connection.
+  FileSpec debugserver_file_spec;
+  if (!GetDebugserverPath(nullptr, debugserver_file_spec))
+return Status::FromErrorString("unable to locate debugserver");
+  return Status();
+}
 
-  // ignore the hostname send from the remote end, just use the ip address that
-  // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+  } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+  return Status::FromErrorString("protocol must be tcp");
+  }
 
   // Spawn a debugserver and try to get the port it listens to.
   ProcessLaunchInfo debugserver_launch_info;
-  if (hostname.empty())
-hostname = "127.0.0.1";
-
   Log *log = GetLog(LLDBLog::Platform);
-  LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+  LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(),
+   (int64_t)fd);

slydiman wrote:

Updated.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -667,7 +669,22 @@ ConnectionStatus
 ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+  int64_t fd = -1;
+  if (!s.getAsInteger(0, fd)) {
+// Assume we own fd.
+std::unique_ptr tcp_socket =
+std::make_unique((NativeSocket)fd, true, false);
+m_io_sp = std::move(tcp_socket);
+m_uri = s.str();
+return eConnectionStatusSuccess;
+  }
+  if (error_ptr)
+*error_ptr = Status::FromErrorStringWithFormat(
+"invalid file descriptor: \"%s\"", s.str().c_str());

slydiman wrote:

ConnectionFileDescriptorPosix.cpp is unchanged now.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [clang-tools-extra] [lld] [lldb] [llvm] [mlir] [polly] [NFC] Add explicit #include llvm-config.h where its macros are used. (PR #106810)

[Daniil Fukalov via lldb-commits]

https://github.com/dfukalov updated 
https://github.com/llvm/llvm-project/pull/106810

>From 0221e97459534f0f7396e7970663e1a4f1f98cca Mon Sep 17 00:00:00 2001
From: dfukalov 
Date: Sat, 31 Aug 2024 01:45:27 +0200
Subject: [PATCH 1/3] [NFC] Add explicit #include llvm-config.h where its
 macros are used, part 2.

Without these explicit includes, removing other headers, who implicitly include 
llvm-config.h, may have non-trivial side effects.
For example, `clagd` may report even `llvm-config.h` as "no used" in case it 
defines a macro, that is expicitly used with #ifdef.
It is actually amplified with different build configs which use different set 
of macros.
---
 bolt/include/bolt/Core/BinaryBasicBlock.h | 1 +
 clang-tools-extra/clangd/Feature.cpp  | 1 +
 clang-tools-extra/clangd/unittests/ClangdTests.cpp| 1 +
 clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp   | 1 +
 clang-tools-extra/clangd/unittests/SerializationTests.cpp | 1 +
 clang/include/clang/Interpreter/Value.h   | 1 +
 clang/lib/Driver/ToolChains/Cuda.cpp  | 1 +
 clang/lib/Driver/ToolChains/MinGW.cpp | 1 +
 clang/lib/Driver/ToolChains/WebAssembly.cpp   | 1 +
 clang/lib/Frontend/FrontendActions.cpp| 1 +
 clang/tools/driver/driver.cpp | 2 ++
 clang/unittests/Driver/GCCVersionTest.cpp | 1 +
 lld/ELF/OutputSections.cpp| 2 +-
 lldb/source/API/SBDebugger.cpp| 1 +
 lldb/source/Host/common/Host.cpp  | 1 +
 .../Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 1 +
 .../Process/gdb-remote/GDBRemoteCommunicationClient.cpp   | 1 +
 lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp  | 1 +
 lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp  | 1 +
 lldb/unittests/Host/MainLoopTest.cpp  | 1 +
 llvm/include/llvm/Debuginfod/HTTPClient.h | 1 +
 llvm/include/llvm/Debuginfod/HTTPServer.h | 1 +
 llvm/include/llvm/Support/ErrorHandling.h | 1 +
 llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp| 1 +
 llvm/lib/Analysis/InlineAdvisor.cpp   | 1 +
 llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp | 1 +
 llvm/lib/Analysis/ModelUnderTrainingRunner.cpp| 1 +
 llvm/lib/Analysis/TFLiteUtils.cpp | 1 +
 llvm/lib/CodeGen/MLRegAllocEvictAdvisor.cpp   | 1 +
 llvm/lib/CodeGen/MLRegAllocPriorityAdvisor.cpp| 1 +
 llvm/lib/CodeGen/RegAllocEvictionAdvisor.cpp  | 1 +
 llvm/lib/CodeGen/RegAllocPriorityAdvisor.cpp  | 1 +
 llvm/lib/DebugInfo/PDB/PDB.cpp| 1 +
 llvm/lib/Debuginfod/HTTPClient.cpp| 1 +
 llvm/lib/Debuginfod/HTTPServer.cpp| 1 +
 llvm/lib/ExecutionEngine/ExecutionEngineBindings.cpp  | 1 +
 llvm/lib/ExecutionEngine/Orc/TargetProcess/JITLoaderVTune.cpp | 1 +
 llvm/lib/IR/Core.cpp  | 1 +
 llvm/lib/MC/SPIRVObjectWriter.cpp | 1 +
 llvm/lib/Support/CRC.cpp  | 1 +
 llvm/lib/Support/Compression.cpp  | 1 +
 llvm/lib/Support/Z3Solver.cpp | 1 +
 llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp | 1 +
 llvm/lib/TargetParser/Unix/Host.inc   | 1 +
 llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp| 1 +
 llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp| 1 +
 .../Analysis/InlineAdvisorPlugin/InlineAdvisorPlugin.cpp  | 4 ++--
 .../Analysis/InlineOrderPlugin/InlineOrderPlugin.cpp  | 4 ++--
 llvm/unittests/Analysis/PluginInlineAdvisorAnalysisTest.cpp   | 1 +
 llvm/unittests/Analysis/PluginInlineOrderAnalysisTest.cpp | 1 +
 llvm/unittests/Debuginfod/HTTPServerTests.cpp | 1 +
 llvm/unittests/Passes/Plugins/PluginsTest.cpp | 1 +
 llvm/unittests/Support/CompressionTest.cpp| 1 +
 llvm/unittests/TargetParser/Host.cpp  | 2 +-
 llvm/unittests/tools/llvm-profdata/OutputSizeLimitTest.cpp| 1 +
 mlir/include/mlir/Bytecode/BytecodeWriter.h   | 1 +
 mlir/lib/Target/SPIRV/SPIRVBinaryUtils.cpp| 1 +
 mlir/unittests/Target/LLVM/SerializeNVVMTarget.cpp| 1 +
 polly/lib/Support/RegisterPasses.cpp  | 1 +
 59 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/bolt/include/bolt/Core/BinaryBasicBlock.h 
b/bolt/include/bolt/Core/BinaryBasicBlock.h
index 9a9d7b8735d714..b4f31cf2bae6f6 100644
--- a/bolt/include/bolt/Core/BinaryBasicBlock.h
+++ 

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/104238

>From 18b8a835a3182e5be5f0dd848977333d9b8a26fa Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 30 Aug 2024 01:08:24 +0400
Subject: [PATCH 1/2] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run lldb-server gdbserver 
--fd on all platforms.
Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../posix/ConnectionFileDescriptorPosix.cpp   |  21 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |  45 ++-
 .../gdb-remote/GDBRemoteCommunication.h   |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 287 +--
 .../GDBRemoteCommunicationServerPlatform.h|  82 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   2 +-
 .../TestPlatformLaunchGDBServer.py|  12 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |  98 -
 lldb/tools/lldb-server/Acceptor.h |  60 
 lldb/tools/lldb-server/CMakeLists.txt |   1 -
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  23 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 335 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 .../tools/lldb-server/tests/LLGSTest.cpp  |   4 -
 .../tools/lldb-server/tests/TestClient.cpp|   4 -
 .../tools/lldb-server/tests/TestClient.h  |   4 +
 20 files changed, 415 insertions(+), 721 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp
 delete mode 100644 lldb/tools/lldb-server/Acceptor.h
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index 6a40f66be39b18..0c9672ea6aa753 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.c

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

@@ -666,7 +756,23 @@ ConnectionStatus
 ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
 socket_id_callback_type socket_id_callback,
 Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+  int64_t fd = -1;
+  if (!s.getAsInteger(0, fd)) {
+// Assume we own fd.
+std::unique_ptr tcp_socket =
+std::make_unique((NativeSocket)fd, true, false);
+m_io_sp = std::move(tcp_socket);
+m_uri = s.str();
+return eConnectionStatusSuccess;
+  }

slydiman wrote:

ConnectionFileDescriptorPosix.cpp is unchanged now.
I have moved the SharedSocket using to lldb-gdbserver.cpp.
I don't like the idea about SharedSocket::GetSocket() because of ambiguity 
Socket/TCPSocket/UDPSocket and additional parameters should_close, 
child_processes_inherit, etc. It may reduce using of NativeSocket, but will not 
reduce the code in general.

https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Reappply SBSaveCore AddMemoryList (PR #107159)

[Jacob Lalonde via lldb-commits]

Jlalond wrote:

Hey @bulbazord, this was reverted because it failed `TestSkinnyCorefile.py` 
[Link](https://github.com/llvm/llvm-project/blob/main/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py#L15),
 I'm running into issues where it keeps coming up as unsupported. Is there a 
way to trigger any Darwin CI before merging? Thanks!

https://github.com/llvm/llvm-project/pull/107159
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

[via lldb-commits]

https://github.com/Da-Viper updated 
https://github.com/llvm/llvm-project/pull/106919

>From d2bddca1753b4c960895f51d7eb80b6efa7dc986 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Sun, 1 Sep 2024 17:26:11 +0100
Subject: [PATCH 1/4] [lldb-dap] Make environment option an object

---
 .../tools/lldb-dap/launch/TestDAP_launch.py   |  4 +-
 lldb/tools/lldb-dap/JSONUtils.cpp | 40 ---
 lldb/tools/lldb-dap/JSONUtils.h   | 22 ++
 lldb/tools/lldb-dap/README.md |  5 ++-
 lldb/tools/lldb-dap/lldb-dap.cpp  |  8 +++-
 lldb/tools/lldb-dap/package.json  | 11 +++--
 6 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py 
b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index a16f2da3c4df71..6b9993a2548b8d 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -229,7 +229,7 @@ def test_environment(self):
 Tests launch of a simple program with environment variables
 """
 program = self.getBuildArtifact("a.out")
-env = ["NO_VALUE", "WITH_VALUE=BAR", "EMPTY_VALUE=", "SPACE=Hello 
World"]
+env = {"NO_VALUE": "", "WITH_VALUE":"BAR", "EMPTY_VALUE": "", "SPACE": 
"Hello World"}
 self.build_and_launch(program, env=env)
 self.continue_to_exit()
 
@@ -242,7 +242,7 @@ def test_environment(self):
 lines.pop(0)
 # Make sure each environment variable in "env" is actually set in the
 # program environment that was printed to STDOUT
-for var in env:
+for var in env.keys():
 found = False
 for program_var in lines:
 if var in program_var:
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 7338e7cf41eb03..29b3ad490af0b6 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -136,6 +136,31 @@ std::vector GetStrings(const 
llvm::json::Object *obj,
   return strs;
 }
 
+std::unordered_map
+GetStringMap(const llvm::json::Object &obj, llvm::StringRef key) {
+  std::unordered_map strs;
+  const auto *const json_object = obj.getObject(key);
+  if (!json_object)
+return strs;
+
+  for (const auto &[key, value] : *json_object) {
+switch (value.kind()) {
+case llvm::json::Value::String:
+  strs.emplace(key.str(), value.getAsString()->str());
+  break;
+case llvm::json::Value::Number:
+case llvm::json::Value::Boolean:
+  strs.emplace(key.str(), llvm::to_string(value));
+  break;
+case llvm::json::Value::Null:
+case llvm::json::Value::Object:
+case llvm::json::Value::Array:
+  break;
+}
+  }
+  return strs;
+}
+
 static bool IsClassStructOrUnionType(lldb::SBType t) {
   return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
   lldb::eTypeClassArray)) != 0;
@@ -1370,13 +1395,16 @@ CreateRunInTerminalReverseRequest(const 
llvm::json::Object &launch_request,
   if (!cwd.empty())
 run_in_terminal_args.try_emplace("cwd", cwd);
 
-  // We need to convert the input list of environments variables into a
-  // dictionary
-  std::vector envs = GetStrings(launch_request_arguments, "env");
+  std::unordered_map envMap =
+  GetStringMap(*launch_request_arguments, "env");
   llvm::json::Object environment;
-  for (const std::string &env : envs) {
-size_t index = env.find('=');
-environment.try_emplace(env.substr(0, index), env.substr(index + 1));
+  for (const auto &[key, value] : envMap) {
+if (key.empty())
+  g_dap.SendOutput(OutputType::Stderr,
+   "empty environment variable for value: \"" + value +
+   '\"');
+else
+  environment.try_emplace(key, value);
   }
   run_in_terminal_args.try_emplace("env",
llvm::json::Value(std::move(environment)));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index b6356630b72682..60d5db06560657 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/JSON.h"
 #include 
 #include 
+#include 
 
 namespace lldb_dap {
 
@@ -152,6 +153,27 @@ bool ObjectContainsKey(const llvm::json::Object &obj, 
llvm::StringRef key);
 std::vector GetStrings(const llvm::json::Object *obj,
 llvm::StringRef key);
 
+/// Extract an object of key value strings for the specified key from an 
object.
+///
+/// String values in the object will be extracted without any quotes
+/// around them. Numbers and Booleans will be converted into
+/// strings. Any NULL, array or objects values in the array will be
+/// ignored.
+///
+/// \param[in] obj
+/// A JSON object that we will attempt to extract the array from
+///
+/// \param[in] key
+/// The key to use when extr

[Lldb-commits] [lldb] [lldb] Do not use LC_FUNCTION_STARTS data to determine symbol size as symbols are created (PR #106791)

[Alex Langford via lldb-commits]

bulbazord wrote:

> OK, I see in `Symtab::InitAddressIndexes` we go through the symbol table and 
> calculate the sizes of any entries that don't have a size based on the next 
> symbol, or the end of the section.
> 
> A little further in ObjectFileMachO::ParseSymtab when we add any 
> LC_FUNCTION_STARTS entries to the symbol table, we calculate the size of the 
> symbols based on the symbols we have parsed so far,
> 
> ```
> uint32_t symbol_byte_size = 0;
> if (symbol_section) {
>   const addr_t section_file_addr = 
> symbol_section->GetFileAddress();
>   const FunctionStarts::Entry *next_func_start_entry =
>   function_starts.FindNextEntry(func_start_entry);
>   const addr_t section_end_file_addr =
>   section_file_addr + symbol_section->GetByteSize();
>   if (next_func_start_entry) {
> addr_t next_symbol_file_addr = next_func_start_entry->addr;
> if (is_arm)   
>   next_symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
> symbol_byte_size = std::min(
> next_symbol_file_addr - symbol_file_addr,
> section_end_file_addr - symbol_file_addr);
>   } else {
> symbol_byte_size = section_end_file_addr - symbol_file_addr;
>   } 
> ```
> 
> and this should probably set the size to 0 as well, leaving it to 
> `Symtab::InitAddressIndexes` to do the same calculation. Does this sound 
> right to you?

@jasonmolenda In this instance it shouldn't matter tremendously where we do the 
work, but doing it in `Symtab::InitAddressIndexes` simplifies the code a lot. 
Great catch! :)

> All symbol sizes are zero for mach-o symbols unless they come from the debug 
> map as there is no size field in nlist entries. This PR seems to just not do 
> the work of setting the symbol size at all. Does something try to set the 
> symbol sizes later? Does the code still try to create symbols from the 
> LC_FUNCTION_STARTS later in this function?
> 
> I worry about not setting the function sizes for symbols as we use this 
> information for back tracing when we lookup a symbol by address. Now if we 
> don't have debug symbols, we won't get any sizes for symbols and I am not 
> sure how much that affects the unwinder. I believe it is pretty important. 
> @jasonmolenda let me know if you agree?
> 
> Can we possibly find a way to calculate a symbol size on demand when we 
> access the address range from a `lldb_private::Symbol`? The idea would be we 
> could mark certain symbols as `can_synthesize_size = true` and then any 
> accessor to a symbol's address range would need to see if this is true and 
> call some function that could calculate this on the fly on demand.

@clayborg Yes, the symtab reaches the same state with or without this patch. 
This is purely for performance. The code I've deleted is an O(log n) operation 
for each symbol processed when the same work is being done later in O(1) time. 
Accessing the symbol's size on-demand would be a much more involved change and 
I'm not convinced it would be faster in the end.

https://github.com/llvm/llvm-project/pull/106791
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Pavel Labath via lldb-commits]

@@ -91,9 +91,9 @@ class Status {
 
   ~Status();
 
-  // llvm::Error support
-  explicit Status(llvm::Error error) { *this = std::move(error); }
-  const Status &operator=(llvm::Error error);
+  /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+  static Status FromError(llvm::Error &&error);

labath wrote:

```suggestion
  static Status FromError(llvm::Error error);
```

https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Pavel Labath via lldb-commits]

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Pavel Labath via lldb-commits]

@@ -145,6 +145,7 @@ class Status {
   bool Success() const;
 
 protected:
+  Status(llvm::Error &&error);

labath wrote:

```suggestion
  Status(llvm::Error error);
```

https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

No, I'm just saying we should ditch the rvalue references. I.e., do this.

https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (PR #106767)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/106767

>From ceb20d62d9cef3090e34e8b9fc0bc620a7d9da3d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Fri, 30 Aug 2024 10:33:08 -0700
Subject: [PATCH 1/3] Extend the minidump x86_64 registers to include fs_base
 and gs_base

---
 .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp| 5 -
 .../Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp   | 2 +-
 .../Process/minidump/RegisterContextMinidump_x86_64.cpp| 7 +++
 .../Process/minidump/RegisterContextMinidump_x86_64.h  | 7 ++-
 .../TestProcessSaveCoreMinidump.py | 7 +++
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 13355afb58dbd1..5c9ba223ad143e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -473,7 +473,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
   lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
   lldb_private::minidump::MinidumpContext_x86_64_Flags::Control |
   lldb_private::minidump::MinidumpContext_x86_64_Flags::Segments |
-  lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer);
+  lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer |
+  lldb_private::minidump::MinidumpContext_x86_64_Flags::LLDBSpecific);
   thread_context.rax = read_register_u64(reg_ctx, "rax");
   thread_context.rbx = read_register_u64(reg_ctx, "rbx");
   thread_context.rcx = read_register_u64(reg_ctx, "rcx");
@@ -499,6 +500,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
   thread_context.gs = read_register_u64(reg_ctx, "gs");
   thread_context.ss = read_register_u64(reg_ctx, "ss");
   thread_context.ds = read_register_u64(reg_ctx, "ds");
+  thread_context.fs_base = read_register_u64(reg_ctx, "fs_base");
+  thread_context.gs_base = read_register_u64(reg_ctx, "gs_base");
   return thread_context;
 }
 
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
index 845312f4c1eddc..f60757a52c6310 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
@@ -21,7 +21,7 @@ 
RegisterContextCorePOSIX_x86_64::RegisterContextCorePOSIX_x86_64(
 
   size = GetGPRSize();
   m_gpregset.reset(new uint8_t[size]);
-  len =
+  len = 
   gpregset.ExtractBytes(0, size, lldb::eByteOrderLittle, m_gpregset.get());
   if (len != size)
 m_gpregset.reset();
diff --git 
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp 
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index 917140cab29767..4db049ff7e64e7 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
@@ -67,6 +67,7 @@ lldb::DataBufferSP 
lldb_private::minidump::ConvertMinidumpContext_x86_64(
   auto ControlFlag = MinidumpContext_x86_64_Flags::Control;
   auto IntegerFlag = MinidumpContext_x86_64_Flags::Integer;
   auto SegmentsFlag = MinidumpContext_x86_64_Flags::Segments;
+  auto LLDBSpecificFlag = MinidumpContext_x86_64_Flags::LLDBSpecific;
 
   if ((context_flags & x86_64_Flag) != x86_64_Flag)
 return nullptr;
@@ -104,6 +105,12 @@ lldb::DataBufferSP 
lldb_private::minidump::ConvertMinidumpContext_x86_64(
 writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]);
   }
 
+  if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) {
+writeRegister(&context->fs_base, result_base, 
reg_info[x86_64_with_base::lldb_fs_base]);
+writeRegister(&context->gs_base, result_base,
+  reg_info[x86_64_with_base::lldb_gs_base]);
+  }
+
   // TODO parse the floating point registers
 
   return result_context_buf;
diff --git 
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h 
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
index d920ea9d823f4f..f214e04a315a8e 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -153,6 +153,10 @@ struct MinidumpContext_x86_64 {
   llvm::support::ulittle64_t last_branch_from_rip;
   llvm::support::ulittle64_t last_exception_to_rip;
   llvm::support::ulittle64_t last_exception_from_rip;
+
+  // These registers are LLDB specific.
+  llvm::support::ulittle64_t fs_base;
+  llvm::support::ulittle64_t gs_base;
 };
 
 // For context_flags. These values indicate the type of
@@ -168,9 +172,10 @@ enum class MinidumpContext_x86_64_Flags : uint32_t {
   FloatingPoint = x86_64_Flag | 0x0008,
   Debug

[Lldb-commits] [lldb] [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (PR #106767)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff c4906588ce47de33d59bcd95f3e82ce2c3e61c23 
3da1e51b77592dc193328ad3b7d64165ccc3c71e --extensions h,cpp -- 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp 
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
``





View the diff from clang-format here.


``diff
diff --git 
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h 
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
index 34b1fbb45f..f8d38e4ba5 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -154,8 +154,8 @@ struct MinidumpContext_x86_64 {
   llvm::support::ulittle64_t last_exception_to_rip;
   llvm::support::ulittle64_t last_exception_from_rip;
 
-  // LLDB can save core files and save extra information that isn't available 
from
-  // Google breakpad, or similar, minidump files. 
+  // LLDB can save core files and save extra information that isn't available
+  // from Google breakpad, or similar, minidump files.
   llvm::support::ulittle64_t fs_base;
   llvm::support::ulittle64_t gs_base;
 };

``




https://github.com/llvm/llvm-project/pull/106767
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Adrian Prantl via lldb-commits]

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/107163

>From 3a133869fd7defac30329ef4c81eb082883e Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Tue, 3 Sep 2024 15:29:25 -0700
Subject: [PATCH] [lldb] Make conversions from llvm::Error explicit with
 Status::FromError() [NFC]

---
 lldb/include/lldb/Utility/Status.h|   7 +-
 lldb/source/API/SBDebugger.cpp|   2 +-
 lldb/source/API/SBTarget.cpp  |   2 +-
 .../Commands/CommandObjectBreakpoint.cpp  | 103 ++
 .../Commands/CommandObjectMemoryTag.cpp   |  10 +-
 lldb/source/Commands/CommandObjectStats.cpp   |   6 +-
 lldb/source/Commands/CommandObjectTrace.cpp   |   2 +-
 lldb/source/Core/PluginManager.cpp|   2 +-
 lldb/source/Core/ThreadedCommunication.cpp|   2 +-
 lldb/source/Core/ValueObjectVTable.cpp|   2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |   2 +-
 lldb/source/DataFormatters/VectorType.cpp |   2 +-
 lldb/source/Host/common/FileCache.cpp |   2 +-
 .../Host/common/NativeProcessProtocol.cpp |   2 +-
 lldb/source/Host/common/TCPSocket.cpp |   6 +-
 lldb/source/Host/macosx/objcxx/Host.mm|   2 +-
 .../posix/ConnectionFileDescriptorPosix.cpp   |   6 +-
 lldb/source/Interpreter/CommandObject.cpp |   2 +-
 lldb/source/Interpreter/OptionValueRegex.cpp  |   2 +-
 .../Language/CPlusPlus/BlockPointer.cpp   |   2 +-
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp |   2 +-
 .../Minidump/ObjectFileMinidump.cpp   |   2 +-
 ...latformiOSSimulatorCoreSimulatorSupport.mm |   2 +-
 .../NativeRegisterContextDBReg_arm64.cpp  |  10 +-
 .../Process/elf-core/ProcessElfCore.cpp   |   2 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |   2 +-
 .../GDBRemoteCommunicationClient.cpp  |   2 +-
 .../GDBRemoteCommunicationServer.cpp  |   2 +-
 .../GDBRemoteCommunicationServerLLGS.cpp  |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  |   2 +-
 .../Process/minidump/ProcessMinidump.cpp  |   2 +-
 .../Interfaces/ScriptedPythonInterface.h  |   2 +-
 .../Python/PythonDataObjects.cpp  |  22 ++--
 .../Python/ScriptInterpreterPython.cpp|  10 +-
 .../DarwinLog/StructuredDataDarwinLog.cpp |   2 +-
 lldb/source/Target/ModuleCache.cpp|   2 +-
 lldb/source/Target/Platform.cpp   |   2 +-
 lldb/source/Target/Process.cpp|   4 +-
 lldb/source/Target/StackFrame.cpp |   2 +-
 lldb/source/Target/Thread.cpp |   3 +-
 lldb/source/Utility/Scalar.cpp|   2 +-
 lldb/source/Utility/Status.cpp|  10 +-
 lldb/source/Utility/StructuredData.cpp|   2 +-
 .../Host/NativeProcessTestUtils.h |   4 +-
 lldb/unittests/Utility/StatusTest.cpp |  14 ++-
 45 files changed, 151 insertions(+), 132 deletions(-)

diff --git a/lldb/include/lldb/Utility/Status.h 
b/lldb/include/lldb/Utility/Status.h
index b304291ffae00e..3813a3c1604700 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -91,9 +91,9 @@ class Status {
 
   ~Status();
 
-  // llvm::Error support
-  explicit Status(llvm::Error error) { *this = std::move(error); }
-  const Status &operator=(llvm::Error error);
+  /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+  static Status FromError(llvm::Error error);
+  /// FIXME: Replace this with a takeError method.
   llvm::Error ToError() const;
 
   /// Get the error string associated with the current error.
@@ -145,6 +145,7 @@ class Status {
   bool Success() const;
 
 protected:
+  Status(llvm::Error error);
   /// Status code as an integer value.
   ValueType m_code = 0;
   /// The type of the above error code.
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 72501570320d57..c226acc15018ef 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -220,7 +220,7 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
   SBError error;
   if (auto e = g_debugger_lifetime->Initialize(
   std::make_unique(), LoadPlugin)) {
-error.SetError(Status(std::move(e)));
+error.SetError(Status::FromError(std::move(e)));
   }
   return error;
 }
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index e927cb854cd88c..41eb77e5506bc5 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1658,7 +1658,7 @@ SBError SBTarget::SetLabel(const char *label) {
   if (!target_sp)
 return Status::FromErrorString("Couldn't get internal target object.");
 
-  return Status(target_sp->SetLabel(label));
+  return Status::FromError(target_sp->SetLabel(label));
 }
 
 uint32_t SBTarget::GetDataByteSize() {
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index ede3dd2f2a864c..494d6c50e94ac3 100644
--- a/lldb/source/Com

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Adrian Prantl via lldb-commits]

adrian-prantl wrote:

@labath Done.

https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 9626e84faeaab200665bae9694458c2beb3d49c7 
3a133869fd7defac30329ef4c81eb082883e --extensions cpp,h -- 
lldb/include/lldb/Utility/Status.h lldb/source/API/SBDebugger.cpp 
lldb/source/API/SBTarget.cpp lldb/source/Commands/CommandObjectBreakpoint.cpp 
lldb/source/Commands/CommandObjectMemoryTag.cpp 
lldb/source/Commands/CommandObjectStats.cpp 
lldb/source/Commands/CommandObjectTrace.cpp lldb/source/Core/PluginManager.cpp 
lldb/source/Core/ThreadedCommunication.cpp 
lldb/source/Core/ValueObjectVTable.cpp lldb/source/Core/ValueObjectVariable.cpp 
lldb/source/DataFormatters/VectorType.cpp lldb/source/Host/common/FileCache.cpp 
lldb/source/Host/common/NativeProcessProtocol.cpp 
lldb/source/Host/common/TCPSocket.cpp 
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
lldb/source/Interpreter/CommandObject.cpp 
lldb/source/Interpreter/OptionValueRegex.cpp 
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp 
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp 
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
 lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp 
lldb/source/Target/ModuleCache.cpp lldb/source/Target/Platform.cpp 
lldb/source/Target/Process.cpp lldb/source/Target/StackFrame.cpp 
lldb/source/Target/Thread.cpp lldb/source/Utility/Scalar.cpp 
lldb/source/Utility/Status.cpp lldb/source/Utility/StructuredData.cpp 
lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h 
lldb/unittests/Utility/StatusTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index cdc2013fb4..131fc662bf 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -94,9 +94,7 @@ Status Status::FromErrorStringWithFormat(const char *format, 
...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) {
-  return Status(std::move(error));
-}
+Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
 
 llvm::Error Status::ToError() const {
   if (Success())

``




https://github.com/llvm/llvm-project/pull/107163
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Adrian Prantl via lldb-commits]

@@ -26,6 +26,36 @@ class raw_ostream;
 
 namespace lldb_private {
 
+class MachKernelError

adrian-prantl wrote:

Updated.

https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 9626e84faeaab200665bae9694458c2beb3d49c7 
47e2879bf9f49ece471d9e0d47d16c175d012703 --extensions cpp,h -- 
lldb/include/lldb/API/SBError.h lldb/include/lldb/API/SBValueList.h 
lldb/include/lldb/Core/ValueObjectConstResult.h 
lldb/include/lldb/Target/Process.h lldb/include/lldb/Utility/Status.h 
lldb/source/API/SBBreakpoint.cpp lldb/source/API/SBBreakpointLocation.cpp 
lldb/source/API/SBBreakpointName.cpp lldb/source/API/SBDebugger.cpp 
lldb/source/API/SBError.cpp lldb/source/API/SBFile.cpp 
lldb/source/API/SBFormat.cpp lldb/source/API/SBFrame.cpp 
lldb/source/API/SBPlatform.cpp lldb/source/API/SBProcess.cpp 
lldb/source/API/SBSaveCoreOptions.cpp lldb/source/API/SBStructuredData.cpp 
lldb/source/API/SBTarget.cpp lldb/source/API/SBThread.cpp 
lldb/source/API/SBValue.cpp lldb/source/API/SBValueList.cpp 
lldb/source/API/SBWatchpoint.cpp 
lldb/source/Commands/CommandObjectBreakpoint.cpp 
lldb/source/Commands/CommandObjectCommands.cpp 
lldb/source/Commands/CommandObjectMemoryTag.cpp 
lldb/source/Commands/CommandObjectStats.cpp 
lldb/source/Commands/CommandObjectTrace.cpp lldb/source/Core/Debugger.cpp 
lldb/source/Core/ModuleList.cpp lldb/source/Core/PluginManager.cpp 
lldb/source/Core/ThreadedCommunication.cpp lldb/source/Core/ValueObject.cpp 
lldb/source/Core/ValueObjectCast.cpp 
lldb/source/Core/ValueObjectConstResult.cpp 
lldb/source/Core/ValueObjectDynamicValue.cpp 
lldb/source/Core/ValueObjectSyntheticFilter.cpp 
lldb/source/Core/ValueObjectVTable.cpp lldb/source/Core/ValueObjectVariable.cpp 
lldb/source/DataFormatters/VectorType.cpp 
lldb/source/Expression/FunctionCaller.cpp 
lldb/source/Expression/LLVMUserExpression.cpp 
lldb/source/Expression/Materializer.cpp 
lldb/source/Expression/UserExpression.cpp lldb/source/Host/common/FileCache.cpp 
lldb/source/Host/common/LockFileBase.cpp 
lldb/source/Host/common/NativeProcessProtocol.cpp 
lldb/source/Host/common/TCPSocket.cpp 
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
lldb/source/Interpreter/CommandInterpreter.cpp 
lldb/source/Interpreter/CommandObject.cpp 
lldb/source/Interpreter/OptionValueRegex.cpp 
lldb/source/Interpreter/ScriptInterpreter.cpp 
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
lldb/source/Plugins/Platform/Android/AdbClient.cpp 
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp 
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.h
 lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp 
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp 
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp 
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.cpp
 
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
 lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp 
lldb/source/Target/ModuleCache.cpp lldb/source/Target/Platform.cpp 
lldb/source/Target/Process.cpp lldb/source/Target/StackFrame.cpp 
lldb/source/Target/Target.cpp lldb/source/Target/Thread.cpp 
lldb/source/Utility/Scalar.cpp lldb/source/Utility/Status.cpp 
lldb/source/Utility/StructuredData.cpp 
lldb/unittests/Target/RemoteAwarePlatformTest.cpp 
lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h 
lldb/unittests/Utility/StatusTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 54b26ad5a5..a165bfe57f 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -94,9 +94,7 @@ Status Status::FromErrorStringWithFormat(const char *format, 
...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) {
-  return Status(std::move(error));
-}
+Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
 
 llvm::Error Sta

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

I'm not if this is your intention, but the way LLVM has configured their 
github, your two nicely separated commits will be squashed :( 

https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

Ah I've just noticed the other PR

https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)

[Adrian Prantl via lldb-commits]

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/107163

>From 1c258c6efb04688ccb2e60a9c472c5c40e93c9ac Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Tue, 3 Sep 2024 15:29:25 -0700
Subject: [PATCH] [lldb] Make conversions from llvm::Error explicit with
 Status::FromError() [NFC]

---
 lldb/include/lldb/Utility/Status.h|   7 +-
 lldb/source/API/SBDebugger.cpp|   2 +-
 lldb/source/API/SBTarget.cpp  |   2 +-
 .../Commands/CommandObjectBreakpoint.cpp  | 103 ++
 .../Commands/CommandObjectMemoryTag.cpp   |  10 +-
 lldb/source/Commands/CommandObjectStats.cpp   |   6 +-
 lldb/source/Commands/CommandObjectTrace.cpp   |   2 +-
 lldb/source/Core/PluginManager.cpp|   2 +-
 lldb/source/Core/ThreadedCommunication.cpp|   2 +-
 lldb/source/Core/ValueObjectVTable.cpp|   2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |   2 +-
 lldb/source/DataFormatters/VectorType.cpp |   2 +-
 lldb/source/Host/common/FileCache.cpp |   2 +-
 .../Host/common/NativeProcessProtocol.cpp |   2 +-
 lldb/source/Host/common/TCPSocket.cpp |   6 +-
 lldb/source/Host/macosx/objcxx/Host.mm|   2 +-
 .../posix/ConnectionFileDescriptorPosix.cpp   |   6 +-
 lldb/source/Interpreter/CommandObject.cpp |   2 +-
 lldb/source/Interpreter/OptionValueRegex.cpp  |   2 +-
 .../Language/CPlusPlus/BlockPointer.cpp   |   2 +-
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp |   2 +-
 .../Minidump/ObjectFileMinidump.cpp   |   2 +-
 ...latformiOSSimulatorCoreSimulatorSupport.mm |   2 +-
 .../NativeRegisterContextDBReg_arm64.cpp  |  10 +-
 .../Process/elf-core/ProcessElfCore.cpp   |   2 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |   2 +-
 .../GDBRemoteCommunicationClient.cpp  |   2 +-
 .../GDBRemoteCommunicationServer.cpp  |   2 +-
 .../GDBRemoteCommunicationServerLLGS.cpp  |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  |   2 +-
 .../Process/minidump/ProcessMinidump.cpp  |   2 +-
 .../Interfaces/ScriptedPythonInterface.h  |   2 +-
 .../Python/PythonDataObjects.cpp  |  22 ++--
 .../Python/ScriptInterpreterPython.cpp|  10 +-
 .../DarwinLog/StructuredDataDarwinLog.cpp |   2 +-
 lldb/source/Target/ModuleCache.cpp|   2 +-
 lldb/source/Target/Platform.cpp   |   2 +-
 lldb/source/Target/Process.cpp|   4 +-
 lldb/source/Target/StackFrame.cpp |   2 +-
 lldb/source/Target/Thread.cpp |   3 +-
 lldb/source/Utility/Scalar.cpp|   2 +-
 lldb/source/Utility/Status.cpp|   8 +-
 lldb/source/Utility/StructuredData.cpp|   2 +-
 .../Host/NativeProcessTestUtils.h |   4 +-
 lldb/unittests/Utility/StatusTest.cpp |  14 ++-
 45 files changed, 149 insertions(+), 132 deletions(-)

diff --git a/lldb/include/lldb/Utility/Status.h 
b/lldb/include/lldb/Utility/Status.h
index b304291ffae00e..3813a3c1604700 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -91,9 +91,9 @@ class Status {
 
   ~Status();
 
-  // llvm::Error support
-  explicit Status(llvm::Error error) { *this = std::move(error); }
-  const Status &operator=(llvm::Error error);
+  /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+  static Status FromError(llvm::Error error);
+  /// FIXME: Replace this with a takeError method.
   llvm::Error ToError() const;
 
   /// Get the error string associated with the current error.
@@ -145,6 +145,7 @@ class Status {
   bool Success() const;
 
 protected:
+  Status(llvm::Error error);
   /// Status code as an integer value.
   ValueType m_code = 0;
   /// The type of the above error code.
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 72501570320d57..c226acc15018ef 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -220,7 +220,7 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
   SBError error;
   if (auto e = g_debugger_lifetime->Initialize(
   std::make_unique(), LoadPlugin)) {
-error.SetError(Status(std::move(e)));
+error.SetError(Status::FromError(std::move(e)));
   }
   return error;
 }
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index e927cb854cd88c..41eb77e5506bc5 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1658,7 +1658,7 @@ SBError SBTarget::SetLabel(const char *label) {
   if (!target_sp)
 return Status::FromErrorString("Couldn't get internal target object.");
 
-  return Status(target_sp->SetLabel(label));
+  return Status::FromError(target_sp->SetLabel(label));
 }
 
 uint32_t SBTarget::GetDataByteSize() {
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index ede3dd2f2a864c..494d6c50e94ac3 100644
--- a/lldb/source/Com

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Adrian Prantl via lldb-commits]

adrian-prantl wrote:

> Ah I've just noticed the other PR

Do we actually have abetter solution for stacked commits than what I'm doing 
here?


https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Change ValueObject::AddressOf() to return Expected (NFC) (PR #106831)

[via lldb-commits]

jimingham wrote:

Are you planning to do this more generally?  There are lots of API's that can 
ValueObjects that might or might not succeed, and it's not clear to me how 
AddressOf is special in this regard.

If we're planning on changing how we hand out ValueObjects more generally in 
the API then we should probably discuss that (maybe an RFC) and make a plan 
rather than just dive into it piecemeal.

https://github.com/llvm/llvm-project/pull/106831
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

[via lldb-commits]

jimingham wrote:

Ah, I thought you were echoing something and pointing into that, I missed that 
you were pointing into the original command line.  Does this also work if I 
have an alias that scrambles the command arguments compared to how they are in 
the command that eventually got invoked.

If we're going to do errors this way, we should change the argument/option 
value setting to recognize which slot a bad option argument/option value came 
from (or unrecognized option flag for that matter) and point at those as well...

Jim

 
> On Aug 30, 2024, at 6:39 PM, Adrian Prantl ***@***.***> wrote:
> 
> 
> @adrian-prantl commented on this pull request.
> 
> In lldb/source/Interpreter/CommandInterpreter.cpp 
> :
> 
> > @@ -2076,7 +2077,11 @@ bool CommandInterpreter::HandleCommand(const char 
> > *command_line,
>  }
>  
>  ElapsedTime elapsed(execute_time);
> -cmd_obj->Execute(remainder.c_str(), result);
> +size_t nchar = real_original_command_string.find(remainder);
> +std::optional pos_in_cmd;
> +if (nchar != std::string::npos)
> +  pos_in_cmd = nchar + GetDebugger().GetPrompt().size();
> +cmd_obj->Execute(remainder.c_str(), pos_in_cmd, result);
> @jimingham  @medismailben 
>  I hopefully have resolved both of your 
> concerns by storing the original command string as a member in CommandObject. 
> This way the expr -i 0 -u 0 -- not a valid expression works.
> 
> (lldb) expr -i 0 -u 0 -- not a valid expression
>  ^
>  ╰─ error: use of undeclared identifier 'not'
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



https://github.com/llvm/llvm-project/pull/106470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

> > Ah I've just noticed the other PR
> 
> Do we actually have abetter solution for stacked commits than what I'm doing 
> here?

short answer is "maybe": 
https://llvm.org/docs/GitHub.html#using-graphite-for-stacked-pull-requests

https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

[via lldb-commits]

https://github.com/Da-Viper updated 
https://github.com/llvm/llvm-project/pull/106919

>From d2bddca1753b4c960895f51d7eb80b6efa7dc986 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Sun, 1 Sep 2024 17:26:11 +0100
Subject: [PATCH 1/5] [lldb-dap] Make environment option an object

---
 .../tools/lldb-dap/launch/TestDAP_launch.py   |  4 +-
 lldb/tools/lldb-dap/JSONUtils.cpp | 40 ---
 lldb/tools/lldb-dap/JSONUtils.h   | 22 ++
 lldb/tools/lldb-dap/README.md |  5 ++-
 lldb/tools/lldb-dap/lldb-dap.cpp  |  8 +++-
 lldb/tools/lldb-dap/package.json  | 11 +++--
 6 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py 
b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index a16f2da3c4df71..6b9993a2548b8d 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -229,7 +229,7 @@ def test_environment(self):
 Tests launch of a simple program with environment variables
 """
 program = self.getBuildArtifact("a.out")
-env = ["NO_VALUE", "WITH_VALUE=BAR", "EMPTY_VALUE=", "SPACE=Hello 
World"]
+env = {"NO_VALUE": "", "WITH_VALUE":"BAR", "EMPTY_VALUE": "", "SPACE": 
"Hello World"}
 self.build_and_launch(program, env=env)
 self.continue_to_exit()
 
@@ -242,7 +242,7 @@ def test_environment(self):
 lines.pop(0)
 # Make sure each environment variable in "env" is actually set in the
 # program environment that was printed to STDOUT
-for var in env:
+for var in env.keys():
 found = False
 for program_var in lines:
 if var in program_var:
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 7338e7cf41eb03..29b3ad490af0b6 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -136,6 +136,31 @@ std::vector GetStrings(const 
llvm::json::Object *obj,
   return strs;
 }
 
+std::unordered_map
+GetStringMap(const llvm::json::Object &obj, llvm::StringRef key) {
+  std::unordered_map strs;
+  const auto *const json_object = obj.getObject(key);
+  if (!json_object)
+return strs;
+
+  for (const auto &[key, value] : *json_object) {
+switch (value.kind()) {
+case llvm::json::Value::String:
+  strs.emplace(key.str(), value.getAsString()->str());
+  break;
+case llvm::json::Value::Number:
+case llvm::json::Value::Boolean:
+  strs.emplace(key.str(), llvm::to_string(value));
+  break;
+case llvm::json::Value::Null:
+case llvm::json::Value::Object:
+case llvm::json::Value::Array:
+  break;
+}
+  }
+  return strs;
+}
+
 static bool IsClassStructOrUnionType(lldb::SBType t) {
   return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
   lldb::eTypeClassArray)) != 0;
@@ -1370,13 +1395,16 @@ CreateRunInTerminalReverseRequest(const 
llvm::json::Object &launch_request,
   if (!cwd.empty())
 run_in_terminal_args.try_emplace("cwd", cwd);
 
-  // We need to convert the input list of environments variables into a
-  // dictionary
-  std::vector envs = GetStrings(launch_request_arguments, "env");
+  std::unordered_map envMap =
+  GetStringMap(*launch_request_arguments, "env");
   llvm::json::Object environment;
-  for (const std::string &env : envs) {
-size_t index = env.find('=');
-environment.try_emplace(env.substr(0, index), env.substr(index + 1));
+  for (const auto &[key, value] : envMap) {
+if (key.empty())
+  g_dap.SendOutput(OutputType::Stderr,
+   "empty environment variable for value: \"" + value +
+   '\"');
+else
+  environment.try_emplace(key, value);
   }
   run_in_terminal_args.try_emplace("env",
llvm::json::Value(std::move(environment)));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index b6356630b72682..60d5db06560657 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/JSON.h"
 #include 
 #include 
+#include 
 
 namespace lldb_dap {
 
@@ -152,6 +153,27 @@ bool ObjectContainsKey(const llvm::json::Object &obj, 
llvm::StringRef key);
 std::vector GetStrings(const llvm::json::Object *obj,
 llvm::StringRef key);
 
+/// Extract an object of key value strings for the specified key from an 
object.
+///
+/// String values in the object will be extracted without any quotes
+/// around them. Numbers and Booleans will be converted into
+/// strings. Any NULL, array or objects values in the array will be
+/// ignored.
+///
+/// \param[in] obj
+/// A JSON object that we will attempt to extract the array from
+///
+/// \param[in] key
+/// The key to use when extr

[Lldb-commits] [lldb] [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (PR #106767)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/106767

>From ceb20d62d9cef3090e34e8b9fc0bc620a7d9da3d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Fri, 30 Aug 2024 10:33:08 -0700
Subject: [PATCH 1/4] Extend the minidump x86_64 registers to include fs_base
 and gs_base

---
 .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp| 5 -
 .../Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp   | 2 +-
 .../Process/minidump/RegisterContextMinidump_x86_64.cpp| 7 +++
 .../Process/minidump/RegisterContextMinidump_x86_64.h  | 7 ++-
 .../TestProcessSaveCoreMinidump.py | 7 +++
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 13355afb58dbd1..5c9ba223ad143e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -473,7 +473,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
   lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
   lldb_private::minidump::MinidumpContext_x86_64_Flags::Control |
   lldb_private::minidump::MinidumpContext_x86_64_Flags::Segments |
-  lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer);
+  lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer |
+  lldb_private::minidump::MinidumpContext_x86_64_Flags::LLDBSpecific);
   thread_context.rax = read_register_u64(reg_ctx, "rax");
   thread_context.rbx = read_register_u64(reg_ctx, "rbx");
   thread_context.rcx = read_register_u64(reg_ctx, "rcx");
@@ -499,6 +500,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
   thread_context.gs = read_register_u64(reg_ctx, "gs");
   thread_context.ss = read_register_u64(reg_ctx, "ss");
   thread_context.ds = read_register_u64(reg_ctx, "ds");
+  thread_context.fs_base = read_register_u64(reg_ctx, "fs_base");
+  thread_context.gs_base = read_register_u64(reg_ctx, "gs_base");
   return thread_context;
 }
 
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
index 845312f4c1eddc..f60757a52c6310 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
@@ -21,7 +21,7 @@ 
RegisterContextCorePOSIX_x86_64::RegisterContextCorePOSIX_x86_64(
 
   size = GetGPRSize();
   m_gpregset.reset(new uint8_t[size]);
-  len =
+  len = 
   gpregset.ExtractBytes(0, size, lldb::eByteOrderLittle, m_gpregset.get());
   if (len != size)
 m_gpregset.reset();
diff --git 
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp 
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index 917140cab29767..4db049ff7e64e7 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
@@ -67,6 +67,7 @@ lldb::DataBufferSP 
lldb_private::minidump::ConvertMinidumpContext_x86_64(
   auto ControlFlag = MinidumpContext_x86_64_Flags::Control;
   auto IntegerFlag = MinidumpContext_x86_64_Flags::Integer;
   auto SegmentsFlag = MinidumpContext_x86_64_Flags::Segments;
+  auto LLDBSpecificFlag = MinidumpContext_x86_64_Flags::LLDBSpecific;
 
   if ((context_flags & x86_64_Flag) != x86_64_Flag)
 return nullptr;
@@ -104,6 +105,12 @@ lldb::DataBufferSP 
lldb_private::minidump::ConvertMinidumpContext_x86_64(
 writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]);
   }
 
+  if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) {
+writeRegister(&context->fs_base, result_base, 
reg_info[x86_64_with_base::lldb_fs_base]);
+writeRegister(&context->gs_base, result_base,
+  reg_info[x86_64_with_base::lldb_gs_base]);
+  }
+
   // TODO parse the floating point registers
 
   return result_context_buf;
diff --git 
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h 
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
index d920ea9d823f4f..f214e04a315a8e 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -153,6 +153,10 @@ struct MinidumpContext_x86_64 {
   llvm::support::ulittle64_t last_branch_from_rip;
   llvm::support::ulittle64_t last_exception_to_rip;
   llvm::support::ulittle64_t last_exception_from_rip;
+
+  // These registers are LLDB specific.
+  llvm::support::ulittle64_t fs_base;
+  llvm::support::ulittle64_t gs_base;
 };
 
 // For context_flags. These values indicate the type of
@@ -168,9 +172,10 @@ enum class MinidumpContext_x86_64_Flags : uint32_t {
   FloatingPoint = x86_64_Flag | 0x0008,
   Debug

[Lldb-commits] [lldb] [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (PR #106767)

[Greg Clayton via lldb-commits]

https://github.com/clayborg approved this pull request.


https://github.com/llvm/llvm-project/pull/106767
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Greg Clayton via lldb-commits]

@@ -174,6 +176,85 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name, std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; };
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+
+  /// Basic RAII class to increment the summary count when the call is 
complete.
+  class SummaryInvocation{
+  public:
+SummaryInvocation(std::shared_ptr summary_stats)
+: m_stats(summary_stats),
+  m_elapsed_time(summary_stats->GetDurationReference()) {}
+~SummaryInvocation() { m_stats->OnInvoked(); }
+
+/// Delete the copy constructor and assignment operator to prevent
+/// accidental double counting.
+/// @{
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+/// @}
+
+  private:
+std::shared_ptr m_stats;
+ElapsedTime m_elapsed_time;
+  };
+
+private:
+  void OnInvoked() noexcept {
+m_count.fetch_add(1, std::memory_order_relaxed);
+  }
+  lldb_private::StatsDuration m_total_time;
+  std::string m_impl_type;
+  std::string m_name;
+  std::atomic m_count;
+};
+
+typedef std::shared_ptr SummaryStatisticsSP;

clayborg wrote:

Move this above the `SummaryStatistics` class by doing:
```
class SummaryStatistics;
typedef std::shared_ptr SummaryStatisticsSP;
```
then use SummaryStatisticsSP in the class instead of manually typing 
`std::shared_ptr`

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Greg Clayton via lldb-commits]

@@ -174,6 +176,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name, std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name), m_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; };
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  /// Basic RAII class to increment the summary count when the call is 
complete.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(std::shared_ptr summary_stats)
+: m_stats(summary_stats),
+  m_elapsed_time(summary_stats->GetDurationReference()) {}
+~SummaryInvocation() { m_stats->OnInvoked(); }
+
+/// Delete the copy constructor and assignment operator to prevent
+/// accidental double counting.
+/// @{
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+/// @}
+
+  private:
+std::shared_ptr m_stats;
+ElapsedTime m_elapsed_time;
+  };
+
+private:
+  void OnInvoked() noexcept { m_count.fetch_add(1, std::memory_order_relaxed); 
}
+  lldb_private::StatsDuration m_total_time;
+  const std::string m_impl_type;
+  const std::string m_name;
+  std::atomic m_count;
+};
+
+typedef std::shared_ptr SummaryStatisticsSP;
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  SummaryStatisticsSP
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{

clayborg wrote:

So with your suggestion the RAII object would need to copy the string so that 
it could call `Update` at the and to find the stats and then update the time. 
This also means the RAII object needs to hold a reference to the 
`SummaryStatisticsCache` global object, which could get destructed as nothing 
is keeping it alive. So we need to have the RAII object copy the string and get 
a weak pointer to the SummaryStatisticsCache so it can safely update the 
results in the end. 

This current solution hands out a `SummaryStatisticsSP` to the RAII object 
which will keep it alive so it can get updated, and if the 
`SummaryStatisticsCache` goes away, it is ok because we have a strong 
reference. Both solutions only do 1 lookup. `std::shared_ptr` objects are 
thread safe.

The StringMap requires a value that can be copied when the map grows and 
re-allocation happen, so if we don't use a shared pointer, then we need 
`SummaryStatistics` to have a move operator.

So I don't mind this solution, but would like to hear other opinions

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Greg Clayton via lldb-commits]

@@ -201,3 +216,12 @@ std::string ScriptSummaryFormat::GetDescription() {
   }
   return std::string(sstr.GetString());
 }
+
+std::string ScriptSummaryFormat::GetName() { return m_script_formatter_name; }
+
+std::string ScriptSummaryFormat::GetSummaryKindName() {
+  if (!m_python_script.empty())

clayborg wrote:

Not sure what the difference between a "python" and "script" is? Lua is a 
supported scripting language, but it isn't used for any data formatting or 
summary providers.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits