[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC) (PR #85892)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes This is another step towards supporting DWARF5 checksums and inline source code in LLDB. This is a reland of #85468 but without the functional change of storing the support file from the line table (yet). --- Patch is 27.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85892.diff 27 Files Affected: - (modified) lldb/include/lldb/Core/Disassembler.h (+1-1) - (modified) lldb/include/lldb/Symbol/LineEntry.h (+4-1) - (modified) lldb/include/lldb/Utility/SupportFile.h (+3) - (modified) lldb/source/API/SBLineEntry.cpp (+5-5) - (modified) lldb/source/API/SBThread.cpp (+1-1) - (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+1-1) - (modified) lldb/source/Breakpoint/BreakpointResolverFileLine.cpp (+4-3) - (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+2-2) - (modified) lldb/source/Commands/CommandObjectSource.cpp (+7-7) - (modified) lldb/source/Commands/CommandObjectThread.cpp (+1-1) - (modified) lldb/source/Core/Address.cpp (+1-1) - (modified) lldb/source/Core/Disassembler.cpp (+4-4) - (modified) lldb/source/Core/FormatEntity.cpp (+1-1) - (modified) lldb/source/Core/IOHandlerCursesGUI.cpp (+6-5) - (modified) lldb/source/Core/SourceManager.cpp (+1-1) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp (+1-1) - (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (+2-2) - (modified) lldb/source/Symbol/CompileUnit.cpp (+1-1) - (modified) lldb/source/Symbol/Function.cpp (+2-2) - (modified) lldb/source/Symbol/LineEntry.cpp (+7-6) - (modified) lldb/source/Symbol/LineTable.cpp (+2-2) - (modified) lldb/source/Symbol/SymbolContext.cpp (+2-2) - (modified) lldb/source/Target/StackFrame.cpp (+1-1) - (modified) lldb/source/Target/StackFrameList.cpp (+2-2) - (modified) lldb/source/Target/Thread.cpp (+4-4) - (modified) lldb/source/Target/TraceDumper.cpp (+2-2) - (modified) lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (+1-1) ``diff diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 885ac1bb4a7ef8..e037a49f152c74 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -538,7 +538,7 @@ class Disassembler : public std::enable_shared_from_this, ElideMixedSourceAndDisassemblyLine(const ExecutionContext _ctx, const SymbolContext , LineEntry ) { SourceLine sl; -sl.file = line.file; +sl.file = line.GetFile(); sl.line = line.line; sl.column = line.column; return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl); diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h index 31e1cd0b36f96e..8da59cf0bd24aa 100644 --- a/lldb/include/lldb/Symbol/LineEntry.h +++ b/lldb/include/lldb/Symbol/LineEntry.h @@ -130,11 +130,14 @@ struct LineEntry { /// Shared pointer to the target this LineEntry belongs to. void ApplyFileMappings(lldb::TargetSP target_sp); + /// Helper to access the file. + const FileSpec () const { return file_sp->GetSpecOnly(); } + /// The section offset address range for this line entry. AddressRange range; /// The source file, possibly mapped by the target.source-map setting. - FileSpec file; + lldb::SupportFileSP file_sp; /// The original source file, from debug info. lldb::SupportFileSP original_file_sp; diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h index 0ea0ca4e7c97a1..7505d7f345c5dd 100644 --- a/lldb/include/lldb/Utility/SupportFile.h +++ b/lldb/include/lldb/Utility/SupportFile.h @@ -45,6 +45,9 @@ class SupportFile { /// Materialize the file to disk and return the path to that temporary file. virtual const FileSpec () { return m_file_spec; } + /// Change the file name. + void Update(const FileSpec _spec) { m_file_spec = file_spec; } + protected: FileSpec m_file_spec; Checksum m_checksum; diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp index 28d12e65fdaf8a..99a7b8fe644cb5 100644 --- a/lldb/source/API/SBLineEntry.cpp +++ b/lldb/source/API/SBLineEntry.cpp @@ -81,8 +81,8 @@ SBFileSpec SBLineEntry::GetFileSpec() const { LLDB_INSTRUMENT_VA(this); SBFileSpec sb_file_spec; - if (m_opaque_up.get() && m_opaque_up->file) -sb_file_spec.SetFileSpec(m_opaque_up->file); + if (m_opaque_up.get() && m_opaque_up->GetFile()) +sb_file_spec.SetFileSpec(m_opaque_up->GetFile()); return sb_file_spec; } @@ -109,9 +109,9 @@ void SBLineEntry::SetFileSpec(lldb::SBFileSpec filespec) { LLDB_INSTRUMENT_VA(this, filespec); if (filespec.IsValid()) -ref().file = filespec.ref(); +ref().file_sp = std::make_shared(filespec.ref()); else -ref().file.Clear(); +ref().file_sp = std::make_shared(); } void SBLineEntry::SetLine(uint32_t
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC) (PR #85892)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC) (PR #85892)
https://github.com/JDevlieghere ready_for_review https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC) (PR #85892)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC (PR #85892)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/85892 >From b4580245ea380c9953e24d8816003ecfcd281155 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 20 Mar 2024 19:31:35 -0700 Subject: [PATCH 1/2] [lldb] Make LineEntry::file private for SupportFile transition --- lldb/include/lldb/Core/Disassembler.h | 2 +- lldb/include/lldb/Symbol/LineEntry.h | 5 + lldb/include/lldb/Utility/SupportFile.h| 3 +++ lldb/source/API/SBLineEntry.cpp| 10 +- lldb/source/API/SBThread.cpp | 2 +- lldb/source/Breakpoint/BreakpointResolver.cpp | 2 +- .../Breakpoint/BreakpointResolverFileLine.cpp | 7 --- lldb/source/Commands/CommandObjectBreakpoint.cpp | 4 ++-- lldb/source/Commands/CommandObjectSource.cpp | 14 +++--- lldb/source/Commands/CommandObjectThread.cpp | 2 +- lldb/source/Core/Address.cpp | 2 +- lldb/source/Core/Disassembler.cpp | 8 lldb/source/Core/FormatEntity.cpp | 2 +- lldb/source/Core/IOHandlerCursesGUI.cpp| 11 ++- lldb/source/Core/SourceManager.cpp | 2 +- .../Clang/ClangExpressionSourceCode.cpp| 2 +- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp | 4 ++-- lldb/source/Symbol/CompileUnit.cpp | 2 +- lldb/source/Symbol/Function.cpp| 4 ++-- lldb/source/Symbol/LineTable.cpp | 4 ++-- lldb/source/Symbol/SymbolContext.cpp | 4 ++-- lldb/source/Target/StackFrame.cpp | 2 +- lldb/source/Target/StackFrameList.cpp | 4 ++-- lldb/source/Target/Thread.cpp | 8 lldb/source/Target/TraceDumper.cpp | 4 ++-- .../SymbolFile/PDB/SymbolFilePDBTests.cpp | 2 +- 26 files changed, 63 insertions(+), 53 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 885ac1bb4a7ef8..e037a49f152c74 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -538,7 +538,7 @@ class Disassembler : public std::enable_shared_from_this, ElideMixedSourceAndDisassemblyLine(const ExecutionContext _ctx, const SymbolContext , LineEntry ) { SourceLine sl; -sl.file = line.file; +sl.file = line.GetFile(); sl.line = line.line; sl.column = line.column; return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl); diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h index 31e1cd0b36f96e..a8c8fa716df2d5 100644 --- a/lldb/include/lldb/Symbol/LineEntry.h +++ b/lldb/include/lldb/Symbol/LineEntry.h @@ -130,12 +130,17 @@ struct LineEntry { /// Shared pointer to the target this LineEntry belongs to. void ApplyFileMappings(lldb::TargetSP target_sp); + const FileSpec () const { return file; } + void SetFile(const FileSpec _spec) { file = file_spec; } + /// The section offset address range for this line entry. AddressRange range; +private: /// The source file, possibly mapped by the target.source-map setting. FileSpec file; +public: /// The original source file, from debug info. lldb::SupportFileSP original_file_sp; diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h index 0ea0ca4e7c97a1..7505d7f345c5dd 100644 --- a/lldb/include/lldb/Utility/SupportFile.h +++ b/lldb/include/lldb/Utility/SupportFile.h @@ -45,6 +45,9 @@ class SupportFile { /// Materialize the file to disk and return the path to that temporary file. virtual const FileSpec () { return m_file_spec; } + /// Change the file name. + void Update(const FileSpec _spec) { m_file_spec = file_spec; } + protected: FileSpec m_file_spec; Checksum m_checksum; diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp index 28d12e65fdaf8a..0941ef20d0ff3b 100644 --- a/lldb/source/API/SBLineEntry.cpp +++ b/lldb/source/API/SBLineEntry.cpp @@ -81,8 +81,8 @@ SBFileSpec SBLineEntry::GetFileSpec() const { LLDB_INSTRUMENT_VA(this); SBFileSpec sb_file_spec; - if (m_opaque_up.get() && m_opaque_up->file) -sb_file_spec.SetFileSpec(m_opaque_up->file); + if (m_opaque_up.get() && m_opaque_up->GetFile()) +sb_file_spec.SetFileSpec(m_opaque_up->GetFile()); return sb_file_spec; } @@ -109,9 +109,9 @@ void SBLineEntry::SetFileSpec(lldb::SBFileSpec filespec) { LLDB_INSTRUMENT_VA(this, filespec); if (filespec.IsValid()) -ref().file = filespec.ref(); +ref().SetFile(filespec.ref()); else -ref().file.Clear(); +ref().SetFile(FileSpec()); } void SBLineEntry::SetLine(uint32_t line) { LLDB_INSTRUMENT_VA(this, line); @@ -168,7 +168,7 @@ bool SBLineEntry::GetDescription(SBStream ) { if (m_opaque_up) {
[Lldb-commits] [lldb] [lldb][progress][NFC] Clarify Doxygen comments for `details` field (PR #86002)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/86002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress][NFC] Clarify Doxygen comments for `details` field (PR #86002)
https://github.com/adrian-prantl approved this pull request. Thanks, that's a great improvement! https://github.com/llvm/llvm-project/pull/86002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [PAC][lldb][Dwarf] Support `__ptrauth`-qualified types in user expressions (PR #84387)
@@ -676,6 +677,62 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext , case DW_TAG_volatile_type: encoding_data_type = Type::eEncodingIsVolatileUID; break; + case DW_TAG_LLVM_ptrauth_type: { +DWARFDIE ptr_die = die.GetReferencedDIE(DW_AT_type); Michael137 wrote: Would be nice to put all of this in a helper function, in order to not bloat the size of `ParseTypeModifier`. But feel free to ignore. https://github.com/llvm/llvm-project/pull/84387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [PAC][lldb][Dwarf] Support `__ptrauth`-qualified types in user expressions (PR #84387)
@@ -216,6 +216,16 @@ class TypeSystem : public PluginInterface, virtual uint32_t GetPointerByteSize() = 0; + // TODO: are we allowed to insert virtual functions in the middle of the class + // interface and break ABI? JDevlieghere wrote: Answer: Yes, there's no ABI stability for lldb_private. https://github.com/llvm/llvm-project/pull/84387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [PAC][lldb][Dwarf] Support `__ptrauth`-qualified types in user expressions (PR #84387)
@@ -664,6 +685,17 @@ CompilerType CompilerType::GetPointerType() const { return CompilerType(); } +CompilerType +CompilerType::AddPtrAuthModifier(unsigned key, bool isAddressDiscriminated, Michael137 wrote: Also, whose this user of this API? Presumably there's a separate PR out there that makes use of it? https://github.com/llvm/llvm-project/pull/84387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [PAC][lldb][Dwarf] Support `__ptrauth`-qualified types in user expressions (PR #84387)
@@ -664,6 +685,17 @@ CompilerType CompilerType::GetPointerType() const { return CompilerType(); } +CompilerType +CompilerType::AddPtrAuthModifier(unsigned key, bool isAddressDiscriminated, Michael137 wrote: Can we add an API test that tests this API? E.g., running `frame var`/`expr` on ptrauth types? https://github.com/llvm/llvm-project/pull/84387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support custom LLVM formatting for variables (PR #81196)
@@ -0,0 +1,11 @@ +import lldb +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil + + +class TestCase(TestBase): +def test(self): +self.build() +lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) +self.runCmd("type summary add -s '${var.ubyte:x-2}${var.sbyte:x-2}!' Bytes") +self.expect("v bytes", substrs=[" = 1001!"]) kastiglione wrote: ill formatted format options cause an assert. https://github.com/llvm/llvm-project/pull/81196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support custom LLVM formatting for variables (PR #81196)
@@ -658,6 +658,33 @@ static char ConvertValueObjectStyleToChar( return '\0'; } +static bool DumpValueWithLLVMFormat(Stream , llvm::StringRef options, +ValueObject ) { + std::string formatted; + std::string llvm_format = ("{0:" + options + "}").str(); kastiglione wrote: > by switching over the supported options? There are many supported options, and they vary from type to type (int has different options than say strings). See [FormatProviders.h](https://llvm.org/doxygen/FormatProviders_8h_source.html) for the builtins. It would be possible to exhaustively iterate all the options llvm includes, but I'm not sure it would be worth it. Switching over them seems fragile to changes made to the source of truth in llvm. I will check/test what llvm does for invalid options. https://github.com/llvm/llvm-project/pull/81196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [PAC][lldb][Dwarf] Support `__ptrauth`-qualified types in user expressions (PR #84387)
kovdan01 wrote: A kind reminder regarding the PR - would be glad to see feedback from everyone interested. https://github.com/llvm/llvm-project/pull/84387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress][NFC] Clarify Doxygen comments for `details` field (PR #86002)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Chelsea Cassanova (chelcassanova) Changes The Doxygen comments for the `details` field of a progress report currently does not specify that this field will act as the initial set of details for a progress report that gets updated with `Progress::Increment()`. This commit clarifies this. --- Full diff: https://github.com/llvm/llvm-project/pull/86002.diff 1 Files Affected: - (modified) lldb/include/lldb/Core/Progress.h (+5-1) ``diff diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index c38f6dd0a140ed..eb3af9816dc6ca 100644 --- a/lldb/include/lldb/Core/Progress.h +++ b/lldb/include/lldb/Core/Progress.h @@ -66,7 +66,11 @@ class Progress { /// @param [in] title The title of this progress activity. /// /// @param [in] details Specific information about what the progress report - /// is currently working on. + /// is currently working on. Although not required, if the progress report is + /// updated with Progress::Increment() then this field will be overwritten + /// with the new set of details passed into that function, and the details + /// passed initially will act as an "item 0" for the total set of + /// items being reported on. /// /// @param [in] total The total units of work to be done if specified, if /// set to std::nullopt then an indeterminate progress indicator should be `` https://github.com/llvm/llvm-project/pull/86002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress][NFC] Clarify Doxygen comments for `details` field (PR #86002)
https://github.com/chelcassanova created https://github.com/llvm/llvm-project/pull/86002 The Doxygen comments for the `details` field of a progress report currently does not specify that this field will act as the initial set of details for a progress report that gets updated with `Progress::Increment()`. This commit clarifies this. >From f651cc7367d30a0718a37cdbd55e9f2718febb68 Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Wed, 20 Mar 2024 13:17:42 -0700 Subject: [PATCH] [lldb][progress][NFC] Clarify Doxygen comments for `details` field The Doxygen comments for the `details` field of a progress report currently does not specify that this field will act as the initial set of details for a progress report that gets updated with `Progress::Increment()`. This commit clarifies this. --- lldb/include/lldb/Core/Progress.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h index c38f6dd0a140ed..eb3af9816dc6ca 100644 --- a/lldb/include/lldb/Core/Progress.h +++ b/lldb/include/lldb/Core/Progress.h @@ -66,7 +66,11 @@ class Progress { /// @param [in] title The title of this progress activity. /// /// @param [in] details Specific information about what the progress report - /// is currently working on. + /// is currently working on. Although not required, if the progress report is + /// updated with Progress::Increment() then this field will be overwritten + /// with the new set of details passed into that function, and the details + /// passed initially will act as an "item 0" for the total set of + /// items being reported on. /// /// @param [in] total The total units of work to be done if specified, if /// set to std::nullopt then an indeterminate progress indicator should be ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC (PR #85892)
slackito wrote: > Could you do me another favor and try swapping out line 292 in LineTable.cpp > for: > ``` > line_entry.SetFile( > > std::make_shared(m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx))); > ``` With that modification the test goes back to passing :) https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC (PR #85892)
JDevlieghere wrote: > I tested the two patches separately. > > With the first one ([Make LineEntry::file > private](https://github.com/llvm/llvm-project/pull/85892/commits/0914ecdf582831ec4da776e26ae5ebd2cf9f984f)) > my problematic test case still passes. > > When I apply the second one ([Swap FileSpec for > SupportFile](https://github.com/llvm/llvm-project/pull/85892/commits/a9a62f28128d9ed667bc311140a994f9e39af7c6)) > on top, the test fails as observed with the original change. @slackito Thanks, that's what I expected, but it still helps narrow things down. Could you do me another favor and try swapping out line 292 in LineTable.cpp for: ``` line_entry.SetFile( std::make_shared(m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx))); ``` I'm wondering if we somehow end up comparing it against the file set in `SymbolContext::GetParentOfInlinedScope`: ``` next_frame_sc.line_entry.SetFile( std::make_shared(curr_inlined_block_inlined_info->GetCallSite().GetFile())); ``` https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reland: Store SupportFile in FileEntry (NFC (PR #85892)
slackito wrote: I tested the two patches separately. With the first one, [Make LineEntry::file private](https://github.com/llvm/llvm-project/pull/85892/commits/0914ecdf582831ec4da776e26ae5ebd2cf9f984f), my problematic test case still passes. When I apply the second one, [Swap FileSpec for SupportFile](https://github.com/llvm/llvm-project/pull/85892/commits/a9a62f28128d9ed667bc311140a994f9e39af7c6), on top, the test fails as observed with the original change. https://github.com/llvm/llvm-project/pull/85892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Invert relationship between Process and AddressableBits (PR #85858)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/85858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 10b0e35 - [lldb] Invert relationship between Process and AddressableBits (#85858)
Author: Alex Langford Date: 2024-03-20T10:46:06-07:00 New Revision: 10b0e355372fab1f4d58536525545eef8523 URL: https://github.com/llvm/llvm-project/commit/10b0e355372fab1f4d58536525545eef8523 DIFF: https://github.com/llvm/llvm-project/commit/10b0e355372fab1f4d58536525545eef8523.diff LOG: [lldb] Invert relationship between Process and AddressableBits (#85858) AddressableBits is in the Utility module of LLDB. It currently directly refers to Process, which is from the Target LLDB module. This is a layering violation which concretely means that it is impossible to link anything that uses Utility without it also using Target as well. This is generally not an issue for LLDB (since everything is built together) but it may make it difficult to write unit tests for AddressableBits later on. Added: Modified: lldb/include/lldb/Target/Process.h lldb/include/lldb/Utility/AddressableBits.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp lldb/source/Target/Process.cpp lldb/source/Utility/AddressableBits.cpp Removed: diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index e260e1b4b797bc..2f3a3c22422efe 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -43,6 +43,7 @@ #include "lldb/Target/ThreadList.h" #include "lldb/Target/ThreadPlanStack.h" #include "lldb/Target/Trace.h" +#include "lldb/Utility/AddressableBits.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Broadcaster.h" #include "lldb/Utility/Event.h" @@ -3219,6 +3220,8 @@ void PruneThreadPlans(); void LoadOperatingSystemPlugin(bool flush); + void SetAddressableBitMasks(AddressableBits bit_masks); + private: Status DestroyImpl(bool force_kill); diff --git a/lldb/include/lldb/Utility/AddressableBits.h b/lldb/include/lldb/Utility/AddressableBits.h index 75752fcf840a44..0d27c3561ec272 100644 --- a/lldb/include/lldb/Utility/AddressableBits.h +++ b/lldb/include/lldb/Utility/AddressableBits.h @@ -32,11 +32,13 @@ class AddressableBits { void SetLowmemAddressableBits(uint32_t lowmem_addressing_bits); + uint32_t GetLowmemAddressableBits() const; + void SetHighmemAddressableBits(uint32_t highmem_addressing_bits); - static lldb::addr_t AddressableBitToMask(uint32_t addressable_bits); + uint32_t GetHighmemAddressableBits() const; - void SetProcessMasks(lldb_private::Process ); + static lldb::addr_t AddressableBitToMask(uint32_t addressable_bits); private: uint32_t m_low_memory_addr_bits; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 5b9a9d71802f86..871683a605686f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -900,7 +900,7 @@ void ProcessGDBRemote::DidLaunchOrAttach(ArchSpec _arch) { } AddressableBits addressable_bits = m_gdb_comm.GetAddressableBits(); - addressable_bits.SetProcessMasks(*this); + SetAddressableBitMasks(addressable_bits); if (process_arch.IsValid()) { const ArchSpec _arch = GetTarget().GetArchitecture(); @@ -2337,7 +2337,7 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor _packet) { } } -addressable_bits.SetProcessMasks(*this); +SetAddressableBitMasks(addressable_bits); ThreadSP thread_sp = SetThreadStopInfo( tid, expedited_register_map, signo, thread_name, reason, description, diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp index 7b9938d4f02020..1da7696c9a352a 100644 --- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp +++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp @@ -574,7 +574,7 @@ Status ProcessMachCore::DoLoadCore() { CleanupMemoryRegionPermissions(); AddressableBits addressable_bits = core_objfile->GetAddressableBits(); - addressable_bits.SetProcessMasks(*this); + SetAddressableBitMasks(addressable_bits); return error; } diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 6d58873b54a3ad..f02ec37cb0f08f 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -63,6 +63,7 @@ #include "lldb/Target/ThreadPlanCallFunction.h" #include "lldb/Target/ThreadPlanStack.h" #include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/AddressableBits.h" #include "lldb/Utility/Event.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" @@ -6453,3 +6454,25 @@ Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, return Status(); // Success! } + +void Process::SetAddressableBitMasks(AddressableBits bit_masks) { + uint32_t
[Lldb-commits] [lldb] 66a2ed5 - [lldb] Remove process restart prompt from TestSourceManager (#85861)
Author: Alex Langford Date: 2024-03-20T10:43:40-07:00 New Revision: 66a2ed50ccb6de64fdf82957ca0d4b55ef76f3cd URL: https://github.com/llvm/llvm-project/commit/66a2ed50ccb6de64fdf82957ca0d4b55ef76f3cd DIFF: https://github.com/llvm/llvm-project/commit/66a2ed50ccb6de64fdf82957ca0d4b55ef76f3cd.diff LOG: [lldb] Remove process restart prompt from TestSourceManager (#85861) In TestSourceManager, test_artificial_source_location will give the process restart prompt if you run the test individually. The reason is that we run the process twice: first using a convenience function to run to a specific breakpoint and then again to check for a specific message emitted when you hit the breakpoint. Instead of running twice and making the test difficult to run individually, we can just check for the specific messages using other commands. Added: Modified: lldb/test/API/source-manager/TestSourceManager.py Removed: diff --git a/lldb/test/API/source-manager/TestSourceManager.py b/lldb/test/API/source-manager/TestSourceManager.py index eab8924d108146..ad7c85aac70eaf 100644 --- a/lldb/test/API/source-manager/TestSourceManager.py +++ b/lldb/test/API/source-manager/TestSourceManager.py @@ -323,13 +323,12 @@ def test_artificial_source_location(self): ) self.expect( -"run", -RUN_SUCCEEDED, +"process status", substrs=[ "stop reason = breakpoint", -"%s:%d" % (src_file, 0), -"Note: this address is compiler-generated code in " "function", -"that has no source code associated " "with it.", +f"{src_file}:0", +"Note: this address is compiler-generated code in function", +"that has no source code associated with it.", ], ) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove process restart prompt from TestSourceManager (PR #85861)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/85861 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DebugInfoD tests + fixing issues exposed by tests (PR #85693)
https://github.com/kevinfrei updated https://github.com/llvm/llvm-project/pull/85693 >From fc0eda99c7cceeaefd33477c9b08e7221a5a6e2a Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Fri, 15 Mar 2024 08:54:04 -0700 Subject: [PATCH 1/5] Tests (w/fixes) for initial DebugInfoD LLDB integration Summary: While adding tests for DebugInfoD integration, there were a couple issues that came up: DWP from /debuginfo for a stripped binary needs to return symbols from /executable Test Plan: Added some API tests Reviewers: gclayton, hyubo Subscribers: Tasks: T179375937 Tags: debuginfod Differential Revision: https://phabricator.intern.facebook.com/D54953389 --- .../Python/lldbsuite/test/make/Makefile.rules | 33 +++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 40 +++-- .../Plugins/SymbolLocator/CMakeLists.txt | 7 +- .../SymbolVendor/ELF/SymbolVendorELF.cpp | 32 +++- lldb/test/API/debuginfod/Normal/Makefile | 25 +++ .../API/debuginfod/Normal/TestDebuginfod.py | 159 + lldb/test/API/debuginfod/Normal/main.c| 7 + lldb/test/API/debuginfod/SplitDWARF/Makefile | 29 +++ .../SplitDWARF/TestDebuginfodDWP.py | 167 ++ lldb/test/API/debuginfod/SplitDWARF/main.c| 7 + 10 files changed, 489 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/debuginfod/Normal/Makefile create mode 100644 lldb/test/API/debuginfod/Normal/TestDebuginfod.py create mode 100644 lldb/test/API/debuginfod/Normal/main.c create mode 100644 lldb/test/API/debuginfod/SplitDWARF/Makefile create mode 100644 lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py create mode 100644 lldb/test/API/debuginfod/SplitDWARF/main.c diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index bfd249ccd43f2e..b33c087357a79b 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # # GNUWin32 uname gives "windows32" or "server version windows32" while # some versions of MSYS uname return "MSYS_NT*", but most environments -# standardize on "Windows_NT", so we'll make it consistent here. +# standardize on "Windows_NT", so we'll make it consistent here. # When running tests from Visual Studio, the environment variable isn't # inherited all the way down to the process spawned for make. #-- @@ -210,6 +210,12 @@ else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug endif + + ifeq "$(MERGE_DWOS)" "YES" + MAKE_DWO := YES + DWP_NAME = $(EXE).dwp + DYLIB_DWP_NAME = $(DYLIB_NAME).dwp + endif endif LIMIT_DEBUG_INFO_FLAGS = @@ -357,6 +363,7 @@ ifneq "$(OS)" "Darwin" OBJCOPY ?= $(call replace_cc_with,objcopy) ARCHIVER ?= $(call replace_cc_with,ar) + DWP ?= $(call replace_cc_with,dwp) override AR = $(ARCHIVER) endif @@ -527,6 +534,10 @@ ifneq "$(CXX)" "" endif endif +ifeq "$(GEN_GNU_BUILD_ID)" "YES" + LDFLAGS += -Wl,--build-id +endif + #-- # DYLIB_ONLY variable can be used to skip the building of a.out. # See the sections below regarding dSYM file as well as the building of @@ -565,11 +576,25 @@ else endif else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" +ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" + cp "$(EXE)" "$(EXE).full" +endif $(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)" endif +ifeq "$(MERGE_DWOS)" "YES" + $(DWP) -o "$(DWP_NAME)" $(DWOS) +endif endif + +#-- +# Support emitting the context of the GNU build-id into a file +# This needs to be used in conjunction with GEN_GNU_BUILD_ID := YES +#-- +$(EXE).uuid : $(EXE) + $(OBJCOPY) --dump-section=.note.gnu.build-id=$@ $< + #-- # Make the dylib #-- @@ -610,9 +635,15 @@ endif else $(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)" ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" + ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" + cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).full" + endif $(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)" endif +ifeq "$(MERGE_DWOS)" "YES" + $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS) +endif endif
[Lldb-commits] [lldb] [lldb] Support custom LLVM formatting for variables (PR #81196)
@@ -658,6 +658,33 @@ static char ConvertValueObjectStyleToChar( return '\0'; } +static bool DumpValueWithLLVMFormat(Stream , llvm::StringRef options, +ValueObject ) { clayborg wrote: Can we named this something other that `target`? `target` usually means a lldb_private::Target in lldb code. Maybe `valobj` https://github.com/llvm/llvm-project/pull/81196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support custom LLVM formatting for variables (PR #81196)
@@ -0,0 +1,11 @@ +import lldb +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil + + +class TestCase(TestBase): +def test(self): +self.build() +lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) +self.runCmd("type summary add -s '${var.ubyte:x-2}${var.sbyte:x-2}!' Bytes") +self.expect("v bytes", substrs=[" = 1001!"]) adrian-prantl wrote: We also need some tests for ill-formatted inputs. https://github.com/llvm/llvm-project/pull/81196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Support custom LLVM formatting for variables (PR #81196)
@@ -658,6 +658,33 @@ static char ConvertValueObjectStyleToChar( return '\0'; } +static bool DumpValueWithLLVMFormat(Stream , llvm::StringRef options, +ValueObject ) { + std::string formatted; + std::string llvm_format = ("{0:" + options + "}").str(); adrian-prantl wrote: Is there any way we can make this string static, by switching over the supported options? Or let me ask another way — what happens if options contained "}{1}" is this well-defined in llvm::formatv because it knows the template arguments and thus will not lead to corruption and crashes? If the answer is yes, then this is okay. https://github.com/llvm/llvm-project/pull/81196 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] DebugInfoD tests + fixing issues exposed by tests (PR #85693)
kevinfrei wrote: @JDevlieghere ping :) https://github.com/llvm/llvm-project/pull/85693 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1232964 - [lldb] Omit --show-globals in `help target var` (#85855)
Author: Felipe de Azevedo Piovezan Date: 2024-03-20T07:03:24-07:00 New Revision: 12329648e2c3f8651228f17d3619b1e1ddab80f0 URL: https://github.com/llvm/llvm-project/commit/12329648e2c3f8651228f17d3619b1e1ddab80f0 DIFF: https://github.com/llvm/llvm-project/commit/12329648e2c3f8651228f17d3619b1e1ddab80f0.diff LOG: [lldb] Omit --show-globals in `help target var` (#85855) This option doesn't exist. It is currently displayed by `help target var` due to a bug introduced by 41ae8e7445 in 2018. Some code for `target var` and `frame var` is shared, and some hard-code constants are used in order to filter out options that belong only to `frame var`. However, the aforementioned commit failed to update these constants properly. This patch addresses the issue by having a _single_ place where the filtering of options needs to be done. Added: Modified: lldb/source/Interpreter/OptionGroupVariable.cpp lldb/test/API/functionalities/target_var/TestTargetVar.py Removed: diff --git a/lldb/source/Interpreter/OptionGroupVariable.cpp b/lldb/source/Interpreter/OptionGroupVariable.cpp index 0e35a641361b82..99644b3f423c81 100644 --- a/lldb/source/Interpreter/OptionGroupVariable.cpp +++ b/lldb/source/Interpreter/OptionGroupVariable.cpp @@ -50,6 +50,11 @@ static constexpr OptionDefinition g_variable_options[] = { "Specify a summary string to use to format the variable output."}, }; +static constexpr auto g_num_frame_options = 4; +static const auto g_variable_options_noframe = +llvm::ArrayRef(g_variable_options) +.drop_front(g_num_frame_options); + static Status ValidateNamedSummary(const char *str, void *) { if (!str || !str[0]) return Status("must specify a valid named summary"); @@ -77,9 +82,9 @@ OptionGroupVariable::SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, ExecutionContext *execution_context) { Status error; - if (!include_frame_options) -option_idx += 3; - const int short_option = g_variable_options[option_idx].short_option; + llvm::ArrayRef variable_options = + include_frame_options ? g_variable_options : g_variable_options_noframe; + const int short_option = variable_options[option_idx].short_option; switch (short_option) { case 'r': use_regex = true; @@ -128,16 +133,9 @@ void OptionGroupVariable::OptionParsingStarting( summary_string.Clear(); } -#define NUM_FRAME_OPTS 3 - llvm::ArrayRef OptionGroupVariable::GetDefinitions() { - auto result = llvm::ArrayRef(g_variable_options); - // Show the "--no-args", "--no-locals" and "--show-globals" options if we are - // showing frame specific options - if (include_frame_options) -return result; - - // Skip the "--no-args", "--no-locals" and "--show-globals" options if we are - // not showing frame specific options (globals only) - return result.drop_front(NUM_FRAME_OPTS); + // Show the "--no-args", "--no-recognized-args", "--no-locals" and + // "--show-globals" options if we are showing frame specific options + return include_frame_options ? g_variable_options + : g_variable_options_noframe; } diff --git a/lldb/test/API/functionalities/target_var/TestTargetVar.py b/lldb/test/API/functionalities/target_var/TestTargetVar.py index a0f3663f036510..54b7b77b6773ce 100644 --- a/lldb/test/API/functionalities/target_var/TestTargetVar.py +++ b/lldb/test/API/functionalities/target_var/TestTargetVar.py @@ -15,6 +15,16 @@ class targetCommandTestCase(TestBase): def testTargetVarExpr(self): self.build() lldbutil.run_to_name_breakpoint(self, "main") +self.expect( +"help target variable", +substrs=[ +"--no-args", +"--no-recognized-args", +"--no-locals", +"--show-globals", +], +matching=False, +) self.expect("target variable i", substrs=["i", "42"]) self.expect( "target variable var", patterns=["\(incomplete \*\) var = 0[xX](0)*dead"] ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Omit --show-globals in `help target var` (PR #85855)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/85855 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 81ec95f - Silence -Wswitch warnings
Author: Benjamin Kramer Date: 2024-03-20T13:06:29+01:00 New Revision: 81ec95ff983afd7f31c2713891bbc37bd630a100 URL: https://github.com/llvm/llvm-project/commit/81ec95ff983afd7f31c2713891bbc37bd630a100 DIFF: https://github.com/llvm/llvm-project/commit/81ec95ff983afd7f31c2713891bbc37bd630a100.diff LOG: Silence -Wswitch warnings TypeSystemClang.cpp:4074:11: error: enumeration value 'CountAttributed' not handled in switch [-Werror,-Wswitch] 4074 | switch (qual_type->getTypeClass()) { | ^ TypeSystemClang.cpp:4755:11: error: enumeration value 'CountAttributed' not handled in switch [-Werror,-Wswitch] 4755 | switch (qual_type->getTypeClass()) { | ^ TypeSystemClang.cpp:5088:11: error: enumeration value 'CountAttributed' not handled in switch [-Werror,-Wswitch] 5088 | switch (qual_type->getTypeClass()) { | ^ Added: Modified: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp Removed: diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 68d9165b90a47b..3ac1cf91932cca 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -4074,6 +4074,7 @@ TypeSystemClang::GetTypeClass(lldb::opaque_compiler_type_t type) { switch (qual_type->getTypeClass()) { case clang::Type::Atomic: case clang::Type::Auto: + case clang::Type::CountAttributed: case clang::Type::Decltype: case clang::Type::Elaborated: case clang::Type::Paren: @@ -4755,6 +4756,7 @@ lldb::Encoding TypeSystemClang::GetEncoding(lldb::opaque_compiler_type_t type, switch (qual_type->getTypeClass()) { case clang::Type::Atomic: case clang::Type::Auto: + case clang::Type::CountAttributed: case clang::Type::Decltype: case clang::Type::Elaborated: case clang::Type::Paren: @@ -5088,6 +5090,7 @@ lldb::Format TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) { switch (qual_type->getTypeClass()) { case clang::Type::Atomic: case clang::Type::Auto: + case clang::Type::CountAttributed: case clang::Type::Decltype: case clang::Type::Elaborated: case clang::Type::Paren: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits