[Lldb-commits] [lldb] [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value (PR #104617)

2024-08-19 Thread David Spickett via lldb-commits


@@ -120,7 +121,10 @@ TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
   ASSERT_EQ(prpsinfo_opt->pr_state, 0);
   ASSERT_EQ(prpsinfo_opt->pr_sname, 'R');
   ASSERT_EQ(prpsinfo_opt->pr_zomb, 0);
-  ASSERT_EQ(prpsinfo_opt->pr_nice, 0);
+  int priority = getpriority(PRIO_PROCESS, getpid());
+  if (priority == -1)

DavidSpickett wrote:

Are you assuming that the priority of the debugged process is the same as the 
test process, or is the debugged process in fact the same as the test process?

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread 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 aad27bf534b59645f47a92f072af798687b1dd0d 
e72ceaada170354aa322b4c6a1787152ac72c65b --extensions cpp -- 
lldb/source/API/SBBreakpoint.cpp lldb/source/API/SBBreakpointLocation.cpp 
lldb/source/API/SBBreakpointName.cpp lldb/source/Expression/DWARFExpression.cpp 
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
 lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
 lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp 
lldb/source/Plugins/Process/Utility/ThreadMemory.cpp 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp 
lldb/source/Symbol/DWARFCallFrameInfo.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index c1feec990f..22d899f799 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -131,8 +131,9 @@ static llvm::Error 
ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
 /// Return the length in bytes of the set of operands for \p op. No guarantees
 /// are made on the state of \p data after this call.
 static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
-  const lldb::offset_t data_offset,
-  const uint8_t op, const DWARFUnit *dwarf_cu) 
{
+const lldb::offset_t data_offset,
+const uint8_t op,
+const DWARFUnit *dwarf_cu) {
   lldb::offset_t offset = data_offset;
   switch (op) {
   case DW_OP_addr:
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
index 876e74056f..e67e60b4a3 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -261,7 +261,8 @@ 
InstrumentationRuntimeMainThreadChecker::GetBacktracesFromExtendedStopInfo(
 
   StructuredData::ObjectSP thread_id_obj =
   info->GetObjectForDotSeparatedPath("tid");
-  lldb::tid_t tid = thread_id_obj ? thread_id_obj->GetUnsignedIntegerValue() : 
0;
+  lldb::tid_t tid =
+  thread_id_obj ? thread_id_obj->GetUnsignedIntegerValue() : 0;
 
   // We gather symbolication addresses above, so no need for HistoryThread to
   // try to infer the call addresses.
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
index 7a827a3ea7..5516de19d8 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -770,15 +770,15 @@ std::string 
InstrumentationRuntimeTSan::GetLocationDescription(
 Sprintf("Location is a %ld-byte heap object at 0x%llx", size, 
addr);
   }
 } else if (type == "stack") {
-lldb::tid_t tid = loc->GetAsDictionary()
-  ->GetValueForKey("thread_id")
-  ->GetUnsignedIntegerValue();
+  lldb::tid_t tid = loc->GetAsDictionary()
+->GetValueForKey("thread_id")
+->GetUnsignedIntegerValue();
 
   result = Sprintf("Location is stack of thread %d", tid);
 } else if (type == "tls") {
-lldb::tid_t tid = loc->GetAsDictionary()
-  ->GetValueForKey("thread_id")
-  ->GetUnsignedIntegerValue();
+  lldb::tid_t tid = loc->GetAsDictionary()
+->GetValueForKey("thread_id")
+->GetUnsignedIntegerValue();
 
   result = Sprintf("Location is TLS of thread %d", tid);
 } else if (type == "fd") {
@@ -979,7 +979,7 @@ static std::string GenerateThreadName(const std::string 
&p

[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

DavidSpickett wrote:

These seem fine but can you provide some details why this might have happened 
in your build and not in others?

Like:
* Are these tid_t and offset_t defined differently on AIX?
* Are you building with the system compiler or clang?

Include issues are not always easily explained, so if you can't, that's ok too. 
I would like to record the reasoning if we can though.

Probably best to be explicit about very common type names like this anyway.

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Normally I would advise you to ignore the formatter for changes like this, but 
this is small enough and the intent will be clear even with the formatting 
changes. So please follow the guidance in the formatting comment to fix it.

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

DavidSpickett wrote:

And I edited the commit title slightly. The usual format is 
`[sub-project][area] `, so if you could keep to that form. Not a hard 
rule but it is useful for skimming logs later.

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

DavidSpickett wrote:

And for the formatting, I usually use the clang-format in my llvm build dir. 
You don't have to go hunt down another specific version.

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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread Dhruv Srivastava via lldb-commits

DhruvSrivastavaX wrote:

Ok Great, @DavidSpickett 
I was hoping for some feedback to get more clarity about llvm PRs.
1. Yes, the definition of tid_t and offset_t can differ in certain scenarios in 
AIX when compared to lldb, so trying to avoid such conflicts. 
2. We have used ibm-clang which is an AIX compatible version of clang, Here is 
the link:
https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.1?topic=cc-highly-configurable-compiler
3. clang-format: Ok, thanks for that. I will use clang-format to avoid such 
formatting errors in the future.
4. [sub-project][area]  : Yes, thanks I had some doubt about that. 
Thanks for clarifying. Also, I am numbering these porting commits to keep 
additional tracking.


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


[Lldb-commits] [lldb] [lldb-dap] Implement value locations for function pointers (PR #104589)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits

vogelsgesang wrote:

Here the screen recording of a fully functional, non-remote session:

https://github.com/user-attachments/assets/64b36078-a57b-440a-85f8-0e73f88c5a56



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


[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 commented:

Haven't thoroughly reviewed the latest callback-based approach but wonder if it 
is just more straightforward to keep the original plan of adding a`search_hint` 
to the `FindFunctions` APIs, and add a flag to `ModuleFunctionSearchOptions` 
that indicates whether we're looking for a single function or multiple. And if 
we're looking for a single function, short-circuit if we find one.

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


[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)

2024-08-19 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] Remove redundant symbol lookups in IRExecutionUnit::FindInSymbols (PR #102835)

2024-08-19 Thread Dmitrii Galimzianov via lldb-commits

DmT021 wrote:

The tricky part is in the post-processing. IRExecutionUnit::FindInSymbols wants 
an external symbol and falls back to an internal one after all the modules are 
scanned. SymbolContext::FindBestGlobalDataSymbol prefers an internal symbol if 
it is found in the hinted module.
Also resolution of a reexported symbol is part of the post-processing

https://github.com/llvm/llvm-project/pull/102835
___
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)

2024-08-19 Thread Dmitry Vasilyev via lldb-commits

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

>From 8491aa073ae240eb5e31df12ac278c70fc9515e7 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 16 Aug 2024 17:03:37 +0400
Subject: [PATCH 1/4] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver 
--fd` on all platforms.
Added acceptor_gdb and gdb_thread to lldb-platform.cpp
SharedSocket has been moved to ConnectionFileDescriptorPosix.

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.h |  26 ++
 .../posix/ConnectionFileDescriptorPosix.cpp   | 110 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |  41 ++-
 .../gdb-remote/GDBRemoteCommunication.h   |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 287 +---
 .../GDBRemoteCommunicationServerPlatform.h|  83 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   2 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  23 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 324 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 ---
 .../tools/lldb-server/tests/TestClient.h  |   4 +
 15 files changed, 450 insertions(+), 608 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 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/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e913..08f486b92e0f07 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -26,6 +26,32 @@ class Status;
 class Socket;
 class SocketAddress;
 
+#ifdef _WIN32
+typedef lldb::pipe_t shared_fd_t;
+#else
+typedef NativeSocket shared_fd_t;
+#endif
+
+class Sha

[Lldb-commits] [lldb] [lldb][NFC] Moved the SharedSocket class to Socket.* (PR #104787)

2024-08-19 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman created 
https://github.com/llvm/llvm-project/pull/104787

This is prerequisite for #104238.

>From 8c92e7b3df1c76c3c092dcd9c6fc90889192dc8b Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Mon, 19 Aug 2024 16:37:10 +0400
Subject: [PATCH] [lldb][NFC] Moved the SharedSocket class to Socket.*

This is prerequisite for #104238.
---
 lldb/include/lldb/Host/Socket.h  |  24 +
 lldb/source/Host/common/Socket.cpp   |  76 ++
 lldb/tools/lldb-server/lldb-platform.cpp | 126 +++
 3 files changed, 114 insertions(+), 112 deletions(-)

diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 573c881f727d8f..304a91bdf6741b 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -19,6 +19,7 @@
 #include "lldb/Utility/Status.h"
 
 #ifdef _WIN32
+#include "lldb/Host/Pipe.h"
 #include "lldb/Host/windows/windows.h"
 #include 
 #include 
@@ -32,12 +33,35 @@ namespace lldb_private {
 
 #if defined(_WIN32)
 typedef SOCKET NativeSocket;
+typedef lldb::pipe_t shared_fd_t;
 #else
 typedef int NativeSocket;
+typedef NativeSocket shared_fd_t;
 #endif
+class Socket;
 class TCPSocket;
 class UDPSocket;
 
+class SharedSocket {
+public:
+  static const shared_fd_t kInvalidFD;
+
+  SharedSocket(const Socket *socket, Status &error);
+
+  shared_fd_t GetSendableFD() { return m_fd; }
+
+  Status CompleteSending(lldb::pid_t child_pid);
+
+  static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket);
+
+private:
+#ifdef _WIN32
+  Pipe m_socket_pipe;
+  NativeSocket m_socket;
+#endif
+  shared_fd_t m_fd;
+};
+
 class Socket : public IOObject {
 public:
   enum SocketProtocol {
diff --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 7364a12280cfdd..aabd562b0557c6 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -56,10 +56,12 @@ using namespace lldb_private;
 typedef const char *set_socket_option_arg_type;
 typedef char *get_socket_option_arg_type;
 const NativeSocket Socket::kInvalidSocketValue = INVALID_SOCKET;
+const shared_fd_t SharedSocket::kInvalidFD = LLDB_INVALID_PIPE;
 #else  // #if defined(_WIN32)
 typedef const void *set_socket_option_arg_type;
 typedef void *get_socket_option_arg_type;
 const NativeSocket Socket::kInvalidSocketValue = -1;
+const shared_fd_t SharedSocket::kInvalidFD = Socket::kInvalidSocketValue;
 #endif // #if defined(_WIN32)
 
 static bool IsInterrupted() {
@@ -70,6 +72,80 @@ static bool IsInterrupted() {
 #endif
 }
 
+SharedSocket::SharedSocket(const Socket *socket, Status &error) {
+#ifdef _WIN32
+  m_socket = socket->GetNativeSocket();
+  m_fd = kInvalidFD;
+
+  // Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
+  error = m_socket_pipe.CreateNew(true);
+  if (error.Fail())
+return;
+
+  m_fd = m_socket_pipe.GetReadPipe();
+#else
+  m_fd = socket->GetNativeSocket();
+  error = Status();
+#endif
+}
+
+Status SharedSocket::CompleteSending(lldb::pid_t child_pid) {
+#ifdef _WIN32
+  // Transfer WSAPROTOCOL_INFO to the child process.
+  m_socket_pipe.CloseReadFileDescriptor();
+
+  WSAPROTOCOL_INFO protocol_info;
+  if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) ==
+  SOCKET_ERROR) {
+int last_error = ::WSAGetLastError();
+return Status("WSADuplicateSocket() failed, error: %d", last_error);
+  }
+
+  size_t num_bytes;
+  Status error =
+  m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(10), num_bytes);
+  if (error.Fail())
+return error;
+  if (num_bytes != sizeof(protocol_info))
+return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes",
+  num_bytes);
+#endif
+  return Status();
+}
+
+Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) {
+#ifdef _WIN32
+  socket = Socket::kInvalidSocketValue;
+  // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket.
+  WSAPROTOCOL_INFO protocol_info;
+  {
+Pipe socket_pipe(fd, LLDB_INVALID_PIPE);
+size_t num_bytes;
+Status error =
+socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info),
+std::chrono::seconds(10), num_bytes);
+if (error.Fail())
+  return error;
+if (num_bytes != sizeof(protocol_info)) {
+  return Status(
+  "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes",
+  num_bytes);
+}
+  }
+  socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO,
+   FROM_PROTOCOL_INFO, &protocol_info, 0, 0);
+  if (socket == INVALID_SOCKET) {
+return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d",
+  ::WSAGetLastError());
+  }
+  return Status();
+#else
+  socket = fd;
+  return Status();
+#endif
+}
+
 struct SocketScheme {
   const char *m_scheme;
   const Socket::SocketProtocol m_protocol;
dif

[Lldb-commits] [lldb] [lldb][NFC] Moved the SharedSocket class to Socket.* (PR #104787)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)


Changes

This is prerequisite for #104238.

---
Full diff: https://github.com/llvm/llvm-project/pull/104787.diff


3 Files Affected:

- (modified) lldb/include/lldb/Host/Socket.h (+24) 
- (modified) lldb/source/Host/common/Socket.cpp (+76) 
- (modified) lldb/tools/lldb-server/lldb-platform.cpp (+14-112) 


``diff
diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 573c881f727d8f..304a91bdf6741b 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -19,6 +19,7 @@
 #include "lldb/Utility/Status.h"
 
 #ifdef _WIN32
+#include "lldb/Host/Pipe.h"
 #include "lldb/Host/windows/windows.h"
 #include 
 #include 
@@ -32,12 +33,35 @@ namespace lldb_private {
 
 #if defined(_WIN32)
 typedef SOCKET NativeSocket;
+typedef lldb::pipe_t shared_fd_t;
 #else
 typedef int NativeSocket;
+typedef NativeSocket shared_fd_t;
 #endif
+class Socket;
 class TCPSocket;
 class UDPSocket;
 
+class SharedSocket {
+public:
+  static const shared_fd_t kInvalidFD;
+
+  SharedSocket(const Socket *socket, Status &error);
+
+  shared_fd_t GetSendableFD() { return m_fd; }
+
+  Status CompleteSending(lldb::pid_t child_pid);
+
+  static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket);
+
+private:
+#ifdef _WIN32
+  Pipe m_socket_pipe;
+  NativeSocket m_socket;
+#endif
+  shared_fd_t m_fd;
+};
+
 class Socket : public IOObject {
 public:
   enum SocketProtocol {
diff --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 7364a12280cfdd..aabd562b0557c6 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -56,10 +56,12 @@ using namespace lldb_private;
 typedef const char *set_socket_option_arg_type;
 typedef char *get_socket_option_arg_type;
 const NativeSocket Socket::kInvalidSocketValue = INVALID_SOCKET;
+const shared_fd_t SharedSocket::kInvalidFD = LLDB_INVALID_PIPE;
 #else  // #if defined(_WIN32)
 typedef const void *set_socket_option_arg_type;
 typedef void *get_socket_option_arg_type;
 const NativeSocket Socket::kInvalidSocketValue = -1;
+const shared_fd_t SharedSocket::kInvalidFD = Socket::kInvalidSocketValue;
 #endif // #if defined(_WIN32)
 
 static bool IsInterrupted() {
@@ -70,6 +72,80 @@ static bool IsInterrupted() {
 #endif
 }
 
+SharedSocket::SharedSocket(const Socket *socket, Status &error) {
+#ifdef _WIN32
+  m_socket = socket->GetNativeSocket();
+  m_fd = kInvalidFD;
+
+  // Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
+  error = m_socket_pipe.CreateNew(true);
+  if (error.Fail())
+return;
+
+  m_fd = m_socket_pipe.GetReadPipe();
+#else
+  m_fd = socket->GetNativeSocket();
+  error = Status();
+#endif
+}
+
+Status SharedSocket::CompleteSending(lldb::pid_t child_pid) {
+#ifdef _WIN32
+  // Transfer WSAPROTOCOL_INFO to the child process.
+  m_socket_pipe.CloseReadFileDescriptor();
+
+  WSAPROTOCOL_INFO protocol_info;
+  if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) ==
+  SOCKET_ERROR) {
+int last_error = ::WSAGetLastError();
+return Status("WSADuplicateSocket() failed, error: %d", last_error);
+  }
+
+  size_t num_bytes;
+  Status error =
+  m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(10), num_bytes);
+  if (error.Fail())
+return error;
+  if (num_bytes != sizeof(protocol_info))
+return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes",
+  num_bytes);
+#endif
+  return Status();
+}
+
+Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) {
+#ifdef _WIN32
+  socket = Socket::kInvalidSocketValue;
+  // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket.
+  WSAPROTOCOL_INFO protocol_info;
+  {
+Pipe socket_pipe(fd, LLDB_INVALID_PIPE);
+size_t num_bytes;
+Status error =
+socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info),
+std::chrono::seconds(10), num_bytes);
+if (error.Fail())
+  return error;
+if (num_bytes != sizeof(protocol_info)) {
+  return Status(
+  "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes",
+  num_bytes);
+}
+  }
+  socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO,
+   FROM_PROTOCOL_INFO, &protocol_info, 0, 0);
+  if (socket == INVALID_SOCKET) {
+return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d",
+  ::WSAGetLastError());
+  }
+  return Status();
+#else
+  socket = fd;
+  return Status();
+#endif
+}
+
 struct SocketScheme {
   const char *m_scheme;
   const Socket::SocketProtocol m_protocol;
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp 
b/lldb/tools/lldb-server/lldb-platform.cpp
index 82a3a0d6b4e51c..75f51132aa9cc6 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools

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

2024-08-19 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman edited 
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] PopulatePrpsInfoTest can fail due to hardcoded priority value (PR #104617)

2024-08-19 Thread Fred Grim via lldb-commits


@@ -120,7 +121,10 @@ TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
   ASSERT_EQ(prpsinfo_opt->pr_state, 0);
   ASSERT_EQ(prpsinfo_opt->pr_sname, 'R');
   ASSERT_EQ(prpsinfo_opt->pr_zomb, 0);
-  ASSERT_EQ(prpsinfo_opt->pr_nice, 0);
+  int priority = getpriority(PRIO_PROCESS, getpid());
+  if (priority == -1)

feg208 wrote:

In this test the process for which the prpsinfo object is being populated is 
the test process. The issue I saw in the build kites was that, rather than the 
test process being prioritized at the default for linux, it got 5. So the fix 
here is just to make sure that, whatever the priority of the test process, what 
is verified that the populated value in the prpsinfo object matches the 
priority that we know the process runs as.

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits

https://github.com/walter-erquinigo requested changes to this pull request.


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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -1,4 +1,5 @@
 import * as vscode from "vscode";
+import * as fs from "node:fs/promises";

walter-erquinigo wrote:

don't use this

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);

walter-erquinigo wrote:

use instead `vscode.workspace.fs.stat`

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);

walter-erquinigo wrote:

`Debug adapter path ${path} is not a valid file`

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },
+  openSettingsAction,
+);
+if (openSettingsAction === callBackValue) {
+  vscode.commands.executeCommand(
+"workbench.action.openSettings",
+"lldb-dap.executable-path",
+  );
+}
   }
-  return packageJSONExecutable;
+  return new vscode.DebugAdapterExecutable(path, []);

walter-erquinigo wrote:

instead of making a try catch block here, why don't you just show the pop up 
right away without throwing an error?

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },

walter-erquinigo wrote:

don't make this modal. It would be too obtrusive

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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Paul Kirth via lldb-commits

ilovepi wrote:

I think the need to add _GNU_SOURCE depends on your libc, it it’s probably 
reasonable to add it based on the platform. 

I’d think you’d want to use an ifdef here to define _GNU_SOURCE if needed. I 
based on the target platform.  You could also probably do something similar in 
CMAKE but you may want to limit that in scope to only this test, since I’m not 
completely sure of the impact, though IIRC it’s safe. 

Regardless, I’d appreciate it if this was reverted until a fixed version ready, 
since is been breaking our CI since it landed. 

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


[Lldb-commits] [lldb] [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value (PR #104617)

2024-08-19 Thread David Spickett via lldb-commits


@@ -120,7 +121,10 @@ TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
   ASSERT_EQ(prpsinfo_opt->pr_state, 0);
   ASSERT_EQ(prpsinfo_opt->pr_sname, 'R');
   ASSERT_EQ(prpsinfo_opt->pr_zomb, 0);
-  ASSERT_EQ(prpsinfo_opt->pr_nice, 0);
+  int priority = getpriority(PRIO_PROCESS, getpid());
+  if (priority == -1)

DavidSpickett wrote:

Sure, makes sense.

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


[Lldb-commits] [lldb] [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value (PR #104617)

2024-08-19 Thread David Spickett via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb] Updated TCPSocket to listen multiple ports on the same single thread (PR #104797)

2024-08-19 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman created 
https://github.com/llvm/llvm-project/pull/104797

This is prerequisite for #104238.

>From c14f06278aff93f0c7433f6d13c3d9d944d7a1cf Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Mon, 19 Aug 2024 18:56:36 +0400
Subject: [PATCH] [lldb] Updated TCPSocket to listen multiple ports on the same
 single thread

This is prerequisite for #104238.
---
 lldb/include/lldb/Host/common/TCPSocket.h |   3 +
 lldb/source/Host/common/TCPSocket.cpp | 105 +++---
 lldb/unittests/Host/SocketTest.cpp|  13 +++
 3 files changed, 87 insertions(+), 34 deletions(-)

diff --git a/lldb/include/lldb/Host/common/TCPSocket.h 
b/lldb/include/lldb/Host/common/TCPSocket.h
index b782c9e6096c44..b45fdae00c6b43 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -24,6 +24,9 @@ class TCPSocket : public Socket {
   // returns port number or 0 if error
   uint16_t GetLocalPortNumber() const;
 
+  // returns port numbers of all listening sockets
+  std::set GetLocalPortNumbers() const;
+
   // returns ip address string or empty string if error
   std::string GetLocalIPAddress() const;
 
diff --git a/lldb/source/Host/common/TCPSocket.cpp 
b/lldb/source/Host/common/TCPSocket.cpp
index df4737216ed3ac..e659b20bb0045e 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/WindowsError.h"
@@ -94,6 +95,25 @@ uint16_t TCPSocket::GetLocalPortNumber() const {
   return 0;
 }
 
+// Return all the port numbers that is being used by the socket.
+std::set TCPSocket::GetLocalPortNumbers() const {
+  std::set ports;
+  if (m_socket != kInvalidSocketValue) {
+SocketAddress sock_addr;
+socklen_t sock_addr_len = sock_addr.GetMaxLength();
+if (::getsockname(m_socket, sock_addr, &sock_addr_len) == 0)
+  ports.insert(sock_addr.GetPort());
+  } else if (!m_listen_sockets.empty()) {
+SocketAddress sock_addr;
+socklen_t sock_addr_len = sock_addr.GetMaxLength();
+for (auto listen_socket : m_listen_sockets) {
+  if (::getsockname(listen_socket.first, sock_addr, &sock_addr_len) == 0)
+ports.insert(sock_addr.GetPort());
+}
+  }
+  return ports;
+}
+
 std::string TCPSocket::GetLocalIPAddress() const {
   // We bound to port zero, so we need to figure out which port we actually
   // bound to
@@ -196,49 +216,66 @@ Status TCPSocket::Listen(llvm::StringRef name, int 
backlog) {
   if (!host_port)
 return Status(host_port.takeError());
 
+  llvm::SmallVector ports;
+  ports.push_back(host_port->port);
+
+  llvm::SmallVector extra_ports;
+  name.split(extra_ports, ',', -1, false);
+  if (extra_ports.size() > 1) {
+for (auto i = extra_ports.begin() + 1; i != extra_ports.end(); ++i) {
+  uint16_t port;
+  if (!llvm::to_integer(*i, port, 10))
+return Status("invalid extra port number %s", i->str().c_str());
+  ports.push_back(port);
+}
+  }
+
   if (host_port->hostname == "*")
 host_port->hostname = "0.0.0.0";
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, 
IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
-int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
-  m_child_processes_inherit, error);
-if (error.Fail() || fd < 0)
-  continue;
-
-// enable local address reuse
-int option_value = 1;
-set_socket_option_arg_type option_value_p =
-reinterpret_cast(&option_value);
-if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
- sizeof(option_value)) == -1) {
-  CLOSE_SOCKET(fd);
-  continue;
-}
-
-SocketAddress listen_address = address;
-if(!listen_address.IsLocalhost())
-  listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
-else
-  listen_address.SetPort(host_port->port);
+for (size_t i = 0; i < ports.size(); ++i) {
+  int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
+m_child_processes_inherit, error);
+  if (error.Fail() || fd < 0)
+continue;
+
+  // enable local address reuse
+  int option_value = 1;
+  set_socket_option_arg_type option_value_p =
+  reinterpret_cast(&option_value);
+  if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
+   sizeof(option_value)) == -1) {
+CLOSE_SOCKET(fd);
+continue;
+  }
 
-int err =
-::bind(fd, &listen_address.sockaddr(), listen_address.GetLength());
-if (err != -1)
-  err = ::listen(fd, backlog);
+  SocketAddress listen_address = address;
+  if (!listen_address.IsLo

[Lldb-commits] [lldb] [lldb] Updated TCPSocket to listen multiple ports on the same single thread (PR #104797)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)


Changes

This is prerequisite for #104238.

---
Full diff: https://github.com/llvm/llvm-project/pull/104797.diff


3 Files Affected:

- (modified) lldb/include/lldb/Host/common/TCPSocket.h (+3) 
- (modified) lldb/source/Host/common/TCPSocket.cpp (+71-34) 
- (modified) lldb/unittests/Host/SocketTest.cpp (+13) 


``diff
diff --git a/lldb/include/lldb/Host/common/TCPSocket.h 
b/lldb/include/lldb/Host/common/TCPSocket.h
index b782c9e6096c44..b45fdae00c6b43 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -24,6 +24,9 @@ class TCPSocket : public Socket {
   // returns port number or 0 if error
   uint16_t GetLocalPortNumber() const;
 
+  // returns port numbers of all listening sockets
+  std::set GetLocalPortNumbers() const;
+
   // returns ip address string or empty string if error
   std::string GetLocalIPAddress() const;
 
diff --git a/lldb/source/Host/common/TCPSocket.cpp 
b/lldb/source/Host/common/TCPSocket.cpp
index df4737216ed3ac..e659b20bb0045e 100644
--- a/lldb/source/Host/common/TCPSocket.cpp
+++ b/lldb/source/Host/common/TCPSocket.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/WindowsError.h"
@@ -94,6 +95,25 @@ uint16_t TCPSocket::GetLocalPortNumber() const {
   return 0;
 }
 
+// Return all the port numbers that is being used by the socket.
+std::set TCPSocket::GetLocalPortNumbers() const {
+  std::set ports;
+  if (m_socket != kInvalidSocketValue) {
+SocketAddress sock_addr;
+socklen_t sock_addr_len = sock_addr.GetMaxLength();
+if (::getsockname(m_socket, sock_addr, &sock_addr_len) == 0)
+  ports.insert(sock_addr.GetPort());
+  } else if (!m_listen_sockets.empty()) {
+SocketAddress sock_addr;
+socklen_t sock_addr_len = sock_addr.GetMaxLength();
+for (auto listen_socket : m_listen_sockets) {
+  if (::getsockname(listen_socket.first, sock_addr, &sock_addr_len) == 0)
+ports.insert(sock_addr.GetPort());
+}
+  }
+  return ports;
+}
+
 std::string TCPSocket::GetLocalIPAddress() const {
   // We bound to port zero, so we need to figure out which port we actually
   // bound to
@@ -196,49 +216,66 @@ Status TCPSocket::Listen(llvm::StringRef name, int 
backlog) {
   if (!host_port)
 return Status(host_port.takeError());
 
+  llvm::SmallVector ports;
+  ports.push_back(host_port->port);
+
+  llvm::SmallVector extra_ports;
+  name.split(extra_ports, ',', -1, false);
+  if (extra_ports.size() > 1) {
+for (auto i = extra_ports.begin() + 1; i != extra_ports.end(); ++i) {
+  uint16_t port;
+  if (!llvm::to_integer(*i, port, 10))
+return Status("invalid extra port number %s", i->str().c_str());
+  ports.push_back(port);
+}
+  }
+
   if (host_port->hostname == "*")
 host_port->hostname = "0.0.0.0";
   std::vector addresses = SocketAddress::GetAddressInfo(
   host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, 
IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
-int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
-  m_child_processes_inherit, error);
-if (error.Fail() || fd < 0)
-  continue;
-
-// enable local address reuse
-int option_value = 1;
-set_socket_option_arg_type option_value_p =
-reinterpret_cast(&option_value);
-if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
- sizeof(option_value)) == -1) {
-  CLOSE_SOCKET(fd);
-  continue;
-}
-
-SocketAddress listen_address = address;
-if(!listen_address.IsLocalhost())
-  listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
-else
-  listen_address.SetPort(host_port->port);
+for (size_t i = 0; i < ports.size(); ++i) {
+  int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
+m_child_processes_inherit, error);
+  if (error.Fail() || fd < 0)
+continue;
+
+  // enable local address reuse
+  int option_value = 1;
+  set_socket_option_arg_type option_value_p =
+  reinterpret_cast(&option_value);
+  if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
+   sizeof(option_value)) == -1) {
+CLOSE_SOCKET(fd);
+continue;
+  }
 
-int err =
-::bind(fd, &listen_address.sockaddr(), listen_address.GetLength());
-if (err != -1)
-  err = ::listen(fd, backlog);
+  SocketAddress listen_address = address;
+  if (!listen_address.IsLocalhost())
+listen_address.SetToAnyAddress(address.GetFamily(), ports[i]);
+  else
+listen_address.SetPort(ports[i]);
+
+  int err =
+  ::bind(fd, &listen_address.sockaddr(),

[Lldb-commits] [lldb] b1d75fe - [lldb][test] Fix cast dropping const warnin in TestBreakpointSetCallback.cpp

2024-08-19 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-08-19T15:16:04Z
New Revision: b1d75fe48c940ba677614db3d891fbebcb8a41a2

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

LOG: [lldb][test] Fix cast dropping const warnin in 
TestBreakpointSetCallback.cpp

When building with gcc I get this warning:
```
<...>TestBreakpointSetCallback.cpp:58:25: warning: cast from type ‘const char*’ 
to type ‘void*’ casts away qualifiers [-Wcast-qual]
   58 |   void *baton = (void *)"hello";
  | ^~~
```

Use the address of a mutable global variable instead. All we care about
is that the address passed as the baton is the same one we get later.

Added: 


Modified: 
lldb/unittests/Callback/TestBreakpointSetCallback.cpp

Removed: 




diff  --git a/lldb/unittests/Callback/TestBreakpointSetCallback.cpp 
b/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
index 2a7070f9349c02..3dba4a9eb719e3 100644
--- a/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
+++ b/lldb/unittests/Callback/TestBreakpointSetCallback.cpp
@@ -30,6 +30,8 @@ using namespace lldb;
 static constexpr lldb::user_id_t expected_breakpoint_id = 1;
 static constexpr lldb::user_id_t expected_breakpoint_location_id = 0;
 
+int baton_value;
+
 class BreakpointSetCallbackTest : public ::testing::Test {
 public:
   static void CheckCallbackArgs(void *baton, StoppointCallbackContext *context,
@@ -37,7 +39,7 @@ class BreakpointSetCallbackTest : public ::testing::Test {
 lldb::user_id_t break_loc_id,
 TargetSP expected_target_sp) {
 EXPECT_EQ(context->exe_ctx_ref.GetTargetSP(), expected_target_sp);
-EXPECT_EQ(baton, "hello");
+EXPECT_EQ(baton, &baton_value);
 EXPECT_EQ(break_id, expected_breakpoint_id);
 EXPECT_EQ(break_loc_id, expected_breakpoint_location_id);
   }
@@ -55,7 +57,6 @@ class BreakpointSetCallbackTest : public ::testing::Test {
 };
 
 TEST_F(BreakpointSetCallbackTest, TestBreakpointSetCallback) {
-  void *baton = (void *)"hello";
   // Set up the debugger, make sure that was done properly.
   TargetSP target_sp;
   ArchSpec arch("x86_64-apple-macosx-");
@@ -78,7 +79,7 @@ TEST_F(BreakpointSetCallbackTest, TestBreakpointSetCallback) {
 CheckCallbackArgs(baton, context, break_id, break_loc_id, target_sp);
 return true;
   },
-  baton, true);
+  (void *)&baton_value, true);
   ExecutionContext exe_ctx(target_sp, false);
   StoppointCallbackContext context(nullptr, exe_ctx, true);
   breakpoint_sp->InvokeCallback(&context, 0);



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


[Lldb-commits] [lldb] c7a54bf - [lldb][ASTUtils] Remove unused SemaSourceWithPriorities::addSource API

2024-08-19 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2024-08-19T16:19:44+01:00
New Revision: c7a54bfd1d25330199c96dd0a46cef1644b1b1ce

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

LOG: [lldb][ASTUtils] Remove unused SemaSourceWithPriorities::addSource API

As far as I can tell, this has always been unused. My hunch is that
this was supposed to mimick the `MultiplexExternalSemaSource::AddSource`
API which `SemaSourceWithPriorities` is based on.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index da2b1a15f74614..79ed74b79f04fd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -264,10 +264,6 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
   ~SemaSourceWithPriorities() override;
 
-  void addSource(clang::ExternalSemaSource &source) {
-Sources.push_back(&source);
-  }
-
   
//======//
   // ExternalASTSource.
   
//======//



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


[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/104799

When we use `SemaSourceWithPriorities` as the `ASTContext`s ExternalASTSource, 
we allocate a `ClangASTSourceProxy` (via `CreateProxy`) and two 
`ExternalASTSourceWrapper`. Then we push these sources into a vector in 
`SemaSourceWithPriorities`. The allocated `SemaSourceWithPriorities` itself 
will get properly deallocated because the `ASTContext` wraps it in an 
`IntrusiveRefCntPtr`. But the three sources we allocated earlier will never get 
released.

This patch fixes this by mimicking what `MultiplexExternalSemaSource` does 
(which is what `SemaSourceWithPriorities` is based on anyway). I.e., when 
`SemaSourceWithPriorities` gets constructed, it increments the use count of its 
sources. And on destruction it decrements them.

Similarly, to make sure we dealloacted the `ClangASTProxy` properly, the 
`ExternalASTSourceWrapper` now optionally owns the source that it wraps. We 
might be able to get away with having `ExternalASTSourceWrapper` 
increment/decrement the use-count of the source, but it's not entirely clear to 
me why this class wanted to avoid sharing ownership of the source, as a first 
measure, I opted for adding an `owns_source` flag.

>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH] [lldb][ClangExpressionParser] Don't leak memory when
 multiplexing ExternalASTSources

---
 .../ExpressionParser/Clang/ASTUtils.cpp   | 10 --
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++-
 .../Clang/ClangExpressionParser.cpp   | 16 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * lo

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

When we use `SemaSourceWithPriorities` as the `ASTContext`s ExternalASTSource, 
we allocate a `ClangASTSourceProxy` (via `CreateProxy`) and two 
`ExternalASTSourceWrapper`. Then we push these sources into a vector in 
`SemaSourceWithPriorities`. The allocated `SemaSourceWithPriorities` itself 
will get properly deallocated because the `ASTContext` wraps it in an 
`IntrusiveRefCntPtr`. But the three sources we allocated earlier will never get 
released.

This patch fixes this by mimicking what `MultiplexExternalSemaSource` does 
(which is what `SemaSourceWithPriorities` is based on anyway). I.e., when 
`SemaSourceWithPriorities` gets constructed, it increments the use count of its 
sources. And on destruction it decrements them.

Similarly, to make sure we dealloacted the `ClangASTProxy` properly, the 
`ExternalASTSourceWrapper` now optionally owns the source that it wraps. We 
might be able to get away with having `ExternalASTSourceWrapper` 
increment/decrement the use-count of the source, but it's not entirely clear to 
me why this class wanted to avoid sharing ownership of the source, as a first 
measure, I opted for adding an `owns_source` flag.

---
Full diff: https://github.com/llvm/llvm-project/pull/104799.diff


3 Files Affected:

- (modified) lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp (+8-2) 
- (modified) lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h (+23-8) 
- (modified) 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+9-7) 


``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa96..c1d34c1d519a8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04f..31345fe35e862 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * low_quality_source) {
+assert(high_quality_source);
+assert(low_quality_source);
+
+high_quality_source->Retain();
+low_qu

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/104799

>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH 1/2] [lldb][ClangExpressionParser] Don't leak memory when
 multiplexing ExternalASTSources

---
 .../ExpressionParser/Clang/ASTUtils.cpp   | 10 --
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++-
 .../Clang/ClangExpressionParser.cpp   | 16 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * low_quality_source) {
+assert(high_quality_source);
+assert(low_quality_source);
+
+high_quality_source->Retain();
+low_quality_source->Retain();
+
+Sources.push_back(high_quality_source);
+Sources.push_back(low_quality_source);
   }
 
   ~SemaSourceWithPriorities() override;
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b64613d214e157 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,18 +1221,20 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
 decl_map->InstallDiagnosticManager(diagnostic_manager);
 
-clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
+IntrusiveRefCntPtr ast_source =
+decl_map->CreateProxy();
 
 if (ast_context.getExternalSource()) {
-  auto module_wrapper =
+  auto *module_wrapper =
   new ExternalASTSourceWrapper

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

Interestingly enough, I just checked at `gettid` _is_ defined in `unistd.h` in 
Bionic. See 
[here](https://android.googlesource.com/platform/bionic/+/master/libc/include/unistd.h).
 Although I am not the original author here, I will definitely take a look at 
seeing what is wrong! Hope some of this discussion is helpful.

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


[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread 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 c7a54bfd1d25330199c96dd0a46cef1644b1b1ce 
0c23c427f7154dabadbca65f64973ec2dbe365d9 --extensions cpp,h -- 
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index c1d34c1d51..43a71e036a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -22,7 +22,7 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() = 
default;
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
 lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
-  for (auto * Source : Sources)
+  for (auto *Source : Sources)
 Source->Release();
 }
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 31345fe35e..4398b7ac2e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -28,7 +28,7 @@ namespace lldb_private {
 /// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
-  
+
   ///< If true, means that this class is responsible for
   ///< decrementing the use count of m_Source.
   bool m_owns_source = false;
@@ -255,8 +255,7 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector
-  Sources;
+  llvm::SmallVector Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
@@ -264,9 +263,8 @@ public:
   /// as a fallback.
   ///
   /// This class assumes shared ownership of the sources provided to it.
-  SemaSourceWithPriorities(
-  clang::ExternalSemaSource * high_quality_source,
-  clang::ExternalSemaSource * low_quality_source) {
+  SemaSourceWithPriorities(clang::ExternalSemaSource *high_quality_source,
+   clang::ExternalSemaSource *low_quality_source) {
 assert(high_quality_source);
 assert(low_quality_source);
 

``




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


[Lldb-commits] [lldb] 08201cb - [lldb][test] Fix GCC warnings in TestGetControlFlowKindX86.cpp

2024-08-19 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-08-19T16:35:47+01:00
New Revision: 08201cb4245b0a03e1af664e00a22ea4db1fc2fb

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

LOG: [lldb][test] Fix GCC warnings in TestGetControlFlowKindX86.cpp

```
<...>/TestGetControlFlowKindx86.cpp:148:8: warning: suggest explicit braces to 
avoid ambiguous ‘else’ [-Wdangling-else]
  148 | if (kind == eInstructionControlFlowKindReturn)
  |^
```

Usually llvm is a "no braces for single line body" project but
for whatever reason gcc objects to it here. Perhaps because it's
within a for loop.

Added the newlines just for readability.

Added: 


Modified: 
lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp

Removed: 




diff  --git a/lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp 
b/lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
index fecd93361e5633..e9d5ae3f7b3cdb 100644
--- a/lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
+++ b/lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
@@ -145,14 +145,19 @@ TEST_F(TestGetControlFlowKindx86, TestX86_64Instruction) {
 EXPECT_EQ(kind, result[i]);
 
 // Also, test the DisassemblerLLVMC::MCDisasmInstance methods.
-if (kind == eInstructionControlFlowKindReturn)
+if (kind == eInstructionControlFlowKindReturn) {
   EXPECT_FALSE(inst_sp->IsCall());
-if (kind == eInstructionControlFlowKindCall)
+}
+
+if (kind == eInstructionControlFlowKindCall) {
   EXPECT_TRUE(inst_sp->IsCall());
+}
+
 if (kind == eInstructionControlFlowKindCall ||
 kind == eInstructionControlFlowKindJump ||
 kind == eInstructionControlFlowKindCondJump ||
-kind == eInstructionControlFlowKindReturn)
+kind == eInstructionControlFlowKindReturn) {
   EXPECT_TRUE(inst_sp->DoesBranch());
+}
   }
 }



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


[Lldb-commits] [lldb] 7a06ebd - [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value (#104617)

2024-08-19 Thread via lldb-commits

Author: Fred Grim
Date: 2024-08-19T08:41:11-07:00
New Revision: 7a06ebdeb6440d80fbcaeccd33314c6e039c6795

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

LOG: [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value 
(#104617)

In implementing this test one of the assertions assumes that the
priority is the default in linux (0) but, evidently, some of the build
runners prioritize the test to, at least, 5. This ensures that
regardless of the priority the test passes by validating that its the
process's priority
```
fgrim@host001 :~/llvm-project/debug_build> nice -n 15 
tools/lldb/unittests/Process/elf-core/ProcessElfCoreTests
[==] Running 2 tests from 1 test suite.
[--] Global test environment set-up.
[--] 2 tests from ElfCoreTest
[ RUN  ] ElfCoreTest.PopulatePrpsInfoTest
[   OK ] ElfCoreTest.PopulatePrpsInfoTest (4 ms)
[ RUN  ] ElfCoreTest.PopulatePrStatusTest
[   OK ] ElfCoreTest.PopulatePrStatusTest (3 ms)
[--] 2 tests from ElfCoreTest (7 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test suite ran. (8 ms total)
[  PASSED  ] 2 tests.
===(10:03)===
fgrim@host001 :~/llvm-project/debug_build>
```

Added: 


Modified: 
lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp

Removed: 




diff  --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp 
b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index ce146f62b0d826..3bc8b9053d2009 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -21,6 +21,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 using namespace lldb_private;
@@ -120,7 +121,10 @@ TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
   ASSERT_EQ(prpsinfo_opt->pr_state, 0);
   ASSERT_EQ(prpsinfo_opt->pr_sname, 'R');
   ASSERT_EQ(prpsinfo_opt->pr_zomb, 0);
-  ASSERT_EQ(prpsinfo_opt->pr_nice, 0);
+  int priority = getpriority(PRIO_PROCESS, getpid());
+  if (priority == -1)
+ASSERT_EQ(errno, 0);
+  ASSERT_EQ(prpsinfo_opt->pr_nice, priority);
   ASSERT_EQ(prpsinfo_opt->pr_flag, 0UL);
   ASSERT_EQ(prpsinfo_opt->pr_uid, getuid());
   ASSERT_EQ(prpsinfo_opt->pr_gid, getgid());



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


[Lldb-commits] [lldb] [lldb] PopulatePrpsInfoTest can fail due to hardcoded priority value (PR #104617)

2024-08-19 Thread Fred Grim via lldb-commits

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


[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/104799

>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH 1/4] [lldb][ClangExpressionParser] Don't leak memory when
 multiplexing ExternalASTSources

---
 .../ExpressionParser/Clang/ASTUtils.cpp   | 10 --
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++-
 .../Clang/ClangExpressionParser.cpp   | 16 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * low_quality_source) {
+assert(high_quality_source);
+assert(low_quality_source);
+
+high_quality_source->Retain();
+low_quality_source->Retain();
+
+Sources.push_back(high_quality_source);
+Sources.push_back(low_quality_source);
   }
 
   ~SemaSourceWithPriorities() override;
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b64613d214e157 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,18 +1221,20 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
 decl_map->InstallDiagnosticManager(diagnostic_manager);
 
-clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
+IntrusiveRefCntPtr ast_source =
+decl_map->CreateProxy();
 
 if (ast_context.getExternalSource()) {
-  auto module_wrapper =
+  auto *module_wrapper =
   new ExternalASTSourceWrapper

[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX updated 
https://github.com/llvm/llvm-project/pull/104679

>From e72ceaada170354aa322b4c6a1787152ac72c65b Mon Sep 17 00:00:00 2001
From: Dhruv-Srivastava 
Date: Sat, 17 Aug 2024 12:16:09 -0500
Subject: [PATCH 1/2] Using lldb's internal typedefs to avoid namespace
 collision

1. tid_t --> lldb::tid_t
2. offset_t --> lldb::ofset_t
---
 lldb/source/API/SBBreakpoint.cpp   |  6 +++---
 lldb/source/API/SBBreakpointLocation.cpp   |  6 +++---
 lldb/source/API/SBBreakpointName.cpp   |  4 ++--
 lldb/source/Expression/DWARFExpression.cpp | 10 +-
 lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp  |  2 +-
 .../Darwin-Kernel/DynamicLoaderDarwinKernel.cpp|  4 ++--
 .../MacOSX-DYLD/DynamicLoaderDarwin.cpp|  2 +-
 .../InstrumentationRuntimeMainThreadChecker.cpp|  2 +-
 .../TSan/InstrumentationRuntimeTSan.cpp| 14 +++---
 .../UBSan/InstrumentationRuntimeUBSan.cpp  |  2 +-
 .../MemoryHistory/asan/MemoryHistoryASan.cpp   |  2 +-
 .../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp  |  6 +++---
 .../Python/OperatingSystemPython.cpp   |  2 +-
 .../Plugins/Process/Utility/ThreadMemory.cpp   |  2 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp|  2 +-
 .../Plugins/Process/mach-core/ProcessMachCore.cpp  |  8 
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp |  2 +-
 .../MacOSX/AppleGetThreadItemInfoHandler.cpp   |  2 +-
 lldb/source/Symbol/DWARFCallFrameInfo.cpp  |  4 ++--
 19 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 3d908047f9455b..728fe04d14d927 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -342,7 +342,7 @@ uint32_t SBBreakpoint::GetIgnoreCount() const {
   return count;
 }
 
-void SBBreakpoint::SetThreadID(tid_t tid) {
+void SBBreakpoint::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointSP bkpt_sp = GetSP();
@@ -353,10 +353,10 @@ void SBBreakpoint::SetThreadID(tid_t tid) {
   }
 }
 
-tid_t SBBreakpoint::GetThreadID() {
+lldb::tid_t SBBreakpoint::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointSP bkpt_sp = GetSP();
   if (bkpt_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointLocation.cpp 
b/lldb/source/API/SBBreakpointLocation.cpp
index 75b66364d4f1ae..fad9a4076a54fb 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -302,7 +302,7 @@ bool 
SBBreakpointLocation::GetCommandLineCommands(SBStringList &commands) {
   return has_commands;
 }
 
-void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
+void SBBreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
   LLDB_INSTRUMENT_VA(this, thread_id);
 
   BreakpointLocationSP loc_sp = GetSP();
@@ -313,10 +313,10 @@ void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
   }
 }
 
-tid_t SBBreakpointLocation::GetThreadID() {
+lldb::tid_t SBBreakpointLocation::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointLocationSP loc_sp = GetSP();
   if (loc_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointName.cpp 
b/lldb/source/API/SBBreakpointName.cpp
index 7f63aaf6fa7d5e..5c7c0a8f6504b0 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/lldb/source/API/SBBreakpointName.cpp
@@ -347,7 +347,7 @@ bool SBBreakpointName::GetAutoContinue() {
   return bp_name->GetOptions().IsAutoContinue();
 }
 
-void SBBreakpointName::SetThreadID(tid_t tid) {
+void SBBreakpointName::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointName *bp_name = GetBreakpointName();
@@ -361,7 +361,7 @@ void SBBreakpointName::SetThreadID(tid_t tid) {
   UpdateName(*bp_name);
 }
 
-tid_t SBBreakpointName::GetThreadID() {
+lldb::tid_t SBBreakpointName::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
   BreakpointName *bp_name = GetBreakpointName();
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 444e44b3928919..c1feec990f989d 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -130,7 +130,7 @@ static llvm::Error 
ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
 
 /// Return the length in bytes of the set of operands for \p op. No guarantees
 /// are made on the state of \p data after this call.
-static offset_t GetOpcodeDataSize(const DataExtractor &data,
+static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
   const lldb::offset_t data_offset,
   const uint8_t op, const DWARFUnit *dwarf_cu) 
{
   lldb::offset_t offset = data_offset;
@@ -358,7 +358,7 @@ lldb::add

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/104799

>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH 1/5] [lldb][ClangExpressionParser] Don't leak memory when
 multiplexing ExternalASTSources

---
 .../ExpressionParser/Clang/ASTUtils.cpp   | 10 --
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++-
 .../Clang/ClangExpressionParser.cpp   | 16 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * low_quality_source) {
+assert(high_quality_source);
+assert(low_quality_source);
+
+high_quality_source->Retain();
+low_quality_source->Retain();
+
+Sources.push_back(high_quality_source);
+Sources.push_back(low_quality_source);
   }
 
   ~SemaSourceWithPriorities() override;
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b64613d214e157 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,18 +1221,20 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
 decl_map->InstallDiagnosticManager(diagnostic_manager);
 
-clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
+IntrusiveRefCntPtr ast_source =
+decl_map->CreateProxy();
 
 if (ast_context.getExternalSource()) {
-  auto module_wrapper =
+  auto *module_wrapper =
   new ExternalASTSourceWrapper

[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX updated 
https://github.com/llvm/llvm-project/pull/104679

>From e72ceaada170354aa322b4c6a1787152ac72c65b Mon Sep 17 00:00:00 2001
From: Dhruv-Srivastava 
Date: Sat, 17 Aug 2024 12:16:09 -0500
Subject: [PATCH] Using lldb's internal typedefs to avoid namespace collision

1. tid_t --> lldb::tid_t
2. offset_t --> lldb::ofset_t
---
 lldb/source/API/SBBreakpoint.cpp   |  6 +++---
 lldb/source/API/SBBreakpointLocation.cpp   |  6 +++---
 lldb/source/API/SBBreakpointName.cpp   |  4 ++--
 lldb/source/Expression/DWARFExpression.cpp | 10 +-
 lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp  |  2 +-
 .../Darwin-Kernel/DynamicLoaderDarwinKernel.cpp|  4 ++--
 .../MacOSX-DYLD/DynamicLoaderDarwin.cpp|  2 +-
 .../InstrumentationRuntimeMainThreadChecker.cpp|  2 +-
 .../TSan/InstrumentationRuntimeTSan.cpp| 14 +++---
 .../UBSan/InstrumentationRuntimeUBSan.cpp  |  2 +-
 .../MemoryHistory/asan/MemoryHistoryASan.cpp   |  2 +-
 .../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp  |  6 +++---
 .../Python/OperatingSystemPython.cpp   |  2 +-
 .../Plugins/Process/Utility/ThreadMemory.cpp   |  2 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp|  2 +-
 .../Plugins/Process/mach-core/ProcessMachCore.cpp  |  8 
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp |  2 +-
 .../MacOSX/AppleGetThreadItemInfoHandler.cpp   |  2 +-
 lldb/source/Symbol/DWARFCallFrameInfo.cpp  |  4 ++--
 19 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 3d908047f9455b..728fe04d14d927 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -342,7 +342,7 @@ uint32_t SBBreakpoint::GetIgnoreCount() const {
   return count;
 }
 
-void SBBreakpoint::SetThreadID(tid_t tid) {
+void SBBreakpoint::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointSP bkpt_sp = GetSP();
@@ -353,10 +353,10 @@ void SBBreakpoint::SetThreadID(tid_t tid) {
   }
 }
 
-tid_t SBBreakpoint::GetThreadID() {
+lldb::tid_t SBBreakpoint::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointSP bkpt_sp = GetSP();
   if (bkpt_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointLocation.cpp 
b/lldb/source/API/SBBreakpointLocation.cpp
index 75b66364d4f1ae..fad9a4076a54fb 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -302,7 +302,7 @@ bool 
SBBreakpointLocation::GetCommandLineCommands(SBStringList &commands) {
   return has_commands;
 }
 
-void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
+void SBBreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
   LLDB_INSTRUMENT_VA(this, thread_id);
 
   BreakpointLocationSP loc_sp = GetSP();
@@ -313,10 +313,10 @@ void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
   }
 }
 
-tid_t SBBreakpointLocation::GetThreadID() {
+lldb::tid_t SBBreakpointLocation::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointLocationSP loc_sp = GetSP();
   if (loc_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointName.cpp 
b/lldb/source/API/SBBreakpointName.cpp
index 7f63aaf6fa7d5e..5c7c0a8f6504b0 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/lldb/source/API/SBBreakpointName.cpp
@@ -347,7 +347,7 @@ bool SBBreakpointName::GetAutoContinue() {
   return bp_name->GetOptions().IsAutoContinue();
 }
 
-void SBBreakpointName::SetThreadID(tid_t tid) {
+void SBBreakpointName::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointName *bp_name = GetBreakpointName();
@@ -361,7 +361,7 @@ void SBBreakpointName::SetThreadID(tid_t tid) {
   UpdateName(*bp_name);
 }
 
-tid_t SBBreakpointName::GetThreadID() {
+lldb::tid_t SBBreakpointName::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
   BreakpointName *bp_name = GetBreakpointName();
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 444e44b3928919..c1feec990f989d 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -130,7 +130,7 @@ static llvm::Error 
ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
 
 /// Return the length in bytes of the set of operands for \p op. No guarantees
 /// are made on the state of \p data after this call.
-static offset_t GetOpcodeDataSize(const DataExtractor &data,
+static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
   const lldb::offset_t data_offset,
   const uint8_t op, const DWARFUnit *dwarf_cu) 
{
   lldb::offset_t offset = data_offset;
@@ -358,7 +358,7 @@ lldb::addr_t D

[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Assuming the formatting is fine I'll merge this tomorrow.

Before I do, please turn off private email addresses in your GitHub settings so 
that we record a proper commit email and you'll get buildbot notifications to 
the right place.

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


[Lldb-commits] [lldb] 0ee0857 - [lldb][Python] Silence GCC warning for modules error workaround

2024-08-19 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-08-19T17:03:58+01:00
New Revision: 0ee0857363aadf9ce0f403e7e0da10f0a9d94887

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

LOG: [lldb][Python] Silence GCC warning for modules error workaround

```
lldb-python.h:16:30: warning: ‘g_fcxx_modules_workaround’ defined but not used 
[-Wunused-variable]
   16 | static llvm::Expected *g_fcxx_modules_workaround;
  |
```

Workaround originally added in 36cb29cbbe1b22dcd298ad65e1fabe899b7d7249.

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h

Removed: 




diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h 
b/lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h
index c99372fa110cd2..378b9fa2a59865 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/lldb-python.h
@@ -13,7 +13,7 @@
 // This declaration works around a clang module build failure.
 // It should be deleted ASAP.
 #include "llvm/Support/Error.h"
-static llvm::Expected *g_fcxx_modules_workaround;
+static llvm::Expected *g_fcxx_modules_workaround [[maybe_unused]];
 // END
 
 #include "lldb/Host/Config.h"



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


[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread Dhruv Srivastava via lldb-commits

https://github.com/DhruvSrivastavaX updated 
https://github.com/llvm/llvm-project/pull/104679

>From e72ceaada170354aa322b4c6a1787152ac72c65b Mon Sep 17 00:00:00 2001
From: Dhruv-Srivastava 
Date: Sat, 17 Aug 2024 12:16:09 -0500
Subject: [PATCH 1/2] Using lldb's internal typedefs to avoid namespace
 collision

1. tid_t --> lldb::tid_t
2. offset_t --> lldb::ofset_t
---
 lldb/source/API/SBBreakpoint.cpp   |  6 +++---
 lldb/source/API/SBBreakpointLocation.cpp   |  6 +++---
 lldb/source/API/SBBreakpointName.cpp   |  4 ++--
 lldb/source/Expression/DWARFExpression.cpp | 10 +-
 lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp  |  2 +-
 .../Darwin-Kernel/DynamicLoaderDarwinKernel.cpp|  4 ++--
 .../MacOSX-DYLD/DynamicLoaderDarwin.cpp|  2 +-
 .../InstrumentationRuntimeMainThreadChecker.cpp|  2 +-
 .../TSan/InstrumentationRuntimeTSan.cpp| 14 +++---
 .../UBSan/InstrumentationRuntimeUBSan.cpp  |  2 +-
 .../MemoryHistory/asan/MemoryHistoryASan.cpp   |  2 +-
 .../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp  |  6 +++---
 .../Python/OperatingSystemPython.cpp   |  2 +-
 .../Plugins/Process/Utility/ThreadMemory.cpp   |  2 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp|  2 +-
 .../Plugins/Process/mach-core/ProcessMachCore.cpp  |  8 
 lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp |  2 +-
 .../MacOSX/AppleGetThreadItemInfoHandler.cpp   |  2 +-
 lldb/source/Symbol/DWARFCallFrameInfo.cpp  |  4 ++--
 19 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 3d908047f9455b..728fe04d14d927 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -342,7 +342,7 @@ uint32_t SBBreakpoint::GetIgnoreCount() const {
   return count;
 }
 
-void SBBreakpoint::SetThreadID(tid_t tid) {
+void SBBreakpoint::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointSP bkpt_sp = GetSP();
@@ -353,10 +353,10 @@ void SBBreakpoint::SetThreadID(tid_t tid) {
   }
 }
 
-tid_t SBBreakpoint::GetThreadID() {
+lldb::tid_t SBBreakpoint::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointSP bkpt_sp = GetSP();
   if (bkpt_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointLocation.cpp 
b/lldb/source/API/SBBreakpointLocation.cpp
index 75b66364d4f1ae..fad9a4076a54fb 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -302,7 +302,7 @@ bool 
SBBreakpointLocation::GetCommandLineCommands(SBStringList &commands) {
   return has_commands;
 }
 
-void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
+void SBBreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
   LLDB_INSTRUMENT_VA(this, thread_id);
 
   BreakpointLocationSP loc_sp = GetSP();
@@ -313,10 +313,10 @@ void SBBreakpointLocation::SetThreadID(tid_t thread_id) {
   }
 }
 
-tid_t SBBreakpointLocation::GetThreadID() {
+lldb::tid_t SBBreakpointLocation::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
-  tid_t tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
   BreakpointLocationSP loc_sp = GetSP();
   if (loc_sp) {
 std::lock_guard guard(
diff --git a/lldb/source/API/SBBreakpointName.cpp 
b/lldb/source/API/SBBreakpointName.cpp
index 7f63aaf6fa7d5e..5c7c0a8f6504b0 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/lldb/source/API/SBBreakpointName.cpp
@@ -347,7 +347,7 @@ bool SBBreakpointName::GetAutoContinue() {
   return bp_name->GetOptions().IsAutoContinue();
 }
 
-void SBBreakpointName::SetThreadID(tid_t tid) {
+void SBBreakpointName::SetThreadID(lldb::tid_t tid) {
   LLDB_INSTRUMENT_VA(this, tid);
 
   BreakpointName *bp_name = GetBreakpointName();
@@ -361,7 +361,7 @@ void SBBreakpointName::SetThreadID(tid_t tid) {
   UpdateName(*bp_name);
 }
 
-tid_t SBBreakpointName::GetThreadID() {
+lldb::tid_t SBBreakpointName::GetThreadID() {
   LLDB_INSTRUMENT_VA(this);
 
   BreakpointName *bp_name = GetBreakpointName();
diff --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 444e44b3928919..c1feec990f989d 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -130,7 +130,7 @@ static llvm::Error 
ReadRegisterValueAsScalar(RegisterContext *reg_ctx,
 
 /// Return the length in bytes of the set of operands for \p op. No guarantees
 /// are made on the state of \p data after this call.
-static offset_t GetOpcodeDataSize(const DataExtractor &data,
+static lldb::offset_t GetOpcodeDataSize(const DataExtractor &data,
   const lldb::offset_t data_offset,
   const uint8_t op, const DWARFUnit *dwarf_cu) 
{
   lldb::offset_t offset = data_offset;
@@ -358,7 +358,7 @@ lldb::add

[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

2024-08-19 Thread Miro Bucko via lldb-commits

mbucko wrote:

Let me know if you guys don't want this patch in. I will closed it and apply it 
to our local branch.

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


[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/104799

>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH 1/6] [lldb][ClangExpressionParser] Don't leak memory when
 multiplexing ExternalASTSources

---
 .../ExpressionParser/Clang/ASTUtils.cpp   | 10 --
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++-
 .../Clang/ClangExpressionParser.cpp   | 16 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
 
 #include "ASTUtils.h"
 
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+  if (m_owns_source)
+m_Source->Release();
+}
 
 void lldb_private::ExternalASTSourceWrapper::PrintStats() {
   m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder() 
= default;
 
 void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
 
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+  for (auto * Source : Sources)
+Source->Release();
+}
 
 void lldb_private::SemaSourceWithPriorities::PrintStats() {
   for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include 
 
 namespace clang {
@@ -24,13 +25,17 @@ class Module;
 
 namespace lldb_private {
 
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
 class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
   ExternalASTSource *m_Source;
+  
+  ///< If true, means that this class is responsible for
+  ///< decrementing the use count of m_Source.
+  bool m_owns_source = false;
 
 public:
-  ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+  ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+  : m_Source(Source), m_owns_source(owns_source) {
 assert(m_Source && "Can't wrap nullptr ExternalASTSource");
   }
 
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public 
clang::ExternalSemaSource {
 
 private:
   /// The sources ordered in decreasing priority.
-  llvm::SmallVector Sources;
+  llvm::SmallVector
+  Sources;
 
 public:
   /// Construct a SemaSourceWithPriorities with a 'high quality' source that
   /// has the higher priority and a 'low quality' source that will be used
   /// as a fallback.
-  SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
-   clang::ExternalSemaSource &low_quality_source) {
-Sources.push_back(&high_quality_source);
-Sources.push_back(&low_quality_source);
+  ///
+  /// This class assumes shared ownership of the sources provided to it.
+  SemaSourceWithPriorities(
+  clang::ExternalSemaSource * high_quality_source,
+  clang::ExternalSemaSource * low_quality_source) {
+assert(high_quality_source);
+assert(low_quality_source);
+
+high_quality_source->Retain();
+low_quality_source->Retain();
+
+Sources.push_back(high_quality_source);
+Sources.push_back(low_quality_source);
   }
 
   ~SemaSourceWithPriorities() override;
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b64613d214e157 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,18 +1221,20 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
 decl_map->InstallDiagnosticManager(diagnostic_manager);
 
-clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
+IntrusiveRefCntPtr ast_source =
+decl_map->CreateProxy();
 
 if (ast_context.getExternalSource()) {
-  auto module_wrapper =
+  auto *module_wrapper =
   new ExternalASTSourceWrapper

[Lldb-commits] [lldb] [lldb][AIX] 1. Avoid namespace collision on other platforms (PR #104679)

2024-08-19 Thread Dhruv Srivastava via lldb-commits

DhruvSrivastavaX wrote:

Great!!

Yes, I have turned off private email addresses now. 

Just a general question. Is it a one time thing or it is advisable to keep it 
off in future as well?


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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Paul Kirth via lldb-commits

ilovepi wrote:


> Update: I see that what we are really compiling for is fuchsia! Interesting. 
> I will investigate further!

No, this is targeting linux for x86_64: The command line doesn't list a 
different target, and we don't try to test LLDB on Fuchsia targets to my 
knowledge. We do run the tests for the host platform though. This is a normal 
linux build, with no downstream packages. Looking at that command, 
`-D_GNU_SOURCE` is already defined for that TU, so maybe there is a different 
reason. Our bots and sysroot are based on the oldest supported versions of 
Debian, based on the project's stated minimum requirements. We do this 
purposefully to make sure we're not out of sync w/ what is intended to be 
supported by LLVM. So perhaps there is an issue in that regard?

```
/b/s/w/ir/x/w/rc/cxx-rbe47grlile/reclient-cxx-wrapper.sh 
/b/s/w/ir/x/w/cipd/bin/clang++ --sysroot=/b/s/w/ir/x/w/cipd/linux 
-DGTEST_HAS_RTTI=0 -DHAVE_ROUND -DLIBXML_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS 
-I/b/s/w/ir/x/w/llvm_build/tools/lldb/unittests/Process/elf-core 
-I/b/s/w/ir/x/w/llvm-llvm-project/lldb/unittests/Process/elf-core 
-I/b/s/w/ir/x/w/llvm-llvm-project/lldb/include 
-I/b/s/w/ir/x/w/llvm_build/tools/lldb/include 
-I/b/s/w/ir/x/w/rc/tensorflow-venv/store/python_venv-u3hh0hkn7c13sepmcfocq14vq0/contents/lib/python3.8/site-packages/tensorflow/include
 -I/b/s/w/ir/x/w/llvm_build/include 
-I/b/s/w/ir/x/w/llvm-llvm-project/llvm/include 
-I/b/s/w/ir/x/w/lldb_install/python3/include/python3.11 
-I/b/s/w/ir/x/w/llvm-llvm-project/llvm/../clang/include 
-I/b/s/w/ir/x/w/llvm_build/tools/lldb/../clang/include 
-I/b/s/w/ir/x/w/llvm-llvm-project/lldb/source 
-I/b/s/w/ir/x/w/llvm-llvm-project/lldb/unittests 
-I/b/s/w/ir/x/w/llvm-llvm-project/third-party/unittest/googletest/include 
-I/b/s/w/ir/x/w/llvm-llvm-project/third-party/unittest/googlemock/include 
-isystem /b/s/w/ir/x/w/libxml2_install_target/include/libxml2 -isystem 
/b/s/w/ir/x/w/zlib_install_target/include -isystem 
/b/s/w/ir/x/w/zstd_install/include -isystem 
/b/s/w/ir/x/w/libedit_install/include -stdlib=libc++ -fPIC 
-fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion 
-Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color 
-ffunction-sections -fdata-sections -ffat-lto-objects 
-ffile-prefix-map=/b/s/w/ir/x/w/llvm_build=../llvm-llvm-project 
-ffile-prefix-map=/b/s/w/ir/x/w/llvm-llvm-project/= -no-canonical-prefixes 
-Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing 
-Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG -std=c++17 
-fvisibility=default  -Wno-variadic-macros 
-Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-unwind-tables 
-fno-asynchronous-unwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD 
-MT 
tools/lldb/unittests/Process/elf-core/CMakeFiles/ProcessElfCoreTests.dir/ThreadElfCoreTest.cpp.o
 -MF 
tools/lldb/unittests/Process/elf-core/CMakeFiles/ProcessElfCoreTests.dir/ThreadElfCoreTest.cpp.o.d
 -o 
tools/lldb/unittests/Process/elf-core/CMakeFiles/ProcessElfCoreTests.dir/ThreadElfCoreTest.cpp.o
 -c 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
```

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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

Thanks, @ilovepi ! Again, I'm just trying to help, so I appreciate the 
backstory. 

I will keep looking!

https://github.com/llvm/llvm-project/pull/104109
___
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)

2024-08-19 Thread Jacob Lalonde via lldb-commits

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

>From b451c1630a9799180a3a976a4ecd86c8d4ec35da Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 01/14] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 7f4d607f5427df..94ce505231e084 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -261,6 +262,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1224,6 +1226,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1557,6 +1561,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/so

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed (PR #104817)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/104817

While parsing an expression, Clang tries to diagnose usage of decls (with 
possibly non-external linkage) for which it hasn't been provided with a 
definition. This is the case, e.g., for functions with parameters that live in 
an anonymous namespace (those will have `UniqueExternal` linkage). Before 
diagnosing such situations, Clang calls 
`ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this API is to 
extend the set of "used but not defined" decls with additional ones that the 
external source knows about. However, in LLDB's case, we never provide 
`FunctionDecl`s with a definition, and instead rely on the expression parser to 
resolve those symbols by linkage name. Thus, to avoid the Clang parser from 
erroring out in these situations, this patch implements `ReadUndefinedButUsed` 
which just removes the "undefined" non-external `FunctionDecl`s that Clang 
found.

We also had to add an `ExternalSemaSource` to the `clang::Sema` instance LLDB 
creates. We previously didn't have any source on `Sema`. Because we add the 
`ExternalASTSourceWrapper` here, that means we'd also technically be adding the 
`ClangExpressionDeclMap` as an `ExternalASTSource` to `Sema`, which is fine 
because `Sema` will only be calling into the `ExternalSemaSource` APIs (though 
nothing currently strictly enforces this, which is a bit worrying).

Fixes https://github.com/llvm/llvm-project/issues/104712

>From 4bb7f6495a2fbf46e01b9f01c46d335d2b680afd Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 17:39:44 +0100
Subject: [PATCH] [lldb][ClangExpressionParser] Implement
 ExternalSemaSource::ReadUndefinedButUsed

While parsing an expression, Clang tries to diagnose usage of decls
(with possibly non-external linkage) for which it hasn't been provided
with a definition. This is the case, e.g., for functions with parameters
that live in an anonymous namespace (those will have `UniqueExternal`
linkage). Before diagnosing such situations, Clang calls
`ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this
API is to extend the set of "used but not defined" decls with additional
ones that the external source knows about. However, in LLDB's case,
we never provide `FunctionDecl`s with a definition, and instead
rely on the expression parser to resolve those symbols by linkage name.
Thus, to avoid the Clang parser from erroring out in these situations,
this patch implements `ReadUndefinedButUsed` which just removes the
"undefined" non-external `FunctionDecl`s that Clang found.

Fixes https://github.com/llvm/llvm-project/issues/104712
---
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 19 +++
 .../Clang/ClangExpressionParser.cpp   |  3 +++
 .../Shell/Expr/TestAnonNamespaceParamFunc.cpp | 19 +++
 3 files changed, 41 insertions(+)
 create mode 100644 lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..9710f7dd4e983c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/Support/Casting.h"
 #include 
 
 namespace clang {
@@ -135,6 +136,24 @@ class ExternalASTSourceWrapper : public 
clang::ExternalSemaSource {
 return m_Source->layoutRecordType(Record, Size, Alignment, FieldOffsets,
   BaseOffsets, VirtualBaseOffsets);
   }
+
+  /// This gets called when Sema is reconciling undefined but used functions.
+  /// For LLDB's use-case, we never provide Clang with function definitions,
+  /// instead we rely on linkage names and symbol resolution to call the
+  /// correct funcitons during JITting. So this implementation clears
+  /// any "undefined" functions Clang found while parsing.
+  ///
+  /// \param[in,out] Undefined A set of used decls for which Clang has not
+  ///  been provided a definition with.
+  ///
+  void ReadUndefinedButUsed(
+  llvm::MapVector &Undefined)
+  override {
+Undefined.remove_if([](auto const &decl_loc_pair) {
+  const clang::NamedDecl *ND = decl_loc_pair.first;
+  return llvm::isa_and_present(ND);
+});
+  }
 };
 
 /// Wraps an ASTConsumer into an SemaConsumer. Doesn't take ownership of the
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b23e851f547ce9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1223,6 +1223,8 @@ ClangExpressionParser::ParseInternal(Diagno

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed (PR #104817)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

While parsing an expression, Clang tries to diagnose usage of decls (with 
possibly non-external linkage) for which it hasn't been provided with a 
definition. This is the case, e.g., for functions with parameters that live in 
an anonymous namespace (those will have `UniqueExternal` linkage). Before 
diagnosing such situations, Clang calls 
`ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this API is to 
extend the set of "used but not defined" decls with additional ones that the 
external source knows about. However, in LLDB's case, we never provide 
`FunctionDecl`s with a definition, and instead rely on the expression parser to 
resolve those symbols by linkage name. Thus, to avoid the Clang parser from 
erroring out in these situations, this patch implements `ReadUndefinedButUsed` 
which just removes the "undefined" non-external `FunctionDecl`s that Clang 
found.

We also had to add an `ExternalSemaSource` to the `clang::Sema` instance LLDB 
creates. We previously didn't have any source on `Sema`. Because we add the 
`ExternalASTSourceWrapper` here, that means we'd also technically be adding the 
`ClangExpressionDeclMap` as an `ExternalASTSource` to `Sema`, which is fine 
because `Sema` will only be calling into the `ExternalSemaSource` APIs (though 
nothing currently strictly enforces this, which is a bit worrying).

Fixes https://github.com/llvm/llvm-project/issues/104712

---
Full diff: https://github.com/llvm/llvm-project/pull/104817.diff


3 Files Affected:

- (modified) lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h (+19) 
- (modified) 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+3) 
- (added) lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp (+19) 


``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..9710f7dd4e983c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/Support/Casting.h"
 #include 
 
 namespace clang {
@@ -135,6 +136,24 @@ class ExternalASTSourceWrapper : public 
clang::ExternalSemaSource {
 return m_Source->layoutRecordType(Record, Size, Alignment, FieldOffsets,
   BaseOffsets, VirtualBaseOffsets);
   }
+
+  /// This gets called when Sema is reconciling undefined but used functions.
+  /// For LLDB's use-case, we never provide Clang with function definitions,
+  /// instead we rely on linkage names and symbol resolution to call the
+  /// correct funcitons during JITting. So this implementation clears
+  /// any "undefined" functions Clang found while parsing.
+  ///
+  /// \param[in,out] Undefined A set of used decls for which Clang has not
+  ///  been provided a definition with.
+  ///
+  void ReadUndefinedButUsed(
+  llvm::MapVector &Undefined)
+  override {
+Undefined.remove_if([](auto const &decl_loc_pair) {
+  const clang::NamedDecl *ND = decl_loc_pair.first;
+  return llvm::isa_and_present(ND);
+});
+  }
 };
 
 /// Wraps an ASTConsumer into an SemaConsumer. Doesn't take ownership of the
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b23e851f547ce9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1223,6 +1223,8 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 
 clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
 
+auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
+
 if (ast_context.getExternalSource()) {
   auto module_wrapper =
   new ExternalASTSourceWrapper(ast_context.getExternalSource());
@@ -1236,6 +1238,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 } else {
   ast_context.setExternalSource(ast_source);
 }
+m_compiler->getSema().addExternalSource(ast_source_wrapper);
 decl_map->InstallASTContext(*m_ast_context);
   }
 
diff --git a/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp 
b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
new file mode 100644
index 00..750c81c0db6b32
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
@@ -0,0 +1,19 @@
+// TODO: Test that ...
+
+// RUN: %build %s -o %t
+// RUN: %lldb %t -o run -o "expression func(a)" -o exit | FileCheck %s
+
+// CHECK: expression func(a)
+// CHECK: (int) $0 = 15
+
+namespace {
+struct InAnon {};
+} // namespace
+
+int func(InAnon a) 

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed (PR #104817)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/104817

>From 4bb7f6495a2fbf46e01b9f01c46d335d2b680afd Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 17:39:44 +0100
Subject: [PATCH 1/2] [lldb][ClangExpressionParser] Implement
 ExternalSemaSource::ReadUndefinedButUsed

While parsing an expression, Clang tries to diagnose usage of decls
(with possibly non-external linkage) for which it hasn't been provided
with a definition. This is the case, e.g., for functions with parameters
that live in an anonymous namespace (those will have `UniqueExternal`
linkage). Before diagnosing such situations, Clang calls
`ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this
API is to extend the set of "used but not defined" decls with additional
ones that the external source knows about. However, in LLDB's case,
we never provide `FunctionDecl`s with a definition, and instead
rely on the expression parser to resolve those symbols by linkage name.
Thus, to avoid the Clang parser from erroring out in these situations,
this patch implements `ReadUndefinedButUsed` which just removes the
"undefined" non-external `FunctionDecl`s that Clang found.

Fixes https://github.com/llvm/llvm-project/issues/104712
---
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 19 +++
 .../Clang/ClangExpressionParser.cpp   |  3 +++
 .../Shell/Expr/TestAnonNamespaceParamFunc.cpp | 19 +++
 3 files changed, 41 insertions(+)
 create mode 100644 lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..9710f7dd4e983c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/Support/Casting.h"
 #include 
 
 namespace clang {
@@ -135,6 +136,24 @@ class ExternalASTSourceWrapper : public 
clang::ExternalSemaSource {
 return m_Source->layoutRecordType(Record, Size, Alignment, FieldOffsets,
   BaseOffsets, VirtualBaseOffsets);
   }
+
+  /// This gets called when Sema is reconciling undefined but used functions.
+  /// For LLDB's use-case, we never provide Clang with function definitions,
+  /// instead we rely on linkage names and symbol resolution to call the
+  /// correct funcitons during JITting. So this implementation clears
+  /// any "undefined" functions Clang found while parsing.
+  ///
+  /// \param[in,out] Undefined A set of used decls for which Clang has not
+  ///  been provided a definition with.
+  ///
+  void ReadUndefinedButUsed(
+  llvm::MapVector &Undefined)
+  override {
+Undefined.remove_if([](auto const &decl_loc_pair) {
+  const clang::NamedDecl *ND = decl_loc_pair.first;
+  return llvm::isa_and_present(ND);
+});
+  }
 };
 
 /// Wraps an ASTConsumer into an SemaConsumer. Doesn't take ownership of the
diff --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b23e851f547ce9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1223,6 +1223,8 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 
 clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
 
+auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
+
 if (ast_context.getExternalSource()) {
   auto module_wrapper =
   new ExternalASTSourceWrapper(ast_context.getExternalSource());
@@ -1236,6 +1238,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 } else {
   ast_context.setExternalSource(ast_source);
 }
+m_compiler->getSema().addExternalSource(ast_source_wrapper);
 decl_map->InstallASTContext(*m_ast_context);
   }
 
diff --git a/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp 
b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
new file mode 100644
index 00..750c81c0db6b32
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
@@ -0,0 +1,19 @@
+// TODO: Test that ...
+
+// RUN: %build %s -o %t
+// RUN: %lldb %t -o run -o "expression func(a)" -o exit | FileCheck %s
+
+// CHECK: expression func(a)
+// CHECK: (int) $0 = 15
+
+namespace {
+struct InAnon {};
+} // namespace
+
+int func(InAnon a) { return 15; }
+
+int main() {
+  InAnon a;
+  __builtin_debugtrap();
+  return func(a);
+}

>From 4bdb9e1763d133f1b0712675a4e135cb95e5e50b Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 19 Aug 2024 18:08:34 +0100
Subject: [PATCH 2/2] fixup! adjust comment

---

[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Don't leak memory when multiplexing ExternalASTSources (PR #104799)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -1224,15 +1224,15 @@ ClangExpressionParser::ParseInternal(DiagnosticManager 
&diagnostic_manager,
 clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
 
 if (ast_context.getExternalSource()) {
-  auto module_wrapper =
+  auto *module_wrapper =
   new ExternalASTSourceWrapper(ast_context.getExternalSource());
 
-  auto ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
+  auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
 
-  auto multiplexer =
-  new SemaSourceWithPriorities(*module_wrapper, *ast_source_wrapper);
-  IntrusiveRefCntPtr Source(multiplexer);

Michael137 wrote:

All changes in this file are drive-by stylistic changes.

https://github.com/llvm/llvm-project/pull/104799
___
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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -175,6 +177,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(std::move(impl_type)), 
m_name(std::move(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) 
{
+std::lock_guard guard(m_map_mutex);
+auto iterator = m_summary_stats_map.find(provider.GetName());
+if (iterator != m_summary_stats_map.end())
+  return iterator->second;
+else {
+  auto it = m_summary_stats_map.try_emplace(
+  provider.GetName(),
+  std::make_shared(provider.GetName(),
+  provider.GetSummaryKindName()));
+  return it.first->second;
+}

Michael137 wrote:

Don't need the `else` here.

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -615,7 +615,19 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+
+TargetSP target_sp = GetExecutionContextRef().GetTargetSP();
+if (target_sp) {
+  // Get Shared pointer to the summary statistics container

Michael137 wrote:

```suggestion
```

Nit: don't need the comment here

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)

2024-08-19 Thread Michael Buch 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) 
{

Michael137 wrote:

Yea it's mostly personal preference, thanks for addressing all the comments so 
far. But this approach seems fine to me too (though I've had enough time to let 
it sink in, not sure it'd be straightforward to reason about for someone new to 
this).

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -0,0 +1,35 @@
+// Test that the lldb command `statistics` works.

Michael137 wrote:

Not aware of anything like this off-the-top. We'd want a test that access the 
same `Target` from multiple threads and formats types. Maybe having some 
callback tied to several threads within the target which trigger a 
`ValueObject::GetSummaryAsCString` call. @clayborg @jimingham might have some 
more concrete ideas on how to do that.

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -175,6 +177,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations

Michael137 wrote:

Can you put a note here that all members of this class need to be accessible in 
a thread-safe manner?

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -175,6 +177,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(std::move(impl_type)), 
m_name(std::move(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) 
{
+std::lock_guard guard(m_map_mutex);
+auto iterator = m_summary_stats_map.find(provider.GetName());
+if (iterator != m_summary_stats_map.end())
+  return iterator->second;

Michael137 wrote:

```suggestion
if (auto iterator = m_summary_stats_map.find(provider.GetName());
 iterator != m_summary_stats_map.end())
  return iterator->second;
```

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -615,7 +615,19 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+
+TargetSP target_sp = GetExecutionContextRef().GetTargetSP();
+if (target_sp) {

Michael137 wrote:

```suggestion
   if (TargetSP target_sp = GetExecutionContextRef().GetTargetSP();) {
```

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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -175,6 +177,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(std::move(impl_type)), 
m_name(std::move(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;

Michael137 wrote:

Not sure what the exact policy is but i think we define all of these in 
`lldb-forward.h` (given you expose it in the header here anyway, probably best 
to move it there, but will defer to @clayborg )

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)

2024-08-19 Thread Michael Buch via lldb-commits

https://github.com/Michael137 edited 
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] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

2024-08-19 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

My comment about hoping to crack open the Target/Process ReadMemory API 
shouldn't have anything to do with this patch, I don't think.  I saw the 
discussion going in to areas of "the ReadMemory API could behave differently", 
and I wanted to note that I'm hoping to start a proposal on changing these API, 
and that would be a good opportunity to make changes like those because I'll 
probably need to update all the callers across the codebase if we find a new 
API design we can all agree on.

https://github.com/llvm/llvm-project/pull/102536
___
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)

2024-08-19 Thread Jacob Lalonde via lldb-commits


@@ -175,6 +177,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(std::move(impl_type)), 
m_name(std::move(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) 
{
+std::lock_guard guard(m_map_mutex);
+auto iterator = m_summary_stats_map.find(provider.GetName());
+if (iterator != m_summary_stats_map.end())
+  return iterator->second;

Jlalond wrote:

@Michael137 any idea why my git-clang-format isn't picking these up? I know 
there was an issue a few weeks ago, but I tried to run it just now on the 
rebase and it seemed to hang and never complete formatting

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] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap (PR #104824)

2024-08-19 Thread via lldb-commits

https://github.com/jeffreytan81 created 
https://github.com/llvm/llvm-project/pull/104824

`SBCommand::AddCommand()` requires `SBCommandPluginInterface` to be heap based 
because it will be stored inside 
`std::shared_ptr` later for reference counting. 
But lldb-dap passes `StartDebuggingRequestHandler/ReplModeRequestHandler` 
static function pointer to it which will cause corruption later during 
destruction. 

This PR fixes this issue by making these two handler heap based. 

>From 90707cae4d2cc4f3dcc5d76c506e3cd53430f3bd Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 19 Aug 2024 10:59:57 -0700
Subject: [PATCH] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in
 lldb-dap

---
 lldb/tools/lldb-dap/DAP.h| 2 --
 lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..7828272aa15a7d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;
   ReplMode repl_mode;
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..f50a6c17310739 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object 
&request) {
   "lldb-dap", "Commands for managing lldb-dap.");
   if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) {
 cmd.AddCommand(
-"startDebugging", &g_dap.start_debugging_request_handler,
+"startDebugging", new StartDebuggingRequestHandler(),
 "Sends a startDebugging request from the debug adapter to the client "
 "to start a child debug session of the same type as the caller.");
   }
   cmd.AddCommand(
-  "repl-mode", &g_dap.repl_mode_request_handler,
+  "repl-mode", new ReplModeRequestHandler(),
   "Get or set the repl behavior of lldb-dap evaluation requests.");
 
   g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);

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


[Lldb-commits] [lldb] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap (PR #104824)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

`SBCommand::AddCommand()` requires `SBCommandPluginInterface` to be heap based 
because it will be stored inside 
`std::shared_ptr` later for reference 
counting. But lldb-dap passes 
`StartDebuggingRequestHandler/ReplModeRequestHandler` static function pointer 
to it which will cause corruption later during destruction. 

This PR fixes this issue by making these two handler heap based. 

---
Full diff: https://github.com/llvm/llvm-project/pull/104824.diff


2 Files Affected:

- (modified) lldb/tools/lldb-dap/DAP.h (-2) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+2-2) 


``diff
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..7828272aa15a7d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;
   ReplMode repl_mode;
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..f50a6c17310739 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object 
&request) {
   "lldb-dap", "Commands for managing lldb-dap.");
   if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) {
 cmd.AddCommand(
-"startDebugging", &g_dap.start_debugging_request_handler,
+"startDebugging", new StartDebuggingRequestHandler(),
 "Sends a startDebugging request from the debug adapter to the client "
 "to start a child debug session of the same type as the caller.");
   }
   cmd.AddCommand(
-  "repl-mode", &g_dap.repl_mode_request_handler,
+  "repl-mode", new ReplModeRequestHandler(),
   "Get or set the repl behavior of lldb-dap evaluation requests.");
 
   g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);

``




https://github.com/llvm/llvm-project/pull/104824
___
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)

2024-08-19 Thread Jacob Lalonde via lldb-commits


@@ -175,6 +177,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations

Jlalond wrote:

Great idea

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] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

> Our bots and sysroot are based on the oldest supported versions of Debian, 
> based on the project's stated minimum requirements.

@ilovepi Could you say which of the versions that you are using? That would 
help, I think. 

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },

Da-Viper wrote:

> don't make this modal. It would be too obtrusive
I was looking at how other debugger does and they make the dialog modal

https://github.com/llvm/llvm-project/pull/104711
___
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)

2024-08-19 Thread Michael Buch via lldb-commits


@@ -175,6 +177,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(std::move(impl_type)), 
m_name(std::move(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) 
{
+std::lock_guard guard(m_map_mutex);
+auto iterator = m_summary_stats_map.find(provider.GetName());
+if (iterator != m_summary_stats_map.end())
+  return iterator->second;

Michael137 wrote:

I don't think `clang-format` would fix up this case here (note, this is not 
just a formatting change, I suggested moving the `iterator` variable 
declaration into the `if-statement`).

Regarding `clang-format` hanging, what version of `clang-format` are you using? 
Maybe updating it fixes it?

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)

2024-08-19 Thread Jacob Lalonde via lldb-commits

https://github.com/Jlalond edited 
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] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread Will Hawkins via lldb-commits

https://github.com/hawkinsw created 
https://github.com/llvm/llvm-project/pull/104831

Only Linux systems do not have gettid. Provide our own gettid in these cases.

Fixes a build failure caused by #104109.

>From 10bc5a512b56238328b025303345e47708927d6e Mon Sep 17 00:00:00 2001
From: Will Hawkins 
Date: Mon, 19 Aug 2024 14:43:45 -0400
Subject: [PATCH] WIP: [lldb][test] Workaround older systems that lack gettid

Only Linux systems do not have gettid. Provide our own gettid in these
cases.

Fixes a build failure caused by #104109.
---
 .../unittests/Process/elf-core/ThreadElfCoreTest.cpp | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp 
b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index ce146f62b0d826..5cf5f9718252d5 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -23,6 +23,12 @@
 #include 
 #include 
 
+#include 
+
+pid_t _workaround_gettid() {
+  return ((pid_t)syscall(SYS_gettid));
+}
+
 using namespace lldb_private;
 
 namespace {
@@ -91,7 +97,7 @@ lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, 
ArchSpec &arch) {
 
 lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
   lldb::ThreadSP thread_sp =
-  std::make_shared(*process_sp.get(), gettid());
+  std::make_shared(*process_sp.get(), _workaround_gettid());
   if (thread_sp == nullptr) {
 return nullptr;
   }
@@ -167,8 +173,8 @@ TEST_F(ElfCoreTest, PopulatePrStatusTest) {
   ASSERT_EQ(prstatus_opt->pr_cursig, 0);
   ASSERT_EQ(prstatus_opt->pr_sigpend, 0UL);
   ASSERT_EQ(prstatus_opt->pr_sighold, 0UL);
-  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(gettid()));
+  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(_workaround_gettid()));
   ASSERT_EQ(prstatus_opt->pr_ppid, static_cast(getppid()));
   ASSERT_EQ(prstatus_opt->pr_pgrp, static_cast(getpgrp()));
-  ASSERT_EQ(prstatus_opt->pr_sid, static_cast(getsid(gettid(;
+  ASSERT_EQ(prstatus_opt->pr_sid, 
static_cast(getsid(_workaround_gettid(;
 }

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },
+  openSettingsAction,
+);
+if (openSettingsAction === callBackValue) {
+  vscode.commands.executeCommand(
+"workbench.action.openSettings",
+"lldb-dap.executable-path",
+  );
+}
   }
-  return packageJSONExecutable;
+  return new vscode.DebugAdapterExecutable(path, []);

Da-Viper wrote:

the first one is when there is a directory / device/ socket (any thing that is 
not an actual file) that exists in that path 

the second one is when there is nothing in that path

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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Will Hawkins (hawkinsw)


Changes

Only Linux systems do not have gettid. Provide our own gettid in these cases.

Fixes a build failure caused by #104109.

---
Full diff: https://github.com/llvm/llvm-project/pull/104831.diff


1 Files Affected:

- (modified) lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp (+9-3) 


``diff
diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp 
b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index ce146f62b0d826..5cf5f9718252d5 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -23,6 +23,12 @@
 #include 
 #include 
 
+#include 
+
+pid_t _workaround_gettid() {
+  return ((pid_t)syscall(SYS_gettid));
+}
+
 using namespace lldb_private;
 
 namespace {
@@ -91,7 +97,7 @@ lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, 
ArchSpec &arch) {
 
 lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
   lldb::ThreadSP thread_sp =
-  std::make_shared(*process_sp.get(), gettid());
+  std::make_shared(*process_sp.get(), _workaround_gettid());
   if (thread_sp == nullptr) {
 return nullptr;
   }
@@ -167,8 +173,8 @@ TEST_F(ElfCoreTest, PopulatePrStatusTest) {
   ASSERT_EQ(prstatus_opt->pr_cursig, 0);
   ASSERT_EQ(prstatus_opt->pr_sigpend, 0UL);
   ASSERT_EQ(prstatus_opt->pr_sighold, 0UL);
-  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(gettid()));
+  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(_workaround_gettid()));
   ASSERT_EQ(prstatus_opt->pr_ppid, static_cast(getppid()));
   ASSERT_EQ(prstatus_opt->pr_pgrp, static_cast(getpgrp()));
-  ASSERT_EQ(prstatus_opt->pr_sid, static_cast(getsid(gettid(;
+  ASSERT_EQ(prstatus_opt->pr_sid, 
static_cast(getsid(_workaround_gettid(;
 }

``




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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread Will Hawkins via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

@ilovepi If you can, try https://github.com/llvm/llvm-project/pull/104831.

I think that the problem is your builder is running with a version of glibc 
that does not provide `gettid`. If the hack implemented in that PR fixes the 
problem, I will work on a nicer fix.

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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

FYI: I understand that the code in this PR is _not_ going to be the final 
answer. I am just hoping to confirm that it does, actually, fix the problem. If 
it does, then I will work on a "better" solution. 

Will


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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread 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 6e0fc155782ff5307245a85c7b037a2998ec6c86 
10bc5a512b56238328b025303345e47708927d6e --extensions cpp -- 
lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp 
b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index 5cf5f97182..0c67a65eb5 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -25,9 +25,7 @@
 
 #include 
 
-pid_t _workaround_gettid() {
-  return ((pid_t)syscall(SYS_gettid));
-}
+pid_t _workaround_gettid() { return ((pid_t)syscall(SYS_gettid)); }
 
 using namespace lldb_private;
 
@@ -176,5 +174,6 @@ TEST_F(ElfCoreTest, PopulatePrStatusTest) {
   ASSERT_EQ(prstatus_opt->pr_pid, static_cast(_workaround_gettid()));
   ASSERT_EQ(prstatus_opt->pr_ppid, static_cast(getppid()));
   ASSERT_EQ(prstatus_opt->pr_pgrp, static_cast(getpgrp()));
-  ASSERT_EQ(prstatus_opt->pr_sid, 
static_cast(getsid(_workaround_gettid(;
+  ASSERT_EQ(prstatus_opt->pr_sid,
+static_cast(getsid(_workaround_gettid(;
 }

``




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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Petr Hosek via lldb-commits

petrhosek wrote:

> > Our bots and sysroot are based on the oldest supported versions of Debian, 
> > based on the project's stated minimum requirements.
> 
> @ilovepi Could you say which of the versions that you are using? That would 
> help, I think.

It's Ubuntu 14.04 which uses glibc 2.19.

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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread Will Hawkins via lldb-commits

https://github.com/hawkinsw updated 
https://github.com/llvm/llvm-project/pull/104831

>From 76f01132b7667be48967b8d9788f610cf1a539d4 Mon Sep 17 00:00:00 2001
From: Will Hawkins 
Date: Mon, 19 Aug 2024 14:43:45 -0400
Subject: [PATCH] WIP: [lldb][test] Workaround older systems that lack gettid

Older glibc versions do not have `gettid`. Provide our own `gettid` in these
cases.

Fixes a build failure caused by #104109.
---
 lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp 
b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index ce146f62b0d826..fc952ff42a91fb 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -23,6 +23,10 @@
 #include 
 #include 
 
+#include 
+
+pid_t _workaround_gettid() { return ((pid_t)syscall(SYS_gettid)); }
+
 using namespace lldb_private;
 
 namespace {
@@ -91,7 +95,7 @@ lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, 
ArchSpec &arch) {
 
 lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
   lldb::ThreadSP thread_sp =
-  std::make_shared(*process_sp.get(), gettid());
+  std::make_shared(*process_sp.get(), _workaround_gettid());
   if (thread_sp == nullptr) {
 return nullptr;
   }
@@ -167,8 +171,8 @@ TEST_F(ElfCoreTest, PopulatePrStatusTest) {
   ASSERT_EQ(prstatus_opt->pr_cursig, 0);
   ASSERT_EQ(prstatus_opt->pr_sigpend, 0UL);
   ASSERT_EQ(prstatus_opt->pr_sighold, 0UL);
-  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(gettid()));
+  ASSERT_EQ(prstatus_opt->pr_pid, static_cast(_workaround_gettid()));
   ASSERT_EQ(prstatus_opt->pr_ppid, static_cast(getppid()));
   ASSERT_EQ(prstatus_opt->pr_pgrp, static_cast(getpgrp()));
-  ASSERT_EQ(prstatus_opt->pr_sid, static_cast(getsid(gettid(;
+  ASSERT_EQ(prstatus_opt->pr_sid, 
static_cast(getsid(_workaround_gettid(;
 }

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread via lldb-commits


@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },
+  openSettingsAction,
+);
+if (openSettingsAction === callBackValue) {
+  vscode.commands.executeCommand(
+"workbench.action.openSettings",
+"lldb-dap.executable-path",
+  );
+}
   }
-  return packageJSONExecutable;
+  return new vscode.DebugAdapterExecutable(path, []);

Da-Viper wrote:

> instead of making a try catch block here, why don't you just show the pop up 
> right away without throwing an error?

I would have to show it twice one for when the path does not exist and the 
other for when it exists and it is not a file


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


[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

2024-08-19 Thread Will Hawkins via lldb-commits

hawkinsw wrote:

> > > Our bots and sysroot are based on the oldest supported versions of 
> > > Debian, based on the project's stated minimum requirements.
> > 
> > 
> > @ilovepi Could you say which of the versions that you are using? That would 
> > help, I think.
> 
> It's Ubuntu 14.04 which uses glibc 2.19.

That's the problem! `gettid` wasn't added to glibc until 2.30. Let me know if 
the referenced PR helps!

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread via lldb-commits

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

>From 0b84d36d320a58a13ca98bd1b1c186c72bbe63e2 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Sun, 18 Aug 2024 15:26:11 +0100
Subject: [PATCH 1/3] [lldb-dap] vscode now shows a dialog when the
 dab-executable is not found

---
 lldb/tools/lldb-dap/src-ts/extension.ts | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts 
b/lldb/tools/lldb-dap/src-ts/extension.ts
index 791175f7b46224..c7802eed2a513b 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -1,4 +1,5 @@
 import * as vscode from "vscode";
+import * as fs from "node:fs/promises";
 import { LLDBDapOptions } from "./types";
 import { DisposableContext } from "./disposable-context";
 import { LLDBDapDescriptorFactory } from "./debug-adapter-factory";
@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
+  }
+
+  try {
+const fileStats = await fs.stat(path);
+if (!fileStats.isFile()) {
+  throw new Error(`Error: ${path} is not a file`);
+}
+  } catch (err) {
+const error: Error = err as Error;
+const openSettingsAction = "Open Settings";
+const callBackValue = await vscode.window.showErrorMessage(
+  error.message,
+  { modal: true },
+  openSettingsAction,
+);
+if (openSettingsAction === callBackValue) {
+  vscode.commands.executeCommand(
+"workbench.action.openSettings",
+"lldb-dap.executable-path",
+  );
+}
   }
-  return packageJSONExecutable;
+  return new vscode.DebugAdapterExecutable(path, []);
 },
   };
 }

>From 2eca3822a8ba5f1a329e459176003a30de40c683 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Sun, 18 Aug 2024 23:25:00 +0100
Subject: [PATCH 2/3] [lldb-dap] handle symbolic link

---
 lldb/tools/lldb-dap/src-ts/extension.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts 
b/lldb/tools/lldb-dap/src-ts/extension.ts
index c7802eed2a513b..7bc380a73bd443 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -25,7 +25,7 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
 
   try {
 const fileStats = await fs.stat(path);
-if (!fileStats.isFile()) {
+if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
   throw new Error(`Error: ${path} is not a file`);
 }
   } catch (err) {

>From 6b436602cd5bae4926bd7d06d9bf9047697547f5 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Mon, 19 Aug 2024 21:01:25 +0100
Subject: [PATCH 3/3] Resolve review changes

---
 lldb/tools/lldb-dap/src-ts/extension.ts | 51 +++--
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts 
b/lldb/tools/lldb-dap/src-ts/extension.ts
index 7bc380a73bd443..d4f8fb1599a3d1 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -1,5 +1,4 @@
 import * as vscode from "vscode";
-import * as fs from "node:fs/promises";
 import { LLDBDapOptions } from "./types";
 import { DisposableContext } from "./disposable-context";
 import { LLDBDapDescriptorFactory } from "./debug-adapter-factory";
@@ -23,31 +22,41 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
 return packageJSONExecutable;
   }
 
-  try {
-const fileStats = await fs.stat(path);
-if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
-  throw new Error(`Error: ${path} is not a file`);
-}
-  } catch (err) {
-const error: Error = err as Error;
-const openSettingsAction = "Open Settings";
-const callBackValue = await vscode.window.showErrorMessage(
-  error.message,
-  { modal: true },
-  openSettingsAction,
-);
-if (openSettingsAction === callBackValue) {
-  vscode.commands.executeCommand(
-"workbench.action.openSettings",
-"lldb-dap.executable-path",
-  );
-}
-  }
+  vscode.workspace.fs.stat(vscode.Uri.file(path)).then(
+(fileStats) => {
+  if (!(fileStats.type & vscode.FileType.File)) {
+showErrorMessage(path);
+  }
+},
+(_) => {
+  showErrorMessage(path);
+},
+  );
   return new vscode.DebugAdapterExecutable(path, []);
 },
   };
 }
 
+/**
+ * Shows a message box when the debug adapter's path is not found
+ */

[Lldb-commits] [lldb] [llvm] [Docs] Use cacheable myst_heading_slug_func value (PR #104847)

2024-08-19 Thread Scott Linder via lldb-commits

https://github.com/slinder1 created 
https://github.com/llvm/llvm-project/pull/104847

Avoid creating an uncacheable conf variable by using a string instead of
a function reference. Also has the effect of avoiding triggering the
"config.cache" sphinx warning.

Requires myst_parser 0.19.0 (specifically
https://github.com/executablebooks/MyST-Parser/pull/696) which is over a
year old by now. Do we mandate any minimum version for these
dependencies?


>From 27adbd2a1b495b66147245b31359eae03ebeabb1 Mon Sep 17 00:00:00 2001
From: Scott Linder 
Date: Mon, 19 Aug 2024 20:01:41 +
Subject: [PATCH] [Docs] Use cacheable myst_heading_slug_func value

Avoid creating an uncacheable conf variable by using a string instead of
a function reference. Also has the effect of avoiding triggering the
"config.cache" sphinx warning.

Requires myst_parser 0.19.0 (specifically
https://github.com/executablebooks/MyST-Parser/pull/696) which is over a
year old by now. Do we mandate any minimum version for these
dependencies?
---
 lldb/docs/conf.py | 4 +---
 llvm/docs/conf.py | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 50f4ab84f1ca66..79cc37c8c45578 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -57,10 +57,8 @@
 raise
 
 # Automatic anchors for markdown titles
-from llvm_slug import make_slug
-
 myst_heading_anchors = 6
-myst_heading_slug_func = make_slug
+myst_heading_slug_func = "llvm_slug.make_slug"
 
 autodoc_default_options = {"special-members": True}
 
diff --git a/llvm/docs/conf.py b/llvm/docs/conf.py
index 7f2ed5309606b5..a40da828ae2a25 100644
--- a/llvm/docs/conf.py
+++ b/llvm/docs/conf.py
@@ -40,10 +40,8 @@
 raise
 
 # Automatic anchors for markdown titles
-from llvm_slug import make_slug
-
 myst_heading_anchors = 6
-myst_heading_slug_func = make_slug
+myst_heading_slug_func = "llvm_slug.make_slug"
 
 
 # Add any paths that contain templates here, relative to this directory.

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


[Lldb-commits] [lldb] [llvm] [Docs] Use cacheable myst_heading_slug_func value (PR #104847)

2024-08-19 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Scott Linder (slinder1)


Changes

Avoid creating an uncacheable conf variable by using a string instead of
a function reference. Also has the effect of avoiding triggering the
"config.cache" sphinx warning.

Requires myst_parser 0.19.0 (specifically
https://github.com/executablebooks/MyST-Parser/pull/696) which is over a
year old by now. Do we mandate any minimum version for these
dependencies?


---
Full diff: https://github.com/llvm/llvm-project/pull/104847.diff


2 Files Affected:

- (modified) lldb/docs/conf.py (+1-3) 
- (modified) llvm/docs/conf.py (+1-3) 


``diff
diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 50f4ab84f1ca66..79cc37c8c45578 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -57,10 +57,8 @@
 raise
 
 # Automatic anchors for markdown titles
-from llvm_slug import make_slug
-
 myst_heading_anchors = 6
-myst_heading_slug_func = make_slug
+myst_heading_slug_func = "llvm_slug.make_slug"
 
 autodoc_default_options = {"special-members": True}
 
diff --git a/llvm/docs/conf.py b/llvm/docs/conf.py
index 7f2ed5309606b5..a40da828ae2a25 100644
--- a/llvm/docs/conf.py
+++ b/llvm/docs/conf.py
@@ -40,10 +40,8 @@
 raise
 
 # Automatic anchors for markdown titles
-from llvm_slug import make_slug
-
 myst_heading_anchors = 6
-myst_heading_slug_func = make_slug
+myst_heading_slug_func = "llvm_slug.make_slug"
 
 
 # Add any paths that contain templates here, relative to this directory.

``




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


[Lldb-commits] [lldb] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap (PR #104824)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;

walter-erquinigo wrote:

btw, do you use the REPL mode at meta?

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


[Lldb-commits] [lldb] Fix StartDebuggingRequestHandler/ReplModeRequestHandler in lldb-dap (PR #104824)

2024-08-19 Thread Walter Erquinigo via lldb-commits

https://github.com/walter-erquinigo approved this pull request.


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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits

https://github.com/vogelsgesang commented:

Thanks for looking into this! The patch already looks very good!

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits




vogelsgesang wrote:

please rebase onto `main`. This unfortunately has a merge conflict with another 
change which I recently merged

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits


@@ -17,14 +17,46 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {

vogelsgesang wrote:

even if the path is not set, we should check if the `packageJSONExecutable` 
actually exists
By default, the `path` is not set. As such, new users would not get this error 
message, although those are the ones which are most likely to run into issues

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Adrian Vogelsgesang via lldb-commits


@@ -17,14 +17,46 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
   const path = vscode.workspace
 .getConfiguration("lldb-dap", session.workspaceFolder)
 .get("executable-path");
-  if (path) {
-return new vscode.DebugAdapterExecutable(path, []);
+
+  if (!path) {
+return packageJSONExecutable;
   }
-  return packageJSONExecutable;
+
+  vscode.workspace.fs.stat(vscode.Uri.file(path)).then(
+(fileStats) => {

vogelsgesang wrote:

in addition to checking the presence of the file on launch, we could also check 
for it in the `onDidChangeConfiguration` event if the `executable-path` gets 
changed.

(100% optional; feel free to just ignore this comment 🙂)

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits


@@ -23,31 +22,41 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
 return packageJSONExecutable;
   }
 
-  try {
-const fileStats = await fs.stat(path);
-if (!fileStats.isFile() && !fileStats.isSymbolicLink()) {
-  throw new Error(`Error: ${path} is not a file`);
-}
-  } catch (err) {
-const error: Error = err as Error;
-const openSettingsAction = "Open Settings";
-const callBackValue = await vscode.window.showErrorMessage(
-  error.message,
-  { modal: true },
-  openSettingsAction,
-);
-if (openSettingsAction === callBackValue) {
-  vscode.commands.executeCommand(
-"workbench.action.openSettings",
-"lldb-dap.executable-path",
-  );
-}
-  }
+  vscode.workspace.fs.stat(vscode.Uri.file(path)).then(

walter-erquinigo wrote:

could you make `createDefaultLLDBDapOptions` be an async function so that you 
can use `await` instead of `then`? it's way more readable

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


[Lldb-commits] [lldb] [lldb-dap] show dialog when executable is not found (PR #104711)

2024-08-19 Thread Walter Erquinigo via lldb-commits

walter-erquinigo wrote:

@Da-Viper , a better way to do this is that, instead of checking for the path 
upon initialization, you add this checking logic in the 
`createDebugAdapterDescriptor` function, which only gets triggered when you 
launch the debugger. This will be less annoying to users who have the extension 
but rarely use it. 

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


[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

2024-08-19 Thread Miro Bucko via lldb-commits


@@ -691,6 +691,19 @@ def request_disassemble(
 for inst in instructions:
 self.disassembled_instructions[inst["address"]] = inst
 
+def request_read_memory(self, memoryReference, offset, count):

mbucko wrote:

for consistency this should be called `request_readMemory`

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


[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

2024-08-19 Thread Miro Bucko via lldb-commits


@@ -4028,6 +4046,161 @@ void request_disassemble(const llvm::json::Object 
&request) {
   response.try_emplace("body", std::move(body));
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
+
+// "ReadMemoryRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Reads bytes from memory at the provided location. 
Clients
+// should only call this request if the corresponding
+// capability `supportsReadMemoryRequest` is true.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "readMemory" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/ReadMemoryArguments"
+//   }
+// },
+// "required": [ "command", "arguments" ]
+//   }]
+// },
+// "ReadMemoryArguments": {
+//   "type": "object",
+//   "description": "Arguments for `readMemory` request.",
+//   "properties": {
+// "memoryReference": {

mbucko wrote:

If you look at the `memory read` command it takes start and end 
`address-expression`. If you limit this API to memoryReference it might be hard 
to extend in the future.

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


[Lldb-commits] [lldb] WIP: [lldb][test] Workaround older systems that lack gettid (PR #104831)

2024-08-19 Thread Petr Hosek via lldb-commits

petrhosek wrote:

I think a more idiomatic approach would be to use CMake to check if `gettid` is 
available:
```
check_symbol_exists(gettid "unistd.h" HAVE_GETTID)
```
Then in `lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp` you'd define 
`gettid` only `#if !HAVE_GETTID`.

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


  1   2   >