[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
@@ -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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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