[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -250,6 +251,13 @@ def which(program): return None +def pickrandomport(): +"""Returns a random open port.""" +with socket.socket() as sock: +sock.bind(("", 0)) +return sock.getsockname()[1] labath wrote: Cool. So I guess this means the function is unused now? https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,182 @@ +//===-- Socket.h *- C++ -*-===// labath wrote: I'm not quite comfortable with this essentially reimplementing large parts of `source/Host/common/*Socket.cpp`. @walter-erquinigo do you have any objections to using the implementation from the lldb `host` library. I see lldb-dap already depends on `lldbHost` on windows (and NetBSD?) -- this would make that dependency unconditional. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -52,11 +51,11 @@ struct StreamDescriptor { struct InputStream { StreamDescriptor descriptor; - bool read_full(std::ofstream *log, size_t length, std::string &text); + bool read_full(llvm::raw_ostream *log, size_t length, std::string &text); - bool read_line(std::ofstream *log, std::string &line); + bool read_line(llvm::raw_ostream *log, std::string &line); - bool read_expected(std::ofstream *log, llvm::StringRef expected); + bool read_expected(llvm::raw_ostream *log, llvm::StringRef expected); labath wrote: I'm not sure why is this being changed, but could that be a separate patch as well? https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,67 @@ +""" +Test lldb-dap server integration. +""" + +import os +import tempfile + +import dap_server +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbdap_testcase + + +class TestDAP_server(lldbdap_testcase.DAPTestCaseBase): +def do_test_server(self, connection): +log_file_path = self.getBuildArtifact("dap.txt") +server, connection = dap_server.DebugAdaptorServer.launch( +self.lldbDAPExec, connection, log_file=log_file_path +) + +def cleanup(): +server.terminate() +server.wait() + +self.addTearDownHook(cleanup) + +self.build() +program = self.getBuildArtifact("a.out") +source = "main.c" +breakpoint_line = line_number(source, "// breakpoint") + +# Initial connection over the port. +self.create_debug_adaptor(launch=False, connection=connection) +self.launch( +program, +disconnectAutomatically=False, +) +self.set_source_breakpoints(source, [breakpoint_line]) +self.continue_to_next_stop() +self.continue_to_exit() +output = self.get_stdout() +self.assertEquals(output, "hello world!\r\n") +self.dap_server.request_disconnect() + +# Second connection over the port. +self.create_debug_adaptor(launch=False, connection=connection) +self.launch(program) +self.set_source_breakpoints(source, [breakpoint_line]) +self.continue_to_next_stop() +self.continue_to_exit() +output = self.get_stdout() +self.assertEquals(output, "hello world!\r\n") + +def test_server_port(self): +""" +Test launching a binary with a lldb-dap in server mode on a specific port. +""" +self.do_test_server(connection="tcp://localhost:0") + +def test_server_unix_socket(self): labath wrote: ```suggestion @skipIfWindows def test_server_unix_socket(self): ``` https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1bc9895 - [lldb/DWARF] Remove duplicate type filtering (#116989)
Author: Pavel Labath Date: 2024-11-25T09:52:19+01:00 New Revision: 1bc98957c898d7e7233746a7b284982d20539593 URL: https://github.com/llvm/llvm-project/commit/1bc98957c898d7e7233746a7b284982d20539593 DIFF: https://github.com/llvm/llvm-project/commit/1bc98957c898d7e7233746a7b284982d20539593.diff LOG: [lldb/DWARF] Remove duplicate type filtering (#116989) In #108907, the index classes started filtering the DIEs according to the full type query (instead of just the base name). This means that the checks in SymbolFileDWARF are now redundant. I've also moved the non-redundant checks so that now all checking is done in the DWARFIndex class and the caller can expect to get the final filtered list of types. Added: Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Removed: diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp index c18edd10b96819..30c890d6d01388 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp @@ -137,9 +137,19 @@ void DWARFIndex::GetTypesWithQuery( bool DWARFIndex::ProcessTypeDIEMatchQuery( TypeQuery &query, DWARFDIE die, llvm::function_ref callback) { - // Nothing to match from query - if (query.GetContextRef().size() <= 1) + // Check the language, but only if we have a language filter. + if (query.HasLanguage() && + !query.LanguageMatches(SymbolFileDWARF::GetLanguageFamily(*die.GetCU( +return true; // Keep iterating over index types, language mismatch. + + // Since mangled names are unique, we only need to check if the names are + // the same. + if (query.GetSearchByMangledName()) { +if (die.GetMangledName(/*substitute_name_allowed=*/false) != +query.GetTypeBasename().GetStringRef()) + return true; // Keep iterating over index types, mangled name mismatch. return callback(die); + } std::vector die_context; if (query.GetModuleSearch()) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 8ce0db4588a46a..c900f330b481bb 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2726,39 +2726,8 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { TypeQuery query_full(query); bool have_index_match = false; m_index->GetTypesWithQuery(query_full, [&](DWARFDIE die) { -// Check the language, but only if we have a language filter. -if (query.HasLanguage()) { - if (!query.LanguageMatches(GetLanguageFamily(*die.GetCU( -return true; // Keep iterating over index types, language mismatch. -} - -// Since mangled names are unique, we only need to check if the names are -// the same. -if (query.GetSearchByMangledName()) { - if (die.GetMangledName(/*substitute_name_allowed=*/false) != - query.GetTypeBasename().GetStringRef()) -return true; // Keep iterating over index types, mangled name mismatch. - if (Type *matching_type = ResolveType(die, true, true)) { -results.InsertUnique(matching_type->shared_from_this()); -return !results.Done(query); // Keep iterating if we aren't done. - } - return true; // Keep iterating over index types, weren't able to resolve - // this type -} - -// Check the context matches -std::vector die_context; -if (query.GetModuleSearch()) - die_context = die.GetDeclContext(); -else - die_context = die.GetTypeLookupContext(); -assert(!die_context.empty()); -if (!query.ContextMatches(die_context)) - return true; // Keep iterating over index types, context mismatch. - -// Try to resolve the type. if (Type *matching_type = ResolveType(die, true, true)) { - if (matching_type->IsTemplateType()) { + if (!query.GetSearchByMangledName() && matching_type->IsTemplateType()) { // We have to watch out for case where we lookup a type by basename and // it matches a template with simple template names. Like looking up // "Foo" and if we have simple template names then we will match @@ -2790,7 +2759,7 @@ void SymbolFileDWARF::FindTypes(const TypeQuery &query, TypeResults &results) { // With -gsimple-template-names, a templated type's DW_AT_name will not // contain the template parameters. Try again stripping '<' and anything // after, filtering out entries with template parameters that don't match. - if (!have_index_match) { + if (!have_index_match && !query.GetSearchByMangledName()) { // Create a type matcher with a compiler context that is tuned for // -gsimple-template-names. We will use this for the index look
[Lldb-commits] [lldb] 84fec77 - [lldb][docs] Clarify unit for SVE P register size
Author: David Spickett Date: 2024-11-25T09:53:15Z New Revision: 84fec7757ea330bbaf82b46ed081ccc45b120e45 URL: https://github.com/llvm/llvm-project/commit/84fec7757ea330bbaf82b46ed081ccc45b120e45 DIFF: https://github.com/llvm/llvm-project/commit/84fec7757ea330bbaf82b46ed081ccc45b120e45.diff LOG: [lldb][docs] Clarify unit for SVE P register size Added: Modified: lldb/docs/use/aarch64-linux.md Removed: diff --git a/lldb/docs/use/aarch64-linux.md b/lldb/docs/use/aarch64-linux.md index 70432f57857a59..393838dc0bb4f5 100644 --- a/lldb/docs/use/aarch64-linux.md +++ b/lldb/docs/use/aarch64-linux.md @@ -17,7 +17,7 @@ In LLDB you will be able to see the following new registers: * `z0-z31` vector registers, each one has size equal to the vector length. * `p0-p15` predicate registers, each one containing 1 bit per byte in the vector - length. Making each one vector length / 8 sized. + length. So each one is `vector length in bits / 8` bits. * `ffr` the first fault register, same size as a predicate register. * `vg`, the vector length in "granules". Each granule is 8 bytes. ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/117581 In case of an error GetBlock would return a reference to a Block without adding it to a parent. This doesn't seem like a good idea, and none of the other plugins do that. This patch fixes that by propagating errors (well, null pointers...) up the stack. I don't know of any specific problem that this solves, but given that this occurs only when something goes very wrong (e.g. a corrupted PDB file), it's quite possible noone has run into this situation, so we can't say the code is correct either. It also gets in the way of a refactor I'm contemplating. >From 704eaf250480e0f74e4f135d61b7e66c3356eb97 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 25 Nov 2024 18:05:54 +0100 Subject: [PATCH] [lldb/NativePDB] Don't create parentless blocks In case of an error GetBlock would return a reference to a Block without adding it to a parent. This doesn't seem like a good idea, and none of the other plugins do that. This patch fixes that by propagating errors (well, null pointers...) up the stack. I don't know of any specific problem that this solves, but given that this occurs only when something goes very wrong (e.g. a corrupted PDB file), it's quite possible noone has run into this situation, so we can't say the code is correct either. It also gets in the way of a refactor I'm contemplating. --- .../NativePDB/SymbolFileNativePDB.cpp | 59 +++ .../NativePDB/SymbolFileNativePDB.h | 4 +- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index c0416b4d06815d..8b02e6369c59c7 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -379,18 +379,17 @@ uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() { return count; } -Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { +Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi); CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset); CompUnitSP comp_unit = GetOrCreateCompileUnit(*cii); lldb::user_id_t opaque_block_uid = toOpaqueUid(block_id); - BlockSP child_block = std::make_shared(opaque_block_uid); auto ts_or_err = GetTypeSystemForLanguage(comp_unit->GetLanguage()); if (auto err = ts_or_err.takeError()) -return *child_block; +return nullptr; auto ts = *ts_or_err; if (!ts) -return *child_block; +return nullptr; PdbAstBuilder* ast_builder = ts->GetNativePDBParser(); switch (sym.kind()) { @@ -403,7 +402,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { Block &block = func->GetBlock(false); if (block.GetNumRanges() == 0) block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize())); - return block; + return █ } break; } @@ -416,13 +415,16 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { cantFail(SymbolDeserializer::deserializeAs(sym, block)); lldbassert(block.Parent != 0); PdbCompilandSymId parent_id(block_id.modi, block.Parent); -Block &parent_block = GetOrCreateBlock(parent_id); -Function *func = parent_block.CalculateSymbolContextFunction(); +Block *parent_block = GetOrCreateBlock(parent_id); +if (!parent_block) + return nullptr; +Function *func = parent_block->CalculateSymbolContextFunction(); lldbassert(func); lldb::addr_t block_base = m_index->MakeVirtualAddress(block.Segment, block.CodeOffset); lldb::addr_t func_base = func->GetAddressRange().GetBaseAddress().GetFileAddress(); +BlockSP child_block = std::make_shared(opaque_block_uid); if (block_base >= func_base) child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize)); else { @@ -435,7 +437,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { block_id.modi, block_id.offset, block_base, block_base + block.CodeSize, func_base); } -parent_block.AddChild(child_block); +parent_block->AddChild(child_block); ast_builder->GetOrCreateBlockDecl(block_id); m_blocks.insert({opaque_block_uid, child_block}); break; @@ -445,8 +447,11 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { comp_unit->GetLineTable(); std::shared_ptr inline_site = m_inline_sites[opaque_block_uid]; -Block &parent_block = GetOrCreateBlock(inline_site->parent_id); -parent_block.AddChild(child_block); +Block *parent_block = GetOrCreateBlock(inline_site->parent_id); +if (!parent_block) + return nullptr; +BlockSP child_block = std::make_shared(opaque_block_u
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes In case of an error GetBlock would return a reference to a Block without adding it to a parent. This doesn't seem like a good idea, and none of the other plugins do that. This patch fixes that by propagating errors (well, null pointers...) up the stack. I don't know of any specific problem that this solves, but given that this occurs only when something goes very wrong (e.g. a corrupted PDB file), it's quite possible noone has run into this situation, so we can't say the code is correct either. It also gets in the way of a refactor I'm contemplating. --- Full diff: https://github.com/llvm/llvm-project/pull/117581.diff 2 Files Affected: - (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp (+36-23) - (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h (+2-2) ``diff diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index c0416b4d06815d..8b02e6369c59c7 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -379,18 +379,17 @@ uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() { return count; } -Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { +Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi); CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset); CompUnitSP comp_unit = GetOrCreateCompileUnit(*cii); lldb::user_id_t opaque_block_uid = toOpaqueUid(block_id); - BlockSP child_block = std::make_shared(opaque_block_uid); auto ts_or_err = GetTypeSystemForLanguage(comp_unit->GetLanguage()); if (auto err = ts_or_err.takeError()) -return *child_block; +return nullptr; auto ts = *ts_or_err; if (!ts) -return *child_block; +return nullptr; PdbAstBuilder* ast_builder = ts->GetNativePDBParser(); switch (sym.kind()) { @@ -403,7 +402,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { Block &block = func->GetBlock(false); if (block.GetNumRanges() == 0) block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize())); - return block; + return █ } break; } @@ -416,13 +415,16 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { cantFail(SymbolDeserializer::deserializeAs(sym, block)); lldbassert(block.Parent != 0); PdbCompilandSymId parent_id(block_id.modi, block.Parent); -Block &parent_block = GetOrCreateBlock(parent_id); -Function *func = parent_block.CalculateSymbolContextFunction(); +Block *parent_block = GetOrCreateBlock(parent_id); +if (!parent_block) + return nullptr; +Function *func = parent_block->CalculateSymbolContextFunction(); lldbassert(func); lldb::addr_t block_base = m_index->MakeVirtualAddress(block.Segment, block.CodeOffset); lldb::addr_t func_base = func->GetAddressRange().GetBaseAddress().GetFileAddress(); +BlockSP child_block = std::make_shared(opaque_block_uid); if (block_base >= func_base) child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize)); else { @@ -435,7 +437,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { block_id.modi, block_id.offset, block_base, block_base + block.CodeSize, func_base); } -parent_block.AddChild(child_block); +parent_block->AddChild(child_block); ast_builder->GetOrCreateBlockDecl(block_id); m_blocks.insert({opaque_block_uid, child_block}); break; @@ -445,8 +447,11 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { comp_unit->GetLineTable(); std::shared_ptr inline_site = m_inline_sites[opaque_block_uid]; -Block &parent_block = GetOrCreateBlock(inline_site->parent_id); -parent_block.AddChild(child_block); +Block *parent_block = GetOrCreateBlock(inline_site->parent_id); +if (!parent_block) + return nullptr; +BlockSP child_block = std::make_shared(opaque_block_uid); +parent_block->AddChild(child_block); ast_builder->GetOrCreateInlinedFunctionDecl(block_id); // Copy ranges from InlineSite to Block. for (size_t i = 0; i < inline_site->ranges.GetSize(); ++i) { @@ -469,7 +474,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { lldbassert(false && "Symbol is not a block!"); } - return *child_block; + return nullptr; } lldb::FunctionSP SymbolFileNativePDB::CreateFunction(PdbCompilandSymId func_id, @@ -997,10 +1002,10 @@ SymbolFileNativePDB::GetOrCreateCompileUnit(const CompilandIndexItem &cci) { return emplace_result.first->second; } -Block &Symb
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
https://github.com/ZequanWu approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/117581 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) Changes On #110065 the changes to LinuxSigInfo Struct introduced some variables that will differ in size on 32b or 64b. I've rectified this by setting them all to build independent types. --- Full diff: https://github.com/llvm/llvm-project/pull/117604.diff 1 Files Affected: - (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.h (+5-4) ``diff diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 4ebbaadebe9f90..e60ee86e5f5a03 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -83,10 +83,10 @@ struct ELFLinuxSigInfo { int32_t si_errno; int32_t si_code; // Copied from siginfo_t so we don't have to include signal.h on non 'Nix - // builds. + // builds. Slight modifications to ensure no 32b vs 64b differences. struct { -lldb::addr_t si_addr; /* faulting insn/memory ref. */ -short int si_addr_lsb; /* Valid LSB of the reported address. */ +lldb::addr_t si_addr; /* faulting insn/memory ref. */ +int16_t si_addr_lsb; /* Valid LSB of the reported address. */ union { /* used when si_code=SEGV_BNDERR */ struct { @@ -98,7 +98,8 @@ struct ELFLinuxSigInfo { } bounds; } sigfault; - enum { eUnspecified, eNT_SIGINFO } note_type; + enum SigInfoNoteType : uint8_t { eUnspecified, eNT_SIGINFO }; + SigInfoNoteType note_type; ELFLinuxSigInfo(); `` https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
Jlalond wrote: @tuliom I don't have a way to verify this, would you mind assisting? https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/117604 On #110065 the changes to LinuxSigInfo Struct introduced some variables that will differ in size on 32b or 64b. I've rectified this by setting them all to build independent types. >From 693f1039e10e5dd26ef0d600b03895f71f340b23 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 25 Nov 2024 10:26:07 -0800 Subject: [PATCH] Set all the properties of ELFLinuxSigInfo to a non build dependent size --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 4ebbaadebe9f90..e60ee86e5f5a03 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -83,10 +83,10 @@ struct ELFLinuxSigInfo { int32_t si_errno; int32_t si_code; // Copied from siginfo_t so we don't have to include signal.h on non 'Nix - // builds. + // builds. Slight modifications to ensure no 32b vs 64b differences. struct { -lldb::addr_t si_addr; /* faulting insn/memory ref. */ -short int si_addr_lsb; /* Valid LSB of the reported address. */ +lldb::addr_t si_addr; /* faulting insn/memory ref. */ +int16_t si_addr_lsb; /* Valid LSB of the reported address. */ union { /* used when si_code=SEGV_BNDERR */ struct { @@ -98,7 +98,8 @@ struct ELFLinuxSigInfo { } bounds; } sigfault; - enum { eUnspecified, eNT_SIGINFO } note_type; + enum SigInfoNoteType : uint8_t { eUnspecified, eNT_SIGINFO }; + SigInfoNoteType note_type; ELFLinuxSigInfo(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
https://github.com/compnerd created https://github.com/llvm/llvm-project/pull/117615 The `find_package(LibXml2 ...)` invocation that we are currently using precludes the use of "CONFIG mode" for libxml2. This is important to allow dependencies to flow through the build with static builds on Windows, which depends on Bcrypt and conditionally on Ws2_32 (in development - current releases are unconditionally dependent on it). If libxml2 is built statically, this dependency would need to be replicated into LLDB's build configuration to ensure that the dependencies are met. Add a workaround to support CONFIG mode search which avoids this need. Additionally, clean up some unnecessary include paths. The use of `LibXml2::LibXml2` with `target_link_libraries` on `libLLDBHost` ensures that the header search path is properly propagated. >From 733687bd4aef042df4d1e7587cd78d6ba9e6c37f Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Mon, 25 Nov 2024 11:21:17 -0800 Subject: [PATCH] build: enable CONFIG mode search for LibXml2 for LLDB The `find_package(LibXml2 ...)` invocation that we are currently using precludes the use of "CONFIG mode" for libxml2. This is important to allow dependencies to flow through the build with static builds on Windows, which depends on Bcrypt and conditionally on Ws2_32 (in development - current releases are unconditionally dependent on it). If libxml2 is built statically, this dependency would need to be replicated into LLDB's build configuration to ensure that the dependencies are met. Add a workaround to support CONFIG mode search which avoids this need. Additionally, clean up some unnecessary include paths. The use of `LibXml2::LibXml2` with `target_link_libraries` on `libLLDBHost` ensures that the header search path is properly propagated. --- lldb/cmake/modules/LLDBConfig.cmake | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 93ccd9c479c2b8..ba3ce6c444f887 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, +# in CONFIG mode, we cannot specify the version as that is an exact match. As a +# mitigation, perform a CONFIG mode search for LibXml2 if LLDB_ENABLE_LIBXML2 +# is ON or AUTO, and fallback to the old path if not found. +if(LLDB_ENABLE_LIBXML2 OR "${LLDB_ENABLE_LIBXML2}" STREQUAL "AUTO") + find_package(LibXml2 CONFIG) + if(NOT TARGET LibXml2::LibXml2) +add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) + endif() +endif() add_optional_dependency(LLDB_ENABLE_FBSDVMCORE "Enable libfbsdvmcore support in LLDB" FBSDVMCore FBSDVMCore_FOUND QUIET) option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON) @@ -239,10 +249,6 @@ if (LLDB_ENABLE_LZMA) include_directories(${LIBLZMA_INCLUDE_DIRS}) endif() -if (LLDB_ENABLE_LIBXML2) - include_directories(${LIBXML2_INCLUDE_DIR}) -endif() - include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR}/include ${CMAKE_CURRENT_SOURCE_DIR}/include @@ -283,7 +289,6 @@ if (APPLE) find_library(FOUNDATION_LIBRARY Foundation) find_library(CORE_FOUNDATION_LIBRARY CoreFoundation) find_library(SECURITY_LIBRARY Security) - include_directories(${LIBXML2_INCLUDE_DIR}) endif() if( WIN32 AND NOT CYGWIN ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] build: enable CONFIG mode search for LibXml2 for LLDB (PR #117615)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Saleem Abdulrasool (compnerd) Changes The `find_package(LibXml2 ...)` invocation that we are currently using precludes the use of "CONFIG mode" for libxml2. This is important to allow dependencies to flow through the build with static builds on Windows, which depends on Bcrypt and conditionally on Ws2_32 (in development - current releases are unconditionally dependent on it). If libxml2 is built statically, this dependency would need to be replicated into LLDB's build configuration to ensure that the dependencies are met. Add a workaround to support CONFIG mode search which avoids this need. Additionally, clean up some unnecessary include paths. The use of `LibXml2::LibXml2` with `target_link_libraries` on `libLLDBHost` ensures that the header search path is properly propagated. --- Full diff: https://github.com/llvm/llvm-project/pull/117615.diff 1 Files Affected: - (modified) lldb/cmake/modules/LLDBConfig.cmake (+11-6) ``diff diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index 93ccd9c479c2b8..ba3ce6c444f887 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -57,7 +57,17 @@ add_optional_dependency(LLDB_ENABLE_CURSES "Enable curses support in LLDB" Curse add_optional_dependency(LLDB_ENABLE_LZMA "Enable LZMA compression support in LLDB" LibLZMA LIBLZMA_FOUND) add_optional_dependency(LLDB_ENABLE_LUA "Enable Lua scripting support in LLDB" LuaAndSwig LUAANDSWIG_FOUND) add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python scripting support in LLDB" PythonAndSwig PYTHONANDSWIG_FOUND) -add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) +# LibXml2 is required if LLDB_ENABLE_LIBXML2 is ON or AUTO. The LibXml2 package +# search is not setup properly to support the CONFIG mode search. Furthermore, +# in CONFIG mode, we cannot specify the version as that is an exact match. As a +# mitigation, perform a CONFIG mode search for LibXml2 if LLDB_ENABLE_LIBXML2 +# is ON or AUTO, and fallback to the old path if not found. +if(LLDB_ENABLE_LIBXML2 OR "${LLDB_ENABLE_LIBXML2}" STREQUAL "AUTO") + find_package(LibXml2 CONFIG) + if(NOT TARGET LibXml2::LibXml2) +add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" LibXml2 LIBXML2_FOUND VERSION 2.8) + endif() +endif() add_optional_dependency(LLDB_ENABLE_FBSDVMCORE "Enable libfbsdvmcore support in LLDB" FBSDVMCore FBSDVMCore_FOUND QUIET) option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" ON) @@ -239,10 +249,6 @@ if (LLDB_ENABLE_LZMA) include_directories(${LIBLZMA_INCLUDE_DIRS}) endif() -if (LLDB_ENABLE_LIBXML2) - include_directories(${LIBXML2_INCLUDE_DIR}) -endif() - include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR}/include ${CMAKE_CURRENT_SOURCE_DIR}/include @@ -283,7 +289,6 @@ if (APPLE) find_library(FOUNDATION_LIBRARY Foundation) find_library(CORE_FOUNDATION_LIBRARY CoreFoundation) find_library(SECURITY_LIBRARY Security) - include_directories(${LIBXML2_INCLUDE_DIR}) endif() if( WIN32 AND NOT CYGWIN ) `` https://github.com/llvm/llvm-project/pull/117615 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
https://github.com/mbucko updated https://github.com/llvm/llvm-project/pull/114120 >From 362d6f52709ddf303d9d9fa256526182ba60fa53 Mon Sep 17 00:00:00 2001 From: Miro Bucko Date: Tue, 29 Oct 2024 13:02:21 -0700 Subject: [PATCH] Fix return value of 'PluginManager::RegisterPlugin()'. --- lldb/source/Core/PluginManager.cpp | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index a5219025495a91..80c9465f9af721 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -206,10 +206,9 @@ template class PluginInstances { if (!callback) return false; assert(!name.empty()); -Instance instance = -Instance(name, description, callback, std::forward(args)...); -m_instances.push_back(instance); -return false; +m_instances.emplace_back(name, description, callback, + std::forward(args)...); +return true; } bool UnregisterPlugin(typename Instance::CallbackType callback) { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
mbucko wrote: > We obviously aren't checking this return anywhere. The only way it can return > false that I can see is if you call RegisterPlugin with no callback pointer - > which seems more like a programming error than a run-time check. Do we > actually need this bool return? It is indeed being checked in many places, I tried removing it and the compiler was not happy :) https://github.com/llvm/llvm-project/pull/114120 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
https://github.com/clayborg approved this pull request. https://github.com/llvm/llvm-project/pull/114120 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,182 @@ +//===-- Socket.h *- C++ -*-===// walter-erquinigo wrote: Thanks for bringing this up. I think that ideally, we should be able to reuse components from LLDB, like the `Host` library, as you point out. So, @ashgti , please do that instead of reimplementing the wheel :) https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -32,35 +34,44 @@ using namespace lldb_dap; namespace lldb_dap { -DAP::DAP(llvm::StringRef path, ReplMode repl_mode) -: debug_adaptor_path(path), broadcaster("lldb-dap"), - exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID), - stop_at_entry(false), is_attach(false), +DAP::DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode, + std::vector pre_init_commands) +: debug_adaptor_path(path), broadcaster("lldb-dap"), log(log), + exception_breakpoints(), pre_init_commands(pre_init_commands), + focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), enable_synthetic_child_debugging(false), display_extended_backtrace(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - reverse_request_seq(0), repl_mode(repl_mode) { - const char *log_file_path = getenv("LLDBDAP_LOG"); -#if defined(_WIN32) - // Windows opens stdout and stdin in text mode which converts \n to 13,10 - // while the value is just 10 on Darwin/Linux. Setting the file mode to binary - // fixes this. - int result = _setmode(fileno(stdout), _O_BINARY); - assert(result); - result = _setmode(fileno(stdin), _O_BINARY); - UNUSED_IF_ASSERT_DISABLED(result); - assert(result); -#endif - if (log_file_path) -log.reset(new std::ofstream(log_file_path)); -} + reverse_request_seq(0), repl_mode(repl_mode) {} DAP::~DAP() = default; +llvm::Error DAP::ConfigureIO(int out_fd, int err_fd) { + llvm::Expected new_stdout_fd = + RedirectFd(out_fd, [this](llvm::StringRef data) { +SendOutput(OutputType::Stdout, data); + }); walter-erquinigo wrote: Yeah, this is maybe the trickiest part of all. The redirection was introduced to prevent LLDB proper from printing messages to stderr and unexpectedly causing the DAP client to abort. And indeed, there are many parts of LLDB that print directly to stderr upon unusual situations (dwarf parsing is one of them IIRC). The ideal solution is to create an actual SB API that can handle these unusual messages and print to stderr is simply the default behavior. You could also do that. If you don't do such refactor, something that you should consider when using multiple connections simultaneously, is that it might be tricky to know which of those stderr messages correspond to which DAP client. You might decide to multicast them, which is better than sending the output to the wrong client. Silencing the messages altogether might not be a good idea. In any case, you can think of the redirection as something that needs to be set up only once per process. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
ashgti wrote: > The patch is still a bit larger than I'd like. I've tried to highlight a few > things that could be split off into separate PRs, but I think the biggest > question now (not really for you, but for lldb-dap maintainers) is whether > this could be using the socket abstractions already present in lldb (instead > of building your own). If the answer to that is "yes", then switching to > those should probably be a separate patch as well. I can split this into smaller patches. As far as the socket abstraction, at the moment lldb-dap doesn't use much (if anything) from lldb outside of the API folder. I saw the `lldb/Host/Socket.h` files but they also are built on top of the `IOObject` and interact with the `MainLoop` which would mean taking on a few new dependencies from the Host folder. I did look at the [`llvm::ListeningSocket`](https://github.com/llvm/llvm-project/blob/99fd1c5536547ed4fc360b16e7fa2e06278707a8/llvm/include/llvm/Support/raw_socket_stream.h#L59) which supports unix sockets / named pipes and the [`lldb_private::SocketAddress`](https://github.com/llvm/llvm-project/blob/99fd1c5536547ed4fc360b16e7fa2e06278707a8/lldb/include/lldb/Host/SocketAddress.h#L34) types when I implemented my socket, but I also tried to keep the socket simple for the lldb-dap's purposes. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [DWARF] Fix DWARTTypePrinter unable to print qualified name for DW_TAG_typedef DIE (PR #117239)
dwblaikie wrote: > > Where those names go from "const string" to "const std::__1::string" (and > > from "string" to "std::__1::string")? or something like that. > > Yes. In some cases, it's getting super verbose, e.g: > `std::__1::basic_string, > std::__1::allocator >::value_type`. Oh yeah, still good though. > > Probably the most common way to test this would be to add something in > > llvm/test/DebugInfo/X86/dwarfdump-*.ll > > Oh, this test might be where we test most of this in the past: > > llvm/test/tools/llvm-dwarfdump/X86/prettyprint_types.s > > Because DWARTTypePrinter is used by both llvm-dwarfump and lldb (after a > reland), how about just test DWARTTypePrinter in > `llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp` as an unit test? Perhaps - if it's easy to write/legible. Though I don't think we should feel bad about somewhat "indirectly" testing libDebugInfoDWARF via llvm-dwarfdump tests, rather than restricting ourselves to API tests for bugs in libDebugInfoDWARF. So whichever's more legible/maintainable. https://github.com/llvm/llvm-project/pull/117239 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
labath wrote: > > The patch is still a bit larger than I'd like. I've tried to highlight a > > few things that could be split off into separate PRs, but I think the > > biggest question now (not really for you, but for lldb-dap maintainers) is > > whether this could be using the socket abstractions already present in lldb > > (instead of building your own). If the answer to that is "yes", then > > switching to those should probably be a separate patch as well. > > I can split this into smaller patches. Thanks. > > As far as the socket abstraction, at the moment lldb-dap doesn't use much (if > anything) from lldb outside of the API folder. I saw the `lldb/Host/Socket.h` > files but they also are built on top of the `IOObject` and interact with the > `MainLoop` which would mean taking on a few new dependencies from the Host > folder. I am aware of that, but I don't think that situation is sustainable in the long run. When lldb-dap was first added, I believe it only supported talking over stdio, so it had like ~zero amount of socket code, and so this kind of purist approach did not hurt (much). However, its getting quite complex now, and I think copying/reimplementing every abstraction it needs is going to come back and bite us. FWIW, the MainLoop class is self-contained, and might actually be useful if you wanted to support things like listening on IPv4 and v6 sockets simultaneously. And IOObject is just a abstract base, so it also doesn't shouldn't pull in much. We're also making sure (it took a lot of effort to reach that point) that the Host library as a whole does not depend on the rest of lldb. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ProcessELFCore] Add Description to ProcessELFCore/ELFThread stop reasons (PR #110065)
Jlalond wrote: Ack, sorry about the delay. I will get this fixed today. https://github.com/llvm/llvm-project/pull/110065 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
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 e5faeb69fbb87722ee315884eeef2089b10b0cee 704eaf250480e0f74e4f135d61b7e66c3356eb97 --extensions cpp,h -- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index 8b02e6369c..0f77b2e280 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -1886,8 +1886,8 @@ VariableSP SymbolFileNativePDB::CreateLocalVariable(PdbCompilandSymId scope_id, bool static_member = false; Variable::RangeList scope_ranges; VariableSP var_sp = std::make_shared( - toOpaqueUid(var_id), name.c_str(), name.c_str(), sftype, var_scope, - block, scope_ranges, &decl, var_info.location, external, artificial, + toOpaqueUid(var_id), name.c_str(), name.c_str(), sftype, var_scope, block, + scope_ranges, &decl, var_info.location, external, artificial, location_is_constant_data, static_member); if (!is_param) { auto ts_or_err = GetTypeSystemForLanguage(comp_unit_sp->GetLanguage()); `` https://github.com/llvm/llvm-project/pull/117581 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/117581 >From 704eaf250480e0f74e4f135d61b7e66c3356eb97 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 25 Nov 2024 18:05:54 +0100 Subject: [PATCH 1/2] [lldb/NativePDB] Don't create parentless blocks In case of an error GetBlock would return a reference to a Block without adding it to a parent. This doesn't seem like a good idea, and none of the other plugins do that. This patch fixes that by propagating errors (well, null pointers...) up the stack. I don't know of any specific problem that this solves, but given that this occurs only when something goes very wrong (e.g. a corrupted PDB file), it's quite possible noone has run into this situation, so we can't say the code is correct either. It also gets in the way of a refactor I'm contemplating. --- .../NativePDB/SymbolFileNativePDB.cpp | 59 +++ .../NativePDB/SymbolFileNativePDB.h | 4 +- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index c0416b4d06815d..8b02e6369c59c7 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -379,18 +379,17 @@ uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() { return count; } -Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { +Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi); CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset); CompUnitSP comp_unit = GetOrCreateCompileUnit(*cii); lldb::user_id_t opaque_block_uid = toOpaqueUid(block_id); - BlockSP child_block = std::make_shared(opaque_block_uid); auto ts_or_err = GetTypeSystemForLanguage(comp_unit->GetLanguage()); if (auto err = ts_or_err.takeError()) -return *child_block; +return nullptr; auto ts = *ts_or_err; if (!ts) -return *child_block; +return nullptr; PdbAstBuilder* ast_builder = ts->GetNativePDBParser(); switch (sym.kind()) { @@ -403,7 +402,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { Block &block = func->GetBlock(false); if (block.GetNumRanges() == 0) block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize())); - return block; + return █ } break; } @@ -416,13 +415,16 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { cantFail(SymbolDeserializer::deserializeAs(sym, block)); lldbassert(block.Parent != 0); PdbCompilandSymId parent_id(block_id.modi, block.Parent); -Block &parent_block = GetOrCreateBlock(parent_id); -Function *func = parent_block.CalculateSymbolContextFunction(); +Block *parent_block = GetOrCreateBlock(parent_id); +if (!parent_block) + return nullptr; +Function *func = parent_block->CalculateSymbolContextFunction(); lldbassert(func); lldb::addr_t block_base = m_index->MakeVirtualAddress(block.Segment, block.CodeOffset); lldb::addr_t func_base = func->GetAddressRange().GetBaseAddress().GetFileAddress(); +BlockSP child_block = std::make_shared(opaque_block_uid); if (block_base >= func_base) child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize)); else { @@ -435,7 +437,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { block_id.modi, block_id.offset, block_base, block_base + block.CodeSize, func_base); } -parent_block.AddChild(child_block); +parent_block->AddChild(child_block); ast_builder->GetOrCreateBlockDecl(block_id); m_blocks.insert({opaque_block_uid, child_block}); break; @@ -445,8 +447,11 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { comp_unit->GetLineTable(); std::shared_ptr inline_site = m_inline_sites[opaque_block_uid]; -Block &parent_block = GetOrCreateBlock(inline_site->parent_id); -parent_block.AddChild(child_block); +Block *parent_block = GetOrCreateBlock(inline_site->parent_id); +if (!parent_block) + return nullptr; +BlockSP child_block = std::make_shared(opaque_block_uid); +parent_block->AddChild(child_block); ast_builder->GetOrCreateInlinedFunctionDecl(block_id); // Copy ranges from InlineSite to Block. for (size_t i = 0; i < inline_site->ranges.GetSize(); ++i) { @@ -469,7 +474,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { lldbassert(false && "Symbol is not a block!"); } - return *child_block; + return nullptr; } lldb::FunctionSP SymbolFileNativePDB::CreateFunction(PdbCompilandSymId func_id, @@ -997,10 +1002,10 @@ SymbolFileNativePDB::GetOrCreateCompileUn
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,182 @@ +//===-- Socket.h *- C++ -*-===// ashgti wrote: Sounds good, I can re-work my patch to use `lldb/Hosts/*` https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix return value of 'PluginManager::RegisterPlugin()'. (PR #114120)
https://github.com/mbucko closed https://github.com/llvm/llvm-project/pull/114120 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
https://github.com/tuliom requested changes to this pull request. This didn't help. The size on 32-bits continue to be 44. I think the difference is not really caused the size of the properties on 32 bits, but the implicit padding on 32/64 bits. Look at the code generated here: https://godbolt.org/z/j5WxTTess By commenting the pads or changing their size you can see the impact on the size of the struct and on the address of each of its member. https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ProcessELFCore] Add Description to ProcessELFCore/ELFThread stop reasons (PR #110065)
tuliom wrote: We're also seeing this issue. We have a full build log available at https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-big-merge-20241123/fedora-rawhide-i386/08305565-llvm/builder-live.log.gz , in case it helps. https://github.com/llvm/llvm-project/pull/110065 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Remove duplicate type filtering (PR #116989)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/116989 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
https://github.com/labath commented: The patch is still a bit larger than I'd like. I've tried to highlight a few things that could be split off into separate PRs, but I think the biggest question now (not really for you, but for lldb-dap maintainers) is whether this could be using the socket abstractions already present in lldb (instead of building your own). If the answer to that is "yes", then switching to those should probably be a separate patch as well. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -1196,6 +1202,62 @@ def terminate(self): self.process.wait() self.process = None +@classmethod +def launch( +cls, executable: str, /, connection=None, log_file=None, env=None +) -> tuple[subprocess.Popen, str]: +adaptor_env = os.environ.copy() +if env: +adaptor_env.update(env) + +if log_file: +adaptor_env["LLDBDAP_LOG"] = log_file + +if os.uname().sysname == "Darwin": +adaptor_env["NSUnbufferedIO"] = "YES" + +args = [executable] +if connection: +args.append("--connection") +args.append(connection) + +proc = subprocess.Popen( +args, +stdin=subprocess.PIPE, +stdout=subprocess.PIPE, +stderr=sys.stdout, +env=adaptor_env, +) + +if connection: +# If a conneciton is specified, lldb-dap will print the listening +# address once the listener is made to stdout. The listener is +# formatted like `tcp://host:port` or `unix:///path`. +with selectors.DefaultSelector() as sel: +print("Reading stdout for the listening connection") +os.set_blocking(proc.stdout.fileno(), False) +stdout_key = sel.register(proc.stdout, selectors.EVENT_READ) +rdy_fds = sel.select(timeout=10.0) labath wrote: It looks like this will be a problem for windows (which can only `select` sockets). I can think of a couple of alternatives (writing the connection spec to a file -- and polling for it; using OS apis directly -- we have a `Pipe` class in `lldbgdbserverutils.py` for the lldb-server use case; doing a blocking read and relying on the test suite level timeout; ...), but neither of them is clearly superior. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -1154,34 +1154,38 @@ class DebugAdaptorServer(DebugCommunication): def __init__( self, executable=None, +launch=True, port=None, +unix_socket=None, init_commands=[], log_file=None, env=None, ): self.process = None -if executable is not None: -adaptor_env = os.environ.copy() -if env is not None: -adaptor_env.update(env) - -if log_file: -adaptor_env["LLDBDAP_LOG"] = log_file -self.process = subprocess.Popen( -[executable], -stdin=subprocess.PIPE, -stdout=subprocess.PIPE, -stderr=subprocess.PIPE, -env=adaptor_env, +if launch: +self.process = DebugAdaptorServer.launch( +executable, +port=port, +unix_socket=unix_socket, +log_file=log_file, +env=env, ) -DebugCommunication.__init__( -self, self.process.stdout, self.process.stdin, init_commands, log_file -) -elif port is not None: + +if port: s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.connect(("127.0.0.1", port)) DebugCommunication.__init__( -self, s.makefile("r"), s.makefile("w"), init_commands +self, s.makefile("rb"), s.makefile("wb"), init_commands, log_file +) +elif unix_socket: +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.connect(unix_socket) +DebugCommunication.__init__( +self, s.makefile("rb"), s.makefile("wb"), init_commands, log_file +) labath wrote: Supporting unix sockets *only* would not be good, as then this wouldn't work on windows. However, TCP/IP works everywhere, so it *might* be sufficient to only support those? I don't think its my place to tell you you shouldn't implement that, but I also don't want you to think that's a prerequisite for this feature (making sure it possible to add the functionality later is nice, but I think we could forego the actual implementation until someone actually needs it). https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -0,0 +1,67 @@ +""" +Test lldb-dap server integration. +""" + +import os +import tempfile + +import dap_server +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbdap_testcase + + +class TestDAP_server(lldbdap_testcase.DAPTestCaseBase): +def do_test_server(self, connection): +log_file_path = self.getBuildArtifact("dap.txt") +server, connection = dap_server.DebugAdaptorServer.launch( +self.lldbDAPExec, connection, log_file=log_file_path +) + +def cleanup(): +server.terminate() +server.wait() + +self.addTearDownHook(cleanup) + +self.build() +program = self.getBuildArtifact("a.out") +source = "main.c" +breakpoint_line = line_number(source, "// breakpoint") + +# Initial connection over the port. +self.create_debug_adaptor(launch=False, connection=connection) +self.launch( +program, +disconnectAutomatically=False, +) +self.set_source_breakpoints(source, [breakpoint_line]) +self.continue_to_next_stop() +self.continue_to_exit() +output = self.get_stdout() +self.assertEquals(output, "hello world!\r\n") +self.dap_server.request_disconnect() + +# Second connection over the port. +self.create_debug_adaptor(launch=False, connection=connection) +self.launch(program) +self.set_source_breakpoints(source, [breakpoint_line]) +self.continue_to_next_stop() +self.continue_to_exit() +output = self.get_stdout() +self.assertEquals(output, "hello world!\r\n") + +def test_server_port(self): +""" +Test launching a binary with a lldb-dap in server mode on a specific port. +""" +self.do_test_server(connection="tcp://localhost:0") + +def test_server_unix_socket(self): +""" +Test launching a binary with a lldb-dap in server mode on a unix socket. +""" +dir = tempfile.gettempdir() labath wrote: Normally, I'd put temporary files in `self.getBuildDir`, but unix sockets have fairly low limits for the socket name/path length (one of the reasons I don't like them), so this may actually be better -- but then I'd recommend to (try to) clean up the socket file afterwards (another reason I don't like unix sockets) https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -1196,6 +1202,62 @@ def terminate(self): self.process.wait() self.process = None +@classmethod +def launch( +cls, executable: str, /, connection=None, log_file=None, env=None +) -> tuple[subprocess.Popen, str]: +adaptor_env = os.environ.copy() +if env: +adaptor_env.update(env) + +if log_file: +adaptor_env["LLDBDAP_LOG"] = log_file + +if os.uname().sysname == "Darwin": +adaptor_env["NSUnbufferedIO"] = "YES" labath wrote: What is this for? Could it be replaced by a well-positioned `flush()` call? https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
@@ -32,35 +34,44 @@ using namespace lldb_dap; namespace lldb_dap { -DAP::DAP(llvm::StringRef path, ReplMode repl_mode) -: debug_adaptor_path(path), broadcaster("lldb-dap"), - exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID), - stop_at_entry(false), is_attach(false), +DAP::DAP(llvm::StringRef path, llvm::raw_ostream *log, ReplMode repl_mode, + std::vector pre_init_commands) +: debug_adaptor_path(path), broadcaster("lldb-dap"), log(log), + exception_breakpoints(), pre_init_commands(pre_init_commands), + focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), enable_synthetic_child_debugging(false), display_extended_backtrace(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }), - reverse_request_seq(0), repl_mode(repl_mode) { - const char *log_file_path = getenv("LLDBDAP_LOG"); -#if defined(_WIN32) - // Windows opens stdout and stdin in text mode which converts \n to 13,10 - // while the value is just 10 on Darwin/Linux. Setting the file mode to binary - // fixes this. - int result = _setmode(fileno(stdout), _O_BINARY); - assert(result); - result = _setmode(fileno(stdin), _O_BINARY); - UNUSED_IF_ASSERT_DISABLED(result); - assert(result); -#endif - if (log_file_path) -log.reset(new std::ofstream(log_file_path)); -} + reverse_request_seq(0), repl_mode(repl_mode) {} DAP::~DAP() = default; +llvm::Error DAP::ConfigureIO(int out_fd, int err_fd) { + llvm::Expected new_stdout_fd = + RedirectFd(out_fd, [this](llvm::StringRef data) { +SendOutput(OutputType::Stdout, data); + }); labath wrote: AFAICT, nothing ensures these callbacks don't run after the DAP object was terminated. This wasn't a big deal when the process was serving only one connection (although making the object stack-allocated in #116272 is kinda problematic), but I think this needs to be sorted out (ideally as a separate patch) for the multi-connection version. It also doesn't look like the pipes created here get cleaned up anywhere. https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose discontinuous functions through SBFunction::GetRanges (PR #117532)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/117532 SBFunction::GetEndAddress doesn't really make sense for discontinuous functions, so I'm declaring it deprecated. GetStartAddress sort of makes sense, if one uses it to find the functions entry point, so I'm keeping that undeprecated. I've made the test a Shell tests because these make it easier to create discontinuous functions regardless of the host os and architecture. They do make testing the python API harder, but I think I've managed to come up with something not entirely unreasonable. >From 03385da68038c3fd40fc085c8fb1914529116837 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 12 Nov 2024 11:30:39 +0100 Subject: [PATCH] [lldb] Expose discontinuous functions through SBFunction::GetRanges SBFunction::GetEndAddress doesn't really make sense for discontinuous functions, so I'm declaring it deprecated. GetStartAddress sort of makes sense, if one uses it to find the functions entry point, so I'm keeping that undeprecated. I've made the test a Shell tests because these make it easier to create discontinuous functions regardless of the host os and architecture. They do make testing the python API harder, but I think I've managed to come up with something not entirely unreasonable. --- lldb/include/lldb/API/SBAddressRangeList.h| 1 + lldb/include/lldb/API/SBFunction.h| 2 + lldb/include/lldb/Core/AddressRangeListImpl.h | 5 +- lldb/include/lldb/Symbol/Function.h | 3 + lldb/source/API/SBFunction.cpp| 17 +- lldb/source/Core/AddressRangeListImpl.cpp | 8 - .../Python/sb_function_ranges.s | 182 ++ 7 files changed, 198 insertions(+), 20 deletions(-) create mode 100644 lldb/test/Shell/ScriptInterpreter/Python/sb_function_ranges.s diff --git a/lldb/include/lldb/API/SBAddressRangeList.h b/lldb/include/lldb/API/SBAddressRangeList.h index 5a4eeecf37dc96..41085b1edf8d7f 100644 --- a/lldb/include/lldb/API/SBAddressRangeList.h +++ b/lldb/include/lldb/API/SBAddressRangeList.h @@ -45,6 +45,7 @@ class LLDB_API SBAddressRangeList { private: friend class SBBlock; friend class SBProcess; + friend class SBFunction; lldb_private::AddressRangeListImpl &ref() const; diff --git a/lldb/include/lldb/API/SBFunction.h b/lldb/include/lldb/API/SBFunction.h index df607fdc7ebf59..0a8aeeff1ea5ad 100644 --- a/lldb/include/lldb/API/SBFunction.h +++ b/lldb/include/lldb/API/SBFunction.h @@ -43,6 +43,8 @@ class LLDB_API SBFunction { lldb::SBAddress GetStartAddress(); + LLDB_DEPRECATED_FIXME("Not compatible with discontinuous functions.", +"GetRanges()") lldb::SBAddress GetEndAddress(); lldb::SBAddressRangeList GetRanges(); diff --git a/lldb/include/lldb/Core/AddressRangeListImpl.h b/lldb/include/lldb/Core/AddressRangeListImpl.h index 6742e6ead87de0..6b88f9b1ac1795 100644 --- a/lldb/include/lldb/Core/AddressRangeListImpl.h +++ b/lldb/include/lldb/Core/AddressRangeListImpl.h @@ -24,9 +24,8 @@ class AddressRangeListImpl { public: AddressRangeListImpl(); - AddressRangeListImpl(const AddressRangeListImpl &rhs) = default; - - AddressRangeListImpl &operator=(const AddressRangeListImpl &rhs); + explicit AddressRangeListImpl(AddressRanges ranges) + : m_ranges(std::move(ranges)) {} size_t GetSize() const; diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h index 70f51a846f8d96..11921398ac7651 100644 --- a/lldb/include/lldb/Symbol/Function.h +++ b/lldb/include/lldb/Symbol/Function.h @@ -444,8 +444,11 @@ class Function : public UserID, public SymbolContextScope { Function *CalculateSymbolContextFunction() override; + // DEPRECATED: Use GetAddressRanges instead. const AddressRange &GetAddressRange() { return m_range; } + const AddressRanges &GetAddressRanges() const { return m_ranges; } + lldb::LanguageType GetLanguage() const; /// Find the file and line number of the source location of the start of the /// function. This will use the declaration if present and fall back on the diff --git a/lldb/source/API/SBFunction.cpp b/lldb/source/API/SBFunction.cpp index ac61220ec8736a..2ef62eea4d1993 100644 --- a/lldb/source/API/SBFunction.cpp +++ b/lldb/source/API/SBFunction.cpp @@ -10,6 +10,7 @@ #include "lldb/API/SBAddressRange.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Core/AddressRangeListImpl.h" #include "lldb/Core/Disassembler.h" #include "lldb/Core/Module.h" #include "lldb/Symbol/CompileUnit.h" @@ -153,10 +154,11 @@ SBAddress SBFunction::GetEndAddress() { SBAddress addr; if (m_opaque_ptr) { -addr_t byte_size = m_opaque_ptr->GetAddressRange().GetByteSize(); -if (byte_size > 0) { - addr.SetAddress(m_opaque_ptr->GetAddressRange().GetBaseAddress()); - addr->Slide(byte_size); +llvm::ArrayRef ranges = m_opaque_ptr->GetAddressRanges(); +if (!ranges.empty()) {
[Lldb-commits] [lldb] [lldb] Expose discontinuous functions through SBFunction::GetRanges (PR #117532)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes SBFunction::GetEndAddress doesn't really make sense for discontinuous functions, so I'm declaring it deprecated. GetStartAddress sort of makes sense, if one uses it to find the functions entry point, so I'm keeping that undeprecated. I've made the test a Shell tests because these make it easier to create discontinuous functions regardless of the host os and architecture. They do make testing the python API harder, but I think I've managed to come up with something not entirely unreasonable. --- Full diff: https://github.com/llvm/llvm-project/pull/117532.diff 7 Files Affected: - (modified) lldb/include/lldb/API/SBAddressRangeList.h (+1) - (modified) lldb/include/lldb/API/SBFunction.h (+2) - (modified) lldb/include/lldb/Core/AddressRangeListImpl.h (+2-3) - (modified) lldb/include/lldb/Symbol/Function.h (+3) - (modified) lldb/source/API/SBFunction.cpp (+8-9) - (modified) lldb/source/Core/AddressRangeListImpl.cpp (-8) - (added) lldb/test/Shell/ScriptInterpreter/Python/sb_function_ranges.s (+182) ``diff diff --git a/lldb/include/lldb/API/SBAddressRangeList.h b/lldb/include/lldb/API/SBAddressRangeList.h index 5a4eeecf37dc96..41085b1edf8d7f 100644 --- a/lldb/include/lldb/API/SBAddressRangeList.h +++ b/lldb/include/lldb/API/SBAddressRangeList.h @@ -45,6 +45,7 @@ class LLDB_API SBAddressRangeList { private: friend class SBBlock; friend class SBProcess; + friend class SBFunction; lldb_private::AddressRangeListImpl &ref() const; diff --git a/lldb/include/lldb/API/SBFunction.h b/lldb/include/lldb/API/SBFunction.h index df607fdc7ebf59..0a8aeeff1ea5ad 100644 --- a/lldb/include/lldb/API/SBFunction.h +++ b/lldb/include/lldb/API/SBFunction.h @@ -43,6 +43,8 @@ class LLDB_API SBFunction { lldb::SBAddress GetStartAddress(); + LLDB_DEPRECATED_FIXME("Not compatible with discontinuous functions.", +"GetRanges()") lldb::SBAddress GetEndAddress(); lldb::SBAddressRangeList GetRanges(); diff --git a/lldb/include/lldb/Core/AddressRangeListImpl.h b/lldb/include/lldb/Core/AddressRangeListImpl.h index 6742e6ead87de0..6b88f9b1ac1795 100644 --- a/lldb/include/lldb/Core/AddressRangeListImpl.h +++ b/lldb/include/lldb/Core/AddressRangeListImpl.h @@ -24,9 +24,8 @@ class AddressRangeListImpl { public: AddressRangeListImpl(); - AddressRangeListImpl(const AddressRangeListImpl &rhs) = default; - - AddressRangeListImpl &operator=(const AddressRangeListImpl &rhs); + explicit AddressRangeListImpl(AddressRanges ranges) + : m_ranges(std::move(ranges)) {} size_t GetSize() const; diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h index 70f51a846f8d96..11921398ac7651 100644 --- a/lldb/include/lldb/Symbol/Function.h +++ b/lldb/include/lldb/Symbol/Function.h @@ -444,8 +444,11 @@ class Function : public UserID, public SymbolContextScope { Function *CalculateSymbolContextFunction() override; + // DEPRECATED: Use GetAddressRanges instead. const AddressRange &GetAddressRange() { return m_range; } + const AddressRanges &GetAddressRanges() const { return m_ranges; } + lldb::LanguageType GetLanguage() const; /// Find the file and line number of the source location of the start of the /// function. This will use the declaration if present and fall back on the diff --git a/lldb/source/API/SBFunction.cpp b/lldb/source/API/SBFunction.cpp index ac61220ec8736a..2ef62eea4d1993 100644 --- a/lldb/source/API/SBFunction.cpp +++ b/lldb/source/API/SBFunction.cpp @@ -10,6 +10,7 @@ #include "lldb/API/SBAddressRange.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Core/AddressRangeListImpl.h" #include "lldb/Core/Disassembler.h" #include "lldb/Core/Module.h" #include "lldb/Symbol/CompileUnit.h" @@ -153,10 +154,11 @@ SBAddress SBFunction::GetEndAddress() { SBAddress addr; if (m_opaque_ptr) { -addr_t byte_size = m_opaque_ptr->GetAddressRange().GetByteSize(); -if (byte_size > 0) { - addr.SetAddress(m_opaque_ptr->GetAddressRange().GetBaseAddress()); - addr->Slide(byte_size); +llvm::ArrayRef ranges = m_opaque_ptr->GetAddressRanges(); +if (!ranges.empty()) { + // Return the end of the first range, use GetRanges to get all ranges. + addr.SetAddress(ranges.front().GetBaseAddress()); + addr->Slide(ranges.front().GetByteSize()); } } return addr; @@ -166,11 +168,8 @@ lldb::SBAddressRangeList SBFunction::GetRanges() { LLDB_INSTRUMENT_VA(this); lldb::SBAddressRangeList ranges; - if (m_opaque_ptr) { -lldb::SBAddressRange range; -(*range.m_opaque_up) = m_opaque_ptr->GetAddressRange(); -ranges.Append(std::move(range)); - } + if (m_opaque_ptr) +ranges.ref() = AddressRangeListImpl(m_opaque_ptr->GetAddressRanges()); return ranges; } diff --git a/lldb/source/Core/AddressRangeListImpl.cpp b/lldb/source/Core/AddressRang
[Lldb-commits] [lldb] [lldb] Expose discontinuous functions through SBFunction::GetRanges (PR #117532)
@@ -24,9 +24,8 @@ class AddressRangeListImpl { public: AddressRangeListImpl(); - AddressRangeListImpl(const AddressRangeListImpl &rhs) = default; labath wrote: I've deleted this because of a combination of a default copy constructor and a non-default assignment operator is very unusual (and it makes the move operations unavailable). https://github.com/llvm/llvm-project/pull/117532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5e3f615 - [lldb/NativePDB] Don't create parentless blocks (#117581)
Author: Pavel Labath Date: 2024-11-26T07:40:34+01:00 New Revision: 5e3f6150b1d490090faf945777985b18db73ea3f URL: https://github.com/llvm/llvm-project/commit/5e3f6150b1d490090faf945777985b18db73ea3f DIFF: https://github.com/llvm/llvm-project/commit/5e3f6150b1d490090faf945777985b18db73ea3f.diff LOG: [lldb/NativePDB] Don't create parentless blocks (#117581) In case of an error GetBlock would return a reference to a Block without adding it to a parent. This doesn't seem like a good idea, and none of the other plugins do that. This patch fixes that by propagating errors (well, null pointers...) up the stack. I don't know of any specific problem that this solves, but given that this occurs only when something goes very wrong (e.g. a corrupted PDB file), it's quite possible noone has run into this situation, so we can't say the code is correct either. It also gets in the way of a refactor I'm contemplating. Added: Modified: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h Removed: diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp index c0416b4d06815d..0f77b2e28004eb 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -379,18 +379,17 @@ uint32_t SymbolFileNativePDB::CalculateNumCompileUnits() { return count; } -Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { +Block *SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi); CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset); CompUnitSP comp_unit = GetOrCreateCompileUnit(*cii); lldb::user_id_t opaque_block_uid = toOpaqueUid(block_id); - BlockSP child_block = std::make_shared(opaque_block_uid); auto ts_or_err = GetTypeSystemForLanguage(comp_unit->GetLanguage()); if (auto err = ts_or_err.takeError()) -return *child_block; +return nullptr; auto ts = *ts_or_err; if (!ts) -return *child_block; +return nullptr; PdbAstBuilder* ast_builder = ts->GetNativePDBParser(); switch (sym.kind()) { @@ -403,7 +402,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { Block &block = func->GetBlock(false); if (block.GetNumRanges() == 0) block.AddRange(Block::Range(0, func->GetAddressRange().GetByteSize())); - return block; + return █ } break; } @@ -416,13 +415,16 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { cantFail(SymbolDeserializer::deserializeAs(sym, block)); lldbassert(block.Parent != 0); PdbCompilandSymId parent_id(block_id.modi, block.Parent); -Block &parent_block = GetOrCreateBlock(parent_id); -Function *func = parent_block.CalculateSymbolContextFunction(); +Block *parent_block = GetOrCreateBlock(parent_id); +if (!parent_block) + return nullptr; +Function *func = parent_block->CalculateSymbolContextFunction(); lldbassert(func); lldb::addr_t block_base = m_index->MakeVirtualAddress(block.Segment, block.CodeOffset); lldb::addr_t func_base = func->GetAddressRange().GetBaseAddress().GetFileAddress(); +BlockSP child_block = std::make_shared(opaque_block_uid); if (block_base >= func_base) child_block->AddRange(Block::Range(block_base - func_base, block.CodeSize)); else { @@ -435,7 +437,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { block_id.modi, block_id.offset, block_base, block_base + block.CodeSize, func_base); } -parent_block.AddChild(child_block); +parent_block->AddChild(child_block); ast_builder->GetOrCreateBlockDecl(block_id); m_blocks.insert({opaque_block_uid, child_block}); break; @@ -445,8 +447,11 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { comp_unit->GetLineTable(); std::shared_ptr inline_site = m_inline_sites[opaque_block_uid]; -Block &parent_block = GetOrCreateBlock(inline_site->parent_id); -parent_block.AddChild(child_block); +Block *parent_block = GetOrCreateBlock(inline_site->parent_id); +if (!parent_block) + return nullptr; +BlockSP child_block = std::make_shared(opaque_block_uid); +parent_block->AddChild(child_block); ast_builder->GetOrCreateInlinedFunctionDecl(block_id); // Copy ranges from InlineSite to Block. for (size_t i = 0; i < inline_site->ranges.GetSize(); ++i) { @@ -469,7 +474,7 @@ Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) { lldbassert(false && "Symbol is not a block!"); } - return *child_block; + return null
[Lldb-commits] [lldb] [lldb/NativePDB] Don't create parentless blocks (PR #117581)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/117581 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
Sirraide wrote: > I image @AaronBallman is on vacation this week. @Sirraide would you be > willing to LGTM this so that I can move it along? I mean, this is stricly a performance issue, so I’m not sure why we’d want to merge it this urgently... https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
bwendling wrote: I image @AaronBallman is on vacation this week. @Sirraide would you be willing to LGTM this so that I can move it along? It's certainly better than what we have and I can address any concerns Aaron has afterwards. https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
Endilll wrote: For the record, Aaron is not on vacation. So if you need his sign-off, you should wait. https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
bwendling wrote: Ah, okay. Thanks. https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
Jlalond wrote: @tuliom Experimented on godbolt a bit, it looks like alignas(8) will handle our padding to 56 on 32 and 64 https://godbolt.org/z/ssqeeEs71 https://github.com/llvm/llvm-project/pull/117604 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][ThreadELFCore] Set all the properties of ELFLinuxSigInfo to a non build dependent size (PR #117604)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/117604 >From 693f1039e10e5dd26ef0d600b03895f71f340b23 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 25 Nov 2024 10:26:07 -0800 Subject: [PATCH 1/2] Set all the properties of ELFLinuxSigInfo to a non build dependent size --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index 4ebbaadebe9f90..e60ee86e5f5a03 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -83,10 +83,10 @@ struct ELFLinuxSigInfo { int32_t si_errno; int32_t si_code; // Copied from siginfo_t so we don't have to include signal.h on non 'Nix - // builds. + // builds. Slight modifications to ensure no 32b vs 64b differences. struct { -lldb::addr_t si_addr; /* faulting insn/memory ref. */ -short int si_addr_lsb; /* Valid LSB of the reported address. */ +lldb::addr_t si_addr; /* faulting insn/memory ref. */ +int16_t si_addr_lsb; /* Valid LSB of the reported address. */ union { /* used when si_code=SEGV_BNDERR */ struct { @@ -98,7 +98,8 @@ struct ELFLinuxSigInfo { } bounds; } sigfault; - enum { eUnspecified, eNT_SIGINFO } note_type; + enum SigInfoNoteType : uint8_t { eUnspecified, eNT_SIGINFO }; + SigInfoNoteType note_type; ELFLinuxSigInfo(); >From 7578b03833c233f79a199adff07dc37519beb7df Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 25 Nov 2024 14:41:52 -0800 Subject: [PATCH 2/2] Add alignas(8) to handle padding of the struct with union --- lldb/source/Plugins/Process/elf-core/ThreadElfCore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index e60ee86e5f5a03..6f8d41351a6bfb 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -84,7 +84,7 @@ struct ELFLinuxSigInfo { int32_t si_code; // Copied from siginfo_t so we don't have to include signal.h on non 'Nix // builds. Slight modifications to ensure no 32b vs 64b differences. - struct { + struct alignas(8) { lldb::addr_t si_addr; /* faulting insn/memory ref. */ int16_t si_addr_lsb; /* Valid LSB of the reported address. */ union { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
https://github.com/Sirraide approved this pull request. The changes seem fine to me now; not sure we’re handling this in all places where it needs to be handled though. I’d say maybe wait a day or two before merging in case anyone else wants to comment on that. https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [Clang] Improve Sema diagnostic performance for __builtin_counted_by_ref (PR #116719)
bwendling wrote: Yeah, but I'd like to get this off my plate. :-) https://github.com/llvm/llvm-project/pull/116719 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits