[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -969,6 +969,66 @@ Status MinidumpFileBuilder::DumpDirectories() const {
   return error;
 }
 
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+lldb_private::DataBufferHeap &data_buffer,
+const lldb_private::CoreFileMemoryRange &range, uint64_t &bytes_read) {
+
+  const lldb::addr_t addr = range.range.start();
+  const lldb::addr_t size = range.range.size();
+  Log *log = GetLog(LLDBLog::Object);
+  Status addDataError;
+  Process::ReadMemoryChunkCallback callback =
+  [&](Status &error, lldb::addr_t current_addr, const void *buf,
+  uint64_t bytes_read_for_chunk) -> lldb_private::IterationAction {
+if (error.Fail() || bytes_read_for_chunk == 0) {
+  LLDB_LOGF(log,
+"Failed to read memory region at: %" PRIx64
+". Bytes read: %zu, error: %s",
+current_addr, bytes_read_for_chunk, error.AsCString());

clayborg wrote:

Don't use `size_t`, use `lldb::offset_t`:
```
". Bytes read: %" PRIu64 ", error: %s",
current_addr, bytes_read_for_chunk, error.AsCString());
```

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -969,6 +969,66 @@ Status MinidumpFileBuilder::DumpDirectories() const {
   return error;
 }
 
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+lldb_private::DataBufferHeap &data_buffer,
+const lldb_private::CoreFileMemoryRange &range, uint64_t &bytes_read) {
+
+  const lldb::addr_t addr = range.range.start();
+  const lldb::addr_t size = range.range.size();
+  Log *log = GetLog(LLDBLog::Object);
+  Status addDataError;
+  Process::ReadMemoryChunkCallback callback =
+  [&](Status &error, lldb::addr_t current_addr, const void *buf,
+  uint64_t bytes_read_for_chunk) -> lldb_private::IterationAction {
+if (error.Fail() || bytes_read_for_chunk == 0) {
+  LLDB_LOGF(log,
+"Failed to read memory region at: %" PRIx64
+". Bytes read: %zu, error: %s",
+current_addr, bytes_read_for_chunk, error.AsCString());
+
+  // If we failed in a memory read, we would normally want to skip
+  // this entire region, if we had already written to the minidump
+  // file, we can't easily rewind that state.
+  //
+  // So if we do encounter an error while reading, we just return
+  // immediately, any prior bytes read will still be included but
+  // any bytes partially read before the error are ignored.
+  return lldb_private::IterationAction::Stop;
+}
+
+// Write to the minidump file with the chunk potentially flushing to
+// disk.
+// This error will be captured by the outer scope and is considered fatal.
+// If we get an error writing to disk we can't easily guarauntee that we
+// won't corrupt the minidump.
+addDataError = AddData(buf, bytes_read_for_chunk);
+if (addDataError.Fail())
+  return lldb_private::IterationAction::Stop;
+
+// If we have a partial read, report it, but only if the partial read
+// didn't finish reading the entire region.
+if (bytes_read_for_chunk != data_buffer.GetByteSize() &&
+current_addr + bytes_read_for_chunk != size) {
+  LLDB_LOGF(log,
+"Memory region at: %" PRIx64 " partiall read %" PRIx64

clayborg wrote:

s/partiall/partial/

You also want all addresses to be prefixed with 0x here, so the format string 
should be:
```
"Memory region at: 0x%" PRIx64 " partiall read 0x%" PRIx64
```
Same for format string below.

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -1589,6 +1589,51 @@ class Process : public 
std::enable_shared_from_this,
   size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
 Status &error);
 
+  // Callback definition for read Memory in chunks
+  //
+  // Status, the status returned from ReadMemoryFromInferior
+  // void*, pointer to the bytes read
+  // addr_t, the bytes_addr, start + bytes read so far.
+  // bytes_size, the count of bytes read for this chunk
+  typedef std::function
+  ReadMemoryChunkCallback;
+
+  /// Read of memory from a process in discrete chunks, terminating
+  /// either when all bytes are read, or the supplied callback returns
+  /// IterationAction::Stop
+  ///
+  /// \param[in] vm_addr
+  /// A virtual load address that indicates where to start reading
+  /// memory from.
+  ///
+  /// \param[in] buf
+  ///If NULL, a buffer of \a chunk_size will be created and used for the
+  ///callback. If non NULL, this buffer must be at least \a chunk_size 
bytes
+  ///and will be used for storing chunked memory reads.
+  ///
+  /// \param[in] chunk_size
+  /// The minimum size of the byte buffer, and the chunk size of memory
+  /// to read.
+  ///
+  /// \param[in] total_size
+  /// The total number of bytes to read.
+  ///
+  /// \param[in] callback
+  /// The callback to invoke when a chunk is read from memory.
+  ///
+  /// \return
+  /// The number of bytes that were actually read into \a buf and
+  /// written to the provided callback.
+  /// If the returned number is greater than zero, yet less than \a
+  /// size, then this function will get called again with \a
+  /// vm_addr, \a buf, and \a size updated appropriately. Zero is
+  /// returned in the case of an error.
+  size_t ReadMemoryInChunks(lldb::addr_t vm_addr, void *buf,
+lldb::addr_t chunk_size, size_t total_size,

clayborg wrote:

Don't use `size_t`, this is 32 bits on 32 bit systems. Use `lldb::offset_t` or 
'lldb::addr_t` which is 64 bits all the time.

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -2233,6 +2233,49 @@ size_t Process::ReadMemoryFromInferior(addr_t addr, void 
*buf, size_t size,
   return bytes_read;
 }
 
+size_t Process::ReadMemoryInChunks(lldb::addr_t vm_addr, void *buf,
+   lldb::addr_t chunk_size, size_t size,
+   ReadMemoryChunkCallback callback) {
+  // Safety check to prevent an infinite loop.
+  if (chunk_size == 0)
+return 0;
+
+  // Buffer for when a NULL buf is provided, initialized
+  // to 0 bytes, we set it to chunk_size and then replace buf
+  // with the new buffer.
+  DataBufferHeap data_buffer;
+  if (!buf) {
+data_buffer.SetByteSize(chunk_size);
+buf = data_buffer.GetBytes();
+  }
+
+  uint64_t bytes_remaining = size;
+  uint64_t bytes_read = 0;
+  Status error;
+  while (bytes_remaining > 0) {
+// Get the next read chunk size as the minimum of the remaining bytes and
+// the write chunk max size.
+const size_t bytes_to_read = std::min(bytes_remaining, chunk_size);
+const lldb::addr_t current_addr = vm_addr + bytes_read;
+const size_t bytes_read_for_chunk =
+ReadMemoryFromInferior(current_addr, buf, bytes_to_read, error);
+
+if (callback(error, current_addr, buf, bytes_read_for_chunk) ==
+IterationAction::Stop)
+  break;
+
+bytes_read += bytes_read_for_chunk;

clayborg wrote:

Move this before the callback or our return value will be wrong

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits

https://github.com/clayborg requested changes to this pull request.


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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -1589,6 +1589,51 @@ class Process : public 
std::enable_shared_from_this,
   size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
 Status &error);
 
+  // Callback definition for read Memory in chunks
+  //
+  // Status, the status returned from ReadMemoryFromInferior
+  // void*, pointer to the bytes read
+  // addr_t, the bytes_addr, start + bytes read so far.
+  // bytes_size, the count of bytes read for this chunk
+  typedef std::function
+  ReadMemoryChunkCallback;
+
+  /// Read of memory from a process in discrete chunks, terminating
+  /// either when all bytes are read, or the supplied callback returns
+  /// IterationAction::Stop
+  ///
+  /// \param[in] vm_addr
+  /// A virtual load address that indicates where to start reading
+  /// memory from.
+  ///
+  /// \param[in] buf
+  ///If NULL, a buffer of \a chunk_size will be created and used for the
+  ///callback. If non NULL, this buffer must be at least \a chunk_size 
bytes
+  ///and will be used for storing chunked memory reads.
+  ///
+  /// \param[in] chunk_size
+  /// The minimum size of the byte buffer, and the chunk size of memory
+  /// to read.
+  ///
+  /// \param[in] total_size
+  /// The total number of bytes to read.
+  ///
+  /// \param[in] callback
+  /// The callback to invoke when a chunk is read from memory.
+  ///
+  /// \return
+  /// The number of bytes that were actually read into \a buf and
+  /// written to the provided callback.
+  /// If the returned number is greater than zero, yet less than \a
+  /// size, then this function will get called again with \a
+  /// vm_addr, \a buf, and \a size updated appropriately. Zero is
+  /// returned in the case of an error.
+  size_t ReadMemoryInChunks(lldb::addr_t vm_addr, void *buf,

clayborg wrote:

Don't use `size_t`, it is 32 bits on 32 bits architectures. Use 
`lldb::offset_t` which is always 64 bits.

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


[Lldb-commits] [lldb] [LLDB] Add unary operators Dereference and AddressOf to DIL (PR #134428)

2025-04-07 Thread Pavel Labath via lldb-commits

labath wrote:

> Trying to add unit tests with this patch. We already have a big test suite 
> that came from `lldb-eval`, and I don't see much reason to rewrite them into 
> Python tests. 

> I didn't find an existing way to build the binary for debugging using in-tree 
> compiler and libc++, so had to improvise how to do it in CMake. The compiler 
> options for building I took from Python testing.

This is the reason. Long-term maintainability of the project. We already have 
(too) many different ways to build test binaries, so you need a very good 
reason to introduce another method, and frankly, I don't think you have one. If 
you don't build the test binaries with the same compiler as the rest of the 
tests, you will expose the test to all of the quirks and bugs of the host 
compiler, which means that this test will be testing a different thing for 
every user.

> > With this patch, the biggest question you will have to answer is "why is 
> > the implementation of `&` more complicated than `inner_value->AddressOf()`
> 
> It's not, the implementation itself is one line, the rest is type and error 
> checking. Same with dereference, only creating an address value first. The 
> node itself is a base for other unary operations in the future.

The fancy reference handling was my main concern, which you've now addressed. I 
haven't looked at the rest of the code too closely, so it *may* be fine, but in 
general, I think you shouldn't be looking at the type of the value in the 
evaluator: You already have the value in hand, so if the expression says you 
should dereference it, you should just ask the value to dereference itself. If 
it (and it's type system) knows how to dereference the value, it will give you 
a result. If not, you'll get an error.


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


[Lldb-commits] [lldb] [RFC][lldb-dap] Always stop on enrty for attaching (PR #134339)

2025-04-07 Thread Greg Clayton via lldb-commits

clayborg wrote:

So the issue from only the lldb-dap side here is VS Code is sending a threads 
request, but we have resumed the process upon attach after sending the 
configuration done request. So the race condition happens like:

- send configuration done
- send continue to process if stop on entry if false
- 
  - Our event thread will at some point consume the `eStateRunning` and cause 
our public API to not allow stop locker requests to succeed, but any API calls 
that come through before we consume this event will succeed,
  - at the same time as the above event thread, we get a "threads" request and 
start calling `SBProcess::GetNumThreads()` and 
`SBProcess::GetThreadAtIndex(...)` and might get valid results if the run 
locker is allowing stopped process calls. so we may start getting the number of 
threads and some threads by index, but any of these calls can fail as soon as 
we consume the `eStateRunning` process event in lldb-dap

So from a DAP only perspective, we need to sync with our lldb-dap event thread. 
Current code looks like:
```
void ConfigurationDoneRequestHandler::operator()(
const llvm::json::Object &request) const {
  llvm::json::Object response;
  FillResponse(request, response);
  dap.SendJSON(llvm::json::Value(std::move(response)));
  dap.configuration_done_sent = true;
  if (dap.stop_at_entry)
SendThreadStoppedEvent(dap);
  else
dap.target.GetProcess().Continue();
}
```
But probably should look like:
```
void ConfigurationDoneRequestHandler::operator()(
const llvm::json::Object &request) const {
  llvm::json::Object response;
  FillResponse(request, response);
  dap.SendJSON(llvm::json::Value(std::move(response)));
  dap.configuration_done_sent = true;
  if (dap.stop_at_entry)
SendThreadStoppedEvent(dap);
  else {
dap.target.GetProcess().Continue();
dap.WaitForProcessResumedEvent(); /// New sync point here!
  }
}

```

The call to `dap.WaitForProcessResumedEvent()` will make sure we have received 
the `eStateRunning` process event before continuing. Then the "threads" call 
can fail as the process stop locker won't succeed for the 
`SBProcess::GetNumThreads()` and `SBProcess::GetThreadAtIndex()` calls. 
Once we get past this race, we are good, but again, only from the DAP 
perspective.

The fact that GetNumThreads() is able to interrupt (send a CTRL+C) to the 
lldb-server is another bug that needs fixing. I thought that only the 
breakpoint setting code would interrupt a process and send GDB remote packets 
while running. Why are we allowing the threads request to interrupt?


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


[Lldb-commits] [lldb] [lldb] Synchronize access to m_statusline in the Debugger (PR #134759)

2025-04-07 Thread Alex Langford via lldb-commits


@@ -391,8 +392,13 @@ bool Debugger::SetTerminalWidth(uint64_t term_width) {
 
   if (auto handler_sp = m_io_handler_stack.Top())
 handler_sp->TerminalSizeChanged();
-  if (m_statusline)
-m_statusline->TerminalSizeChanged();
+
+  {
+// This might get called from a signal handler.
+std::unique_lock lock(m_statusline_mutex, std::try_to_lock);
+if (m_statusline)

bulbazord wrote:

I assume you only intend to call `TerminalSizeChanged` when the lock is 
acquired? This then needs to be `if (lock && m_statusline)`. Otherwise this has 
the exact same behavior with or without your change.

But one thing I'm not sure about is, if you see `SIGWINCH` and it tries to 
change the window size but fails because it can't acquire the lock, when does 
the terminal size get updated again?  Does it have to wait for another 
`SIGWINCH` or does this code run again later when the lock is free?

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


[Lldb-commits] [lldb] [lldb] Synchronize access to m_statusline in the Debugger (PR #134759)

2025-04-07 Thread Alex Langford via lldb-commits


@@ -391,8 +392,13 @@ bool Debugger::SetTerminalWidth(uint64_t term_width) {
 
   if (auto handler_sp = m_io_handler_stack.Top())
 handler_sp->TerminalSizeChanged();
-  if (m_statusline)
-m_statusline->TerminalSizeChanged();
+
+  {
+// This might get called from a signal handler.
+std::unique_lock lock(m_statusline_mutex, std::try_to_lock);
+if (m_statusline)

bulbazord wrote:

I do recognize that this does protect other threads from touching 
`m_statusline` some of the time, but it seems like this can't protect you from 
every race?

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


[Lldb-commits] [lldb] [LLDB] Fix warnings in DIL. (PR #134778)

2025-04-07 Thread via lldb-commits

https://github.com/cmtice created 
https://github.com/llvm/llvm-project/pull/134778

This fixes 3 warnings from compiling the DILParser:

DILParser.h:53:12: warning: returning address of local temporary object 
[-Wreturn-stack-address]

DILParser.h:119:8: warning: private field 'm_fragile_ivar' is not used 
[-Wunused-private-field]

DILParser.h:120:8: warning: private field 'm_check_ptr_vs_member' is not used 
[-Wunused-private-field]

>From a683bad89abeef9d1adacc99bb6ba3aa3b75a35e Mon Sep 17 00:00:00 2001
From: Caroline Tice 
Date: Mon, 7 Apr 2025 20:51:08 -0700
Subject: [PATCH] [LLDB] Fix warnings in DIL.

This fixes 3 warnings from compiling the DILParser:

DILParser.h:53:12: warning: returning address of local temporary object
[-Wreturn-stack-address]

DILParser.h:119:8: warning: private field 'm_fragile_ivar' is not used
[-Wunused-private-field]

DILParser.h:120:8: warning: private field 'm_check_ptr_vs_member' is not used
[-Wunused-private-field]
---
 lldb/include/lldb/ValueObject/DILParser.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/ValueObject/DILParser.h 
b/lldb/include/lldb/ValueObject/DILParser.h
index 9b7a6cd487939..d1a991c2cc94c 100644
--- a/lldb/include/lldb/ValueObject/DILParser.h
+++ b/lldb/include/lldb/ValueObject/DILParser.h
@@ -50,7 +50,7 @@ class DILDiagnosticError
   }
 
   llvm::ArrayRef GetDetails() const override {
-return {m_detail};
+return m_detail;
   }
 
   std::string message() const override { return m_detail.rendered; }
@@ -116,8 +116,10 @@ class DILParser {
 
   lldb::DynamicValueType m_use_dynamic;
   bool m_use_synthetic;
-  bool m_fragile_ivar;
-  bool m_check_ptr_vs_member;
+  // The following are not currently used, but will be used as more
+  // functionality is added to DIL.
+  bool m_fragile_ivar __attribute__((unused));
+  bool m_check_ptr_vs_member __attribute__((unused));
 }; // class DILParser
 
 } // namespace lldb_private::dil

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


[Lldb-commits] [lldb] [lldb] Use correct path for debugserver (PR #131609)

2025-04-07 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Use correct path for debugserver (PR #131609)

2025-04-07 Thread Pavel Labath via lldb-commits


@@ -4,6 +4,7 @@
 """
 
 import os
+import signal

labath wrote:

I guess this isn't used anymore.

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


[Lldb-commits] [lldb] [lldb][lldb-dap] explicitly set the expr as an alias for expression. (PR #134562)

2025-04-07 Thread Ebuka Ezike via lldb-commits

da-viper wrote:

Another solution I could think of is prioritising, the repl command type 
instead of the repl variable type. 
in 

https://github.com/llvm/llvm-project/blob/4607d39c7eded3ff6d425cbc502e30349078365c/lldb/tools/lldb-dap/DAP.cpp#L553-L558

or add the previously defined aliases in docs explicitly as aliases. 

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Jacob Lalonde via lldb-commits


@@ -969,6 +969,64 @@ Status MinidumpFileBuilder::DumpDirectories() const {
   return error;
 }
 
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+lldb_private::DataBufferHeap &data_buffer,
+const lldb_private::CoreFileMemoryRange &range, uint64_t &bytes_read) {
+
+  Log *log = GetLog(LLDBLog::Object);
+  Status addDataError;
+  Process::ReadMemoryChunkCallback callback =
+  [&](Status &error, DataBufferHeap &data, lldb::addr_t current_addr,
+  uint64_t bytes_to_read,
+  uint64_t bytes_read_for_chunk) -> lldb_private::IterationAction {
+if (error.Fail() || bytes_read_for_chunk == 0) {
+  LLDB_LOGF(log,
+"Failed to read memory region at: %" PRIx64
+". Bytes read: %zu, error: %s",
+current_addr, bytes_read_for_chunk, error.AsCString());
+
+  // If we failed in a memory read, we would normally want to skip
+  // this entire region, if we had already written to the minidump
+  // file, we can't easily rewind that state.
+  //
+  // So if we do encounter an error while reading, we just return
+  // immediately, any prior bytes read will still be included but
+  // any bytes partially read before the error are ignored.
+  return lldb_private::IterationAction::Stop;
+}
+
+// Write to the minidump file with the chunk potentially flushing to
+// disk.
+// This error will be captured by the outer scope and is considered fatal.
+// If we get an error writing to disk we can't easily guarauntee that we
+// won't corrupt the minidump.
+addDataError = AddData(data_buffer.GetBytes(), bytes_read_for_chunk);
+if (addDataError.Fail())
+  return lldb_private::IterationAction::Stop;
+
+if (bytes_read_for_chunk != bytes_to_read) {

Jlalond wrote:

We're already passing `bytes_read_for_chunk` and `bytes_to_read` which is the 
expected size, so I didn't need to change the lambda here

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


[Lldb-commits] [lldb] [LLDB][NFC] Remove Debugger dependency in SystemLifetimeManager (PR #134383)

2025-04-07 Thread Pavel Labath via lldb-commits

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

> So, I will move LoadPlugin lambda to SystemInitializerFull.

Thanks, this LGTM now, but do wait for @JDevlieghere's review as well.

> Note lldbInitialization depends on lldbPluginProcessGDBRemote, 
> lldbPluginProcessPOSIX and lldbPluginProcessWindowsCommon. 
> SystemInitializerCommon::Initialize() calls 
> process_gdb_remote::ProcessGDBRemoteLog::Initialize(), 
> ProcessPOSIXLog::Initialize() and ProcessWindowsLog::Initialize(). If it is 
> OK for Utility, I will move lldbInitialization into lldbUtility. 

Ah yes, I'm not sure how I missed these, but that explains why these are a 
separate library. These deps aren't OK for Utililty, though I am not sure they 
actually need to be here. I think the log classes should be initialized from 
their respective plugins' `Initialize` methods. It also calls FileSystem and 
HostInfo initialization functions, which also aren't OK, but maybe we could do 
something about these too (in particular, we already need to jump through some 
hoops in order to pass arguments into HostInfo::Initialize). I may have some 
time to do some "casual coding" next week, so I'll try to come up with 
something here.

> I would prefer to move lldbInitialization into lldbUtility with a separate 
> patch.

definitely

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


[Lldb-commits] [lldb] [LLDB] Refactored CPlusPlusLanguage::MethodName to break lldb-server dependencies (PR #132274)

2025-04-07 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [LLDB] Refactored CPlusPlusLanguage::MethodName to break lldb-server dependencies (PR #132274)

2025-04-07 Thread Pavel Labath via lldb-commits


@@ -222,6 +222,21 @@ ObjCLanguage::GetMethodNameVariants(ConstString 
method_name) const {
   return variant_names;
 }
 
+std::pair
+ObjCLanguage::GetFunctionNameInfo(ConstString name) const {
+  FunctionNameType func_name_type = eFunctionNameTypeNone;
+
+  if (ObjCLanguage::IsPossibleObjCMethodName(name.GetCString())) {
+func_name_type = eFunctionNameTypeFull;
+  }
+
+  if (ObjCLanguage::IsPossibleObjCSelector(name.GetCString())) {
+func_name_type |= eFunctionNameTypeSelector;
+  }

labath wrote:

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

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


[Lldb-commits] [lldb] [LLDB] Refactored CPlusPlusLanguage::MethodName to break lldb-server dependencies (PR #132274)

2025-04-07 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

I like the version a lot, but I'd like to leave to final approval to @bulbazord 
and/or @JDevlieghere.

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -71,6 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
  debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
+  module.try_emplace("symbolDownloadTime", symbol_download_time);

youngd007 wrote:

I think the PR summary should make explicit mention that THIS is the new key so 
anyone looking at history can figure out if without needing to examine the diff 
contents itself.

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


[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -188,17 +188,27 @@ GetFileForModule(const ModuleSpec &module_spec,
   std::string cache_file_name = llvm::toHex(build_id, true);
   if (!file_name.empty())
 cache_file_name += "-" + file_name.str();
-  llvm::Expected result = llvm::getCachedOrDownloadArtifact(
-  cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
-  if (result)
-return FileSpec(*result);
-
-  Log *log = GetLog(LLDBLog::Symbols);
-  auto err_message = llvm::toString(result.takeError());
-  LLDB_LOGV(log,
-"Debuginfod failed to download symbol artifact {0} with error {1}",
-url_path, err_message);
-  return {};
+  StatsDuration duration;
+  ElapsedTime elapased(duration);

youngd007 wrote:

Also, notice this has 'elapsed' misspelled as *elapased*, which is DIFFERENT 
than the variable below.  I assume this is wrong.

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


[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)

2025-04-07 Thread via lldb-commits

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r HEAD~1...HEAD 
lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
``





View the diff from darker here.


``diff
--- TestTargetReadInstructionsFlavor.py 2025-04-07 13:23:51.00 +
+++ TestTargetReadInstructionsFlavor.py 2025-04-07 13:29:57.237559 +
@@ -5,11 +5,10 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
 class TargetReadInstructionsFlavor(TestBase):
-
 @skipIf(archs=no_match(["x86_64", "x86", "i386"]), oslist=["windows"])
 def test_read_instructions_with_flavor(self):
 self.build()
 executable = self.getBuildArtifact("a.out")
 

``




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


[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)

2025-04-07 Thread Ebuka Ezike via lldb-commits

https://github.com/da-viper updated 
https://github.com/llvm/llvm-project/pull/134626

>From 6c8b0a3dcb33eeb2fe57325a792ff5a70225d18e Mon Sep 17 00:00:00 2001
From: Ebuka Ezike 
Date: Mon, 7 Apr 2025 10:24:02 +0100
Subject: [PATCH 1/3] [lldb] Fix SBTarget::ReadInstruction

The disassemblyBytes parameters are not ordered correctly and crashes when the 
read instruction is called
---
 lldb/source/API/SBTarget.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 0fed1bbfed6a7..b42ada42b0931 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2021,9 +2021,9 @@ lldb::SBInstructionList 
SBTarget::ReadInstructions(lldb::SBAddress base_addr,
 error, force_live_memory, &load_addr);
   const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
   sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
-  target_sp->GetArchitecture(), nullptr, 
target_sp->GetDisassemblyCPU(),
-  target_sp->GetDisassemblyFeatures(), flavor_string, *addr_ptr,
-  data.GetBytes(), bytes_read, count, data_from_file));
+  target_sp->GetArchitecture(), nullptr, flavor_string,
+  target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(),
+  *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file));
 }
   }
 

>From 4be5e33bf7b986e35808b83758e4a8d85b646a81 Mon Sep 17 00:00:00 2001
From: Ebuka Ezike 
Date: Mon, 7 Apr 2025 14:23:51 +0100
Subject: [PATCH 2/3] [lldb] Add test for SBTarget reading instructions with
 flavor

This commit introduces a new test for verifying the `SBTarget` API's ability to 
read and validate disassembled instructions with a specified flavor ("intel").

Signed-off-by: Ebuka Ezike 
---
 .../target/read-instructions-flavor/Makefile  |  3 ++
 .../TestTargetReadInstructionsFlavor.py   | 40 +++
 .../target/read-instructions-flavor/main.c| 21 ++
 3 files changed, 64 insertions(+)
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/Makefile
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/main.c

diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/Makefile 
b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git 
a/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
new file mode 100644
index 0..e93c8476d8e00
--- /dev/null
+++ 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
@@ -0,0 +1,40 @@
+"""
+Test SBTarget Read Instruction.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TargetReadInstructionsFlavor(TestBase):
+
+@skipIf(archs=no_match(["x86_64", "x86", "i386"]), oslist=["windows"])
+def test_read_instructions_with_flavor(self):
+self.build()
+executable = self.getBuildArtifact("a.out")
+
+# create a target
+target = self.dbg.CreateTarget(executable)
+self.assertTrue(target.IsValid(), "target is not valid")
+
+functions = target.FindFunctions("test_add")
+self.assertEqual(len(functions), 1)
+test_add = functions[0]
+
+test_add_symbols = test_add.GetSymbol()
+self.assertTrue(
+test_add_symbols.IsValid(), "test_add function symbols is not 
valid"
+)
+
+expected_instructions = (("mov", "eax, edi"), ("add", "eax, esi"), 
("ret", ""))
+test_add_insts = test_add_symbols.GetInstructions(target, "intel")
+# clang adds an extra nop instruction but gcc does not. It makes more 
sense
+# to check if it is at least 3
+self.assertLessEqual(len(expected_instructions), len(test_add_insts))
+
+# compares only the expected instructions
+for expected_instr, instr in zip(expected_instructions, 
test_add_insts):
+self.assertTrue(instr.IsValid(), "instruction is not valid")
+expected_mnemonic, expected_op_str = expected_instr
+self.assertEqual(instr.GetMnemonic(target), expected_mnemonic)
+self.assertEqual(instr.GetOperands(target), expected_op_str)
diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/main.c 
b/lldb/test/API/python_api/target/read-instructions-flavor/main.c
new file mode 100644
index 0..6022d63fb6ed7
--- /dev/null
+++ b/lldb/test/API/python_api/target/read-instructions-fla

[Lldb-commits] [lldb] [lldb] Remove unused UnwindPlan functions (PR #134630)

2025-04-07 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

`GetLSDAAddress` and `GetPersonalityRoutinePtrAddress` are unused and they 
create a bit of a problem for discontinuous functions, because the unwind plan 
for these consists of multiple eh_frame descriptors and (at least in theory) 
each of them could have a different value for these entities.

We could say we only support functions for which these are always the same, or 
create some sort of a Address2LSDA lookup map, but I think it's better to leave 
this question to someone who actually needs this.

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


6 Files Affected:

- (modified) lldb/include/lldb/Symbol/FuncUnwinders.h (-13) 
- (modified) lldb/include/lldb/Symbol/UnwindPlan.h (+1-22) 
- (modified) lldb/source/Symbol/CompactUnwindInfo.cpp (-12) 
- (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+4-34) 
- (modified) lldb/source/Symbol/FuncUnwinders.cpp (-36) 
- (modified) lldb/source/Symbol/UnwindPlan.cpp (-13) 


``diff
diff --git a/lldb/include/lldb/Symbol/FuncUnwinders.h 
b/lldb/include/lldb/Symbol/FuncUnwinders.h
index 479ccf87b6e2c..c21a1af5c56a2 100644
--- a/lldb/include/lldb/Symbol/FuncUnwinders.h
+++ b/lldb/include/lldb/Symbol/FuncUnwinders.h
@@ -61,19 +61,6 @@ class FuncUnwinders {
 });
   }
 
-  // A function may have a Language Specific Data Area specified -- a block of
-  // data in
-  // the object file which is used in the processing of an exception throw /
-  // catch. If any of the UnwindPlans have the address of the LSDA region for
-  // this function, this will return it.
-  Address GetLSDAAddress(Target &target);
-
-  // A function may have a Personality Routine associated with it -- used in 
the
-  // processing of throwing an exception.  If any of the UnwindPlans have the
-  // address of the personality routine, this will return it.  Read the target-
-  // pointer at this address to get the personality function address.
-  Address GetPersonalityRoutinePtrAddress(Target &target);
-
   // The following methods to retrieve specific unwind plans should rarely be
   // used. Instead, clients should ask for the *behavior* they are looking for,
   // using one of the above UnwindPlan retrieval methods.
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h 
b/lldb/include/lldb/Symbol/UnwindPlan.h
index 6640a23a3e868..eee932492a550 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -445,9 +445,7 @@ class UnwindPlan {
 m_plan_is_sourced_from_compiler(rhs.m_plan_is_sourced_from_compiler),
 m_plan_is_valid_at_all_instruction_locations(
 rhs.m_plan_is_valid_at_all_instruction_locations),
-m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap),
-m_lsda_address(rhs.m_lsda_address),
-m_personality_func_addr(rhs.m_personality_func_addr) {
+m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap) {
 m_row_list.reserve(rhs.m_row_list.size());
 for (const RowSP &row_sp : rhs.m_row_list)
   m_row_list.emplace_back(new Row(*row_sp));
@@ -553,22 +551,10 @@ class UnwindPlan {
 m_plan_is_sourced_from_compiler = eLazyBoolCalculate;
 m_plan_is_valid_at_all_instruction_locations = eLazyBoolCalculate;
 m_plan_is_for_signal_trap = eLazyBoolCalculate;
-m_lsda_address.Clear();
-m_personality_func_addr.Clear();
   }
 
   const RegisterInfo *GetRegisterInfo(Thread *thread, uint32_t reg_num) const;
 
-  Address GetLSDAAddress() const { return m_lsda_address; }
-
-  void SetLSDAAddress(Address lsda_addr) { m_lsda_address = lsda_addr; }
-
-  Address GetPersonalityFunctionPtr() const { return m_personality_func_addr; }
-
-  void SetPersonalityFunctionPtr(Address presonality_func_ptr) {
-m_personality_func_addr = presonality_func_ptr;
-  }
-
 private:
   std::vector m_row_list;
   std::vector m_plan_valid_ranges;
@@ -583,13 +569,6 @@ class UnwindPlan {
   lldb_private::LazyBool m_plan_is_sourced_from_compiler;
   lldb_private::LazyBool m_plan_is_valid_at_all_instruction_locations;
   lldb_private::LazyBool m_plan_is_for_signal_trap;
-
-  Address m_lsda_address; // Where the language specific data area exists in 
the
-  // module - used
-  // in exception handling.
-  Address m_personality_func_addr; // The address of a pointer to the
-   // personality function - used in
-   // exception handling.
 }; // class UnwindPlan
 
 } // namespace lldb_private
diff --git a/lldb/source/Symbol/CompactUnwindInfo.cpp 
b/lldb/source/Symbol/CompactUnwindInfo.cpp
index 3c97d2ca11fbc..cdbbeb554c688 100644
--- a/lldb/source/Symbol/CompactUnwindInfo.cpp
+++ b/lldb/source/Symbol/CompactUnwindInfo.cpp
@@ -741,9 +741,6 @@ bool CompactUnwindInfo::CreateUnwindPlan_x86_64(Target 
&target,
   unwind_plan.SetUnwindPlanForSignalTrap(eLazyBool

[Lldb-commits] [lldb] [lldb] Clear thread name container before writing UTF8 bytes (PR #134150)

2025-04-07 Thread Saleem Abdulrasool via lldb-commits

compnerd wrote:

UTF-8 is a multibyte encoding, and if there is existing content in the output 
string, the generated result may not be a valid string, if you passed in a 
buffer with `\xe0` and then convert the input of `a`, we would get something 
that is invalid.

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


[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [lldb] [LLDB][NFC] Remove Debugger dependency in SystemLifetimeManager (PR #134383)

2025-04-07 Thread Pavel Labath via lldb-commits

labath wrote:

I agree with everything Jonas said here (and that's why I am very reluctant to 
approve #132274). The layering should be between libraries (like they are in 
the rest of llvm), not individual object files. My mental model for a 
"dependency" from library `A` to library `B` is "library `A` (it's source 
files)  "mentions" an entity defined in library `B`". It doesn't matter whether 
this pulls in some object files/functions/etc. from the other library during 
linking or not. If there's no dependency, it shouldn't even be aware of its 
existence. You should be able to delete all files from library B, and library A 
should still build fine. I'm not saying I won't approve anything else, but I 
consider anything below this a compromise.

Now, when it comes to the "Initialization" library(*), before it "plugin load" 
callback was introduced, it actually satisfied this criterion. The overall idea 
was that the individual users (lldb-server, liblldb, etc.) put their 
initialization code into their SystemInitializer subclass. 
SystemLifetimeManager was just a RAII wrapper around that class. The plugin 
load patch broke that by adding the dependency directly into the 
SystemLifetimeManager.

This patch removes the dependency as far as the linker is concerned, but 
doesn't remove the dependency according to the definition above. The 
SystemLifetimeManager (and the Initialization library by extension) still 
"depends" on the the Debugger object through `LoadPluginCallbackType` typedef 
(the debugger is an argument of the callback). (The `LoadPluginCallbackType` 
type itself is also a problem since it's an liblldb-specific concept and 
lldb-server wants nothing to do with that).

The right way to fix this is to bypass the SystemLifetimeManager class 
completely, and pass the callback directly into `SystemInitializerFull`. The 
only thing which makes that difficult is that the callback is a lambda defined 
in SBDebugger.cpp. However, there's no reason it has to be defined there -- the 
lambda itself doesn't depend on anything, and it could easily be moved to 
SystemInitializerFull.cpp.

TL;DR: I believe you should move the lambda to SystemInitializerFull.cpp, so 
that it can be directly referenced from `SystemInitializerFull::Initialize`.  I 
believe this should also address Jonas's concerns about library dependencies.

(*) I think having an entire library dedicated to "initialization" is overkill. 
AFAICT, after doing the above, this code will not depend on anything else, so I 
think it'd be best to just move it into the `Utility` library. I'm saying this 
is because I think the main cause of this problem is that people aren't aware 
the intention behind this library (just in case you're wondering, this wasn't 
my idea), but I think that by now, people have internalized the idea that 
"Utility" should have no external dependencies.

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


[Lldb-commits] [lldb] [LLDB] Add unary operators Dereference and AddressOf to DIL (PR #134428)

2025-04-07 Thread Pavel Labath via lldb-commits

labath wrote:

I see this is just a draft, but in order to save time for everyone, I want to 
say this as early as possible.

With this patch, the biggest question you will have to answer is "why is the 
implementation of `&` more complicated than `inner_value->AddressOf()`" (and a 
similar question for dereferencing). I think I know why you did this (because 
you want `&variable_of_reference_type` to behave the same way as is C++). I 
understand that desire, but I don't think this is the way to achieve that. The 
implementation of `AddressOf` is intentional and it kind of makes sense -- 
despite of what the C++ standard says, (most) references do have an address, so 
it makes sense for a debugger to have a way to return that. I think it would 
also make sense (and I might even prefer that) if `AddressOf` returned the 
address of the referenced object (the "value" of the reference), but it's not 
how it works right now -- neither with `AddressOf` nor with the existing 
implementation of `frame var`. What *doesn't* make sense to me is for 
`AddressOf` and `frame var` do to something different. If we want to change how 
this works, the change should probably be inside `AddressOf`, so that it 
applies the same everywhere. Either way, it will require more discussion. If 
that's something you want to take on now, then we probably ought to move this 
to the RFC. If not, then I'd suggest sticking with the status quo (and *maybe* 
coming back to this later).

I'm also not very convinced by this "FlowAnalysis" thingy. Is it supposed to be 
a performance optimization (collapsing `*&foo` to `foo`) or does it do 
something more? I mean, the optimization conceptually makes sense, but I'm 
somewhat surprised that it makes a difference in practice. Is there a 
particular pattern you want to optimize with this? I'd be surprised if users 
actually typed expressions like this.

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


[Lldb-commits] [lldb] [lldb] Use correct path for debugserver (PR #131609)

2025-04-07 Thread Pavel Labath via lldb-commits


@@ -58,3 +59,45 @@ def test_platform_process_launch_gdb_server(self):
 
 self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
 self.expect("run", substrs=["unable to launch a GDB server on"], 
error=True)
+
+@skipIfRemote
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_lldb_server_weird_symlinks(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+]
+
+# Run lldb-server from a symlink without any binary called 
"lldb-server" in the directory.
+llgs_hiding_directory = self.getBuildArtifact("hiding-directory")
+new_lldb_server_link = self.getBuildArtifact(
+"lldb-server-with-an-unconventional-name"
+)
+new_lldb_server = os.path.join(llgs_hiding_directory, "lldb-server")
+os.makedirs(llgs_hiding_directory)
+shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+os.symlink(new_lldb_server, new_lldb_server_link)
+
+proc = self.spawnSubprocess(new_lldb_server_link, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# So that lldb-server doesn't crash over SIGHUP
+os.kill(proc.pid, signal.SIGTERM)

labath wrote:

Okay, yes, I think I've seen this error as well. It probably is another bug (in 
lldb-server, or in the test suite). If it does not impact the result of the 
test, let's remove this, as this should be handled generally.

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


[Lldb-commits] [lldb] [lldb] Use correct path for debugserver (PR #131609)

2025-04-07 Thread Yuval Deutscher via lldb-commits

https://github.com/yuvald-sweet-security updated 
https://github.com/llvm/llvm-project/pull/131609

>From 6f2d070facaced221295a5b0c48ccb3a41a5048d Mon Sep 17 00:00:00 2001
From: Yuval Deutscher 
Date: Mon, 17 Mar 2025 14:37:26 +0200
Subject: [PATCH 1/2] [lldb] Use correct path for debugserver

---
 lldb/tools/lldb-server/SystemInitializerLLGS.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-server/SystemInitializerLLGS.h 
b/lldb/tools/lldb-server/SystemInitializerLLGS.h
index 4469a8ba5f60a..c6020b0dd37da 100644
--- a/lldb/tools/lldb-server/SystemInitializerLLGS.h
+++ b/lldb/tools/lldb-server/SystemInitializerLLGS.h
@@ -11,10 +11,17 @@
 
 #include "lldb/Initialization/SystemInitializer.h"
 #include "lldb/Initialization/SystemInitializerCommon.h"
+#include "lldb/Utility/FileSpec.h"
 
 class SystemInitializerLLGS : public lldb_private::SystemInitializerCommon {
 public:
-  SystemInitializerLLGS() : SystemInitializerCommon(nullptr) {}
+  SystemInitializerLLGS()
+  : SystemInitializerCommon(
+// Finding the shared libraries directory on lldb-server is broken
+// since lldb-server isn't dynamically linked with liblldb.so.
+// Clearing the filespec here causes GetShlibDir to fail and
+// GetSupportExeDir to fall-back to using the binary path instead.
+[](lldb_private::FileSpec &file) { file.Clear(); }) {}
 
   llvm::Error Initialize() override;
   void Terminate() override;

>From 67b33af6291120c712b6219d5faa0f633f6be392 Mon Sep 17 00:00:00 2001
From: Yuval Deutscher 
Date: Thu, 3 Apr 2025 14:52:43 +0300
Subject: [PATCH 2/2] [lldb] Test running lldb-server through symlink

---
 .../TestPlatformLaunchGDBServer.py| 40 +++
 1 file changed, 40 insertions(+)

diff --git 
a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
 
b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
index c365bc993e338..27c39a9e29cf5 100644
--- 
a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
+++ 
b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -4,6 +4,7 @@
 """
 
 import os
+import signal
 import socket
 import shutil
 import lldbgdbserverutils
@@ -58,3 +59,42 @@ def test_platform_process_launch_gdb_server(self):
 
 self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
 self.expect("run", substrs=["unable to launch a GDB server on"], 
error=True)
+
+@skipIfRemote
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_lldb_server_weird_symlinks(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+]
+
+# Run lldb-server from a symlink without any binary called 
"lldb-server" in the directory.
+new_lldb_server = self.getBuildArtifact(
+"lldb-server-with-an-unconventional-name"
+)
+os.symlink(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+
+proc = self.spawnSubprocess(new_lldb_server, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+self.expect(
+"process status",
+patterns=["Process .* exited with status = 0"],
+)

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


[Lldb-commits] [lldb] [lldb] Use correct path for debugserver (PR #131609)

2025-04-07 Thread Yuval Deutscher via lldb-commits


@@ -58,3 +59,45 @@ def test_platform_process_launch_gdb_server(self):
 
 self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
 self.expect("run", substrs=["unable to launch a GDB server on"], 
error=True)
+
+@skipIfRemote
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_lldb_server_weird_symlinks(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+]
+
+# Run lldb-server from a symlink without any binary called 
"lldb-server" in the directory.
+llgs_hiding_directory = self.getBuildArtifact("hiding-directory")
+new_lldb_server_link = self.getBuildArtifact(
+"lldb-server-with-an-unconventional-name"
+)
+new_lldb_server = os.path.join(llgs_hiding_directory, "lldb-server")
+os.makedirs(llgs_hiding_directory)
+shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+os.symlink(new_lldb_server, new_lldb_server_link)
+
+proc = self.spawnSubprocess(new_lldb_server_link, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.runCmd("run")
+
+# So that lldb-server doesn't crash over SIGHUP
+os.kill(proc.pid, signal.SIGTERM)

yuvald-sweet-security wrote:

Alright, done.

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


[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)

2025-04-07 Thread Ebuka Ezike via lldb-commits

https://github.com/da-viper created 
https://github.com/llvm/llvm-project/pull/134626

When you call the `SBTarget::ReadInstructions` with flavor from lldb crashes. 
This is because the wrong order of the `DisassemblyBytes` constructor this 
fixes that

>From 6c8b0a3dcb33eeb2fe57325a792ff5a70225d18e Mon Sep 17 00:00:00 2001
From: Ebuka Ezike 
Date: Mon, 7 Apr 2025 10:24:02 +0100
Subject: [PATCH 1/2] [lldb] Fix SBTarget::ReadInstruction

The disassemblyBytes parameters are not ordered correctly and crashes when the 
read instruction is called
---
 lldb/source/API/SBTarget.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 0fed1bbfed6a7..b42ada42b0931 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2021,9 +2021,9 @@ lldb::SBInstructionList 
SBTarget::ReadInstructions(lldb::SBAddress base_addr,
 error, force_live_memory, &load_addr);
   const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
   sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
-  target_sp->GetArchitecture(), nullptr, 
target_sp->GetDisassemblyCPU(),
-  target_sp->GetDisassemblyFeatures(), flavor_string, *addr_ptr,
-  data.GetBytes(), bytes_read, count, data_from_file));
+  target_sp->GetArchitecture(), nullptr, flavor_string,
+  target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(),
+  *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file));
 }
   }
 

>From 4be5e33bf7b986e35808b83758e4a8d85b646a81 Mon Sep 17 00:00:00 2001
From: Ebuka Ezike 
Date: Mon, 7 Apr 2025 14:23:51 +0100
Subject: [PATCH 2/2] [lldb] Add test for SBTarget reading instructions with
 flavor

This commit introduces a new test for verifying the `SBTarget` API's ability to 
read and validate disassembled instructions with a specified flavor ("intel").

Signed-off-by: Ebuka Ezike 
---
 .../target/read-instructions-flavor/Makefile  |  3 ++
 .../TestTargetReadInstructionsFlavor.py   | 40 +++
 .../target/read-instructions-flavor/main.c| 21 ++
 3 files changed, 64 insertions(+)
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/Makefile
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 create mode 100644 
lldb/test/API/python_api/target/read-instructions-flavor/main.c

diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/Makefile 
b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git 
a/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
new file mode 100644
index 0..e93c8476d8e00
--- /dev/null
+++ 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
@@ -0,0 +1,40 @@
+"""
+Test SBTarget Read Instruction.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TargetReadInstructionsFlavor(TestBase):
+
+@skipIf(archs=no_match(["x86_64", "x86", "i386"]), oslist=["windows"])
+def test_read_instructions_with_flavor(self):
+self.build()
+executable = self.getBuildArtifact("a.out")
+
+# create a target
+target = self.dbg.CreateTarget(executable)
+self.assertTrue(target.IsValid(), "target is not valid")
+
+functions = target.FindFunctions("test_add")
+self.assertEqual(len(functions), 1)
+test_add = functions[0]
+
+test_add_symbols = test_add.GetSymbol()
+self.assertTrue(
+test_add_symbols.IsValid(), "test_add function symbols is not 
valid"
+)
+
+expected_instructions = (("mov", "eax, edi"), ("add", "eax, esi"), 
("ret", ""))
+test_add_insts = test_add_symbols.GetInstructions(target, "intel")
+# clang adds an extra nop instruction but gcc does not. It makes more 
sense
+# to check if it is at least 3
+self.assertLessEqual(len(expected_instructions), len(test_add_insts))
+
+# compares only the expected instructions
+for expected_instr, instr in zip(expected_instructions, 
test_add_insts):
+self.assertTrue(instr.IsValid(), "instruction is not valid")
+expected_mnemonic, expected_op_str = expected_instr
+self.assertEqual(instr.GetMnemonic(target), expected_mnemonic)
+self.assertEqual(instr.GetOperands(target), expected_op_str)
diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/main.c 
b/lldb/test/API/python_api/t

[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)

2025-04-07 Thread Ebuka Ezike via lldb-commits

da-viper wrote:

I was also wondering if this should be backported as it also affects llvm 20.x 
branch 

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


[Lldb-commits] [lldb] [lldb][lldb-dap] explicitly set the expr as an alias for expression. (PR #134562)

2025-04-07 Thread via lldb-commits

jimingham wrote:

The lldb command line was designed so that it always does shortest complete 
match, and there are a lot of common commands that people only type partially.  
Adding one more expr alias isn't such a big deal, but adding special purpose 
aliases for all the shortenings people want to use is not supportable (and 
might collide with other aliases users have set).
So we need a better solution to this DAP problem than just adding aliases.

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits

https://github.com/clayborg requested changes to this pull request.


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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -969,6 +969,64 @@ Status MinidumpFileBuilder::DumpDirectories() const {
   return error;
 }
 
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+lldb_private::DataBufferHeap &data_buffer,
+const lldb_private::CoreFileMemoryRange &range, uint64_t &bytes_read) {
+
+  Log *log = GetLog(LLDBLog::Object);
+  Status addDataError;
+  Process::ReadMemoryChunkCallback callback =
+  [&](Status &error, DataBufferHeap &data, lldb::addr_t current_addr,
+  uint64_t bytes_to_read,
+  uint64_t bytes_read_for_chunk) -> lldb_private::IterationAction {

clayborg wrote:

Update to suggested callback prototype above.

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


[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits

clayborg wrote:

> Either no one is going to use this except experts, or people will start 
> cargo-culting "turn off plugin X and lldb will be faster" and then we get 
> "Why doesn't 'obvious thing'" work bug reports, to which the answer would be 
> "turn on plugin X" but people may not even know they have this off (a 
> surprising number of folks don't really know what's in the .lldbinit they got 
> from their helpful colleagues...) I don't want to have to have another round 
> of information gathering every time we get a report of some weird behavior.
> 
> So we should make sure that any way that we have of gathering report 
> information includes which plugins are disabled. For instance, if you turn on 
> any log channel, there should be a crunchy frog notice at the beginning of 
> the log for what plugins have been disabled.
> 
> Another way to do this would be to have the list of disabled plugins show in 
> the result of the `version` command, since that's a commonly provided bit of 
> info for bug reports.

Can we include the plug-in info in the "statistics dump" output? We could 
easily JSON'ify the enabled/disabled status of all plug-ins there? Let us know 
if that would be acceptable

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -422,6 +422,11 @@ class SymbolFile : public PluginInterface {
   /// hasn't been indexed yet, or a valid duration if it has.
   virtual StatsDuration::Duration GetDebugInfoIndexTime() { return {}; }
 
+  /// Return the time it took to download any extra symbol files.
+  ///
+  /// \returns 0.0 if no extra symbol files need to be downloaded

youngd007 wrote:

not sure of standard for sentinel values in LLVM nor LLDB, but using 0.0 can be 
tricky if a file downloads so fast that it is effectively zero.  Can we have it 
be null?  Might make the C++ a little rougher, but a thought.  Other option is 
-1, which has its own drawbacks if just logged.  need to filter out those 
greater than -1 before any average or other aggregation.
If zero is standard, I am fine with that.

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -391,6 +398,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   }
 
   json::Object global_stats{
+  {"totalSymbolDownloadTime", symbol_download_time},

youngd007 wrote:

And this key is new.

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -188,17 +188,27 @@ GetFileForModule(const ModuleSpec &module_spec,
   std::string cache_file_name = llvm::toHex(build_id, true);
   if (!file_name.empty())
 cache_file_name += "-" + file_name.str();
-  llvm::Expected result = llvm::getCachedOrDownloadArtifact(
-  cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
-  if (result)
-return FileSpec(*result);
-
-  Log *log = GetLog(LLDBLog::Symbols);
-  auto err_message = llvm::toString(result.takeError());
-  LLDB_LOGV(log,
-"Debuginfod failed to download symbol artifact {0} with error {1}",
-url_path, err_message);
-  return {};
+  StatsDuration duration;
+  ElapsedTime elapased(duration);
+  std::string local_path;
+  {
+ElapsedTime elapsed(duration);
+llvm::Expected result = llvm::getCachedOrDownloadArtifact(
+cache_file_name, url_path, cache_path, debuginfod_urls, timeout);

youngd007 wrote:

Not knowing the exact API of StatsDuration & ElapsedTime, is this getting the 
correct value for duration?  Why are we getting it once at start and then 
before the getCachedOrDownload?  Shouldn't the second time be _after_ the 
download?

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits

https://github.com/youngd007 commented:

Overall seems fine to me.  Will let the others take a crack.  Please confirm 
the duration is accurately getting calculated.

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


[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [lldb] [lldb][lldb-dap] Added support for "WriteMemory" request. (PR #131820)

2025-04-07 Thread Santhosh Kumar Ellendula via lldb-commits


@@ -0,0 +1,175 @@
+//===-- WriteMemoryRequestHandler.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Base64.h"
+
+namespace lldb_dap {
+
+// "WriteMemoryRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Writes bytes to memory at the provided location.\n
+// Clients should only call this request if the corresponding
+// capability `supportsWriteMemoryRequest` is true.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "writeMemory" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/WriteMemoryArguments"
+//   }
+// },
+// "required": [ "command", "arguments" ]
+//   }]
+// },
+// "WriteMemoryArguments": {
+//   "type": "object",
+//   "description": "Arguments for `writeMemory` request.",
+//   "properties": {
+// "memoryReference": {
+//   "type": "string",
+//   "description": "Memory reference to the base location to which
+//   data should be written."
+// },
+// "offset": {
+//   "type": "integer",
+//   "description": "Offset (in bytes) to be applied to the reference
+//   location before writing data. Can be negative."
+// },
+// "allowPartial": {
+//   "type": "boolean",
+//   "description": "Property to control partial writes. If true, the
+//   debug adapter should attempt to write memory even if the entire
+//   memory region is not writable. In such a case the debug adapter
+//   should stop after hitting the first byte of memory that cannot be
+//   written and return the number of bytes written in the response
+//   via the `offset` and `bytesWritten` properties.\nIf false or
+//   missing, a debug adapter should attempt to verify the region is
+//   writable before writing, and fail the response if it is not."
+// },
+// "data": {
+//   "type": "string",
+//   "description": "Bytes to write, encoded using base64."
+// }
+//   },
+//   "required": [ "memoryReference", "data" ]
+// },
+// "WriteMemoryResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `writeMemory` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "offset": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the offset of the first
+// byte of data successfully written. Can be negative."
+//   },
+//   "bytesWritten": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the number of bytes
+// starting from address that were successfully written."
+//   }
+// }
+//   }
+// }
+//   }]
+// },
+void WriteMemoryRequestHandler::operator()(
+const llvm::json::Object &request) const {
+  llvm::json::Object response;
+  FillResponse(request, response);
+
+  auto arguments = request.getObject("arguments");
+  llvm::StringRef memoryReference =

santhoshe447 wrote:

I have updated this to memory_reference from memoryReference.

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


[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [lldb] [lldb][lldb-dap] Added support for "WriteMemory" request. (PR #131820)

2025-04-07 Thread Santhosh Kumar Ellendula via lldb-commits


@@ -0,0 +1,175 @@
+//===-- WriteMemoryRequestHandler.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Base64.h"
+
+namespace lldb_dap {
+
+// "WriteMemoryRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Writes bytes to memory at the provided location.\n
+// Clients should only call this request if the corresponding
+// capability `supportsWriteMemoryRequest` is true.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "writeMemory" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/WriteMemoryArguments"
+//   }
+// },
+// "required": [ "command", "arguments" ]
+//   }]
+// },
+// "WriteMemoryArguments": {
+//   "type": "object",
+//   "description": "Arguments for `writeMemory` request.",
+//   "properties": {
+// "memoryReference": {
+//   "type": "string",
+//   "description": "Memory reference to the base location to which
+//   data should be written."
+// },
+// "offset": {
+//   "type": "integer",
+//   "description": "Offset (in bytes) to be applied to the reference
+//   location before writing data. Can be negative."
+// },
+// "allowPartial": {
+//   "type": "boolean",
+//   "description": "Property to control partial writes. If true, the
+//   debug adapter should attempt to write memory even if the entire
+//   memory region is not writable. In such a case the debug adapter
+//   should stop after hitting the first byte of memory that cannot be
+//   written and return the number of bytes written in the response
+//   via the `offset` and `bytesWritten` properties.\nIf false or
+//   missing, a debug adapter should attempt to verify the region is
+//   writable before writing, and fail the response if it is not."
+// },
+// "data": {
+//   "type": "string",
+//   "description": "Bytes to write, encoded using base64."
+// }
+//   },
+//   "required": [ "memoryReference", "data" ]
+// },
+// "WriteMemoryResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `writeMemory` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "offset": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the offset of the first
+// byte of data successfully written. Can be negative."
+//   },
+//   "bytesWritten": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the number of bytes
+// starting from address that were successfully written."
+//   }
+// }
+//   }
+// }
+//   }]
+// },
+void WriteMemoryRequestHandler::operator()(
+const llvm::json::Object &request) const {
+  llvm::json::Object response;
+  FillResponse(request, response);
+
+  auto arguments = request.getObject("arguments");
+  llvm::StringRef memoryReference =
+  GetString(arguments, "memoryReference").value_or("");
+
+  auto addr_opt = DecodeMemoryReference(memoryReference);
+  if (!addr_opt.has_value()) {
+dap.SendErrorResponse(response, "Malformed memory reference: " +
+memoryReference.str());
+return;
+  }
+  lldb::addr_t address =
+  *addr_opt + GetInteger(arguments, "offset").value_or(0);
+
+  llvm::StringRef data64 = GetString(arguments, "data").value_or("");
+  if (data64.empty()) {
+dap.SendErrorResponse(response,
+  "Data cannot be empty value. Provide valid data");
+return;
+  }
+
+  // The VSCode IDE or other DAP clients send memory data as a Base64 string.
+  // This function decodes it into raw binary before writing it to the target
+  // process memory.
+  std::vector output;
+  auto decode_error = llvm::decodeBase64(data64, output);
+
+  if (decode_error) {
+dap.SendErrorResponse(response,
+

[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)

2025-04-07 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)


Changes

When you call the `SBTarget::ReadInstructions` with flavor from lldb crashes. 
This is because the wrong order of the `DisassemblyBytes` constructor this 
fixes that

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


4 Files Affected:

- (modified) lldb/source/API/SBTarget.cpp (+3-3) 
- (added) lldb/test/API/python_api/target/read-instructions-flavor/Makefile 
(+3) 
- (added) 
lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 (+40) 
- (added) lldb/test/API/python_api/target/read-instructions-flavor/main.c (+21) 


``diff
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 0fed1bbfed6a7..b42ada42b0931 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2021,9 +2021,9 @@ lldb::SBInstructionList 
SBTarget::ReadInstructions(lldb::SBAddress base_addr,
 error, force_live_memory, &load_addr);
   const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
   sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
-  target_sp->GetArchitecture(), nullptr, 
target_sp->GetDisassemblyCPU(),
-  target_sp->GetDisassemblyFeatures(), flavor_string, *addr_ptr,
-  data.GetBytes(), bytes_read, count, data_from_file));
+  target_sp->GetArchitecture(), nullptr, flavor_string,
+  target_sp->GetDisassemblyCPU(), target_sp->GetDisassemblyFeatures(),
+  *addr_ptr, data.GetBytes(), bytes_read, count, data_from_file));
 }
   }
 
diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/Makefile 
b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/python_api/target/read-instructions-flavor/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git 
a/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
new file mode 100644
index 0..e93c8476d8e00
--- /dev/null
+++ 
b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py
@@ -0,0 +1,40 @@
+"""
+Test SBTarget Read Instruction.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TargetReadInstructionsFlavor(TestBase):
+
+@skipIf(archs=no_match(["x86_64", "x86", "i386"]), oslist=["windows"])
+def test_read_instructions_with_flavor(self):
+self.build()
+executable = self.getBuildArtifact("a.out")
+
+# create a target
+target = self.dbg.CreateTarget(executable)
+self.assertTrue(target.IsValid(), "target is not valid")
+
+functions = target.FindFunctions("test_add")
+self.assertEqual(len(functions), 1)
+test_add = functions[0]
+
+test_add_symbols = test_add.GetSymbol()
+self.assertTrue(
+test_add_symbols.IsValid(), "test_add function symbols is not 
valid"
+)
+
+expected_instructions = (("mov", "eax, edi"), ("add", "eax, esi"), 
("ret", ""))
+test_add_insts = test_add_symbols.GetInstructions(target, "intel")
+# clang adds an extra nop instruction but gcc does not. It makes more 
sense
+# to check if it is at least 3
+self.assertLessEqual(len(expected_instructions), len(test_add_insts))
+
+# compares only the expected instructions
+for expected_instr, instr in zip(expected_instructions, 
test_add_insts):
+self.assertTrue(instr.IsValid(), "instruction is not valid")
+expected_mnemonic, expected_op_str = expected_instr
+self.assertEqual(instr.GetMnemonic(target), expected_mnemonic)
+self.assertEqual(instr.GetOperands(target), expected_op_str)
diff --git a/lldb/test/API/python_api/target/read-instructions-flavor/main.c 
b/lldb/test/API/python_api/target/read-instructions-flavor/main.c
new file mode 100644
index 0..6022d63fb6ed7
--- /dev/null
+++ b/lldb/test/API/python_api/target/read-instructions-flavor/main.c
@@ -0,0 +1,21 @@
+
+// This simple program is to test the lldb Python API SBTarget ReadInstruction
+// function.
+//
+// When the target is create we get all the instructions using the intel
+// flavor and see if it is correct.
+
+int test_add(int a, int b);
+
+__asm__("test_add:\n"
+"movl%edi, %eax\n"
+"addl%esi, %eax\n"
+"ret \n");
+
+int main(int argc, char **argv) {
+  int a = 10;
+  int b = 20;
+  int result = test_add(a, b);
+
+  return 0;
+}
\ No newline at end of file

``




https://github.com/llvm/llvm-project/pull/134626
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] 4b90f24 - [LLDB] Add integration test for libsanitizers trace collection (#134323)

2025-04-07 Thread via lldb-commits

Author: Julian Lettner
Date: 2025-04-07T08:33:27-07:00
New Revision: 4b90f24db81fb4378d9f4816f31e16195d8adb0f

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

LOG: [LLDB] Add integration test for libsanitizers trace collection (#134323)

Add integration test for libsanitizers trace collection
(`SanitizersAllocationTraces=all`).

rdar://144244084

Added: 


Modified: 
lldb/test/API/functionalities/asan/Makefile
lldb/test/API/functionalities/asan/TestMemoryHistory.py
lldb/test/API/functionalities/asan/TestReportData.py

Removed: 




diff  --git a/lldb/test/API/functionalities/asan/Makefile 
b/lldb/test/API/functionalities/asan/Makefile
index d66696fed7078..eae5ca3e4626c 100644
--- a/lldb/test/API/functionalities/asan/Makefile
+++ b/lldb/test/API/functionalities/asan/Makefile
@@ -1,8 +1,11 @@
 C_SOURCES := main.c
-asan: CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info
-asan: all
+compiler_rt-asan: CFLAGS_EXTRAS := -fsanitize=address -g -gcolumn-info
+compiler_rt-asan: all
 
-libsanitizers: CFLAGS_EXTRAS := -fsanitize=address -fsanitize-stable-abi -g 
-gcolumn-info
-libsanitizers: all
+libsanitizers-asan: CFLAGS_EXTRAS := -fsanitize=address -fsanitize-stable-abi 
-g -gcolumn-info
+libsanitizers-asan: all
+
+libsanitizers-traces: CFLAGS_EXTRAS := -g -gcolumn-info
+libsanitizers-traces: all
 
 include Makefile.rules

diff  --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py 
b/lldb/test/API/functionalities/asan/TestMemoryHistory.py
index b04182a543719..1568140b355dc 100644
--- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py
+++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py
@@ -10,22 +10,28 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test_event.build_exception import BuildError
 
-class AsanTestCase(TestBase):
+class MemoryHistoryTestCase(TestBase):
 @skipIfFreeBSD  # llvm.org/pr21136 runtimes not yet available by default
 @expectedFailureNetBSD
 @skipUnlessAddressSanitizer
-def test(self):
-self.build(make_targets=["asan"])
-self.asan_tests()
+def test_compiler_rt_asan(self):
+self.build(make_targets=["compiler_rt-asan"])
+self.compiler_rt_asan_tests()
 
-@skipIf(oslist=no_match(["macosx"]))
-@skipIf(bugnumber="rdar://144997976")
+@skipUnlessDarwin
+@skipIf(bugnumber="rdar://109913184&143590169")
 def test_libsanitizers_asan(self):
 try:
-self.build(make_targets=["libsanitizers"])
+self.build(make_targets=["libsanitizers-asan"])
 except BuildError as e:
 self.skipTest("failed to build with libsanitizers")
-self.libsanitizer_tests()
+self.libsanitizers_asan_tests()
+
+@skipUnlessDarwin
+@skipIf(macos_version=["<", "15.5"])
+def test_libsanitizers_traces(self):
+self.build(make_targets=["libsanitizers-traces"])
+self.libsanitizers_traces_tests()
 
 def setUp(self):
 # Call super's setUp().
@@ -36,35 +42,60 @@ def setUp(self):
 self.line_breakpoint = line_number("main.c", "// break line")
 
 # Test line numbers: rdar://126237493
-def libsanitizer_tests(self):
-target = self.createTestTarget()
-
-self.runCmd(
-"env SanitizersAddress=1 MallocSanitizerZone=1 
MallocSecureAllocator=0"
-)
-
-self.runCmd("run")
-
-# In libsanitizers, memory history is not supported until a report has 
been generated
-self.expect(
-"thread list",
-"Process should be stopped due to ASan report",
-substrs=["stopped", "stop reason = Use of deallocated memory"],
-)
-
-# test the 'memory history' command
+# for libsanitizers and remove `skip_line_numbers` parameter
+def check_traces(self, skip_line_numbers=False):
 self.expect(
 "memory history 'pointer'",
 substrs=[
 "Memory deallocated by Thread",
 "a.out`f2",
-"main.c",
+"main.c" if skip_line_numbers else f"main.c:{self.line_free}",
 "Memory allocated by Thread",
 "a.out`f1",
-"main.c",
+"main.c" if skip_line_numbers else 
f"main.c:{self.line_malloc}",
 ],
 )
 
+# Set breakpoint: after free, but before bug
+def set_breakpoint(self, target):
+bkpt = target.BreakpointCreateByLocation("main.c", 
self.line_breakpoint)
+self.assertGreater(bkpt.GetNumLocations(), 0, "Set the breakpoint 
successfully")
+
+def run_to_breakpoint(self, target):
+self.set_breakpoint(target)
+self.runCmd("run")
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT

[Lldb-commits] [lldb] [LLDB] Add integration test for libsanitizers trace collection (PR #134323)

2025-04-07 Thread Julian Lettner via lldb-commits

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -1589,6 +1589,48 @@ class Process : public 
std::enable_shared_from_this,
   size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
 Status &error);
 
+  // Callback definition for read Memory in chunks
+  //
+  // Status, the status returned from ReadMemoryFromInferior
+  // DataBufferHeap, buffer with bytes potentially written to it
+  // addr_t, the current_addr, start + bytes read so far.
+  // uint64_t bytes_to_read, the expected bytes read, this
+  // is important if it's a partial read
+  // uint64_t bytes_read_for_chunk, the actual count of bytes read for this
+  // chunk
+  typedef std::function
+  ReadMemoryChunkCallback;
+
+  /// Read of memory from a process in discrete chunks, terminating
+  /// either when all bytes are read, or the supplied callback returns
+  /// IterationAction::Stop
+  ///
+  /// \param[in] vm_addr
+  /// A virtual load address that indicates where to start reading
+  /// memory from.
+  ///
+  /// \param[in] data
+  /// The data buffer heap to use to read the chunk. The chunk size
+  /// depends upon the byte size of the buffer.
+  ///
+  /// \param[in] size
+  /// The number of bytes to read.
+  ///
+  /// \param[in] callback
+  /// The callback to invoke when a chunk is read from memory.
+  ///
+  /// \return
+  /// The number of bytes that were actually read into \a buf and
+  /// written to the provided callback.
+  /// If the returned number is greater than zero, yet less than \a
+  /// size, then this function will get called again with \a
+  /// vm_addr, \a buf, and \a size updated appropriately. Zero is
+  /// returned in the case of an error.
+  size_t ReadMemoryInChunks(lldb::addr_t vm_addr, DataBufferHeap &data,
+size_t size, ReadMemoryChunkCallback callback);

clayborg wrote:

No need to have people give is a DataBufferHeap, just need the chunk size:
```
size_t ReadMemoryInChunks(lldb::addr_t vm_addr, lldb::offset_t vm_size, 
lldb::offset_t chunk_size, ReadMemoryChunkCallback callback);
```
The `ReadMemoryInChunks` function can create a local DataBufferHeap outside of 
the loop.

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


[Lldb-commits] [lldb] [lldb][debugserver] Fix an off-by-one error in watchpoint identification (PR #134314)

2025-04-07 Thread Jason Molenda via lldb-commits

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


[Lldb-commits] [lldb] 21d9121 - [lldb][debugserver] Fix an off-by-one error in watchpoint identification (#134314)

2025-04-07 Thread via lldb-commits

Author: Jason Molenda
Date: 2025-04-07T11:11:31-07:00
New Revision: 21d912121c9f41385b165a736be787527f5bd7c2

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

LOG: [lldb][debugserver] Fix an off-by-one error in watchpoint identification 
(#134314)

debugserver takes the address of a watchpoint exception and calculates
which watchpoint was responsible for it. There was an off-by-one error
in the range calculation which causes two watchpoints on consecutive
ranges to not correctly identify hits to the second watchpoint. The
result is that lldb wouldn't show the second watchpoint as ever being
hit.

rdar://145107575

Added: 
lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile

lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/main.c

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

Removed: 




diff  --git 
a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile 
b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile
new file mode 100644
index 0..10495940055b6
--- /dev/null
+++ b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
 
b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
new file mode 100644
index 0..229172e6ce0aa
--- /dev/null
+++ 
b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
@@ -0,0 +1,87 @@
+"""
+Watch contiguous memory regions with separate watchpoints, check that lldb
+correctly detect which watchpoint was hit for each one.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ConsecutiveWatchpointsTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def continue_and_report_stop_reason(self, process, iter_str):
+process.Continue()
+self.assertIn(
+process.GetState(), [lldb.eStateStopped, lldb.eStateExited], 
iter_str
+)
+thread = process.GetSelectedThread()
+return thread.GetStopReason()
+
+# debugserver only gained the ability to watch larger regions
+# with this patch.
+def test_large_watchpoint(self):
+"""Test watchpoint that covers a large region of memory."""
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.c")
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "break here", self.main_source_file
+)
+
+frame = thread.GetFrameAtIndex(0)
+
+field2_wp = (
+frame.locals["var"][0]
+.GetChildMemberWithName("field2")
+.Watch(True, False, True)
+)
+field3_wp = (
+frame.locals["var"][0]
+.GetChildMemberWithName("field3")
+.Watch(True, False, True)
+)
+field4_wp = (
+frame.locals["var"][0]
+.GetChildMemberWithName("field4")
+.Watch(True, False, True)
+)
+field5_wp = (
+frame.locals["var"][0]
+.GetChildMemberWithName("field5")
+.Watch(True, False, True)
+)
+
+self.assertTrue(field2_wp.IsValid())
+self.assertTrue(field3_wp.IsValid())
+self.assertTrue(field4_wp.IsValid())
+self.assertTrue(field5_wp.IsValid())
+
+reason = self.continue_and_report_stop_reason(process, "continue to 
field2 wp")
+self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+stop_reason_watchpoint_id = (
+process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+)
+self.assertEqual(stop_reason_watchpoint_id, field2_wp.GetID())
+
+reason = self.continue_and_report_stop_reason(process, "continue to 
field3 wp")
+self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+stop_reason_watchpoint_id = (
+process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+)
+self.assertEqual(stop_reason_watchpoint_id, field3_wp.GetID())
+
+reason = self.continue_and_report_stop_reason(process, "continue to 
field4 wp")
+self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+stop_reason_watchpoint_id = (
+process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+)
+self.assertEqual(stop_reason_watchpoint_id, field4_wp.GetID())
+
+reason = self.continue_and_report_stop_re

[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -188,17 +188,27 @@ GetFileForModule(const ModuleSpec &module_spec,
   std::string cache_file_name = llvm::toHex(build_id, true);
   if (!file_name.empty())
 cache_file_name += "-" + file_name.str();
-  llvm::Expected result = llvm::getCachedOrDownloadArtifact(
-  cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
-  if (result)
-return FileSpec(*result);
-
-  Log *log = GetLog(LLDBLog::Symbols);
-  auto err_message = llvm::toString(result.takeError());
-  LLDB_LOGV(log,
-"Debuginfod failed to download symbol artifact {0} with error {1}",
-url_path, err_message);
-  return {};
+  StatsDuration duration;
+  ElapsedTime elapased(duration);

GeorgeHuyubo wrote:

Yes this is redundant and not used. Deleted.

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


[Lldb-commits] [lldb] [LLDB][NFC] Remove Debugger dependency in SystemLifetimeManager (PR #134383)

2025-04-07 Thread Dmitry Vasilyev via lldb-commits

slydiman wrote:

> and that's why I am very reluctant to approve #132274

#132274 is most important now because it gives the significant effect. 
I have refactored `CPlusPlusLanguage::MethodName` and don't just splite a CU 
now. RichManglingContext.cpp uses auto 
`Language::FindPlugin(eLanguageTypeC_plus_plus)`, etc.

This patch is almost cosmetics. It removes unnecessary Debugger from 
lldb-server initialization chain. This speeds up and reduces memory usage fot 
now, but the effect is not noticeable.

So, I will move LoadPlugin lambda to SystemInitializerFull.

Note lldbInitialization depends on lldbPluginProcessGDBRemote, 
lldbPluginProcessPOSIX and lldbPluginProcessWindowsCommon. 
SystemInitializerCommon::Initialize() calls 
process_gdb_remote::ProcessGDBRemoteLog::Initialize(), 
ProcessPOSIXLog::Initialize() and ProcessWindowsLog::Initialize(). 
If it is OK for Utility, I will move lldbInitialization into lldbUtility.

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


[Lldb-commits] [lldb] [lldb][lldb-dap] Added support for "WriteMemory" request. (PR #131820)

2025-04-07 Thread Santhosh Kumar Ellendula via lldb-commits


@@ -0,0 +1,175 @@
+//===-- WriteMemoryRequestHandler.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DAP.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Base64.h"
+
+namespace lldb_dap {
+
+// "WriteMemoryRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Writes bytes to memory at the provided location.\n
+// Clients should only call this request if the corresponding
+// capability `supportsWriteMemoryRequest` is true.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "writeMemory" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/WriteMemoryArguments"
+//   }
+// },
+// "required": [ "command", "arguments" ]
+//   }]
+// },
+// "WriteMemoryArguments": {
+//   "type": "object",
+//   "description": "Arguments for `writeMemory` request.",
+//   "properties": {
+// "memoryReference": {
+//   "type": "string",
+//   "description": "Memory reference to the base location to which
+//   data should be written."
+// },
+// "offset": {
+//   "type": "integer",
+//   "description": "Offset (in bytes) to be applied to the reference
+//   location before writing data. Can be negative."
+// },
+// "allowPartial": {
+//   "type": "boolean",
+//   "description": "Property to control partial writes. If true, the
+//   debug adapter should attempt to write memory even if the entire
+//   memory region is not writable. In such a case the debug adapter
+//   should stop after hitting the first byte of memory that cannot be
+//   written and return the number of bytes written in the response
+//   via the `offset` and `bytesWritten` properties.\nIf false or
+//   missing, a debug adapter should attempt to verify the region is
+//   writable before writing, and fail the response if it is not."
+// },
+// "data": {
+//   "type": "string",
+//   "description": "Bytes to write, encoded using base64."
+// }
+//   },
+//   "required": [ "memoryReference", "data" ]
+// },
+// "WriteMemoryResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to `writeMemory` request.",
+// "properties": {
+//   "body": {
+// "type": "object",
+// "properties": {
+//   "offset": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the offset of the first
+// byte of data successfully written. Can be negative."
+//   },
+//   "bytesWritten": {
+// "type": "integer",
+// "description": "Property that should be returned when
+// `allowPartial` is true to indicate the number of bytes
+// starting from address that were successfully written."
+//   }
+// }
+//   }
+// }
+//   }]
+// },
+void WriteMemoryRequestHandler::operator()(
+const llvm::json::Object &request) const {
+  llvm::json::Object response;
+  FillResponse(request, response);
+
+  auto arguments = request.getObject("arguments");
+  llvm::StringRef memoryReference =
+  GetString(arguments, "memoryReference").value_or("");
+
+  auto addr_opt = DecodeMemoryReference(memoryReference);
+  if (!addr_opt.has_value()) {
+dap.SendErrorResponse(response, "Malformed memory reference: " +
+memoryReference.str());
+return;
+  }
+  lldb::addr_t address =
+  *addr_opt + GetInteger(arguments, "offset").value_or(0);
+
+  llvm::StringRef data64 = GetString(arguments, "data").value_or("");
+  if (data64.empty()) {
+dap.SendErrorResponse(response,
+  "Data cannot be empty value. Provide valid data");
+return;
+  }
+
+  // The VSCode IDE or other DAP clients send memory data as a Base64 string.
+  // This function decodes it into raw binary before writing it to the target
+  // process memory.
+  std::vector output;
+  auto decode_error = llvm::decodeBase64(data64, output);
+
+  if (decode_error) {
+dap.SendErrorResponse(response,
+

[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add option to highlight function names in backtraces (PR #131836)

2025-04-07 Thread Michael Buch via lldb-commits

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

>From 65aab6b727cc430a9e826c7eeda67259e041a462 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Mon, 7 Apr 2025 13:21:25 +0100
Subject: [PATCH 1/5] [ItaniumDemangle] Add printLeft/printRight APIs to
 OutputBuffer

This patch includes the necessary changes for the LLDB feature proposed in 
https://discourse.llvm.org/t/rfc-lldb-highlighting-function-names-in-lldb-backtraces/85309.
 The TL;DR is that we want to track where certain parts of a demangled name 
begin/end so we can highlight them in backtraces.

We introduce a new `printLeft`/`printRight` API that a client (in our case 
LLDB) can implement to track state while printing the demangle tree. This 
requires redirecting all calls to to `printLeft`/`printRight` to the 
`OutputBuffer`. One quirk with the new API is that `Utility.h` would now depend 
on `ItaniumDemangle.h` and vice-versa. To keep these files header-only I made 
the definitions `inline` and implement the new APIs in `ItaniumDemangle.h` (so 
the definition of `Node` is available to them).
---
 libcxxabi/src/demangle/ItaniumDemangle.h | 64 +++-
 libcxxabi/src/demangle/Utility.h |  7 +++
 llvm/include/llvm/Demangle/ItaniumDemangle.h | 64 +++-
 llvm/include/llvm/Demangle/Utility.h |  7 +++
 4 files changed, 86 insertions(+), 56 deletions(-)

diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h 
b/libcxxabi/src/demangle/ItaniumDemangle.h
index 3df41b5f4d7d0..89a24def830f2 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -281,9 +281,9 @@ class Node {
   }
 
   void print(OutputBuffer &OB) const {
-printLeft(OB);
+OB.printLeft(*this);
 if (RHSComponentCache != Cache::No)
-  printRight(OB);
+  OB.printRight(*this);
   }
 
   // Print the "left" side of this Node into OutputBuffer.
@@ -458,11 +458,11 @@ class QualType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-Child->printLeft(OB);
+OB.printLeft(*Child);
 printQuals(OB);
   }
 
-  void printRight(OutputBuffer &OB) const override { Child->printRight(OB); }
+  void printRight(OutputBuffer &OB) const override { OB.printRight(*Child); }
 };
 
 class ConversionOperatorType final : public Node {
@@ -491,7 +491,7 @@ class PostfixQualifiedType final : public Node {
   template void match(Fn F) const { F(Ty, Postfix); }
 
   void printLeft(OutputBuffer &OB) const override {
-Ty->printLeft(OB);
+OB.printLeft(*Ty);
 OB += Postfix;
   }
 };
@@ -577,7 +577,7 @@ struct AbiTagAttr : Node {
   std::string_view getBaseName() const override { return Base->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
-Base->printLeft(OB);
+OB.printLeft(*Base);
 OB += "[abi:";
 OB += Tag;
 OB += "]";
@@ -644,7 +644,7 @@ class PointerType final : public Node {
 // We rewrite objc_object* into id.
 if (Pointee->getKind() != KObjCProtoName ||
 !static_cast(Pointee)->isObjCObject()) {
-  Pointee->printLeft(OB);
+  OB.printLeft(*Pointee);
   if (Pointee->hasArray(OB))
 OB += " ";
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
@@ -663,7 +663,7 @@ class PointerType final : public Node {
 !static_cast(Pointee)->isObjCObject()) {
   if (Pointee->hasArray(OB) || Pointee->hasFunction(OB))
 OB += ")";
-  Pointee->printRight(OB);
+  OB.printRight(*Pointee);
 }
   }
 };
@@ -729,7 +729,7 @@ class ReferenceType : public Node {
 std::pair Collapsed = collapse(OB);
 if (!Collapsed.second)
   return;
-Collapsed.second->printLeft(OB);
+OB.printLeft(*Collapsed.second);
 if (Collapsed.second->hasArray(OB))
   OB += " ";
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
@@ -746,7 +746,7 @@ class ReferenceType : public Node {
   return;
 if (Collapsed.second->hasArray(OB) || Collapsed.second->hasFunction(OB))
   OB += ")";
-Collapsed.second->printRight(OB);
+OB.printRight(*Collapsed.second);
   }
 };
 
@@ -766,7 +766,7 @@ class PointerToMemberType final : public Node {
   }
 
   void printLeft(OutputBuffer &OB) const override {
-MemberType->printLeft(OB);
+OB.printLeft(*MemberType);
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += "(";
 else
@@ -778,7 +778,7 @@ class PointerToMemberType final : public Node {
   void printRight(OutputBuffer &OB) const override {
 if (MemberType->hasArray(OB) || MemberType->hasFunction(OB))
   OB += ")";
-MemberType->printRight(OB);
+OB.printRight(*MemberType);
   }
 };
 
@@ -798,7 +798,7 @@ class ArrayType final : public Node {
   bool hasRHSComponentSlow(OutputBuffer &) const override { return true; }
   bool hasArraySlow(OutputBuffer &) const override { return true; }
 
-  void printLeft(OutputBuffer &OB) const override { Base->print

[Lldb-commits] [lldb] [LLDB][NFC] Remove Debugger dependency in SystemLifetimeManager (PR #134383)

2025-04-07 Thread Dmitry Vasilyev via lldb-commits

slydiman wrote:

@labath 

> The only thing which makes that difficult is that the callback is a lambda 
> defined in SBDebugger.cpp. However, there's no reason it has to be defined 
> there -- the lambda itself doesn't depend on anything, and it could easily be 
> moved to SystemInitializerFull.cpp.

I have moved The lambda LoadPlugin to SystemInitializerFull. But this lambda 
calls the protected constructor lldb::SBDebugger(const lldb::DebuggerSP 
&debugger_sp).

Any suggestions?

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


[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -2233,6 +2233,40 @@ size_t Process::ReadMemoryFromInferior(addr_t addr, void 
*buf, size_t size,
   return bytes_read;
 }
 
+size_t Process::ReadMemoryInChunks(lldb::addr_t vm_addr, DataBufferHeap &data,

clayborg wrote:

There is no need to pass in a DataBufferHeap into this API. The API should just 
take the chunk size as a parameter:
```
size_t Process::ReadMemoryInChunks(lldb::addr_t vm_addr, lldb::addr_t vm_size, 
lldb::addr_t chunk_size, ReadMemoryChunkCallback callback);
```
Then you can create a local DataBufferHeap object in this function outside of 
the loop that reads the chunks, so the same buffer gets re-used.

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


[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -1589,6 +1589,48 @@ class Process : public 
std::enable_shared_from_this,
   size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
 Status &error);
 
+  // Callback definition for read Memory in chunks
+  //
+  // Status, the status returned from ReadMemoryFromInferior
+  // DataBufferHeap, buffer with bytes potentially written to it
+  // addr_t, the current_addr, start + bytes read so far.
+  // uint64_t bytes_to_read, the expected bytes read, this
+  // is important if it's a partial read
+  // uint64_t bytes_read_for_chunk, the actual count of bytes read for this
+  // chunk
+  typedef std::function

clayborg wrote:

We should just pass the bytes without requiring someone to use a 
DataBufferHeap. I suggest a change below that passes the chunk size into the 
`Process::ReadMemoryInChunks(...)` call, so we don't need the `bytes_to_read` 
argument. So this callback can be:
```
typedef std::function
```


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


[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -46,12 +49,333 @@ class CommandObjectPluginLoad : public CommandObjectParsed 
{
   }
 };
 
+namespace {
+#define LLDB_OPTIONS_plugin_list
+#include "CommandOptions.inc"
+
+// These option definitions are shared by the plugin list/enable/disable
+// commands.
+class PluginListCommandOptions : public Options {
+public:
+  PluginListCommandOptions() = default;
+
+  ~PluginListCommandOptions() override = default;
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ExecutionContext *execution_context) override {
+Status error;
+const int short_option = m_getopt_table[option_idx].val;
+
+switch (short_option) {
+case 'x':
+  m_exact_name_match = true;
+  break;
+default:
+  llvm_unreachable("Unimplemented option");
+}
+
+return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+m_exact_name_match = false;
+  }
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(g_plugin_list_options);
+  }
+
+  // Instance variables to hold the values for command options.
+  bool m_exact_name_match = false;
+};
+
+// Define some data structures to describe known plugin "namespaces".
+// The PluginManager is organized into a series of static functions
+// that operate on different types of plugin. For example SystemRuntime
+// and ObjectFile plugins.
+//
+// The namespace name is used a prefix when matching plugin names. For example,
+// if we have an "elf" plugin in the "object-file" namespace then we will
+// match a plugin name pattern against the "object-file.elf" name.
+//
+// The plugin namespace here is used so we can operate on all the plugins
+// of a given type so it is easy to enable or disable them as a group.
+using GetPluginInfo = std::function()>;
+using SetPluginEnabled = std::function;
+struct PluginNamespace {
+  llvm::StringRef name;
+  GetPluginInfo get_info;
+  SetPluginEnabled set_enabled;
+};
+
+// Currently supported set of plugin namespaces. This will be expanded
+// over time.
+PluginNamespace PluginNamespaces[] = {
+{"system-runtime", PluginManager::GetSystemRuntimePluginInfo,
+ PluginManager::SetSystemRuntimePluginEnabled}};
+
+// Helper function to perform an action on each matching plugin.
+// The action callback is given the containing namespace along with plugin info
+// for each matching plugin.
+static int ActOnMatchingPlugins(
+llvm::GlobPattern pattern,
+std::function &plugin_info)>
+action) {
+  int num_matching = 0;
+
+  for (const PluginNamespace &plugin_namespace : PluginNamespaces) {
+std::vector matching_plugins;
+for (const RegisteredPluginInfo &plugin_info :
+ plugin_namespace.get_info()) {
+  std::string qualified_name =
+  (plugin_namespace.name + "." + plugin_info.name).str();
+  if (pattern.match(qualified_name))
+matching_plugins.push_back(plugin_info);
+}
+
+if (!matching_plugins.empty()) {
+  num_matching += matching_plugins.size();
+  action(plugin_namespace, matching_plugins);
+}
+  }
+
+  return num_matching;
+}
+
+// Return a string in glob syntax for matching plugins.
+static std::string GetPluginNamePatternString(llvm::StringRef user_input,
+  bool add_default_glob) {
+  std::string pattern_str = user_input.empty() ? "*" : user_input.str();
+
+  if (add_default_glob && pattern_str != "*")
+pattern_str = "*" + pattern_str + "*";
+
+  return pattern_str;
+}
+
+// Attempts to create a glob pattern for a plugin name based on plugin command
+// input. Writes an error message to the `result` object if the glob cannot be
+// created successfully.
+//
+// The `glob_storage` is used to hold the string data for the glob pattern. The
+// llvm::GlobPattern only contains pointers into the string data so we need a
+// stable location that can outlive the glob pattern itself.
+std::optional
+TryCreatePluginPattern(const char *plugin_command_name, const Args &command,
+   const PluginListCommandOptions &options,
+   CommandReturnObject &result, std::string &glob_storage) 
{
+  size_t argc = command.GetArgumentCount();
+  if (argc > 1) {
+result.AppendErrorWithFormat("'%s' requires one argument",
+ plugin_command_name);
+return {};
+  }
+
+  llvm::StringRef user_pattern = argc == 1 ? command[0].ref() : "";
+
+  glob_storage =
+  GetPluginNamePatternString(user_pattern, !options.m_exact_name_match);
+
+  auto glob_pattern = llvm::GlobPattern::create(glob_storage);
+
+  if (auto error = glob_pattern.takeError()) {
+std::string error_message =
+(llvm::Twine("Invalid plugin glob pattern: '") + glob_storage +
+ "': " + llvm::toString(std::move(error)))
+.str();
+result.AppendError(error_message);
+return {};
+  }
+
+  return *glob_pattern;
+}
+
+// Call the "SetEnable"

[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -46,12 +49,333 @@ class CommandObjectPluginLoad : public CommandObjectParsed 
{
   }
 };
 
+namespace {
+#define LLDB_OPTIONS_plugin_list
+#include "CommandOptions.inc"
+
+// These option definitions are shared by the plugin list/enable/disable
+// commands.
+class PluginListCommandOptions : public Options {
+public:
+  PluginListCommandOptions() = default;
+
+  ~PluginListCommandOptions() override = default;
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ExecutionContext *execution_context) override {
+Status error;
+const int short_option = m_getopt_table[option_idx].val;
+
+switch (short_option) {
+case 'x':
+  m_exact_name_match = true;
+  break;
+default:
+  llvm_unreachable("Unimplemented option");
+}
+
+return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+m_exact_name_match = false;
+  }
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(g_plugin_list_options);
+  }
+
+  // Instance variables to hold the values for command options.
+  bool m_exact_name_match = false;
+};
+
+// Define some data structures to describe known plugin "namespaces".
+// The PluginManager is organized into a series of static functions
+// that operate on different types of plugin. For example SystemRuntime
+// and ObjectFile plugins.
+//
+// The namespace name is used a prefix when matching plugin names. For example,
+// if we have an "elf" plugin in the "object-file" namespace then we will
+// match a plugin name pattern against the "object-file.elf" name.
+//
+// The plugin namespace here is used so we can operate on all the plugins
+// of a given type so it is easy to enable or disable them as a group.
+using GetPluginInfo = std::function()>;
+using SetPluginEnabled = std::function;
+struct PluginNamespace {
+  llvm::StringRef name;
+  GetPluginInfo get_info;
+  SetPluginEnabled set_enabled;
+};
+
+// Currently supported set of plugin namespaces. This will be expanded
+// over time.
+PluginNamespace PluginNamespaces[] = {
+{"system-runtime", PluginManager::GetSystemRuntimePluginInfo,
+ PluginManager::SetSystemRuntimePluginEnabled}};
+
+// Helper function to perform an action on each matching plugin.
+// The action callback is given the containing namespace along with plugin info
+// for each matching plugin.
+static int ActOnMatchingPlugins(
+llvm::GlobPattern pattern,
+std::function &plugin_info)>
+action) {
+  int num_matching = 0;
+
+  for (const PluginNamespace &plugin_namespace : PluginNamespaces) {
+std::vector matching_plugins;
+for (const RegisteredPluginInfo &plugin_info :
+ plugin_namespace.get_info()) {
+  std::string qualified_name =
+  (plugin_namespace.name + "." + plugin_info.name).str();
+  if (pattern.match(qualified_name))
+matching_plugins.push_back(plugin_info);
+}
+
+if (!matching_plugins.empty()) {
+  num_matching += matching_plugins.size();
+  action(plugin_namespace, matching_plugins);
+}
+  }
+
+  return num_matching;
+}
+
+// Return a string in glob syntax for matching plugins.
+static std::string GetPluginNamePatternString(llvm::StringRef user_input,
+  bool add_default_glob) {
+  std::string pattern_str = user_input.empty() ? "*" : user_input.str();
+
+  if (add_default_glob && pattern_str != "*")
+pattern_str = "*" + pattern_str + "*";
+
+  return pattern_str;
+}
+
+// Attempts to create a glob pattern for a plugin name based on plugin command
+// input. Writes an error message to the `result` object if the glob cannot be
+// created successfully.
+//
+// The `glob_storage` is used to hold the string data for the glob pattern. The
+// llvm::GlobPattern only contains pointers into the string data so we need a
+// stable location that can outlive the glob pattern itself.
+std::optional
+TryCreatePluginPattern(const char *plugin_command_name, const Args &command,
+   const PluginListCommandOptions &options,
+   CommandReturnObject &result, std::string &glob_storage) 
{
+  size_t argc = command.GetArgumentCount();
+  if (argc > 1) {
+result.AppendErrorWithFormat("'%s' requires one argument",
+ plugin_command_name);
+return {};
+  }
+
+  llvm::StringRef user_pattern = argc == 1 ? command[0].ref() : "";
+
+  glob_storage =
+  GetPluginNamePatternString(user_pattern, !options.m_exact_name_match);
+
+  auto glob_pattern = llvm::GlobPattern::create(glob_storage);
+
+  if (auto error = glob_pattern.takeError()) {
+std::string error_message =
+(llvm::Twine("Invalid plugin glob pattern: '") + glob_storage +
+ "': " + llvm::toString(std::move(error)))
+.str();
+result.AppendError(error_message);
+return {};
+  }
+
+  return *glob_pattern;
+}
+
+// Call the "SetEnable"

[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -46,12 +49,333 @@ class CommandObjectPluginLoad : public CommandObjectParsed 
{
   }
 };
 
+namespace {
+#define LLDB_OPTIONS_plugin_list
+#include "CommandOptions.inc"
+
+// These option definitions are shared by the plugin list/enable/disable
+// commands.
+class PluginListCommandOptions : public Options {
+public:
+  PluginListCommandOptions() = default;
+
+  ~PluginListCommandOptions() override = default;
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ExecutionContext *execution_context) override {
+Status error;
+const int short_option = m_getopt_table[option_idx].val;
+
+switch (short_option) {
+case 'x':
+  m_exact_name_match = true;
+  break;
+default:
+  llvm_unreachable("Unimplemented option");
+}
+
+return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+m_exact_name_match = false;
+  }
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(g_plugin_list_options);
+  }
+
+  // Instance variables to hold the values for command options.
+  bool m_exact_name_match = false;
+};
+
+// Define some data structures to describe known plugin "namespaces".
+// The PluginManager is organized into a series of static functions
+// that operate on different types of plugin. For example SystemRuntime
+// and ObjectFile plugins.
+//
+// The namespace name is used a prefix when matching plugin names. For example,
+// if we have an "elf" plugin in the "object-file" namespace then we will
+// match a plugin name pattern against the "object-file.elf" name.

clayborg wrote:

Do we want to pick a different example here? We really want to discourage 
people from disabling the ELF plug-in! Maybe just the the system-runtime 
example?

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


[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits

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


[Lldb-commits] [lldb] Add commands to list/enable/disable plugins (PR #134418)

2025-04-07 Thread Greg Clayton via lldb-commits


@@ -46,12 +49,333 @@ class CommandObjectPluginLoad : public CommandObjectParsed 
{
   }
 };
 
+namespace {
+#define LLDB_OPTIONS_plugin_list
+#include "CommandOptions.inc"
+
+// These option definitions are shared by the plugin list/enable/disable
+// commands.
+class PluginListCommandOptions : public Options {
+public:
+  PluginListCommandOptions() = default;
+
+  ~PluginListCommandOptions() override = default;
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+ExecutionContext *execution_context) override {
+Status error;
+const int short_option = m_getopt_table[option_idx].val;
+
+switch (short_option) {
+case 'x':
+  m_exact_name_match = true;
+  break;
+default:
+  llvm_unreachable("Unimplemented option");
+}
+
+return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+m_exact_name_match = false;
+  }
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(g_plugin_list_options);
+  }
+
+  // Instance variables to hold the values for command options.
+  bool m_exact_name_match = false;
+};
+
+// Define some data structures to describe known plugin "namespaces".
+// The PluginManager is organized into a series of static functions
+// that operate on different types of plugin. For example SystemRuntime
+// and ObjectFile plugins.
+//
+// The namespace name is used a prefix when matching plugin names. For example,
+// if we have an "elf" plugin in the "object-file" namespace then we will
+// match a plugin name pattern against the "object-file.elf" name.
+//
+// The plugin namespace here is used so we can operate on all the plugins
+// of a given type so it is easy to enable or disable them as a group.
+using GetPluginInfo = std::function()>;
+using SetPluginEnabled = std::function;
+struct PluginNamespace {
+  llvm::StringRef name;
+  GetPluginInfo get_info;
+  SetPluginEnabled set_enabled;
+};
+
+// Currently supported set of plugin namespaces. This will be expanded
+// over time.
+PluginNamespace PluginNamespaces[] = {
+{"system-runtime", PluginManager::GetSystemRuntimePluginInfo,
+ PluginManager::SetSystemRuntimePluginEnabled}};
+
+// Helper function to perform an action on each matching plugin.
+// The action callback is given the containing namespace along with plugin info
+// for each matching plugin.
+static int ActOnMatchingPlugins(
+llvm::GlobPattern pattern,
+std::function &plugin_info)>
+action) {
+  int num_matching = 0;
+
+  for (const PluginNamespace &plugin_namespace : PluginNamespaces) {
+std::vector matching_plugins;
+for (const RegisteredPluginInfo &plugin_info :
+ plugin_namespace.get_info()) {
+  std::string qualified_name =
+  (plugin_namespace.name + "." + plugin_info.name).str();
+  if (pattern.match(qualified_name))
+matching_plugins.push_back(plugin_info);
+}
+
+if (!matching_plugins.empty()) {
+  num_matching += matching_plugins.size();
+  action(plugin_namespace, matching_plugins);
+}
+  }
+
+  return num_matching;
+}
+
+// Return a string in glob syntax for matching plugins.
+static std::string GetPluginNamePatternString(llvm::StringRef user_input,
+  bool add_default_glob) {
+  std::string pattern_str = user_input.empty() ? "*" : user_input.str();
+
+  if (add_default_glob && pattern_str != "*")
+pattern_str = "*" + pattern_str + "*";
+
+  return pattern_str;
+}
+
+// Attempts to create a glob pattern for a plugin name based on plugin command
+// input. Writes an error message to the `result` object if the glob cannot be
+// created successfully.
+//
+// The `glob_storage` is used to hold the string data for the glob pattern. The
+// llvm::GlobPattern only contains pointers into the string data so we need a
+// stable location that can outlive the glob pattern itself.
+std::optional
+TryCreatePluginPattern(const char *plugin_command_name, const Args &command,
+   const PluginListCommandOptions &options,
+   CommandReturnObject &result, std::string &glob_storage) 
{
+  size_t argc = command.GetArgumentCount();
+  if (argc > 1) {
+result.AppendErrorWithFormat("'%s' requires one argument",
+ plugin_command_name);
+return {};
+  }
+
+  llvm::StringRef user_pattern = argc == 1 ? command[0].ref() : "";
+
+  glob_storage =
+  GetPluginNamePatternString(user_pattern, !options.m_exact_name_match);
+
+  auto glob_pattern = llvm::GlobPattern::create(glob_storage);
+
+  if (auto error = glob_pattern.takeError()) {
+std::string error_message =
+(llvm::Twine("Invalid plugin glob pattern: '") + glob_storage +
+ "': " + llvm::toString(std::move(error)))
+.str();
+result.AppendError(error_message);
+return {};
+  }
+
+  return *glob_pattern;
+}
+
+// Call the "SetEnable"

[Lldb-commits] [lldb] Add download time for each module in statistics (PR #134563)

2025-04-07 Thread via lldb-commits


@@ -188,17 +188,27 @@ GetFileForModule(const ModuleSpec &module_spec,
   std::string cache_file_name = llvm::toHex(build_id, true);
   if (!file_name.empty())
 cache_file_name += "-" + file_name.str();
-  llvm::Expected result = llvm::getCachedOrDownloadArtifact(
-  cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
-  if (result)
-return FileSpec(*result);
-
-  Log *log = GetLog(LLDBLog::Symbols);
-  auto err_message = llvm::toString(result.takeError());
-  LLDB_LOGV(log,
-"Debuginfod failed to download symbol artifact {0} with error {1}",
-url_path, err_message);
-  return {};
+  StatsDuration duration;
+  ElapsedTime elapased(duration);
+  std::string local_path;
+  {
+ElapsedTime elapsed(duration);
+llvm::Expected result = llvm::getCachedOrDownloadArtifact(
+cache_file_name, url_path, cache_path, debuginfod_urls, timeout);

GeorgeHuyubo wrote:

This ElapsedTime work as a scoped timer, it should try to store the time into 
duration in its destructor.

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


[Lldb-commits] [lldb] [lldb][lldb-dap] Respect x86 disassembly flavor setting (PR #134722)

2025-04-07 Thread Ebuka Ezike via lldb-commits

https://github.com/da-viper created 
https://github.com/llvm/llvm-project/pull/134722

Ensure the disassembly respects the "target.x86-disassembly-flavor" setting for 
x86 and x86_64 targets.

Depends on #134626

>From c3b28161884d44d1c0c0e45ef4025bea24bc3bc3 Mon Sep 17 00:00:00 2001
From: Ebuka Ezike 
Date: Mon, 7 Apr 2025 20:43:30 +0100
Subject: [PATCH] [lldb][lldb-dap] Respect x86 disassembly flavor setting

Ensure the disassembly respects the "target.x86-disassembly-flavor" setting for 
x86 and x86_64 targets.

Depends on #134626
---
 .../Handler/DisassembleRequestHandler.cpp   | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp 
b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index f0cb7be70210d..0fd9390623046 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -116,7 +116,22 @@ void DisassembleRequestHandler::operator()(
 
   const auto inst_count =
   GetInteger(arguments, "instructionCount").value_or(0);
-  lldb::SBInstructionList insts = dap.target.ReadInstructions(addr, 
inst_count);
+
+  std::string flavor_string{};
+  const auto target_triple = llvm::StringRef(dap.target.GetTriple());
+  if (target_triple.starts_with("x86_64") || target_triple.starts_with("x86")) 
{
+const lldb::SBStructuredData flavor =
+dap.debugger.GetSetting("target.x86-disassembly-flavor");
+
+const size_t str_length = flavor.GetStringValue(nullptr, 0);
+if (str_length != 0) {
+  flavor_string.resize(str_length + 1);
+  flavor.GetStringValue(flavor_string.data(), flavor_string.length());
+}
+  }
+
+  lldb::SBInstructionList insts =
+  dap.target.ReadInstructions(addr, inst_count, flavor_string.c_str());
 
   if (!insts.IsValid()) {
 response["success"] = false;

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


[Lldb-commits] [lldb] [lldb][lldb-dap] Respect x86 disassembly flavor setting (PR #134722)

2025-04-07 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)


Changes

Ensure the disassembly respects the "target.x86-disassembly-flavor" setting for 
x86 and x86_64 targets.

Depends on #134626

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


1 Files Affected:

- (modified) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+16-1) 


``diff
diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp 
b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index f0cb7be70210d..0fd9390623046 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -116,7 +116,22 @@ void DisassembleRequestHandler::operator()(
 
   const auto inst_count =
   GetInteger(arguments, "instructionCount").value_or(0);
-  lldb::SBInstructionList insts = dap.target.ReadInstructions(addr, 
inst_count);
+
+  std::string flavor_string{};
+  const auto target_triple = llvm::StringRef(dap.target.GetTriple());
+  if (target_triple.starts_with("x86_64") || target_triple.starts_with("x86")) 
{
+const lldb::SBStructuredData flavor =
+dap.debugger.GetSetting("target.x86-disassembly-flavor");
+
+const size_t str_length = flavor.GetStringValue(nullptr, 0);
+if (str_length != 0) {
+  flavor_string.resize(str_length + 1);
+  flavor.GetStringValue(flavor_string.data(), flavor_string.length());
+}
+  }
+
+  lldb::SBInstructionList insts =
+  dap.target.ReadInstructions(addr, inst_count, flavor_string.c_str());
 
   if (!insts.IsValid()) {
 response["success"] = false;

``




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


[Lldb-commits] [lldb] [LLDB][NFC] Remove Debugger dependency in SystemLifetimeManager (PR #134383)

2025-04-07 Thread Dmitry Vasilyev via lldb-commits

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

>From 590d5b47b9f98a8e5f19945334b2a1c34248f9d8 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 4 Apr 2025 17:49:07 +0400
Subject: [PATCH 1/3] [LLDB][NFC] Remove Debugger dependency in
 SystemLifetimeManager

It reduces the memory usage in lldb-server.
Later I will try to remove the rest Debugger dependencies to reduce lldb-server 
size.
---
 .../Initialization/SystemLifetimeManager.h|  5 ++-
 .../Initialization/SystemLifetimeManagerDbg.h | 36 +++
 lldb/source/API/SBDebugger.cpp|  4 +--
 .../Initialization/SystemLifetimeManager.cpp  |  5 ++-
 lldb/tools/lldb-test/lldb-test.cpp|  4 +--
 5 files changed, 46 insertions(+), 8 deletions(-)
 create mode 100644 lldb/include/lldb/Initialization/SystemLifetimeManagerDbg.h

diff --git a/lldb/include/lldb/Initialization/SystemLifetimeManager.h 
b/lldb/include/lldb/Initialization/SystemLifetimeManager.h
index 06328e60133fe..55138b33be712 100644
--- a/lldb/include/lldb/Initialization/SystemLifetimeManager.h
+++ b/lldb/include/lldb/Initialization/SystemLifetimeManager.h
@@ -21,7 +21,7 @@ namespace lldb_private {
 class SystemLifetimeManager {
 public:
   SystemLifetimeManager();
-  ~SystemLifetimeManager();
+  virtual ~SystemLifetimeManager();
 
   llvm::Error Initialize(std::unique_ptr initializer,
  LoadPluginCallbackType plugin_callback);
@@ -32,6 +32,9 @@ class SystemLifetimeManager {
   std::unique_ptr m_initializer;
   bool m_initialized = false;
 
+  virtual void InitializeDebugger(LoadPluginCallbackType plugin_callback) {};
+  virtual void TerminateDebugger() {};
+
   // Noncopyable.
   SystemLifetimeManager(const SystemLifetimeManager &other) = delete;
   SystemLifetimeManager &operator=(const SystemLifetimeManager &other) = 
delete;
diff --git a/lldb/include/lldb/Initialization/SystemLifetimeManagerDbg.h 
b/lldb/include/lldb/Initialization/SystemLifetimeManagerDbg.h
new file mode 100644
index 0..5e728398f71bd
--- /dev/null
+++ b/lldb/include/lldb/Initialization/SystemLifetimeManagerDbg.h
@@ -0,0 +1,36 @@
+//===-- SystemLifetimeManagerDbg.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_INITIALIZATION_SYSTEMLIFETIMEMANAGERDBG_H
+#define LLDB_INITIALIZATION_SYSTEMLIFETIMEMANAGERDBG_H
+
+#include "SystemLifetimeManager.h"
+#include "lldb/Core/Debugger.h"
+
+namespace lldb_private {
+
+class SystemLifetimeManagerDbg : public SystemLifetimeManager {
+public:
+  SystemLifetimeManagerDbg() : SystemLifetimeManager() {};
+
+private:
+  virtual void
+  InitializeDebugger(LoadPluginCallbackType plugin_callback) override {
+Debugger::Initialize(plugin_callback);
+  };
+
+  virtual void TerminateDebugger() override { Debugger::Terminate(); };
+
+  // Noncopyable.
+  SystemLifetimeManagerDbg(const SystemLifetimeManagerDbg &other) = delete;
+  SystemLifetimeManagerDbg &
+  operator=(const SystemLifetimeManagerDbg &other) = delete;
+};
+} // namespace lldb_private
+
+#endif
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index e646b09e05852..55ccfd415ca47 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -44,7 +44,7 @@
 #include "lldb/Host/Config.h"
 #include "lldb/Host/StreamFile.h"
 #include "lldb/Host/XML.h"
-#include "lldb/Initialization/SystemLifetimeManager.h"
+#include "lldb/Initialization/SystemLifetimeManagerDbg.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupPlatform.h"
@@ -66,7 +66,7 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static llvm::ManagedStatic g_debugger_lifetime;
+static llvm::ManagedStatic g_debugger_lifetime;
 
 SBError SBInputReader::Initialize(
 lldb::SBDebugger &sb_debugger,
diff --git a/lldb/source/Initialization/SystemLifetimeManager.cpp 
b/lldb/source/Initialization/SystemLifetimeManager.cpp
index f9de41a675356..b07fe71affec7 100644
--- a/lldb/source/Initialization/SystemLifetimeManager.cpp
+++ b/lldb/source/Initialization/SystemLifetimeManager.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Initialization/SystemLifetimeManager.h"
 
-#include "lldb/Core/Debugger.h"
 #include "lldb/Initialization/SystemInitializer.h"
 
 #include 
@@ -36,7 +35,7 @@ llvm::Error SystemLifetimeManager::Initialize(
 if (auto e = m_initializer->Initialize())
   return e;
 
-Debugger::Initialize(plugin_callback);
+InitializeDebugger(plugin_callback);
   }
 
   return llvm::Error::success();
@@ -46,7 +45,7 @@ void SystemLifetimeManager::Terminate() {
   std::lock_guard guard(m_mutex);
 
   if (m_initialized)

[Lldb-commits] [lldb] [lldb] Support negative function offsets in UnwindPlans (PR #134662)

2025-04-07 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

These are needed for functions whose entry point is not their lowest address.

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


5 Files Affected:

- (modified) lldb/include/lldb/Symbol/UnwindPlan.h (+5-5) 
- (modified) 
lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp (+3-2) 
- (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+1-1) 
- (modified) lldb/source/Symbol/UnwindPlan.cpp (+3-3) 
- (modified) lldb/unittests/Symbol/UnwindPlanTest.cpp (+5) 


``diff
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h 
b/lldb/include/lldb/Symbol/UnwindPlan.h
index 6640a23a3e868..410289851a879 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -356,11 +356,11 @@ class UnwindPlan {
 
 void RemoveRegisterInfo(uint32_t reg_num);
 
-lldb::addr_t GetOffset() const { return m_offset; }
+int64_t GetOffset() const { return m_offset; }
 
-void SetOffset(lldb::addr_t offset) { m_offset = offset; }
+void SetOffset(int64_t offset) { m_offset = offset; }
 
-void SlideOffset(lldb::addr_t offset) { m_offset += offset; }
+void SlideOffset(int64_t offset) { m_offset += offset; }
 
 const FAValue &GetCFAValue() const { return m_cfa_value; }
 FAValue &GetCFAValue() { return m_cfa_value; }
@@ -420,7 +420,7 @@ class UnwindPlan {
 
   protected:
 typedef std::map collection;
-lldb::addr_t m_offset = 0; // Offset into the function for this row
+int64_t m_offset = 0; // Offset into the function for this row
 
 FAValue m_cfa_value;
 FAValue m_afa_value;
@@ -472,7 +472,7 @@ class UnwindPlan {
   // practice, the UnwindPlan for a function with no known start address will 
be
   // the architectural default UnwindPlan which will only have one row.
   const UnwindPlan::Row *
-  GetRowForFunctionOffset(std::optional offset) const;
+  GetRowForFunctionOffset(std::optional offset) const;
 
   lldb::RegisterKind GetRegisterKind() const { return m_register_kind; }
 
diff --git 
a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp 
b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
index 3eaa2f33fce3e..aaff278ca31e2 100644
--- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
+++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp
@@ -1390,11 +1390,12 @@ bool 
x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(
 
 // If we already have one row for this instruction, we can continue.
 while (row_id < unwind_plan.GetRowCount() &&
-   unwind_plan.GetRowAtIndex(row_id)->GetOffset() <= offset) {
+   unwind_plan.GetRowAtIndex(row_id)->GetOffset() <=
+   static_cast(offset)) {
   row_id++;
 }
 const UnwindPlan::Row *original_row = unwind_plan.GetRowAtIndex(row_id - 
1);
-if (original_row->GetOffset() == offset) {
+if (original_row->GetOffset() == static_cast(offset)) {
   row = *original_row;
   continue;
 }
diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp 
b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
index 957818e8d077f..dca3f665b0b80 100644
--- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp
+++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp
@@ -765,7 +765,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t 
dwarf_offset,
  __FUNCTION__, dwarf_offset, startaddr.GetFileAddress());
 break;
   }
-  lldb::addr_t offset = row.GetOffset();
+  int64_t offset = row.GetOffset();
   row = std::move(stack.back());
   stack.pop_back();
   row.SetOffset(offset);
diff --git a/lldb/source/Symbol/UnwindPlan.cpp 
b/lldb/source/Symbol/UnwindPlan.cpp
index cfa8eefaa55bb..33aba0a859d5c 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -398,10 +398,10 @@ void UnwindPlan::AppendRow(Row row) {
 }
 
 struct RowLess {
-  bool operator()(addr_t a, const UnwindPlan::RowSP &b) const {
+  bool operator()(int64_t a, const UnwindPlan::RowSP &b) const {
 return a < b->GetOffset();
   }
-  bool operator()(const UnwindPlan::RowSP &a, addr_t b) const {
+  bool operator()(const UnwindPlan::RowSP &a, int64_t b) const {
 return a->GetOffset() < b;
   }
 };
@@ -418,7 +418,7 @@ void UnwindPlan::InsertRow(Row row, bool replace_existing) {
 }
 
 const UnwindPlan::Row *
-UnwindPlan::GetRowForFunctionOffset(std::optional offset) const {
+UnwindPlan::GetRowForFunctionOffset(std::optional offset) const {
   auto it = offset ? llvm::upper_bound(m_row_list, *offset, RowLess())
: m_row_list.end();
   if (it == m_row_list.begin())
diff --git a/lldb/unittests/Symbol/UnwindPlanTest.cpp 
b/lldb/unittests/Symbol/UnwindPlanTest.cpp
index fa8bb153e9247..08aa5b2dd84bb 100644
--- a/lldb/unittests/Symbol/UnwindPlanTest.cpp
+++ b/lldb/unittests/Symbol/UnwindPlanTest.cp