[Lldb-commits] [lldb] 23c9cfc - [lldb] Small refactor of eh_frame parsing (#134806)
Author: Pavel Labath Date: 2025-04-11T09:27:56+02:00 New Revision: 23c9cfcb7494b2331a80661a0cb83f95a422833c URL: https://github.com/llvm/llvm-project/commit/23c9cfcb7494b2331a80661a0cb83f95a422833c DIFF: https://github.com/llvm/llvm-project/commit/23c9cfcb7494b2331a80661a0cb83f95a422833c.diff LOG: [lldb] Small refactor of eh_frame parsing (#134806) .. which prepares us for handling of discontinuous functions. The main change there is that we can have multiple FDEs contributing towards an unwind plan of a single function. This patch separates the logic for parsing of a single FDE from the construction of an UnwindPlan (so that another patch can change the latter to combine several unwind plans). Co-authored-by: Jonas Devlieghere Added: Modified: lldb/include/lldb/Symbol/DWARFCallFrameInfo.h lldb/source/Symbol/DWARFCallFrameInfo.cpp Removed: diff --git a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h index 6cc24a02de257..679f652c7f2e0 100644 --- a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h +++ b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h @@ -128,8 +128,14 @@ class DWARFCallFrameInfo { void GetFDEIndex(); - bool FDEToUnwindPlan(dw_offset_t offset, Address startaddr, - UnwindPlan &unwind_plan); + /// Parsed representation of a Frame Descriptor Entry. + struct FDE { +AddressRange range; +bool for_signal_trap = false; +uint32_t return_addr_reg_num = LLDB_INVALID_REGNUM; +std::vector rows; + }; + std::optional ParseFDE(dw_offset_t offset, const Address &startaddr); const CIE *GetCIE(dw_offset_t cie_offset); diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index dac4cd9745f96..fb57c61d413aa 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -168,9 +168,32 @@ bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range, module_sp->GetObjectFile() != &m_objfile) return false; - if (std::optional entry = GetFirstFDEEntryInRange(range)) -return FDEToUnwindPlan(entry->data, addr, unwind_plan); - return false; + std::optional entry = GetFirstFDEEntryInRange(range); + if (!entry) +return false; + + std::optional fde = ParseFDE(entry->data, addr); + if (!fde) +return false; + + unwind_plan.SetSourceName(m_type == EH ? "eh_frame CFI" : "DWARF CFI"); + // In theory the debug_frame info should be valid at all call sites + // ("asynchronous unwind info" as it is sometimes called) but in practice + // gcc et al all emit call frame info for the prologue and call sites, but + // not for the epilogue or all the other locations during the function + // reliably. + unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); + unwind_plan.SetSourcedFromCompiler(eLazyBoolYes); + unwind_plan.SetRegisterKind(GetRegisterKind()); + + unwind_plan.SetPlanValidAddressRanges({fde->range}); + unwind_plan.SetUnwindPlanForSignalTrap(fde->for_signal_trap ? eLazyBoolYes + : eLazyBoolNo); + unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num); + for (UnwindPlan::Row &row : fde->rows) +unwind_plan.AppendRow(std::move(row)); + + return true; } bool DWARFCallFrameInfo::GetAddressRange(Address addr, AddressRange &range) { @@ -522,15 +545,15 @@ void DWARFCallFrameInfo::GetFDEIndex() { m_fde_index_initialized = true; } -bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, - Address startaddr, - UnwindPlan &unwind_plan) { +std::optional +DWARFCallFrameInfo::ParseFDE(dw_offset_t dwarf_offset, + const Address &startaddr) { Log *log = GetLog(LLDBLog::Unwind); lldb::offset_t offset = dwarf_offset; lldb::offset_t current_entry = offset; - if (m_section_sp.get() == nullptr || m_section_sp->IsEncrypted()) -return false; + if (!m_section_sp || m_section_sp->IsEncrypted()) +return std::nullopt; if (!m_cfi_data_initialized) GetCFIData(); @@ -550,20 +573,8 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // Translate the CIE_id from the eh_frame format, which is relative to the // FDE offset, into a __eh_frame section offset - if (m_type == EH) { -unwind_plan.SetSourceName("eh_frame CFI"); + if (m_type == EH) cie_offset = current_entry + (is_64bit ? 12 : 4) - cie_offset; -unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); - } else { -unwind_plan.SetSourceName("DWARF CFI"); -// In theory the debug_frame info should be valid at all call sites -// ("asynchronous unwind info" as it is sometimes called) but in practice -// gcc et al all emit call frame info for the prologue and call
[Lldb-commits] [lldb] [lldb] Small refactor of eh_frame parsing (PR #134806)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/134806 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (PR #135331)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/135331 Same cleanup as in https://github.com/llvm/llvm-project/pull/135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication. >From 3c2abf4fb8ddf6f8f8df604034d4769403dedc77 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 09:45:03 +0100 Subject: [PATCH] [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style Same cleanup as in https://github.com/llvm/llvm-project/pull/135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication. --- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 129 +++--- 1 file changed, 76 insertions(+), 53 deletions(-) diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index 4b045d12ad494..09fe9be665df7 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -1697,65 +1697,88 @@ bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const { return file_path.contains("/usr/include/c++/"); } +static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { + assert(sc.function); + + if (sc.block) +if (Block *inline_block = sc.block->GetContainingInlinedBlock()) + return inline_block->GetBlockVariableList(true); + + return sc.function->GetBlock(true).GetBlockVariableList(true); +} + +static char const *GetInlinedFunctionName(const SymbolContext &sc) { + if (!sc.block) +return nullptr; + + const Block *inline_block = sc.block->GetContainingInlinedBlock(); + if (!inline_block) +return nullptr; + + const InlineFunctionInfo *inline_info = + inline_block->GetInlinedFunctionInfo(); + if (!inline_info) +return nullptr; + + return inline_info->GetName().AsCString(nullptr); +} + +static bool PrintFunctionNameWithArgs(Stream &s, + const ExecutionContext *exe_ctx, + const SymbolContext &sc) { + assert(sc.function); + + ExecutionContextScope *exe_scope = + exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; + + const char *cstr = sc.function->GetName().AsCString(nullptr); + if (!cstr) +return false; + + if (const char *inlined_name = GetInlinedFunctionName(sc)) { +s.PutCString(cstr); +s.PutCString(" [inlined] "); +cstr = inlined_name; + } + + VariableList args; + if (auto variable_list_sp = GetFunctionVariableList(sc)) +variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, + args); + + if (args.GetSize() > 0) { +PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args); + } else { +s.PutCString(cstr); + } + + return true; +} + bool CPlusPlusLanguage::GetFunctionDisplayName( const SymbolContext *sc, const ExecutionContext *exe_ctx, FunctionNameRepresentation representation, Stream &s) { switch (representation) { case FunctionNameRepresentation::eNameWithArgs: { +assert(sc); + // Print the function name with arguments in it -if (sc->function) { - ExecutionContextScope *exe_scope = - exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc->function->GetName().AsCString(nullptr); - if (cstr) { -const InlineFunctionInfo *inline_info = nullptr; -VariableListSP variable_list_sp; -bool get_function_vars = true; -if (sc->block) { - Block *inline_block = sc->block->GetContainingInlinedBlock(); - - if (inline_block) { -get_function_vars = false; -inline_info = inline_block->GetInlinedFunctionInfo(); -if (inline_info) - variable_list_sp = inline_block->GetBlockVariableList(true); - } -} - -if (get_function_vars) { - variable_list_sp = - sc->function->GetBlock(true).GetBlockVariableList(true); -} - -if (inline_info) { - s.PutCString(cstr); - s.PutCString(" [inlined] "); - cstr = inline_info->GetName().GetCString(); -} - -VariableList args; -if (variable_list_sp) - variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, - args); -if (args.GetSize() > 0) { - if (!PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args)) -return false; -} else { - s.PutCString(cstr); -} -return true; - } -} else if (sc->symbol) { - const char *cstr = sc->symbol->GetName().AsCString(nullptr); - if (cstr) { -
[Lldb-commits] [lldb] [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (PR #135331)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes Same cleanup as in https://github.com/llvm/llvm-project/pull/135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication. --- Full diff: https://github.com/llvm/llvm-project/pull/135331.diff 1 Files Affected: - (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (+76-53) ``diff diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index 4b045d12ad494..09fe9be665df7 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -1697,65 +1697,88 @@ bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const { return file_path.contains("/usr/include/c++/"); } +static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { + assert(sc.function); + + if (sc.block) +if (Block *inline_block = sc.block->GetContainingInlinedBlock()) + return inline_block->GetBlockVariableList(true); + + return sc.function->GetBlock(true).GetBlockVariableList(true); +} + +static char const *GetInlinedFunctionName(const SymbolContext &sc) { + if (!sc.block) +return nullptr; + + const Block *inline_block = sc.block->GetContainingInlinedBlock(); + if (!inline_block) +return nullptr; + + const InlineFunctionInfo *inline_info = + inline_block->GetInlinedFunctionInfo(); + if (!inline_info) +return nullptr; + + return inline_info->GetName().AsCString(nullptr); +} + +static bool PrintFunctionNameWithArgs(Stream &s, + const ExecutionContext *exe_ctx, + const SymbolContext &sc) { + assert(sc.function); + + ExecutionContextScope *exe_scope = + exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; + + const char *cstr = sc.function->GetName().AsCString(nullptr); + if (!cstr) +return false; + + if (const char *inlined_name = GetInlinedFunctionName(sc)) { +s.PutCString(cstr); +s.PutCString(" [inlined] "); +cstr = inlined_name; + } + + VariableList args; + if (auto variable_list_sp = GetFunctionVariableList(sc)) +variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, + args); + + if (args.GetSize() > 0) { +PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args); + } else { +s.PutCString(cstr); + } + + return true; +} + bool CPlusPlusLanguage::GetFunctionDisplayName( const SymbolContext *sc, const ExecutionContext *exe_ctx, FunctionNameRepresentation representation, Stream &s) { switch (representation) { case FunctionNameRepresentation::eNameWithArgs: { +assert(sc); + // Print the function name with arguments in it -if (sc->function) { - ExecutionContextScope *exe_scope = - exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc->function->GetName().AsCString(nullptr); - if (cstr) { -const InlineFunctionInfo *inline_info = nullptr; -VariableListSP variable_list_sp; -bool get_function_vars = true; -if (sc->block) { - Block *inline_block = sc->block->GetContainingInlinedBlock(); - - if (inline_block) { -get_function_vars = false; -inline_info = inline_block->GetInlinedFunctionInfo(); -if (inline_info) - variable_list_sp = inline_block->GetBlockVariableList(true); - } -} - -if (get_function_vars) { - variable_list_sp = - sc->function->GetBlock(true).GetBlockVariableList(true); -} - -if (inline_info) { - s.PutCString(cstr); - s.PutCString(" [inlined] "); - cstr = inline_info->GetName().GetCString(); -} - -VariableList args; -if (variable_list_sp) - variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, - args); -if (args.GetSize() > 0) { - if (!PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args)) -return false; -} else { - s.PutCString(cstr); -} -return true; - } -} else if (sc->symbol) { - const char *cstr = sc->symbol->GetName().AsCString(nullptr); - if (cstr) { -s.PutCString(cstr); -return true; - } -} - } break; - default: +if (sc->function) + return PrintFunctionNameWithArgs(s, exe_ctx, *sc); + +if (!sc->symbol) + return false; + +const char *cstr = sc->symbol->GetName().AsCString(nullptr); +if (!cstr) + return false; + +s.PutCString(cstr); + +return true; + } + case FunctionNameRepresentation::e
[Lldb-commits] [lldb] [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (PR #135331)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/135331 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Slide eh_frame unwind plan if it doesn't begin at function boundary (PR #135333)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes This is mainly useful for discontinuous functions because individual parts of the function will have separate FDE entries, which can begin many megabytes from the start of the function. However, I'm separating it out, because it turns out we already have a test case for the situation where the FDE does not begin exactly at the function boundary. The test works mostly by accident because the FDE starts only one byte after the beginning of the function so it doesn't really matter whether one looks up the unwind row using the function or fde offset. In this patch, I beef up the test to catch this problem more reliably. To make this work I've also needed to change a couple of places which that an unwind plan always has a row at offset zero. --- Full diff: https://github.com/llvm/llvm-project/pull/135333.diff 6 Files Affected: - (modified) lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp (+1-1) - (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+5-1) - (modified) lldb/source/Symbol/UnwindPlan.cpp (+1-1) - (modified) lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s (+10-2) - (modified) lldb/test/Shell/Unwind/eh-frame-small-fde.test (+4-3) - (modified) lldb/unittests/Symbol/UnwindPlanTest.cpp (+6-1) ``diff diff --git a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp index 5f0a863e936d4..f5279758a147c 100644 --- a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp +++ b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp @@ -73,7 +73,7 @@ bool UnwindAssembly_x86::AugmentUnwindPlanFromCallSite( int wordsize = 8; ProcessSP process_sp(thread.GetProcess()); - if (process_sp.get() == nullptr) + if (!process_sp || !first_row || !last_row) return false; wordsize = process_sp->GetTarget().GetArchitecture().GetAddressByteSize(); diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index fb57c61d413aa..a763acb1fdf9e 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -190,8 +190,12 @@ bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range, unwind_plan.SetUnwindPlanForSignalTrap(fde->for_signal_trap ? eLazyBoolYes : eLazyBoolNo); unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num); - for (UnwindPlan::Row &row : fde->rows) + int64_t slide = + fde->range.GetBaseAddress().GetFileAddress() - addr.GetFileAddress(); + for (UnwindPlan::Row &row : fde->rows) { +row.SlideOffset(slide); unwind_plan.AppendRow(std::move(row)); + } return true; } diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp index 94c23137e8f12..b1a96b5e26840 100644 --- a/lldb/source/Symbol/UnwindPlan.cpp +++ b/lldb/source/Symbol/UnwindPlan.cpp @@ -474,7 +474,7 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) const { // If the 0th Row of unwind instructions is missing, or if it doesn't provide // a register to use to find the Canonical Frame Address, this is not a valid // UnwindPlan. - const Row *row0 = GetRowForFunctionOffset(0); + const Row *row0 = GetRowAtIndex(0); if (!row0 || row0->GetCFAValue().GetValueType() == Row::FAValue::unspecified) { Log *log = GetLog(LLDBLog::Unwind); diff --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s index b08436af3f2ea..29decefad5e51 100644 --- a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s +++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s @@ -10,12 +10,20 @@ bar: .type foo, @function foo: -nop # Make the FDE entry start one byte later than the actual function. +# Make the FDE entry start 16 bytes later than the actual function. The +# size is chosen such that it is larger than the size of the FDE entry. +# This allows us to test that we're using the correct offset for +# unwinding (we'll stop 21 bytes into the function, but only 5 bytes +# into the FDE). +.nops 16 .cfi_startproc .cfi_register %rip, %r13 callbar addl$1, %eax -jmp *%r13 # Return +movq%r13, %r14 +.cfi_register %rip, %r14 +movq$0, %r13 +jmp *%r14 # Return .cfi_endproc .size foo, .-foo diff --git a/lldb/test/Shell/Unwind/eh-frame-small-fde.test b/lldb/test/Shell/Unwind/eh-frame-small-fde.test index 0ece6c2a12a3e..d86d41f73f1c1 100644 --- a/lldb/test/Shell/Unwind/eh-frame-small-fde.test +++ b/lldb/test/Shell/Unwind/eh-frame-small-fde.test @@ -14,9 +14,10 @@ process launch thread backtrace # CHECK: frame #0: {{.*}}`bar -# CHECK: frame #1: {{.*}}`foo + 6 +# CHECK: frame #1
[Lldb-commits] [lldb] [lldb] Slide eh_frame unwind plan if it doesn't begin at function boundary (PR #135333)
@@ -474,7 +474,7 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) const { // If the 0th Row of unwind instructions is missing, or if it doesn't provide // a register to use to find the Canonical Frame Address, this is not a valid // UnwindPlan. - const Row *row0 = GetRowForFunctionOffset(0); + const Row *row0 = GetRowAtIndex(0); labath wrote: This actually changes code back to the state before I started messing with it (#127661). In that patch, I changed it to use `GetRowForFunctionOffset` because I thought it better captures the original intent of the code ("check the value of CFA at function entry"). I still sort of think that, but now I also think that check is too strict (just because we don't have a CFA at offset zero, it doesn't mean we won't have it at some other offset). So this goes back to checking the first row (whatever it's offset), but I'm also open to other options (checking that *any* row contains the CFA, or maybe removing the check entirely -- just checking that *a* row exists) https://github.com/llvm/llvm-project/pull/135333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Slide eh_frame unwind plan if it doesn't begin at function boundary (PR #135333)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/135333 This is mainly useful for discontinuous functions because individual parts of the function will have separate FDE entries, which can begin many megabytes from the start of the function. However, I'm separating it out, because it turns out we already have a test case for the situation where the FDE does not begin exactly at the function boundary. The test works mostly by accident because the FDE starts only one byte after the beginning of the function so it doesn't really matter whether one looks up the unwind row using the function or fde offset. In this patch, I beef up the test to catch this problem more reliably. To make this work I've also needed to change a couple of places which that an unwind plan always has a row at offset zero. >From e706d5e63a4d9cf588674b38ecd617ae4c7e1200 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 11 Apr 2025 10:31:32 +0200 Subject: [PATCH] [lldb] Slide eh_frame unwind plan if it doesn't begin at function boundary This is mainly useful for discontinuous functions because individual parts of the function will have separate FDE entries, which can begin many megabytes from the start of the function. However, I'm separating it out, because it turns out we already have a test case for the situation where the FDE does not begin exactly at the function boundary. The test works mostly by accident because the FDE starts only one byte after the beginning of the function so it doesn't really matter whether one looks up the unwind row using the function or fde offset. In this patch, I beef up the test to catch this problem more reliably. To make this work I've also needed to change a couple of places which that an unwind plan always has a row at offset zero. --- .../UnwindAssembly/x86/UnwindAssembly-x86.cpp| 2 +- lldb/source/Symbol/DWARFCallFrameInfo.cpp| 6 +- lldb/source/Symbol/UnwindPlan.cpp| 2 +- lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s | 12 ++-- lldb/test/Shell/Unwind/eh-frame-small-fde.test | 7 --- lldb/unittests/Symbol/UnwindPlanTest.cpp | 7 ++- 6 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp index 5f0a863e936d4..f5279758a147c 100644 --- a/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp +++ b/lldb/source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp @@ -73,7 +73,7 @@ bool UnwindAssembly_x86::AugmentUnwindPlanFromCallSite( int wordsize = 8; ProcessSP process_sp(thread.GetProcess()); - if (process_sp.get() == nullptr) + if (!process_sp || !first_row || !last_row) return false; wordsize = process_sp->GetTarget().GetArchitecture().GetAddressByteSize(); diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index fb57c61d413aa..a763acb1fdf9e 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -190,8 +190,12 @@ bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range, unwind_plan.SetUnwindPlanForSignalTrap(fde->for_signal_trap ? eLazyBoolYes : eLazyBoolNo); unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num); - for (UnwindPlan::Row &row : fde->rows) + int64_t slide = + fde->range.GetBaseAddress().GetFileAddress() - addr.GetFileAddress(); + for (UnwindPlan::Row &row : fde->rows) { +row.SlideOffset(slide); unwind_plan.AppendRow(std::move(row)); + } return true; } diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp index 94c23137e8f12..b1a96b5e26840 100644 --- a/lldb/source/Symbol/UnwindPlan.cpp +++ b/lldb/source/Symbol/UnwindPlan.cpp @@ -474,7 +474,7 @@ bool UnwindPlan::PlanValidAtAddress(Address addr) const { // If the 0th Row of unwind instructions is missing, or if it doesn't provide // a register to use to find the Canonical Frame Address, this is not a valid // UnwindPlan. - const Row *row0 = GetRowForFunctionOffset(0); + const Row *row0 = GetRowAtIndex(0); if (!row0 || row0->GetCFAValue().GetValueType() == Row::FAValue::unspecified) { Log *log = GetLog(LLDBLog::Unwind); diff --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s index b08436af3f2ea..29decefad5e51 100644 --- a/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s +++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-small-fde.s @@ -10,12 +10,20 @@ bar: .type foo, @function foo: -nop # Make the FDE entry start one byte later than the actual function. +# Make the FDE entry start 16 bytes later than the actual function. The +# size is chosen such that it is larger than the size of the
[Lldb-commits] [lldb] [lldb][lldb-dap] fix repeating commands in repl mode (PR #135008)
https://github.com/da-viper updated https://github.com/llvm/llvm-project/pull/135008 >From 296019edb5edba4a21e040feb154b1ef83f1e64d Mon Sep 17 00:00:00 2001 From: Ebuka Ezike Date: Wed, 9 Apr 2025 14:35:09 +0100 Subject: [PATCH 1/2] [lldb][lldb-dap] fix repeating commands in repl mode Fixes #131589 Add a new option to the RunCommands* functions to control the echoing of commands --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 2 +- .../tools/lldb-dap/launch/TestDAP_launch.py | 4 +-- .../repl-mode/TestDAP_repl_mode_detection.py | 11 +++--- lldb/tools/lldb-dap/DAP.cpp | 6 ++-- lldb/tools/lldb-dap/DAP.h | 3 +- .../Handler/EvaluateRequestHandler.cpp| 3 +- lldb/tools/lldb-dap/LLDBUtils.cpp | 35 ++- lldb/tools/lldb-dap/LLDBUtils.h | 19 +++--- 8 files changed, 54 insertions(+), 29 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 251d77d79d080..e2f843bd337a6 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -101,7 +101,7 @@ def run_test_evaluate_expressions( if context == "repl": # In the repl context expressions may be interpreted as lldb # commands since no variables have the same name as the command. -self.assertEvaluate("list", r"\(lldb\) list\n.*") +self.assertEvaluate("list", r".*") else: self.assertEvaluateFailure("list") # local variable of a_function diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py index 64c99019a1c9b..eceba2f8a13cb 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -522,11 +522,9 @@ def test_version(self): ) version_eval_output = version_eval_response["body"]["result"] -# The first line is the prompt line like "(lldb) version", so we skip it. -version_eval_output_without_prompt_line = version_eval_output.splitlines()[1:] version_string = self.dap_server.get_initialize_value("$__lldb_version") self.assertEqual( -version_eval_output_without_prompt_line, +version_eval_output.splitlines(), version_string.splitlines(), "version string does not match", ) diff --git a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py index 7c77fc8541b93..09ca725ee8883 100644 --- a/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py +++ b/lldb/test/API/tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py @@ -28,15 +28,12 @@ def test_completions(self): self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line]) -self.assertEvaluate( -"`command regex user_command s/^$/platform/", r"\(lldb\) command regex" -) -self.assertEvaluate( -"`command alias alias_command platform", r"\(lldb\) command alias" -) +# the result of the commands should return the empty string. +self.assertEvaluate("`command regex user_command s/^$/platform/", r"^$") +self.assertEvaluate("`command alias alias_command platform", r"^$") self.assertEvaluate( "`command alias alias_command_with_arg platform select --sysroot %1 remote-linux", -r"\(lldb\) command alias", +r"^$", ) self.continue_to_next_stop() diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 9361ba968e9c2..03b9dc7135ef7 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -561,10 +561,12 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression, } bool DAP::RunLLDBCommands(llvm::StringRef prefix, - llvm::ArrayRef commands) { + llvm::ArrayRef commands, + bool echo_commands) { bool required_command_failed = false; std::string output = - ::RunLLDBCommands(debugger, prefix, commands, required_command_failed); + ::RunLLDBCommands(debugger, prefix, commands, required_command_failed, +/*parse_command_directives*/ true, echo_commands); SendOutput(OutputType::Console, output); return !required_command_failed; } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index fc43d988f3a09..cb3431cc87fd1 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -290,7 +290,8 @@ struct DAP { /// \b false if a fatal error was found while executing these commands, /// according to the rules of \a LLDBUtils::RunLLDBCommands. bool RunLL
[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)
https://github.com/da-viper milestoned 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)
da-viper wrote: /cherry-pick 6c8b0a3dcb33eeb2fe57325a792ff5a70225d18e 4be5e33bf7b986e35808b83758e4a8d85b646a81 7417e79323c67ab478a0d41757ebb1679f4cb28c 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)
llvmbot wrote: Failed to cherry-pick: 6c8b0a3dcb33eeb2fe57325a792ff5a70225d18e https://github.com/llvm/llvm-project/actions/runs/14400492146 Please manually backport the fix and push it to your github fork. Once this is done, please create a [pull request](https://github.com/llvm/llvm-project/compare) 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][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (PR #135331)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/135331 ___ 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] b656915 - [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (#135331)
Author: Michael Buch Date: 2025-04-11T11:01:27+01:00 New Revision: b656915d5a4cbb35392d340142340bf9bef8d8d1 URL: https://github.com/llvm/llvm-project/commit/b656915d5a4cbb35392d340142340bf9bef8d8d1 DIFF: https://github.com/llvm/llvm-project/commit/b656915d5a4cbb35392d340142340bf9bef8d8d1.diff LOG: [lldb][Format][NFCI] Refactor CPlusPlusLanguage::GetFunctionDisplayName into helpers and use LLVM style (#135331) Same cleanup as in https://github.com/llvm/llvm-project/pull/135031. It pretty much is the same code that we had to duplicate in the language plugin. Maybe eventually we'll find a way of getting rid of the duplication. Added: Modified: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp Removed: diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp index 4b045d12ad494..a6fdf66f13e4d 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -1697,65 +1697,89 @@ bool CPlusPlusLanguage::IsSourceFile(llvm::StringRef file_path) const { return file_path.contains("/usr/include/c++/"); } +static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { + assert(sc.function); + + if (sc.block) +if (Block *inline_block = sc.block->GetContainingInlinedBlock()) + return inline_block->GetBlockVariableList(true); + + return sc.function->GetBlock(true).GetBlockVariableList(true); +} + +static char const *GetInlinedFunctionName(const SymbolContext &sc) { + if (!sc.block) +return nullptr; + + const Block *inline_block = sc.block->GetContainingInlinedBlock(); + if (!inline_block) +return nullptr; + + const InlineFunctionInfo *inline_info = + inline_block->GetInlinedFunctionInfo(); + if (!inline_info) +return nullptr; + + return inline_info->GetName().AsCString(nullptr); +} + +static bool PrintFunctionNameWithArgs(Stream &s, + const ExecutionContext *exe_ctx, + const SymbolContext &sc) { + assert(sc.function); + + ExecutionContextScope *exe_scope = + exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; + + const char *cstr = sc.function->GetName().AsCString(nullptr); + if (!cstr) +return false; + + if (const char *inlined_name = GetInlinedFunctionName(sc)) { +s.PutCString(cstr); +s.PutCString(" [inlined] "); +cstr = inlined_name; + } + + VariableList args; + if (auto variable_list_sp = GetFunctionVariableList(sc)) +variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, + args); + + if (args.GetSize() > 0) +return PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args); + + // FIXME: can we just unconditionally call PrettyPrintFunctionNameWithArgs? + // It should be able to handle the "no arguments" case. + s.PutCString(cstr); + + return true; +} + bool CPlusPlusLanguage::GetFunctionDisplayName( const SymbolContext *sc, const ExecutionContext *exe_ctx, FunctionNameRepresentation representation, Stream &s) { switch (representation) { case FunctionNameRepresentation::eNameWithArgs: { +assert(sc); + // Print the function name with arguments in it -if (sc->function) { - ExecutionContextScope *exe_scope = - exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc->function->GetName().AsCString(nullptr); - if (cstr) { -const InlineFunctionInfo *inline_info = nullptr; -VariableListSP variable_list_sp; -bool get_function_vars = true; -if (sc->block) { - Block *inline_block = sc->block->GetContainingInlinedBlock(); - - if (inline_block) { -get_function_vars = false; -inline_info = inline_block->GetInlinedFunctionInfo(); -if (inline_info) - variable_list_sp = inline_block->GetBlockVariableList(true); - } -} - -if (get_function_vars) { - variable_list_sp = - sc->function->GetBlock(true).GetBlockVariableList(true); -} - -if (inline_info) { - s.PutCString(cstr); - s.PutCString(" [inlined] "); - cstr = inline_info->GetName().GetCString(); -} - -VariableList args; -if (variable_list_sp) - variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, - args); -if (args.GetSize() > 0) { - if (!PrettyPrintFunctionNameWithArgs(s, cstr, exe_scope, args)) -return false; -} else { - s.PutCString(cstr); -} -return true; - } -} else if (sc->symbol) { - const char *cstr = sc->symbol->GetName().
[Lldb-commits] [lldb] [lldb][Format] Display only the inlined frame name in backtraces if available (PR #135343)
labath wrote: I think this is a good idea. https://github.com/llvm/llvm-project/pull/135343 ___ 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 frame-format option to highlight function names in backtraces (PR #131836)
https://github.com/labath 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -122,7 +122,14 @@ constexpr Definition g_function_child_entries[] = { Definition("pc-offset", EntryType::FunctionPCOffset), Definition("initial-function", EntryType::FunctionInitial), Definition("changed", EntryType::FunctionChanged), -Definition("is-optimized", EntryType::FunctionIsOptimized)}; +Definition("is-optimized", EntryType::FunctionIsOptimized), +Definition("scope", EntryType::FunctionScope), +Definition("basename", EntryType::FunctionBasename), +Definition("template-arguments", EntryType::FunctionTemplateArguments), +Definition("arguments", EntryType::FunctionArguments), +Definition("return-left", EntryType::FunctionReturnLeft), +Definition("return-right", EntryType::FunctionReturnRight), +Definition("qualifiers", EntryType::FunctionQualifiers)}; labath wrote: This should make it easier to add new entries. ```suggestion Definition("qualifiers", EntryType::FunctionQualifiers), }; ``` 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -0,0 +1,60 @@ +# Test the ${function.arguments} frame-format variable. labath wrote: Probably better to factor this according to the language dimension, since the objc parts will most likely need to be disabled on non-macosx. 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -208,6 +209,152 @@ static bool PrettyPrintFunctionNameWithArgs(Stream &out_stream, return true; } +static std::optional GetDemangledBasename(Function &function) { + auto demangled_name = function.GetName().GetStringRef(); + if (demangled_name.empty()) +return std::nullopt; + + const std::optional &info = + function.GetMangled().GetDemangledInfo(); + if (!info) +return std::nullopt; + + // Function without a basename is nonsense. + if (!info->hasBasename()) +return std::nullopt; + + assert(info->BasenameRange.first < demangled_name.size()); + assert(info->BasenameRange.second < demangled_name.size()); + + return demangled_name.substr(info->BasenameRange.first, + info->BasenameRange.second - + info->BasenameRange.first); labath wrote: ```suggestion return demangled_name.slice(info->BasenameRange.first, info->BasenameRange.second); ``` 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
https://github.com/labath commented: Just some drive-by comments. 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -208,6 +209,152 @@ static bool PrettyPrintFunctionNameWithArgs(Stream &out_stream, return true; } +static std::optional GetDemangledBasename(Function &function) { + auto demangled_name = function.GetName().GetStringRef(); + if (demangled_name.empty()) +return std::nullopt; + + const std::optional &info = + function.GetMangled().GetDemangledInfo(); + if (!info) +return std::nullopt; + + // Function without a basename is nonsense. + if (!info->hasBasename()) +return std::nullopt; + + assert(info->BasenameRange.first < demangled_name.size()); + assert(info->BasenameRange.second < demangled_name.size()); + + return demangled_name.substr(info->BasenameRange.first, + info->BasenameRange.second - + info->BasenameRange.first); +} + +static std::optional +GetDemangledTemplateArguments(Function &function) { + auto demangled_name = function.GetName().GetStringRef(); + if (demangled_name.empty()) +return std::nullopt; + + const std::optional &info = + function.GetMangled().GetDemangledInfo(); + if (!info) +return std::nullopt; + + // Function without a basename is nonsense. + if (!info->hasBasename()) +return std::nullopt; + + assert(info->BasenameRange.second < demangled_name.size()); + assert(info->ArgumentsRange.first < demangled_name.size()); + assert(info->ArgumentsRange.first >= info->BasenameRange.second); + + return demangled_name.substr(info->BasenameRange.second, + info->ArgumentsRange.first - + info->BasenameRange.second); +} + +static std::optional +GetDemangledReturnTypeLHS(Function &function) { + auto demangled_name = function.GetName().GetStringRef(); + if (demangled_name.empty()) +return std::nullopt; + + const std::optional &info = + function.GetMangled().GetDemangledInfo(); + if (!info) +return std::nullopt; + + // Function without a basename is nonsense. + if (!info->hasBasename()) +return std::nullopt; + + assert(info->ScopeRange.first < demangled_name.size()); + + return demangled_name.substr(0, info->ScopeRange.first); +} + +static std::optional +GetDemangledFunctionQualifiers(Function &function) { + auto demangled_name = function.GetName().GetStringRef(); + if (demangled_name.empty()) +return std::nullopt; + + const std::optional &info = + function.GetMangled().GetDemangledInfo(); + if (!info) +return std::nullopt; + + // Function without a basename is nonsense. + if (!info->hasBasename()) +return std::nullopt; + + assert(info->QualifiersRange.first <= demangled_name.size()); + assert(info->QualifiersRange.second <= demangled_name.size()); + assert(info->QualifiersRange.second >= info->QualifiersRange.first); + + return demangled_name.substr(info->QualifiersRange.first, + info->QualifiersRange.second - + info->QualifiersRange.first); labath wrote: The only thing that changes here is the DemangledNameInfo field, right? Maybe you could turn deduplicate that using a utility function that takes a member pointer as an argument? 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -2074,6 +2076,64 @@ static const Definition *FindEntry(const llvm::StringRef &format_str, return parent; } +/// Parses a single highlighting format specifier. +/// +/// Example syntax for such specifier: +/// \code +/// ${function.name-with-args:%highlight_basename(ansi.fg.green)} labath wrote: Does the setting affect the format of the whole frame, or just the function name. Because if it's the latter (which I also think would be a better design), then it probably should be called something else... `function-format`, `function-name-format`, ... ? 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] [lldb][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/2] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [lldb] [lldb] Make sure the process is stopped when computing the symbol context (PR #134757)
labath wrote: I can see this too, and can reproduce it pretty much 100% of the time. The `GetRunLock` hypothesis seems believable because the process launching code (Process::LaunchPrivate) is mucking around with the private state thread. It's also possible this doesn't reproduce on darwin because it goes through a different launching code path (basically it never launches, only attaches). I think we'll need to revert this for now, as it makes it more or less impossible to have a normal debug session (things actually work if you type "continue" after this error, but there's no indication that that is an option). https://github.com/llvm/llvm-project/pull/134757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove vestigial remnants of reproducers (PR #135361)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes Not touching the SB API. --- Full diff: https://github.com/llvm/llvm-project/pull/135361.diff 15 Files Affected: - (modified) lldb/include/lldb/API/SBReproducer.h (-6) - (modified) lldb/include/lldb/Core/Debugger.h (-9) - (removed) lldb/scripts/reproducer-replay.py (-121) - (modified) lldb/source/API/SBReproducer.cpp (+2-18) - (modified) lldb/source/Core/Debugger.cpp (-3) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (-3) - (modified) lldb/unittests/Core/DiagnosticEventTest.cpp (-1) - (modified) lldb/unittests/Interpreter/TestCommandPaths.cpp (-1) - (modified) lldb/unittests/Platform/PlatformSiginfoTest.cpp (-1) - (modified) lldb/unittests/Process/ProcessEventDataTest.cpp (-1) - (modified) lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp (-1) - (modified) lldb/unittests/Target/ExecutionContextTest.cpp (-1) - (modified) lldb/unittests/Target/MemoryTest.cpp (-1) - (modified) lldb/unittests/Target/StackFrameRecognizerTest.cpp (-1) - (modified) lldb/unittests/Thread/ThreadTest.cpp (-1) ``diff diff --git a/lldb/include/lldb/API/SBReproducer.h b/lldb/include/lldb/API/SBReproducer.h index c48b4747afe57..e11accf052f46 100644 --- a/lldb/include/lldb/API/SBReproducer.h +++ b/lldb/include/lldb/API/SBReproducer.h @@ -11,12 +11,6 @@ #include "lldb/API/SBDefines.h" -namespace lldb_private { -namespace repro { -struct ReplayOptions; -} -} // namespace lldb_private - namespace lldb { #ifndef SWIG diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 448e8d6a8fc6a..e9e4cabdb5732 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -67,10 +67,6 @@ class Stream; class SymbolContext; class Target; -namespace repro { -class DataRecorder; -} - /// \class Debugger Debugger.h "lldb/Core/Debugger.h" /// A class to manage flag bits. /// @@ -143,8 +139,6 @@ class Debugger : public std::enable_shared_from_this, return m_error_stream_sp->GetUnlockedFileSP(); } - repro::DataRecorder *GetInputRecorder(); - Status SetInputString(const char *data); void SetInputFile(lldb::FileSP file); @@ -720,9 +714,6 @@ class Debugger : public std::enable_shared_from_this, lldb::LockableStreamFileSP m_error_stream_sp; LockableStreamFile::Mutex m_output_mutex; - /// Used for shadowing the input file when capturing a reproducer. - repro::DataRecorder *m_input_recorder; - lldb::BroadcasterManagerSP m_broadcaster_manager_sp; // The debugger acts as a // broadcaster manager of // last resort. diff --git a/lldb/scripts/reproducer-replay.py b/lldb/scripts/reproducer-replay.py deleted file mode 100755 index f44e3cf493538..0 --- a/lldb/scripts/reproducer-replay.py +++ /dev/null @@ -1,121 +0,0 @@ -#!/usr/bin/env python3 - -from multiprocessing import Pool -import multiprocessing -import argparse -import tempfile -import logging -import os -import subprocess - - -def run_reproducer(path): -proc = subprocess.Popen( -[LLDB, "--replay", path], stdout=subprocess.PIPE, stderr=subprocess.PIPE -) -reason = None -try: -outs, errs = proc.communicate(timeout=TIMEOUT) -success = proc.returncode == 0 -result = "PASSED" if success else "FAILED" -if not success: -outs = outs.decode() -errs = errs.decode() -# Do some pattern matching to find out the cause of the failure. -if "Encountered unexpected packet during replay" in errs: -reason = "Unexpected packet" -elif "Assertion failed" in errs: -reason = "Assertion failed" -elif "UNREACHABLE" in errs: -reason = "Unreachable executed" -elif "Segmentation fault" in errs: -reason = "Segmentation fault" -elif "Illegal instruction" in errs: -reason = "Illegal instruction" -else: -reason = f"Exit code {proc.returncode}" -except subprocess.TimeoutExpired: -proc.kill() -success = False -outs, errs = proc.communicate() -result = "TIMEOUT" - -if not FAILURE_ONLY or not success: -reason_str = f" ({reason})" if reason else "" -print(f"{result}: {path}{reason_str}") -if VERBOSE: -if outs: -print(outs) -if errs: -print(errs) - - -def find_reproducers(path): -for root, dirs, files in os.walk(path): -for dir in dirs: -_, extension = os.path.splitext(dir) -if dir.startswith("Test") and extension == ".py": -yield os.path.join(root, dir) - - -if __name__ == "__main__": -parser = argparse.ArgumentParser( -descript
[Lldb-commits] [lldb] [lldb] Remove vestigial remnants of reproducers (PR #135361)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/135361 Not touching the SB API. >From 092a72d271372030f530521dabb3559aebb306fe Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 11 Apr 2025 14:51:59 +0200 Subject: [PATCH] [lldb] Remove vestigial remnants of reproducers Not touching the SB API. --- lldb/include/lldb/API/SBReproducer.h | 6 - lldb/include/lldb/Core/Debugger.h | 9 -- lldb/scripts/reproducer-replay.py | 121 -- lldb/source/API/SBReproducer.cpp | 20 +-- lldb/source/Core/Debugger.cpp | 3 - .../Process/gdb-remote/ProcessGDBRemote.h | 3 - lldb/unittests/Core/DiagnosticEventTest.cpp | 1 - .../Interpreter/TestCommandPaths.cpp | 1 - .../Platform/PlatformSiginfoTest.cpp | 1 - .../Process/ProcessEventDataTest.cpp | 1 - .../Lua/ScriptInterpreterTests.cpp| 1 - .../unittests/Target/ExecutionContextTest.cpp | 1 - lldb/unittests/Target/MemoryTest.cpp | 1 - .../Target/StackFrameRecognizerTest.cpp | 1 - lldb/unittests/Thread/ThreadTest.cpp | 1 - 15 files changed, 2 insertions(+), 169 deletions(-) delete mode 100755 lldb/scripts/reproducer-replay.py diff --git a/lldb/include/lldb/API/SBReproducer.h b/lldb/include/lldb/API/SBReproducer.h index c48b4747afe57..e11accf052f46 100644 --- a/lldb/include/lldb/API/SBReproducer.h +++ b/lldb/include/lldb/API/SBReproducer.h @@ -11,12 +11,6 @@ #include "lldb/API/SBDefines.h" -namespace lldb_private { -namespace repro { -struct ReplayOptions; -} -} // namespace lldb_private - namespace lldb { #ifndef SWIG diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 448e8d6a8fc6a..e9e4cabdb5732 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -67,10 +67,6 @@ class Stream; class SymbolContext; class Target; -namespace repro { -class DataRecorder; -} - /// \class Debugger Debugger.h "lldb/Core/Debugger.h" /// A class to manage flag bits. /// @@ -143,8 +139,6 @@ class Debugger : public std::enable_shared_from_this, return m_error_stream_sp->GetUnlockedFileSP(); } - repro::DataRecorder *GetInputRecorder(); - Status SetInputString(const char *data); void SetInputFile(lldb::FileSP file); @@ -720,9 +714,6 @@ class Debugger : public std::enable_shared_from_this, lldb::LockableStreamFileSP m_error_stream_sp; LockableStreamFile::Mutex m_output_mutex; - /// Used for shadowing the input file when capturing a reproducer. - repro::DataRecorder *m_input_recorder; - lldb::BroadcasterManagerSP m_broadcaster_manager_sp; // The debugger acts as a // broadcaster manager of // last resort. diff --git a/lldb/scripts/reproducer-replay.py b/lldb/scripts/reproducer-replay.py deleted file mode 100755 index f44e3cf493538..0 --- a/lldb/scripts/reproducer-replay.py +++ /dev/null @@ -1,121 +0,0 @@ -#!/usr/bin/env python3 - -from multiprocessing import Pool -import multiprocessing -import argparse -import tempfile -import logging -import os -import subprocess - - -def run_reproducer(path): -proc = subprocess.Popen( -[LLDB, "--replay", path], stdout=subprocess.PIPE, stderr=subprocess.PIPE -) -reason = None -try: -outs, errs = proc.communicate(timeout=TIMEOUT) -success = proc.returncode == 0 -result = "PASSED" if success else "FAILED" -if not success: -outs = outs.decode() -errs = errs.decode() -# Do some pattern matching to find out the cause of the failure. -if "Encountered unexpected packet during replay" in errs: -reason = "Unexpected packet" -elif "Assertion failed" in errs: -reason = "Assertion failed" -elif "UNREACHABLE" in errs: -reason = "Unreachable executed" -elif "Segmentation fault" in errs: -reason = "Segmentation fault" -elif "Illegal instruction" in errs: -reason = "Illegal instruction" -else: -reason = f"Exit code {proc.returncode}" -except subprocess.TimeoutExpired: -proc.kill() -success = False -outs, errs = proc.communicate() -result = "TIMEOUT" - -if not FAILURE_ONLY or not success: -reason_str = f" ({reason})" if reason else "" -print(f"{result}: {path}{reason_str}") -if VERBOSE: -if outs: -print(outs) -if errs: -print(errs) - - -def find_reproducers(path): -for root, dirs, files in os.walk(path): -for dir in dirs: -_, extension = os.path.splitext(dir) -if dir.startswith("Test") and extension == ".py": -
[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)
@@ -0,0 +1,39 @@ +""" +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"]) DavidSpickett wrote: https://lab.llvm.org/buildbot/#/builders/59/builds/15856 ``` gmake: Entering directory '/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.test_read_instructions_with_flavor_dwarf' /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang -g -O0 -I/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/make/../../../../..//include -I/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/include -I/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/python_api/target/read-instructions-flavor -I/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/make -include /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/make/test_common.h -fno-limit-debug-info -MT main.o -MD -MP -MF main.d -c -o main.o /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/python_api/target/read-instructions-flavor/main.c :2:13: error: unknown token in expression 2 | movl%edi, %eax | ^ :2:13: error: invalid operand 2 | movl%edi, %eax | ^ :3:13: error: unknown token in expression 3 | addl%esi, %eax | ^ :3:13: error: invalid operand 3 | addl%esi, %eax | ^ ``` I think this skipIf might be doing X and Y, not X or Y. Checking now. 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)
@@ -0,0 +1,39 @@ +""" +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"]) DavidSpickett wrote: Yep, it's "and": ``` # @skipIf(bugnumber, ["linux"], "gcc", ['>=', '4.9'], ['i386']), skip for gcc>=4.9 on linux with i386 (all conditions must be true) ``` That's why it's running on AArch64 linux, that matches the "not these architectures" but not the "is on windows" bit. 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] 5b384c3 - [lldb][test] Adjust TestTargetReadInstructionsFlavor skipIfs
Author: David Spickett Date: 2025-04-11T13:14:16Z New Revision: 5b384c3015100ad815f4d994d7ef35cc947db711 URL: https://github.com/llvm/llvm-project/commit/5b384c3015100ad815f4d994d7ef35cc947db711 DIFF: https://github.com/llvm/llvm-project/commit/5b384c3015100ad815f4d994d7ef35cc947db711.diff LOG: [lldb][test] Adjust TestTargetReadInstructionsFlavor skipIfs Original in https://github.com/llvm/llvm-project/pull/134626 was written as if it was "this or this" but it's "this and this". So the test ran on AArch64 Linux, because Linux is not Windows. Split out the Windows check to fix that. Added: Modified: lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py Removed: 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 index 40edc57df21ce..12805985798de 100644 --- a/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py +++ b/lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py @@ -7,7 +7,8 @@ class TargetReadInstructionsFlavor(TestBase): -@skipIf(archs=no_match(["x86_64", "x86", "i386"]), oslist=["windows"]) +@skipIfWindows +@skipIf(archs=no_match(["x86_64", "x86", "i386"])) def test_read_instructions_with_flavor(self): self.build() executable = self.getBuildArtifact("a.out") ___ 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)
@@ -0,0 +1,39 @@ +""" +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"]) DavidSpickett wrote: Fix: https://github.com/llvm/llvm-project/commit/5b384c3015100ad815f4d994d7ef35cc947db711 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)
da-viper wrote: separate it like this then it does not and it. ``` @skipIf(archs=no_match(["x86_64", "x86", "i386"])) @skipIfWindows ``` 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)
da-viper wrote: @DavidSpickett is there a way to run the merge build bot before merging to check if the test passes on other platforms ? 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)
DavidSpickett wrote: At this time no, it's a feature I'd like to have one day. My advice would have been to land it and see what happens :) Unless we had pre-commit Windows lldb, but I think right now it's only Linux and it gets disabled some times due to instability. It's fixed for AArch64 Linux at least - https://lab.llvm.org/buildbot/#/builders/59/builds/15858. 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][Format] Display only the inlined frame name in backtraces if available (PR #135343)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- lldb/include/lldb/Symbol/SymbolContext.h lldb/source/Core/FormatEntity.cpp lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp lldb/source/Symbol/SymbolContext.cpp lldb/test/API/functionalities/param_entry_vals/basic_entry_values/main.cpp lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp index 45f30e186..a9626bbc3 100644 --- a/lldb/source/Symbol/SymbolContext.cpp +++ b/lldb/source/Symbol/SymbolContext.cpp @@ -893,9 +893,8 @@ char const *SymbolContext::GetPossiblyInlinedFunctionName( return name; // If we do have an inlined frame name, return that. - if (char const *inline_name = inline_info->GetMangled() -.GetName(mangling_preference) -.AsCString()) + if (char const *inline_name = + inline_info->GetMangled().GetName(mangling_preference).AsCString()) return inline_name; // Sometimes an inline frame may not have mangling information, diff --git a/lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp b/lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp index 5e5c0aa1d..e0c8f3cd7 100644 --- a/lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp +++ b/lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp @@ -2,8 +2,9 @@ volatile int x; void __attribute__((noinline)) tail_call_sink() { x++; //% self.filecheck("bt", "main.cpp", "-check-prefix=TAIL-CALL-SINK") - // TAIL-CALL-SINK: frame #0: 0x{{[0-9a-f]+}} a.out`tail_call_sink() at main.cpp:[[@LINE-1]]:4 - // TAIL-CALL-SINK-NEXT: inlinable_function_which_tail_calls() at main.cpp{{.*}} [artificial] + // TAIL-CALL-SINK: frame #0: 0x{{[0-9a-f]+}} a.out`tail_call_sink() at + // main.cpp:[[@LINE-1]]:4 TAIL-CALL-SINK-NEXT: + // inlinable_function_which_tail_calls() at main.cpp{{.*}} [artificial] // TAIL-CALL-SINK-NEXT: main{{.*}} } @@ -17,10 +18,9 @@ void __attribute__((noinline)) func3() { void __attribute__((always_inline)) inline_sink() { x++; //% self.filecheck("bt", "main.cpp", "-check-prefix=INLINE-SINK") - // INLINE-SINK: frame #0: 0x{{[0-9a-f]+}} a.out`inline_sink() at main.cpp:[[@LINE-1]]:4 - // INLINE-SINK-NEXT: func2{{.*}} - // INLINE-SINK-NEXT: func1{{.*}} [artificial] - // INLINE-SINK-NEXT: main{{.*}} + // INLINE-SINK: frame #0: 0x{{[0-9a-f]+}} a.out`inline_sink() at + // main.cpp:[[@LINE-1]]:4 INLINE-SINK-NEXT: func2{{.*}} INLINE-SINK-NEXT: + // func1{{.*}} [artificial] INLINE-SINK-NEXT: main{{.*}} } void __attribute__((noinline)) func2() { inline_sink(); /* inlined */ } `` https://github.com/llvm/llvm-project/pull/135343 ___ 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)
santhoshe447 wrote: Thank you so much for your valuable feedback and suggestions. I have incorporated most of the requested changes, with just two remaining suggestions that I will address in a follow-up PR. 1. Moving to non-legacy approach to handle memoryWrite/memoryWrite. 2. Adding a utility function for memoryReference argument. Would you be kind enough to review the current changes at your convenience time? Please suggest if I am missing anything to mention here Thanks & Best regards, 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] [lldb] [lldb] Remove vestigial remnants of reproducers (PR #135361)
https://github.com/JDevlieghere approved this pull request. 🪦 https://github.com/llvm/llvm-project/pull/135361 ___ 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)
https://github.com/JDevlieghere closed 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] 70627af - [lldb] Synchronize access to m_statusline in the Debugger (#134759)
Author: Jonas Devlieghere Date: 2025-04-11T08:53:49-07:00 New Revision: 70627af91f577f5195a11ee44c6af39503e1ce52 URL: https://github.com/llvm/llvm-project/commit/70627af91f577f5195a11ee44c6af39503e1ce52 DIFF: https://github.com/llvm/llvm-project/commit/70627af91f577f5195a11ee44c6af39503e1ce52.diff LOG: [lldb] Synchronize access to m_statusline in the Debugger (#134759) Eliminate the potential for a race between the main thread, the default event handler thread and the signal handling thread, when accessing the m_statusline member. Added: Modified: lldb/include/lldb/Core/Debugger.h lldb/source/Core/Debugger.cpp Removed: diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 448e8d6a8fc6a..f36202edbbde0 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -752,6 +752,8 @@ class Debugger : public std::enable_shared_from_this, IOHandlerStack m_io_handler_stack; std::recursive_mutex m_io_handler_synchronous_mutex; + /// Mutex protecting the m_statusline member. + std::mutex m_statusline_mutex; std::optional m_statusline; llvm::StringMap> m_stream_handlers; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index e65d71ddf6960..ce6fb6ed5ec54 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -251,6 +251,7 @@ Status Debugger::SetPropertyValue(const ExecutionContext *exe_ctx, g_debugger_properties[ePropertyShowStatusline].name) { // Statusline setting changed. If we have a statusline instance, update it // now. Otherwise it will get created in the default event handler. + std::lock_guard guard(m_statusline_mutex); if (StatuslineSupported()) m_statusline.emplace(*this); else @@ -391,8 +392,12 @@ 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(); + + { +std::lock_guard guard(m_statusline_mutex); +if (m_statusline) + m_statusline->TerminalSizeChanged(); + } return success; } @@ -409,8 +414,12 @@ bool Debugger::SetTerminalHeight(uint64_t term_height) { if (auto handler_sp = m_io_handler_stack.Top()) handler_sp->TerminalSizeChanged(); - if (m_statusline) -m_statusline->TerminalSizeChanged(); + + { +std::lock_guard guard(m_statusline_mutex); +if (m_statusline) + m_statusline->TerminalSizeChanged(); + } return success; } @@ -1148,8 +1157,11 @@ void Debugger::SetErrorFile(FileSP file_sp) { } void Debugger::SaveInputTerminalState() { - if (m_statusline) -m_statusline->Disable(); + { +std::lock_guard guard(m_statusline_mutex); +if (m_statusline) + m_statusline->Disable(); + } int fd = GetInputFile().GetDescriptor(); if (fd != File::kInvalidDescriptor) m_terminal_state.Save(fd, true); @@ -1157,11 +1169,15 @@ void Debugger::SaveInputTerminalState() { void Debugger::RestoreInputTerminalState() { m_terminal_state.Restore(); - if (m_statusline) -m_statusline->Enable(); + { +std::lock_guard guard(m_statusline_mutex); +if (m_statusline) + m_statusline->Enable(); + } } void Debugger::RedrawStatusline(bool update) { + std::lock_guard guard(m_statusline_mutex); if (m_statusline) m_statusline->Redraw(update); } @@ -2039,8 +2055,11 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { // are now listening to all required events so no events get missed m_sync_broadcaster.BroadcastEvent(eBroadcastBitEventThreadIsListening); - if (!m_statusline && StatuslineSupported()) -m_statusline.emplace(*this); + if (StatuslineSupported()) { +std::lock_guard guard(m_statusline_mutex); +if (!m_statusline) + m_statusline.emplace(*this); + } bool done = false; while (!done) { @@ -2101,8 +2120,11 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { } } - if (m_statusline) -m_statusline.reset(); + { +std::lock_guard guard(m_statusline_mutex); +if (m_statusline) + m_statusline.reset(); + } return {}; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (PR #129092)
https://github.com/jimingham edited https://github.com/llvm/llvm-project/pull/129092 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (PR #129092)
@@ -296,6 +296,9 @@ let Definition = "process" in { DefaultEnumValue<"eFollowParent">, EnumValues<"OptionEnumValues(g_follow_fork_mode_values)">, Desc<"Debugger's behavior upon fork or vfork.">; + def TrackMemoryCacheChanges: Property<"track-memory-cache-changes", "Boolean">, +DefaultTrue, +Desc<"If true, memory cache modifications (which happen often during expressions evaluation) will bump process state ID (and invalidate all synthetic children). Disabling this option helps to avoid synthetic children reevaluation when pretty printers heavily use expressions, but convenience variables won't reevaluate synthetic children automatically.">; jimingham wrote: It's a little unclear that the "but" in the last sentence is is the downside to the "helps". So instead of "but convenience..." you did ". The downside of this setting is" https://github.com/llvm/llvm-project/pull/129092 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (PR #129092)
@@ -1827,6 +1827,33 @@ class CommandObjectMultiwordProcessTrace : public CommandObjectMultiword { ~CommandObjectMultiwordProcessTrace() override = default; }; +// CommandObjectProcessDumpModificationId +class CommandObjectProcessDumpModificationId : public CommandObjectParsed { +public: + CommandObjectProcessDumpModificationId(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "process dump-modification-id", jimingham wrote: I don't think it's a good idea to clutter up the lldb command interface with debugging specific commands. Maybe you could do this by adding a `-d` flag to `process status` that dumps this info? https://github.com/llvm/llvm-project/pull/129092 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not bump memory modificator ID when "internal" debugger memory is updated (PR #129092)
jimingham wrote: This looks pretty good to me. I had one trivial doc comment, and I think it's better not to start sprinkling internal debugging commands into the lldb command set. The debugger has enough commands already, I like to avoid adding ones that nobody but us will ever use. If we start needing to do this a lot, then we should probably follow gdb and add a "maintenance" command to gather them all up. That way normal user would just have to overlook one top-level command. But in this case, we already have `process status` so the debugging dump would fit naturally as an option to that command. https://github.com/llvm/llvm-project/pull/129092 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -870,6 +870,16 @@ llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) { return llvm::json::Value(std::move(object)); } +llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format) { + llvm::json::Array threads; + const uint32_t num_threads = process.GetNumThreads(); + for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { +lldb::SBThread thread = process.GetThreadAtIndex(thread_idx); +threads.emplace_back(CreateThread(thread, format)); + } + return threads; JDevlieghere wrote: This should take the TargetAPI lock as it's doing two SB API calls consecutively. ```suggestion lldb::SBMutex lock = process.GetTarget().GetAPIMutex(); std::lock_guard guard(lock); llvm::json::Array threads; const uint32_t num_threads = process.GetNumThreads(); for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { lldb::SBThread thread = process.GetThreadAtIndex(thread_idx); threads.emplace_back(CreateThread(thread, format)); } return threads; ``` 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] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -50,16 +50,22 @@ namespace lldb_dap { // } void ThreadsRequestHandler::operator()( const llvm::json::Object &request) const { - lldb::SBProcess process = dap.target.GetProcess(); llvm::json::Object response; FillResponse(request, response); - const uint32_t num_threads = process.GetNumThreads(); llvm::json::Array threads; - for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { -lldb::SBThread thread = process.GetThreadAtIndex(thread_idx); -threads.emplace_back(CreateThread(thread, dap.thread_format)); - } + // Client requests the baseline of currently existing threads after + // a successful launch or attach by sending a 'threads' request + // right after receiving the configurationDone response. + // If no thread has reported to the client, it prevents something + // like the pause request from working in the running state. + // Return the cache of initial threads as the process might have resumed + if (dap.initial_thread_list) { +threads = dap.initial_thread_list.value(); +dap.initial_thread_list.reset(); + } else +threads = GetThreads(dap.target.GetProcess(), dap.thread_format); JDevlieghere wrote: The no-braces rule doesn't apply if the other block has braces: ```suggestion } else { threads = GetThreads(dap.target.GetProcess(), dap.thread_format); } ``` 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] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -207,6 +207,9 @@ struct DAP { /// The set of features supported by the connected client. llvm::DenseSet clientFeatures; + /// The initial thread list upon attaching JDevlieghere wrote: ```suggestion /// The initial thread list upon attaching. ``` 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] Make SBProcess thread related actions listen to StopLocker (PR #134339)
https://github.com/JDevlieghere edited 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] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -870,6 +870,16 @@ llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) { return llvm::json::Value(std::move(object)); } +llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format) { + llvm::json::Array threads; + const uint32_t num_threads = process.GetNumThreads(); + for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { +lldb::SBThread thread = process.GetThreadAtIndex(thread_idx); +threads.emplace_back(CreateThread(thread, format)); + } + return threads; clayborg wrote: Might be nice to add a `lldb::SBThreadList` to the API and then add: ``` lldb::SBThreadList lldb::SBProcess::GetThreads(); ``` 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] Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" (PR #135408)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/135408 This reverts commit e84a80408523a48d6eaacd795f1615e821ffb233 because on Linux there seems to be a race around GetRunLock. See #134757 for more context. >From 13689bce25a51d5054601b346b09c249afa31d3b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 11 Apr 2025 10:15:41 -0700 Subject: [PATCH] Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" This reverts commit e84a80408523a48d6eaacd795f1615e821ffb233 because on Linux there seems to be a race around GetRunLock. --- lldb/source/Core/Statusline.cpp | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index e14691e2538a2..ed5308ef53eb0 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -12,7 +12,6 @@ #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolContext.h" -#include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/StreamString.h" @@ -135,15 +134,8 @@ void Statusline::Redraw(bool update) { exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget()); SymbolContext symbol_ctx; - if (ProcessSP process_sp = exe_ctx.GetProcessSP()) { -// Check if the process is stopped, and if it is, make sure it remains -// stopped until we've computed the symbol context. -Process::StopLocker stop_locker; -if (stop_locker.TryLock(&process_sp->GetRunLock())) { - if (auto frame_sp = exe_ctx.GetFrameSP()) -symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); -} - } + if (auto frame_sp = exe_ctx.GetFrameSP()) +symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); StreamString stream; if (auto *format = m_debugger.GetStatuslineFormat()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" (PR #135408)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes This reverts commit e84a80408523a48d6eaacd795f1615e821ffb233 because on Linux there seems to be a race around GetRunLock. See #134757 for more context. --- Full diff: https://github.com/llvm/llvm-project/pull/135408.diff 1 Files Affected: - (modified) lldb/source/Core/Statusline.cpp (+2-10) ``diff diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index e14691e2538a2..ed5308ef53eb0 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -12,7 +12,6 @@ #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolContext.h" -#include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/StreamString.h" @@ -135,15 +134,8 @@ void Statusline::Redraw(bool update) { exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget()); SymbolContext symbol_ctx; - if (ProcessSP process_sp = exe_ctx.GetProcessSP()) { -// Check if the process is stopped, and if it is, make sure it remains -// stopped until we've computed the symbol context. -Process::StopLocker stop_locker; -if (stop_locker.TryLock(&process_sp->GetRunLock())) { - if (auto frame_sp = exe_ctx.GetFrameSP()) -symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); -} - } + if (auto frame_sp = exe_ctx.GetFrameSP()) +symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); StreamString stream; if (auto *format = m_debugger.GetStatuslineFormat()) `` https://github.com/llvm/llvm-project/pull/135408 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -52,8 +64,14 @@ void ConfigurationDoneRequestHandler::operator()( dap.configuration_done_sent = true; if (dap.stop_at_entry) SendThreadStoppedEvent(dap); - else + else { +// Client requests the baseline of currently existing threads after +// a successful launch or attach. +// A 'threads' request will be sent after configurationDone response +// Obtain the list of threads before we resume the process clayborg wrote: yeah, we just need to change our code to always send it even if the `dap.focus_tid` is not valid. 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] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -870,6 +870,16 @@ llvm::json::Value CreateThread(lldb::SBThread &thread, lldb::SBFormat &format) { return llvm::json::Value(std::move(object)); } +llvm::json::Array GetThreads(lldb::SBProcess process, lldb::SBFormat &format) { + llvm::json::Array threads; + const uint32_t num_threads = process.GetNumThreads(); + for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { +lldb::SBThread thread = process.GetThreadAtIndex(thread_idx); +threads.emplace_back(CreateThread(thread, format)); + } + return threads; clayborg wrote: (we don't need to add the API for this patch, I was just mentioning this). The API lock fix will work here nicely. 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][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/5] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [lldb] Reland "[lldb] Clear thread-creation breakpoints in ProcessGDBRemote::Clear (#134397)" (PR #135296)
https://github.com/jimingham approved this pull request. Yes, that is a more appropriate place to do that work. https://github.com/llvm/llvm-project/pull/135296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure the process is stopped when computing the symbol context (PR #134757)
JDevlieghere wrote: I put up a revert in #135408. I'm going to see if I can reproduce this myself on Linux. I want to see what happens if the statusline uses the public API mutex, I don't think it needs the private one at all. If I can't reproduce I might ask one of you to try out a small patch. https://github.com/llvm/llvm-project/pull/134757 ___ 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)
@@ -18,6 +18,22 @@ namespace lldb_private::dil { +static lldb::ValueObjectSP +ArrayToPointerConversion(lldb::ValueObjectSP valobj, + std::shared_ptr ctx) { + assert(valobj->IsArrayType() && + "an argument to array-to-pointer conversion must be an array"); + + uint64_t addr = valobj->GetLoadAddress(); + llvm::StringRef name = "result"; + ExecutionContext exe_ctx; + ctx->CalculateExecutionContext(exe_ctx); + return ValueObject::CreateValueObjectFromAddress( + name, addr, exe_ctx, + valobj->GetCompilerType().GetArrayElementType(ctx.get()).GetPointerType(), + /* do_deref */ false); +} + jimingham wrote: Is there any downside to the second option? It seems to me the general philosophy here is to have DIL do as little as possible and the type system as much as possible, since then we're asking the entity that actually knows what it is doing... 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] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -2074,6 +2076,64 @@ static const Definition *FindEntry(const llvm::StringRef &format_str, return parent; } +/// Parses a single highlighting format specifier. +/// +/// Example syntax for such specifier: +/// \code +/// ${function.name-with-args:%highlight_basename(ansi.fg.green)} jimingham wrote: Shouldn't that be `plugin.language.cplusplus.frame-format`? 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] [lldb] Make sure the process is stopped when computing the symbol context (PR #134757)
JDevlieghere wrote: I assume something else changed in the meantime (maybe the extra locking added in #134759?) but now if I revert this change I always crash on `run`: ``` * thread #51, name = '', stop reason = EXC_BAD_ACCESS (code=1, address=0xfffed77f38f8) frame #0: 0x00011332b66c liblldb.21.0.0git.dylib`lldb_private::DynamicRegisterInfo::IsReconfigurable(this=0xfffed77f383f) at DynamicRegisterInfo.cpp:682:55 [opt] frame #1: 0x0001136046a8 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ThreadGDBRemote::ThreadGDBRemote(this=0x000121704098, process=0x00012880b400, tid=) at ThreadGDBRemote.cpp:48:40 [opt] frame #2: 0x0001135ec940 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] void std::__1::allocator::construct[abi:nn190102](this=, __p=0x000121704098, __args=0x00012880b400, __args=) at allocator.h:165:24 [opt] frame #3: 0x0001135ec930 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] void std::__1::allocator_traits>::construct[abi:nn190102](__a=, __p=0x000121704098, __args=0x00012880b400, __args=) at allocator_traits.h:319:9 [opt] * frame #4: 0x0001135ec930 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] std::__1::__shared_ptr_emplace>::__shared_ptr_emplace[abi:nn190102], 0>(this=0x000121704080, __a=, __args=0x00012880b400, __args=) at shared_ptr.h:266:5 [opt] frame #5: 0x0001135ec918 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] std::__1::__shared_ptr_emplace>::__shared_ptr_emplace[abi:nn190102], 0>(this=0x000121704080, __a=, __args=0x00012880b400, __args=) at shared_ptr.h:263:115 [opt] frame #6: 0x0001135ec918 liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] std::__1::shared_ptr std::__1::allocate_shared[abi:nn190102], lldb_private::process_gdb_remote::ProcessGDBRemote&, unsigned long long&, 0>(__a=, __args=0x00012880b400, __args=) at shared_ptr.h:845:51 [opt] frame #7: 0x0001135ec90c liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(lldb_private::ThreadList&, lldb_private::ThreadList&) [inlined] std::__1::shared_ptr std::__1::make_shared[abi:nn190102](__args=0x00012880b400, __args=) at shared_ptr.h:853:10 [opt] frame #8: 0x0001135ec90c liblldb.21.0.0git.dylib`lldb_private::process_gdb_remote::ProcessGDBRemote::DoUpdateThreadList(this=0x00012880b400, old_thread_list=, new_thread_list=0x00017d1cac28) at ProcessGDBRemote.cpp:1589:21 [opt] frame #9: 0x000113349880 liblldb.21.0.0git.dylib`lldb_private::Process::UpdateThreadListIfNeeded() [inlined] lldb_private::Process::UpdateThreadList(this=0x00012880b400, old_thread_list=, new_thread_list=0x00017d1cac28) at Process.cpp:1124:10 [opt] frame #10: 0x000113349874 liblldb.21.0.0git.dylib`lldb_private::Process::UpdateThreadListIfNeeded(this=0x00012880b400) at Process.cpp:1145:11 [opt] frame #11: 0x0001133b4410 liblldb.21.0.0git.dylib`lldb_private::ThreadList::SetShouldReportStop(this=0x00012880b750, vote=eVoteNo) at ThreadList.cpp:413:13 [opt] frame #12: 0x000113350190 liblldb.21.0.0git.dylib`lldb_private::Process::AttachCompletionHandler::PerformAction(this=0x60404330, event_sp=) at Process.cpp:2907:32 [opt] frame #13: 0x00011334e858 liblldb.21.0.0git.dylib`lldb_private::Process::HandlePrivateEvent(this=0x00012880b400, event_sp=std::__1::shared_ptr::element_type @ 0x638ac900 strong=2 weak=2) at Process.cpp:3941:33 [opt] ``` https://github.com/llvm/llvm-project/pull/134757 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 68ab45f - Revert "[lldb] ProcessGdbRemote header gardning"
Author: Jonas Devlieghere Date: 2025-04-11T10:44:43-07:00 New Revision: 68ab45f0533f3bbfc1c96bddd53de7e769180219 URL: https://github.com/llvm/llvm-project/commit/68ab45f0533f3bbfc1c96bddd53de7e769180219 DIFF: https://github.com/llvm/llvm-project/commit/68ab45f0533f3bbfc1c96bddd53de7e769180219.diff LOG: Revert "[lldb] ProcessGdbRemote header gardning" This reverts commit 2fd860c1f559c0b0be66cc000e38270a04d0a1a3 as this is causing a EXC_BAD_ACCESS on Darwin: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/23807/ https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/11255/ Added: Modified: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Removed: diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h index af2abdf4da5cf..b47fee76a2ab5 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -10,13 +10,8 @@ #define LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECLIENTBASE_H #include "GDBRemoteCommunication.h" -#include "lldb/Utility/Broadcaster.h" -#include "llvm/ADT/STLFunctionalExtras.h" -#include "llvm/ADT/StringRef.h" -#include + #include -#include -#include namespace lldb_private { namespace process_gdb_remote { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 12ae5ce467723..77eadfc8c9f6c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -7,7 +7,14 @@ //===--===// #include "GDBRemoteCommunication.h" -#include "ProcessGDBRemoteLog.h" + +#include +#include +#include +#include + +#include "lldb/Host/Config.h" +#include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Host.h" #include "lldb/Host/HostInfo.h" @@ -23,14 +30,13 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/StreamString.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/Config/llvm-config.h" // for LLVM_ENABLE_ZLIB #include "llvm/Support/ScopedPrinter.h" -#include -#include -#include -#include + +#include "ProcessGDBRemoteLog.h" #if defined(__APPLE__) #define DEBUGSERVER_BASENAME "debugserver" diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index f51b94360d0be..107c0896c4e61 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -11,16 +11,27 @@ #include "GDBRemoteCommunicationHistory.h" +#include #include #include +#include #include +#include + #include "lldb/Core/Communication.h" +#include "lldb/Host/Config.h" #include "lldb/Host/HostThread.h" #include "lldb/Host/Socket.h" #include "lldb/Utility/Args.h" +#include "lldb/Utility/Listener.h" +#include "lldb/Utility/Predicate.h" #include "lldb/Utility/StringExtractorGDBRemote.h" +#include "lldb/lldb-public.h" namespace lldb_private { +namespace repro { +class PacketRecorder; +} namespace process_gdb_remote { enum GDBStoppointType { @@ -151,6 +162,8 @@ class GDBRemoteCommunication : public Communication { void DumpHistory(Stream &strm); + void SetPacketRecorder(repro::PacketRecorder *recorder); + static llvm::Error ConnectLocally(GDBRemoteCommunication &client, GDBRemoteCommunication &server); diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp index ed77e86ac3701..99d1e12359e72 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp @@ -5,20 +5,16 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===--===// +#include -#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h" #include "GDBRemoteTestUtils.h" + #include "Plugins/Process/Utility/LinuxSignals.h" +#include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h" #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h" #include "lldb/Utility/GDBRemote.h" -#include "lldb/Utility/Listener.h" -#include "llvm/ADT
[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)
ilovepi wrote: I think we're seeing this persist on x86_64 mac bots https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/lldb-mac-x64/b8717871333718166241/infra I'm not sure what the right spelling is to opt out.. I think @DavidSpickett's original suggestion may work better if we need to opt out more than just windows. 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)
da-viper wrote: Macos x86_64 also follows the systemV amd64 abi calling convention. So it should not fail or be skipped. In the linked build bot, I could not see the test (lldb/test/API/python_api/target/read-instructions-flavor/TestTargetReadInstructionsFlavor.py) failing could you link it just to confirm. 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] Clean up StartDebugserverProcess before I start refactoring it (PR #135342)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/135342 ___ 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] c2939b9 - Reland "[lldb] Clear thread-creation breakpoints in ProcessGDBRemote::Clear (#134397)" (#135296)
Author: Felipe de Azevedo Piovezan Date: 2025-04-11T11:46:22-07:00 New Revision: c2939b9bf672713ac36910a3e6d00c19ace1bd44 URL: https://github.com/llvm/llvm-project/commit/c2939b9bf672713ac36910a3e6d00c19ace1bd44 DIFF: https://github.com/llvm/llvm-project/commit/c2939b9bf672713ac36910a3e6d00c19ace1bd44.diff LOG: Reland "[lldb] Clear thread-creation breakpoints in ProcessGDBRemote::Clear (#134397)" (#135296) This reapplies commit https://github.com/llvm/llvm-project/commit/232525f06942adb3b9977632e38dcd5f08c0642d. The original commit triggered a sanitizer failure when `Target` was destroyed. In `Target::Destroy`, `DeleteCurrentProcess` was called, but it did not destroy the thread creation breakpoints for the underlying `ProcessGDBRemote` because `ProcessGDBRemote::Clear` was not called in that path. `Target `then proceeded to destroy its breakpoints, which resulted in a call to the destructor of a `std::vector` containing the breakpoints. Through a sequence of complicated events, destroying breakpoints caused the reference count of the underlying `ProcessGDBRemote` to finally reach zero. This, in turn, called `ProcessGDBRemote::Clear`, which attempted to destroy the breakpoints. To do that, it would go back into the Target's vector of breakpoints, which we are in the middle of destroying. We solve this by moving the breakpoint deletion into `Process:DoDestroy`, which is a virtual Process method that will be called much earlier. Added: Modified: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/test/API/macosx/thread_start_bps/TestBreakpointsThreadInit.py Removed: diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 68360788c96e6..b616e99be83b2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2571,9 +2571,18 @@ Status ProcessGDBRemote::DoDestroy() { StopAsyncThread(); KillDebugserverProcess(); + RemoveNewThreadBreakpoints(); return Status(); } +void ProcessGDBRemote::RemoveNewThreadBreakpoints() { + if (m_thread_create_bp_sp) { +if (TargetSP target_sp = m_target_wp.lock()) + target_sp->RemoveBreakpointByID(m_thread_create_bp_sp->GetID()); +m_thread_create_bp_sp.reset(); + } +} + void ProcessGDBRemote::SetLastStopPacket( const StringExtractorGDBRemote &response) { const bool did_exec = diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 1cbd1e82b381d..20d7fc0801eb3 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -436,6 +436,9 @@ class ProcessGDBRemote : public Process, lldb::user_id_t break_id, lldb::user_id_t break_loc_id); + /// Remove the breakpoints associated with thread creation from the Target. + void RemoveNewThreadBreakpoints(); + // ContinueDelegate interface void HandleAsyncStdout(llvm::StringRef out) override; void HandleAsyncMisc(llvm::StringRef data) override; diff --git a/lldb/test/API/macosx/thread_start_bps/TestBreakpointsThreadInit.py b/lldb/test/API/macosx/thread_start_bps/TestBreakpointsThreadInit.py index 1c6fd4f91c73e..bf667f6f7d336 100644 --- a/lldb/test/API/macosx/thread_start_bps/TestBreakpointsThreadInit.py +++ b/lldb/test/API/macosx/thread_start_bps/TestBreakpointsThreadInit.py @@ -35,3 +35,23 @@ def test_internal_bps_resolved(self): for bp in bps: num_resolved += bp.GetNumResolvedLocations() self.assertGreater(num_resolved, 0) + +@skipUnlessDarwin +def test_internal_bps_deleted_on_relaunch(self): +self.build() + +source_file = lldb.SBFileSpec("main.c") +target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( +self, "initial hello", source_file +) + +self.runCmd("break list --internal") +output = self.res.GetOutput() +self.assertEqual(output.count("thread-creation"), 1) + +process.Kill() +self.runCmd("run", RUN_SUCCEEDED) + +self.runCmd("break list --internal") +output = self.res.GetOutput() +self.assertEqual(output.count("thread-creation"), 1) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Clear thread-creation breakpoints in ProcessGDBRemote::Clear (#134397)" (PR #135296)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/135296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 179d30f - Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" (#135408)
Author: Jonas Devlieghere Date: 2025-04-11T11:50:42-07:00 New Revision: 179d30f8c3fddd3c85056fd2b8e877a4a8513158 URL: https://github.com/llvm/llvm-project/commit/179d30f8c3fddd3c85056fd2b8e877a4a8513158 DIFF: https://github.com/llvm/llvm-project/commit/179d30f8c3fddd3c85056fd2b8e877a4a8513158.diff LOG: Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" (#135408) This reverts commit e84a80408523a48d6eaacd795f1615e821ffb233 because on Linux there seems to be a race around GetRunLock. See #134757 for more context. Added: Modified: lldb/source/Core/Statusline.cpp Removed: diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index e14691e2538a2..ed5308ef53eb0 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -12,7 +12,6 @@ #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolContext.h" -#include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/StreamString.h" @@ -135,15 +134,8 @@ void Statusline::Redraw(bool update) { exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget()); SymbolContext symbol_ctx; - if (ProcessSP process_sp = exe_ctx.GetProcessSP()) { -// Check if the process is stopped, and if it is, make sure it remains -// stopped until we've computed the symbol context. -Process::StopLocker stop_locker; -if (stop_locker.TryLock(&process_sp->GetRunLock())) { - if (auto frame_sp = exe_ctx.GetFrameSP()) -symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); -} - } + if (auto frame_sp = exe_ctx.GetFrameSP()) +symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); StreamString stream; if (auto *format = m_debugger.GetStatuslineFormat()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Revert "[lldb] Make sure the process is stopped when computing the symbol context (#134757)" (PR #135408)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/135408 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Clean up StartDebugserverProcess before I start refactoring it (PR #135342)
https://github.com/slydiman approved this pull request. LGTM, thanks. Currently `GDBRemoteCommunication::ListenThread()` to get the bound port is weird. https://github.com/llvm/llvm-project/pull/135342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/6] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [lldb] [lldb] Fix SBTarget::ReadInstruction with flavor (PR #134626)
ilovepi wrote: @da-viper Ah, shoot. Sorry for the noise, I think it may have been https://github.com/llvm/llvm-project/commit/2fd860c1f559c0b0be66cc000e38270a04d0a1a3, which was just reverted. thanks for taking a look. :) 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][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/7] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [libcxxabi] [lldb] [llvm] [lldb] Add frame-format option to highlight function names in backtraces (PR #131836)
@@ -2074,6 +2076,64 @@ static const Definition *FindEntry(const llvm::StringRef &format_str, return parent; } +/// Parses a single highlighting format specifier. +/// +/// Example syntax for such specifier: +/// \code +/// ${function.name-with-args:%highlight_basename(ansi.fg.green)} Michael137 wrote: > Does the setting affect the format of the whole frame, or just the function > name. Because if it's the latter (which I also think would be a better > design), then it probably should be called something else... function-format, > function-name-format, ... ? Yup it just affects the function name. I renamed it to `function-name-format` > Shouldn't that be plugin.language.cplusplus.frame-format? That would've been my preferred order too. But currently the setting names for plugins are derived as: ``` plugin... ``` For `CPlusPlusLanguage`, the plugin name is `cplusplus`. So i decided to call the settings for it `language` (for the lack of a better settings name). Any suggestions for what the `` should be here? 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] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/134339 >From 0c4f6ca21ba6ae37e3a884f4bcc356b6f741ede7 Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Thu, 3 Apr 2025 22:29:13 -0700 Subject: [PATCH 1/3] [lldb] Make SBProcess thread related actions listen to StopLocker --- lldb/source/API/SBProcess.cpp | 20 ++- .../tools/lldb-dap/attach/TestDAP_attach.py | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 23ea449b30cca..ba77b2beed5ea 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -193,10 +193,11 @@ uint32_t SBProcess::GetNumThreads() { if (process_sp) { Process::StopLocker stop_locker; -const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock()); -std::lock_guard guard( -process_sp->GetTarget().GetAPIMutex()); -num_threads = process_sp->GetThreadList().GetSize(can_update); +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + std::lock_guard guard( + process_sp->GetTarget().GetAPIMutex()); + num_threads = process_sp->GetThreadList().GetSize(); +} } return num_threads; @@ -393,11 +394,12 @@ SBThread SBProcess::GetThreadAtIndex(size_t index) { ProcessSP process_sp(GetSP()); if (process_sp) { Process::StopLocker stop_locker; -const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock()); -std::lock_guard guard( -process_sp->GetTarget().GetAPIMutex()); -thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, can_update); -sb_thread.SetThread(thread_sp); +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + std::lock_guard guard( + process_sp->GetTarget().GetAPIMutex()); + thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, false); + sb_thread.SetThread(thread_sp); +} } return sb_thread; diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index 9df44cc454d5d..b9fbf2c8d14f9 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -1,5 +1,5 @@ """ -Test lldb-dap setBreakpoints request +Test lldb-dap attach request """ >From 931b3fbf9c31e4ea4e2c3bfe822ce0784c714c5a Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Tue, 8 Apr 2025 22:52:19 -0700 Subject: [PATCH 2/3] [lldb-dap] Client expects initial threads request not empty --- .../test/tools/lldb-dap/dap_server.py | 4 lldb/tools/lldb-dap/DAP.h | 3 +++ .../ConfigurationDoneRequestHandler.cpp| 10 +- .../lldb-dap/Handler/ThreadsRequestHandler.cpp | 18 -- lldb/tools/lldb-dap/JSONUtils.cpp | 10 ++ lldb/tools/lldb-dap/JSONUtils.h| 2 ++ 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 45403e9df8525..61d7fa94479b8 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -649,6 +649,10 @@ def request_configurationDone(self): response = self.send_recv(command_dict) if response: self.configuration_done_sent = True +# Client requests the baseline of currently existing threads after +# a successful launch or attach. +# Kick off the threads request that follows +self.request_threads() return response def _process_stopped(self): diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index fc43d988f3a09..feeb040546c76 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -207,6 +207,9 @@ struct DAP { /// The set of features supported by the connected client. llvm::DenseSet clientFeatures; + /// The initial thread list upon attaching + std::optional initial_thread_list; + /// Creates a new DAP sessions. /// /// \param[in] log diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp index cd120e1fdfaba..f39bbdefdbb95 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -44,6 +44,7 @@ namespace lldb_dap { // just an acknowledgement, so no body field is required." // }] // }, + void ConfigurationDoneRequestHandler::operator()( const llvm::json::Object &request) const { llvm::json::Object response; @@ -52,8 +53,15 @@ void ConfigurationDoneRequestHandler::operator()( dap.configuration_done_sent = true; if (dap.stop_at_entry) SendThreadStop
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
mizvekov wrote: @eaeltsin speculative fix here, but can you try with this patch? https://github.com/llvm/llvm-project/pull/135434 https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/134339 >From c03108499d3e33b67846f6360ba71270b4139915 Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Thu, 3 Apr 2025 22:29:13 -0700 Subject: [PATCH 1/3] [lldb] Make SBProcess thread related actions listen to StopLocker --- lldb/source/API/SBProcess.cpp | 20 ++- .../tools/lldb-dap/attach/TestDAP_attach.py | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 23ea449b30cca..ba77b2beed5ea 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -193,10 +193,11 @@ uint32_t SBProcess::GetNumThreads() { if (process_sp) { Process::StopLocker stop_locker; -const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock()); -std::lock_guard guard( -process_sp->GetTarget().GetAPIMutex()); -num_threads = process_sp->GetThreadList().GetSize(can_update); +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + std::lock_guard guard( + process_sp->GetTarget().GetAPIMutex()); + num_threads = process_sp->GetThreadList().GetSize(); +} } return num_threads; @@ -393,11 +394,12 @@ SBThread SBProcess::GetThreadAtIndex(size_t index) { ProcessSP process_sp(GetSP()); if (process_sp) { Process::StopLocker stop_locker; -const bool can_update = stop_locker.TryLock(&process_sp->GetRunLock()); -std::lock_guard guard( -process_sp->GetTarget().GetAPIMutex()); -thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, can_update); -sb_thread.SetThread(thread_sp); +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + std::lock_guard guard( + process_sp->GetTarget().GetAPIMutex()); + thread_sp = process_sp->GetThreadList().GetThreadAtIndex(index, false); + sb_thread.SetThread(thread_sp); +} } return sb_thread; diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index 9df44cc454d5d..b9fbf2c8d14f9 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -1,5 +1,5 @@ """ -Test lldb-dap setBreakpoints request +Test lldb-dap attach request """ >From 2a0ef64e1afddc4782107a0747f0e2faece00c90 Mon Sep 17 00:00:00 2001 From: Wanyi Ye Date: Tue, 8 Apr 2025 22:52:19 -0700 Subject: [PATCH 2/3] [lldb-dap] Client expects initial threads request not empty --- .../test/tools/lldb-dap/dap_server.py | 4 lldb/tools/lldb-dap/DAP.h | 3 +++ .../ConfigurationDoneRequestHandler.cpp| 10 +- .../lldb-dap/Handler/ThreadsRequestHandler.cpp | 18 -- lldb/tools/lldb-dap/JSONUtils.cpp | 10 ++ lldb/tools/lldb-dap/JSONUtils.h| 2 ++ 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 45403e9df8525..61d7fa94479b8 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -649,6 +649,10 @@ def request_configurationDone(self): response = self.send_recv(command_dict) if response: self.configuration_done_sent = True +# Client requests the baseline of currently existing threads after +# a successful launch or attach. +# Kick off the threads request that follows +self.request_threads() return response def _process_stopped(self): diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index fc43d988f3a09..feeb040546c76 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -207,6 +207,9 @@ struct DAP { /// The set of features supported by the connected client. llvm::DenseSet clientFeatures; + /// The initial thread list upon attaching + std::optional initial_thread_list; + /// Creates a new DAP sessions. /// /// \param[in] log diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp index cd120e1fdfaba..f39bbdefdbb95 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -44,6 +44,7 @@ namespace lldb_dap { // just an acknowledgement, so no body field is required." // }] // }, + void ConfigurationDoneRequestHandler::operator()( const llvm::json::Object &request) const { llvm::json::Object response; @@ -52,8 +53,15 @@ void ConfigurationDoneRequestHandler::operator()( dap.configuration_done_sent = true; if (dap.stop_at_entry) SendThreadStop
[Lldb-commits] [lldb] [lldb] Make SBProcess thread related actions listen to StopLocker (PR #134339)
@@ -52,8 +64,14 @@ void ConfigurationDoneRequestHandler::operator()( dap.configuration_done_sent = true; if (dap.stop_at_entry) SendThreadStoppedEvent(dap); - else + else { +// Client requests the baseline of currently existing threads after +// a successful launch or attach. +// A 'threads' request will be sent after configurationDone response +// Obtain the list of threads before we resume the process kusmour wrote: According to DAP > Please note: a debug adapter is not expected to send this event in response > to a request that implies that execution continues, e.g. launch or continue. It is only necessary to send a continued event if there was no previous request that implied this. In this case, if the `attach` is expecting the execution continues then we should treat it the same way as `launch`. I am happy to discuss more about this as well as if we should support `stopOnEntry` for attaching :) going forward. 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][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/9] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [lldb] [lldb][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/8] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [lldb] 715c61e - [lldb][lldbp-dap] On Windoows, silence warnings when building with MSVC
Author: Alexandre Ganea Date: 2025-04-11T17:50:15-04:00 New Revision: 715c61e9a7cc631fd0965b887941ccfd8c0133d6 URL: https://github.com/llvm/llvm-project/commit/715c61e9a7cc631fd0965b887941ccfd8c0133d6 DIFF: https://github.com/llvm/llvm-project/commit/715c61e9a7cc631fd0965b887941ccfd8c0133d6.diff LOG: [lldb][lldbp-dap] On Windoows, silence warnings when building with MSVC Fixes: ``` [6373/7138] Building CXX object tools\lldb\tools\lldb-dap\CMakeFiles\lldb-dap.dir\DAP.cpp.obj C:\git\llvm-project\lldb\tools\lldb-dap\DAP.cpp(725) : warning C4715: '`lldb_dap::DAP::HandleObject'::`30'operator()': not all control paths return a value [6421/7138] Building CXX object tools\lldb\tools\lldb-dap\CMakeFiles\lldb-dap.dir\Protocol\ProtocolTypes.cpp.obj C:\git\llvm-project\lldb\tools\lldb-dap\Protocol\ProtocolTypes.cpp(203) : warning C4715: 'lldb_dap::protocol::ToString': not all control paths return a value C:\git\llvm-project\lldb\tools\lldb-dap\Protocol\ProtocolTypes.cpp(98) : warning C4715: 'lldb_dap::protocol::toJSON': not all control paths return a value C:\git\llvm-project\lldb\tools\lldb-dap\Protocol\ProtocolTypes.cpp(72) : warning C4715: 'lldb_dap::protocol::toJSON': not all control paths return a value C:\git\llvm-project\lldb\tools\lldb-dap\Protocol\ProtocolTypes.cpp(111) : warning C4715: 'lldb_dap::protocol::toJSON': not all control paths return a value [6426/7138] Building CXX object tools\lldb\tools\lldb-dap\CMakeFiles\lldb-dap.dir\Protocol\ProtocolBase.cpp.obj C:\git\llvm-project\lldb\tools\lldb-dap\Protocol\ProtocolBase.cpp(287) : warning C4715: 'lldb_dap::protocol::fromJSON': not all control paths return a value ``` Added: Modified: lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp Removed: diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 9361ba968e9c2..1f49d70ab3ac2 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -722,6 +722,7 @@ bool DAP::HandleObject(const protocol::Message &M) { case protocol::eResponseMessageNotStopped: return "notStopped"; } + llvm_unreachable("unknown response message kind."); }), *resp->message); } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp index 87fd0df018b65..af63cc803e545 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp @@ -284,6 +284,7 @@ bool fromJSON(const json::Value &Params, Message &PM, json::Path P) { PM = std::move(evt); return true; } + llvm_unreachable("unhandled message type request."); } json::Value toJSON(const Message &M) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index d1dd9ad9c5fee..f4f0bf8dcea84 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -69,6 +69,7 @@ json::Value toJSON(const ColumnType &T) { case eColumnTypeTimestamp: return "unixTimestampUTC"; } + llvm_unreachable("unhandled column type."); } json::Value toJSON(const ColumnDescriptor &CD) { @@ -95,6 +96,7 @@ json::Value toJSON(const ChecksumAlgorithm &CA) { case eChecksumAlgorithmTimestamp: return "timestamp"; } + llvm_unreachable("unhandled checksum algorithm."); } json::Value toJSON(const BreakpointModeApplicability &BMA) { @@ -108,6 +110,7 @@ json::Value toJSON(const BreakpointModeApplicability &BMA) { case eBreakpointModeApplicabilityInstruction: return "instruction"; } + llvm_unreachable("unhandled breakpoint mode applicability."); } json::Value toJSON(const BreakpointMode &BM) { @@ -200,6 +203,7 @@ static llvm::StringLiteral ToString(AdapterFeature feature) { case eAdapterFeatureTerminateDebuggee: return "supportTerminateDebuggee"; } + llvm_unreachable("unhandled adapter feature."); } json::Value toJSON(const Capabilities &C) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 84ed81b - [lldb] On Windows, silence warning when building with Clang ToT
Author: Alexandre Ganea Date: 2025-04-11T17:49:26-04:00 New Revision: 84ed81bc8a5359f33818c876e43f6450e98deb35 URL: https://github.com/llvm/llvm-project/commit/84ed81bc8a5359f33818c876e43f6450e98deb35 DIFF: https://github.com/llvm/llvm-project/commit/84ed81bc8a5359f33818c876e43f6450e98deb35.diff LOG: [lldb] On Windows, silence warning when building with Clang ToT Fixes: ``` [930/2017] Building CXX object tools\lldb\unittests\Thread\CMakeFiles\ThreadTests.dir\ThreadTest.cpp.obj C:\git\llvm-project\lldb\unittests\Thread\ThreadTest.cpp(51,23): warning: cast from 'FARPROC' (aka 'long long (*)()') to 'SetThreadDescriptionFunctionPtr' (aka 'long (*)(void *, const wchar_t *)') converts to incompatible function type [-Wcast-function-type-mismatch] 51 | SetThreadName = reinterpret_cast( | ^~ 52 | ::GetProcAddress(hModule, "SetThreadDescription")); | ~~ 1 warning generated. ``` Added: Modified: lldb/unittests/Thread/ThreadTest.cpp Removed: diff --git a/lldb/unittests/Thread/ThreadTest.cpp b/lldb/unittests/Thread/ThreadTest.cpp index 542585969c07b..4e87b3eb435b2 100644 --- a/lldb/unittests/Thread/ThreadTest.cpp +++ b/lldb/unittests/Thread/ThreadTest.cpp @@ -49,7 +49,7 @@ class ThreadTest : public ::testing::Test { HMODULE hModule = ::LoadLibraryW(L"Kernel32.dll"); if (hModule) { SetThreadName = reinterpret_cast( - ::GetProcAddress(hModule, "SetThreadDescription")); + (void *)::GetProcAddress(hModule, "SetThreadDescription")); } PlatformWindows::Initialize(); #endif ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e1d91ba - [lldb] Fix erroneous return value
Author: Alexandre Ganea Date: 2025-04-11T17:50:15-04:00 New Revision: e1d91ba06d250dd8bbd5bded4824c5b210c2667a URL: https://github.com/llvm/llvm-project/commit/e1d91ba06d250dd8bbd5bded4824c5b210c2667a DIFF: https://github.com/llvm/llvm-project/commit/e1d91ba06d250dd8bbd5bded4824c5b210c2667a.diff LOG: [lldb] Fix erroneous return value Found when building with MSVC on Windows, was seeing: ``` [2703/7138] Building CXX object tools\lldb\source\Plugins\Process\Utility\CMakeFiles\lldbPluginProcessUtility.dir\NativeRegisterContextDBReg.cpp.obj C:\git\llvm-project\lldb\source\Plugins\Process\Utility\NativeRegisterContextDBReg.cpp(286): warning C4305: 'return': truncation from 'unsigned int' to 'bool' ``` Added: Modified: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp Removed: diff --git a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp index 1e0c1a5db09ca..19601b7f35d47 100644 --- a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp +++ b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp @@ -283,7 +283,7 @@ bool NativeRegisterContextDBReg::ClearHardwareWatchpoint(uint32_t wp_index) { LLDB_LOG_ERROR( log, std::move(error), "unable to set watchpoint: failed to read debug registers: {0}"); -return LLDB_INVALID_INDEX32; +return false; } if (wp_index >= m_max_hwp_supported) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
eaeltsin wrote: Memory Sanitizer complains about initialized value [here](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Serialization/ASTReader.h#L2438) Will try the patch now. https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
eaeltsin wrote: @mizvekov - no, the patch doesn't help, or I did something wrong. https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
mizvekov wrote: Do you have a backtrace of that uninitialized read? https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Format] Display only the inlined frame name in backtraces if available (PR #135343)
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/135343 >From 7d8455bd0b30ee5cd49788243646e75ae8938159 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 11 Apr 2025 11:02:19 +0100 Subject: [PATCH 1/9] [lldb][Format] Only display inlined frame name in backtraces if available --- lldb/include/lldb/Symbol/SymbolContext.h | 7 ++ lldb/source/Core/FormatEntity.cpp | 95 +-- .../Language/CPlusPlus/CPlusPlusLanguage.cpp | 25 + lldb/source/Symbol/SymbolContext.cpp | 26 + .../Shell/Settings/TestFrameFormatName.test | 8 +- 5 files changed, 63 insertions(+), 98 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 8b6317c6f33c2..4f8405f1f0db5 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -307,6 +307,13 @@ class SymbolContext { SymbolContext &next_frame_sc, Address &inlined_frame_addr) const; + /// If available, will return the function name according to the specified + /// mangling preference. If this object represents an inlined function, + /// returns the name of the inlined function. Returns nullptr if no function + /// name could be determined. + const char *GetPossiblyInlinedFunctionName( + Mangled::NamePreference mangling_preference) const; + // Member variables lldb::TargetSP target_sp; ///< The Target for a given query lldb::ModuleSP module_sp; ///< The Module for a given query diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index a9370595c11e7..c3068a9cfaeab 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1147,19 +1147,6 @@ static void PrettyPrintFunctionNameWithArgs(Stream &out_stream, out_stream.PutChar(')'); } -static void FormatInlinedBlock(Stream &out_stream, Block *block) { - if (!block) -return; - Block *inline_block = block->GetContainingInlinedBlock(); - if (inline_block) { -if (const InlineFunctionInfo *inline_info = -inline_block->GetInlinedFunctionInfo()) { - out_stream.PutCString(" [inlined] "); - inline_info->GetName().Dump(&out_stream); -} - } -} - static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { assert(sc.function); @@ -1170,22 +1157,6 @@ static VariableListSP GetFunctionVariableList(const SymbolContext &sc) { return sc.function->GetBlock(true).GetBlockVariableList(true); } -static char const *GetInlinedFunctionName(const SymbolContext &sc) { - if (!sc.block) -return nullptr; - - const Block *inline_block = sc.block->GetContainingInlinedBlock(); - if (!inline_block) -return nullptr; - - const InlineFunctionInfo *inline_info = - inline_block->GetInlinedFunctionInfo(); - if (!inline_info) -return nullptr; - - return inline_info->GetName().AsCString(nullptr); -} - static bool PrintFunctionNameWithArgs(Stream &s, const ExecutionContext *exe_ctx, const SymbolContext &sc) { @@ -1194,16 +1165,11 @@ static bool PrintFunctionNameWithArgs(Stream &s, ExecutionContextScope *exe_scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr; - const char *cstr = sc.function->GetName().AsCString(nullptr); + const char *cstr = + sc.GetPossiblyInlinedFunctionName(Mangled::ePreferDemangled); if (!cstr) return false; - if (const char *inlined_name = GetInlinedFunctionName(sc)) { -s.PutCString(cstr); -s.PutCString(" [inlined] "); -cstr = inlined_name; - } - VariableList args; if (auto variable_list_sp = GetFunctionVariableList(sc)) variable_list_sp->AppendVariablesWithScope(eValueTypeVariableArgument, @@ -1724,21 +1690,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - const char *name = nullptr; - if (sc->function) -name = sc->function->GetName().AsCString(nullptr); - else if (sc->symbol) -name = sc->symbol->GetName().AsCString(nullptr); - - if (name) { -s.PutCString(name); -FormatInlinedBlock(s, sc->block); -return true; - } } + +const char *name = GetPossiblyInlinedFunctionName( +*sc, Mangled::NamePreference::ePreferDemangled); +if (!name) + return false; + +s.PutCString(name); + +return true; } -return false; case Entry::Type::FunctionNameNoArgs: { if (!sc) @@ -1760,20 +1722,17 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, if (language_plugin_handled) { s << ss.GetString(); return true; -} else { - ConstString name; - if (sc->function) -name = sc->function->GetNameNoArguments(); - else if (sc->symbol) -
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
mizvekov wrote: Okay, if the problem is an uninitialized source location somewhere, then that patch doesn't help at all. https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
eaeltsin wrote: Well, to be honest, I'm not completely sure these are the same problems, these are just sanitizer finding for the same compilation. Here is the [MSan finding](https://gist.github.com/eaeltsin/834192fe5d8bbbc061e1488f113e34e1). Please note my source is somewhat behind the head, so locations might be off a bit https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] Reland: [clang] preserve class type sugar when taking pointer to member (PR #132401)
mizvekov wrote: Thanks for that stack trace, could be unrelated to this, but that still helped find a bug: https://github.com/llvm/llvm-project/pull/135450 https://github.com/llvm/llvm-project/pull/132401 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/135455 I traced the issue reported by Caroline and Pavel in #134757 back to the call to ProcessRunLock::TrySetRunning. When that fails, we get a somewhat misleading error message: > process resume at entry point failed: Resume request failed - process still > running. This is incorrect: the problem was not that the process was in a running state, but rather that the RunLock was being held by another thread (i.e. the Statusline). TrySetRunning would return false in both cases and the call site only accounted for the former. Besides the odd semantics, the current implementation is inherently race-y and I believe incorrect. If someone is holding the RunLock, the resume call should block, rather than give up, and with the lock held, switch the running state and report the old running state. This patch removes ProcessRunLock::TrySetRunning and updates all callers to use ProcessRunLock::SetRunning instead. To support that, ProcessRunLock::SetRunning (and ProcessRunLock::SetStopped, for consistency) now report whether the process was stopped or running respectively. Previously, both methods returned true unconditionally. The old code has been around pretty much pretty much forever, there's nothing in the git history to indicate that this was done purposely to solve a particular issue. I've tested this on both Linux and macOS and confirmed that this solves the statusline issue. A big thank you to Jim for reviewing my proposed solution offline and trying to poke holes in it. >From b1416c5e13207eaa56985c84e9c2ac74db6a4d2b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 11 Apr 2025 22:48:44 + Subject: [PATCH] [lldb] Remove ProcessRunLock::TrySetRunning I traced the issue reported by Caroline and Pavel in #134757 back to the call to ProcessRunLock::TrySetRunning. When that fails, we get a somewhat misleading error message: > process resume at entry point failed: Resume request failed - process still > running. This is incorrect: the problem was not that the process was in a running state, but rather that the RunLock was being held by another thread (i.e. the Statusline). TrySetRunning would return false in both cases and the call site only accounted for the former. Besides the odd semantics, the current implementation is inherently race-y and I believe incorrect. If someone is holding the RunLock, the resume call should block, rather than give up, and with the lock held, switch the running state and report the old running state. This patch removes ProcessRunLock::TrySetRunning and updates all callers to use ProcessRunLock::SetRunning instead. To support that, ProcessRunLock::SetRunning (and ProcessRunLock::SetStopped, for consistency) now report whether the process was stopped or running respectively. Previously, both methods returned true unconditionally. The old code has been around pretty much pretty much forever, there's nothing in the git history to indicate that this was done purposely to solve a particular issue. I've tested this on both Linux and macOS and confirmed that this solves the statusline issue. A big thank you to Jim for reviewing my proposed solution offline and trying to poke holes in it. --- lldb/include/lldb/Host/ProcessRunLock.h | 7 ++- lldb/source/Host/common/ProcessRunLock.cpp | 18 ++- lldb/source/Host/windows/ProcessRunLock.cpp | 16 ++ lldb/source/Target/Process.cpp | 59 - 4 files changed, 35 insertions(+), 65 deletions(-) diff --git a/lldb/include/lldb/Host/ProcessRunLock.h b/lldb/include/lldb/Host/ProcessRunLock.h index b5b5328b4a33f..c83cab53a9a65 100644 --- a/lldb/include/lldb/Host/ProcessRunLock.h +++ b/lldb/include/lldb/Host/ProcessRunLock.h @@ -29,8 +29,13 @@ class ProcessRunLock { bool ReadTryLock(); bool ReadUnlock(); + + /// Set the process to running. Returns true if the process was stopped. + /// Return true if the process was running. bool SetRunning(); - bool TrySetRunning(); + + /// Set the process to stopped. Returns true if the process was stopped. + /// Returns false if the process was running. bool SetStopped(); class ProcessRunLocker { diff --git a/lldb/source/Host/common/ProcessRunLock.cpp b/lldb/source/Host/common/ProcessRunLock.cpp index da59f40576978..f9bde96ae8ac9 100644 --- a/lldb/source/Host/common/ProcessRunLock.cpp +++ b/lldb/source/Host/common/ProcessRunLock.cpp @@ -37,21 +37,10 @@ bool ProcessRunLock::ReadUnlock() { bool ProcessRunLock::SetRunning() { ::pthread_rwlock_wrlock(&m_rwlock); + bool was_stopped = !m_running; m_running = true; ::pthread_rwlock_unlock(&m_rwlock); - return true; -} - -bool ProcessRunLock::TrySetRunning() { - bool r; - - if (::pthread_rwlock_trywrlock(&m_rwlock) == 0) { -r = !m_running; -m_running = true; -::pthread_rwlock_unlock(&m_rwlock); -return r; - } - return false; + return was_stopped;
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes I traced the issue reported by Caroline and Pavel in #134757 back to the call to ProcessRunLock::TrySetRunning. When that fails, we get a somewhat misleading error message: > process resume at entry point failed: Resume request failed - process still running. This is incorrect: the problem was not that the process was in a running state, but rather that the RunLock was being held by another thread (i.e. the Statusline). TrySetRunning would return false in both cases and the call site only accounted for the former. Besides the odd semantics, the current implementation is inherently race-y and I believe incorrect. If someone is holding the RunLock, the resume call should block, rather than give up, and with the lock held, switch the running state and report the old running state. This patch removes ProcessRunLock::TrySetRunning and updates all callers to use ProcessRunLock::SetRunning instead. To support that, ProcessRunLock::SetRunning (and ProcessRunLock::SetStopped, for consistency) now report whether the process was stopped or running respectively. Previously, both methods returned true unconditionally. The old code has been around pretty much pretty much forever, there's nothing in the git history to indicate that this was done purposely to solve a particular issue. I've tested this on both Linux and macOS and confirmed that this solves the statusline issue. A big thank you to Jim for reviewing my proposed solution offline and trying to poke holes in it. --- Full diff: https://github.com/llvm/llvm-project/pull/135455.diff 4 Files Affected: - (modified) lldb/include/lldb/Host/ProcessRunLock.h (+6-1) - (modified) lldb/source/Host/common/ProcessRunLock.cpp (+4-14) - (modified) lldb/source/Host/windows/ProcessRunLock.cpp (+4-12) - (modified) lldb/source/Target/Process.cpp (+21-38) ``diff diff --git a/lldb/include/lldb/Host/ProcessRunLock.h b/lldb/include/lldb/Host/ProcessRunLock.h index b5b5328b4a33f..c83cab53a9a65 100644 --- a/lldb/include/lldb/Host/ProcessRunLock.h +++ b/lldb/include/lldb/Host/ProcessRunLock.h @@ -29,8 +29,13 @@ class ProcessRunLock { bool ReadTryLock(); bool ReadUnlock(); + + /// Set the process to running. Returns true if the process was stopped. + /// Return true if the process was running. bool SetRunning(); - bool TrySetRunning(); + + /// Set the process to stopped. Returns true if the process was stopped. + /// Returns false if the process was running. bool SetStopped(); class ProcessRunLocker { diff --git a/lldb/source/Host/common/ProcessRunLock.cpp b/lldb/source/Host/common/ProcessRunLock.cpp index da59f40576978..f9bde96ae8ac9 100644 --- a/lldb/source/Host/common/ProcessRunLock.cpp +++ b/lldb/source/Host/common/ProcessRunLock.cpp @@ -37,21 +37,10 @@ bool ProcessRunLock::ReadUnlock() { bool ProcessRunLock::SetRunning() { ::pthread_rwlock_wrlock(&m_rwlock); + bool was_stopped = !m_running; m_running = true; ::pthread_rwlock_unlock(&m_rwlock); - return true; -} - -bool ProcessRunLock::TrySetRunning() { - bool r; - - if (::pthread_rwlock_trywrlock(&m_rwlock) == 0) { -r = !m_running; -m_running = true; -::pthread_rwlock_unlock(&m_rwlock); -return r; - } - return false; + return was_stopped; } bool ProcessRunLock::SetStopped() { @@ -60,6 +49,7 @@ bool ProcessRunLock::SetStopped() { ::pthread_rwlock_unlock(&m_rwlock); return true; } -} + +} // namespace lldb_private #endif diff --git a/lldb/source/Host/windows/ProcessRunLock.cpp b/lldb/source/Host/windows/ProcessRunLock.cpp index 693641e42ed73..9f144b4c918f8 100644 --- a/lldb/source/Host/windows/ProcessRunLock.cpp +++ b/lldb/source/Host/windows/ProcessRunLock.cpp @@ -58,24 +58,16 @@ bool ProcessRunLock::ReadUnlock() { return ::ReadUnlock(m_rwlock); } bool ProcessRunLock::SetRunning() { WriteLock(m_rwlock); + bool was_stopped = !m_running; m_running = true; WriteUnlock(m_rwlock); - return true; -} - -bool ProcessRunLock::TrySetRunning() { - if (WriteTryLock(m_rwlock)) { -bool was_running = m_running; -m_running = true; -WriteUnlock(m_rwlock); -return !was_running; - } - return false; + return was_stopped; } bool ProcessRunLock::SetStopped() { WriteLock(m_rwlock); + bool was_running = m_running; m_running = false; WriteUnlock(m_rwlock); - return true; + return was_running; } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index a9787823b9108..cd5b8c1e8a6fa 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -577,9 +577,7 @@ void Process::Finalize(bool destructing) { // contain events that have ProcessSP values in them which can keep this // process around forever. These events need to be cleared out. m_private_state_listener_sp->Clear(); - m_public_run_lock.TrySetRunning(); // This will do nothing if alrea
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
@@ -29,8 +29,13 @@ class ProcessRunLock { bool ReadTryLock(); bool ReadUnlock(); + + /// Set the process to running. Returns true if the process was stopped. + /// Return true if the process was running. jimingham wrote: One of those two `true`'s has to be a `false`. https://github.com/llvm/llvm-project/pull/135455 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
@@ -1325,9 +1323,9 @@ void Process::SetPublicState(StateType new_state, bool restarted) { Status Process::Resume() { Log *log(GetLog(LLDBLog::State | LLDBLog::Process)); LLDB_LOGF(log, "(plugin = %s) -- locking run lock", GetPluginName().data()); - if (!m_public_run_lock.TrySetRunning()) { -LLDB_LOGF(log, "(plugin = %s) -- TrySetRunning failed, not resuming.", - GetPluginName().data()); + if (!m_public_run_lock.SetRunning()) { +LLDB_LOGF(log, "(plugin = %s) -- SetRunning failed, not resuming.", + GetPluginName().data()); return Status::FromErrorString( "Resume request failed - process still running."); jimingham wrote: To be really accurate, this should be: Resume request failed - process was already running." https://github.com/llvm/llvm-project/pull/135455 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
jimingham wrote: This looks good to me, but we've discussed this already so I'm a prejudiced judge. We should maybe wait on a second opinion... https://github.com/llvm/llvm-project/pull/135455 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove ProcessRunLock::TrySetRunning (PR #135455)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/135455 >From b1416c5e13207eaa56985c84e9c2ac74db6a4d2b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 11 Apr 2025 22:48:44 + Subject: [PATCH 1/2] [lldb] Remove ProcessRunLock::TrySetRunning I traced the issue reported by Caroline and Pavel in #134757 back to the call to ProcessRunLock::TrySetRunning. When that fails, we get a somewhat misleading error message: > process resume at entry point failed: Resume request failed - process still > running. This is incorrect: the problem was not that the process was in a running state, but rather that the RunLock was being held by another thread (i.e. the Statusline). TrySetRunning would return false in both cases and the call site only accounted for the former. Besides the odd semantics, the current implementation is inherently race-y and I believe incorrect. If someone is holding the RunLock, the resume call should block, rather than give up, and with the lock held, switch the running state and report the old running state. This patch removes ProcessRunLock::TrySetRunning and updates all callers to use ProcessRunLock::SetRunning instead. To support that, ProcessRunLock::SetRunning (and ProcessRunLock::SetStopped, for consistency) now report whether the process was stopped or running respectively. Previously, both methods returned true unconditionally. The old code has been around pretty much pretty much forever, there's nothing in the git history to indicate that this was done purposely to solve a particular issue. I've tested this on both Linux and macOS and confirmed that this solves the statusline issue. A big thank you to Jim for reviewing my proposed solution offline and trying to poke holes in it. --- lldb/include/lldb/Host/ProcessRunLock.h | 7 ++- lldb/source/Host/common/ProcessRunLock.cpp | 18 ++- lldb/source/Host/windows/ProcessRunLock.cpp | 16 ++ lldb/source/Target/Process.cpp | 59 - 4 files changed, 35 insertions(+), 65 deletions(-) diff --git a/lldb/include/lldb/Host/ProcessRunLock.h b/lldb/include/lldb/Host/ProcessRunLock.h index b5b5328b4a33f..c83cab53a9a65 100644 --- a/lldb/include/lldb/Host/ProcessRunLock.h +++ b/lldb/include/lldb/Host/ProcessRunLock.h @@ -29,8 +29,13 @@ class ProcessRunLock { bool ReadTryLock(); bool ReadUnlock(); + + /// Set the process to running. Returns true if the process was stopped. + /// Return true if the process was running. bool SetRunning(); - bool TrySetRunning(); + + /// Set the process to stopped. Returns true if the process was stopped. + /// Returns false if the process was running. bool SetStopped(); class ProcessRunLocker { diff --git a/lldb/source/Host/common/ProcessRunLock.cpp b/lldb/source/Host/common/ProcessRunLock.cpp index da59f40576978..f9bde96ae8ac9 100644 --- a/lldb/source/Host/common/ProcessRunLock.cpp +++ b/lldb/source/Host/common/ProcessRunLock.cpp @@ -37,21 +37,10 @@ bool ProcessRunLock::ReadUnlock() { bool ProcessRunLock::SetRunning() { ::pthread_rwlock_wrlock(&m_rwlock); + bool was_stopped = !m_running; m_running = true; ::pthread_rwlock_unlock(&m_rwlock); - return true; -} - -bool ProcessRunLock::TrySetRunning() { - bool r; - - if (::pthread_rwlock_trywrlock(&m_rwlock) == 0) { -r = !m_running; -m_running = true; -::pthread_rwlock_unlock(&m_rwlock); -return r; - } - return false; + return was_stopped; } bool ProcessRunLock::SetStopped() { @@ -60,6 +49,7 @@ bool ProcessRunLock::SetStopped() { ::pthread_rwlock_unlock(&m_rwlock); return true; } -} + +} // namespace lldb_private #endif diff --git a/lldb/source/Host/windows/ProcessRunLock.cpp b/lldb/source/Host/windows/ProcessRunLock.cpp index 693641e42ed73..9f144b4c918f8 100644 --- a/lldb/source/Host/windows/ProcessRunLock.cpp +++ b/lldb/source/Host/windows/ProcessRunLock.cpp @@ -58,24 +58,16 @@ bool ProcessRunLock::ReadUnlock() { return ::ReadUnlock(m_rwlock); } bool ProcessRunLock::SetRunning() { WriteLock(m_rwlock); + bool was_stopped = !m_running; m_running = true; WriteUnlock(m_rwlock); - return true; -} - -bool ProcessRunLock::TrySetRunning() { - if (WriteTryLock(m_rwlock)) { -bool was_running = m_running; -m_running = true; -WriteUnlock(m_rwlock); -return !was_running; - } - return false; + return was_stopped; } bool ProcessRunLock::SetStopped() { WriteLock(m_rwlock); + bool was_running = m_running; m_running = false; WriteUnlock(m_rwlock); - return true; + return was_running; } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index a9787823b9108..cd5b8c1e8a6fa 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -577,9 +577,7 @@ void Process::Finalize(bool destructing) { // contain events that have ProcessSP values in them which can keep this // process around forever. These events need to be
[Lldb-commits] [lldb] [lldb] Make sure the process is stopped when computing the symbol context (PR #135458)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/135458 Make sure the process is stopped when computing the symbol context. Both Adrian and Felipe reported a handful of crashes in GetSymbolContext called from Statusline::Redraw on the default event thread. Given that we're handling a StackFrameSP, it's not clear to me how that could have gotten invalidated, but Jim points out that it doesn't make sense to compute the symbol context for the frame when the process isn't stopped. Depends on #135455 >From 19d761478bdad5615e3f3c7f4a1b71c1c698725b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 11 Apr 2025 16:56:53 -0700 Subject: [PATCH] =?UTF-8?q?Revert=20"Revert=20"[lldb]=20Make=20sure=20the?= =?UTF-8?q?=20process=20is=20stopped=20when=20computing=20the=20sy?= =?UTF-8?q?=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 179d30f8c3fddd3c85056fd2b8e877a4a8513158. --- lldb/source/Core/Statusline.cpp | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index ed5308ef53eb0..e14691e2538a2 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -12,6 +12,7 @@ #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/StreamString.h" @@ -134,8 +135,15 @@ void Statusline::Redraw(bool update) { exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget()); SymbolContext symbol_ctx; - if (auto frame_sp = exe_ctx.GetFrameSP()) -symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); + if (ProcessSP process_sp = exe_ctx.GetProcessSP()) { +// Check if the process is stopped, and if it is, make sure it remains +// stopped until we've computed the symbol context. +Process::StopLocker stop_locker; +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + if (auto frame_sp = exe_ctx.GetFrameSP()) +symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); +} + } StreamString stream; if (auto *format = m_debugger.GetStatuslineFormat()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make sure the process is stopped when computing the symbol context (PR #135458)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes Make sure the process is stopped when computing the symbol context. Both Adrian and Felipe reported a handful of crashes in GetSymbolContext called from Statusline::Redraw on the default event thread. Given that we're handling a StackFrameSP, it's not clear to me how that could have gotten invalidated, but Jim points out that it doesn't make sense to compute the symbol context for the frame when the process isn't stopped. Depends on #135455 --- Full diff: https://github.com/llvm/llvm-project/pull/135458.diff 1 Files Affected: - (modified) lldb/source/Core/Statusline.cpp (+10-2) ``diff diff --git a/lldb/source/Core/Statusline.cpp b/lldb/source/Core/Statusline.cpp index ed5308ef53eb0..e14691e2538a2 100644 --- a/lldb/source/Core/Statusline.cpp +++ b/lldb/source/Core/Statusline.cpp @@ -12,6 +12,7 @@ #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/StreamString.h" @@ -134,8 +135,15 @@ void Statusline::Redraw(bool update) { exe_ctx.SetTargetPtr(&m_debugger.GetSelectedOrDummyTarget()); SymbolContext symbol_ctx; - if (auto frame_sp = exe_ctx.GetFrameSP()) -symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); + if (ProcessSP process_sp = exe_ctx.GetProcessSP()) { +// Check if the process is stopped, and if it is, make sure it remains +// stopped until we've computed the symbol context. +Process::StopLocker stop_locker; +if (stop_locker.TryLock(&process_sp->GetRunLock())) { + if (auto frame_sp = exe_ctx.GetFrameSP()) +symbol_ctx = frame_sp->GetSymbolContext(eSymbolContextEverything); +} + } StreamString stream; if (auto *format = m_debugger.GetStatuslineFormat()) `` https://github.com/llvm/llvm-project/pull/135458 ___ 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/dmpots updated https://github.com/llvm/llvm-project/pull/134418 >From e240bda8fcea9db4d9c456929ba811feb8d4152b Mon Sep 17 00:00:00 2001 From: David Peixotto Date: Tue, 11 Mar 2025 13:02:14 -0700 Subject: [PATCH 1/7] Add commands to list/enable/disable plugins This commit adds three new commands for managing plugins. The `list` command will show which plugins are currently registered and their enabled state. The `enable` and `disable` commands can be used to enable or disable plugins. A disabled plugin will not show up to the PluginManager when it iterates over available plugins of a particular type. The purpose of these commands is to provide more visibility into registered plugins and allow users to disable plugins for experimental perf reasons. There are a few limitations to the current implementation 1. Only SystemRuntime plugins are currently supported. We can easily extend the existing implementation to support more types. 2. Only "statically" know plugin types are supported (i.e. those managed by the PluginManager and not from `plugin load`). It is possibly we could support dynamic plugins as well, but I have not looked into it yet. --- lldb/source/Commands/CommandObjectPlugin.cpp | 335 ++ .../source/Commands/CommandObjectSettings.cpp | 1 + lldb/source/Commands/Options.td | 5 + .../command-plugin-enable+disable.test| 53 +++ .../Shell/Commands/command-plugin-list.test | 51 +++ 5 files changed, 445 insertions(+) create mode 100644 lldb/test/Shell/Commands/command-plugin-enable+disable.test create mode 100644 lldb/test/Shell/Commands/command-plugin-list.test diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp index f3108b8a768d2..68261d24ffe1f 100644 --- a/lldb/source/Commands/CommandObjectPlugin.cpp +++ b/lldb/source/Commands/CommandObjectPlugin.cpp @@ -7,8 +7,11 @@ //===--===// #include "CommandObjectPlugin.h" +#include "lldb/Core/PluginManager.h" +#include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "llvm/Support/GlobPattern.h" using namespace lldb; using namespace lldb_private; @@ -46,12 +49,344 @@ 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
[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. dmpots wrote: Updated to use the system-runtime as an 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)
@@ -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"