https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/77999
This is a series of 4 NFC commit to store `SupportFile`s rather than `FileSpec`s in `LineEntry`. This is work towards having the `SourceManager`operate on `SupportFile`s so that it can (1) validate the Checksum and (2) materialize the content of inline source information. I highly recommend looking at the individual commits rather than the unified diff. >From 2966f226fb052a4265abef4fa132cdc102cb1f8e Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 12 Jan 2024 15:04:35 -0800 Subject: [PATCH 1/4] [lldb] Hoist SupportFile out of FileSpecList (NFC) This hoists SupportFile out of FileSpecList. SupportFileList is still implemented there as it's very similar to FileSpecList. --- lldb/include/lldb/Utility/FileSpecList.h | 29 +------------ lldb/include/lldb/Utility/SupportFile.h | 52 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 28 deletions(-) create mode 100644 lldb/include/lldb/Utility/SupportFile.h diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h index c1107ad1135dd2..6ddb9d1aa646a2 100644 --- a/lldb/include/lldb/Utility/FileSpecList.h +++ b/lldb/include/lldb/Utility/FileSpecList.h @@ -9,8 +9,8 @@ #ifndef LLDB_CORE_FILESPECLIST_H #define LLDB_CORE_FILESPECLIST_H -#include "lldb/Utility/Checksum.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/SupportFile.h" #include <cstddef> #include <vector> @@ -18,33 +18,6 @@ namespace lldb_private { class Stream; -/// Wraps either a FileSpec that represents a local file or a source -/// file whose contents is known (for example because it can be -/// reconstructed from debug info), but that hasn't been written to a -/// file yet. -class SupportFile { -protected: - FileSpec m_file_spec; - Checksum m_checksum; - -public: - SupportFile(const FileSpec &spec) : m_file_spec(spec), m_checksum() {} - SupportFile(const FileSpec &spec, const Checksum &checksum) - : m_file_spec(spec), m_checksum(checksum) {} - SupportFile(const SupportFile &other) = delete; - SupportFile(SupportFile &&other) = default; - virtual ~SupportFile() = default; - bool operator==(const SupportFile &other) { - return m_file_spec == other.m_file_spec && m_checksum == other.m_checksum; - } - /// Return the file name only. Useful for resolving breakpoints by file name. - const FileSpec &GetSpecOnly() const { return m_file_spec; }; - /// Return the checksum or all zeros if there is none. - const Checksum &GetChecksum() const { return m_checksum; }; - /// Materialize the file to disk and return the path to that temporary file. - virtual const FileSpec &Materialize() { return m_file_spec; } -}; - /// A list of support files for a CompileUnit. class SupportFileList { public: diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h new file mode 100644 index 00000000000000..5156b3e72b32d4 --- /dev/null +++ b/lldb/include/lldb/Utility/SupportFile.h @@ -0,0 +1,52 @@ +//===-- SupportFile.h -------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_UTILITY_SUPPORTFILE_H +#define LLDB_UTILITY_SUPPORTFILE_H + +#include "lldb/Utility/Checksum.h" +#include "lldb/Utility/FileSpec.h" + +namespace lldb_private { + +/// Wraps either a FileSpec that represents a local file or a source +/// file whose contents is known (for example because it can be +/// reconstructed from debug info), but that hasn't been written to a +/// file yet. This also stores an optional checksum of the on-disk content. +class SupportFile { +public: + SupportFile(const FileSpec &spec) : m_file_spec(spec), m_checksum() {} + SupportFile(const FileSpec &spec, const Checksum &checksum) + : m_file_spec(spec), m_checksum(checksum) {} + + SupportFile(const SupportFile &other) = delete; + SupportFile(SupportFile &&other) = default; + + virtual ~SupportFile() = default; + + bool operator==(const SupportFile &other) { + return m_file_spec == other.m_file_spec && m_checksum == other.m_checksum; + } + + /// Return the file name only. Useful for resolving breakpoints by file name. + const FileSpec &GetSpecOnly() const { return m_file_spec; }; + + /// Return the checksum or all zeros if there is none. + const Checksum &GetChecksum() const { return m_checksum; }; + + /// Materialize the file to disk and return the path to that temporary file. + virtual const FileSpec &Materialize() { return m_file_spec; } + +protected: + FileSpec m_file_spec; + Checksum m_checksum; +}; + +} // namespace lldb_private + +#endif // LLDB_UTILITY_SUPPORTFILE_H >From cdaf541b7bb5550d8260ea2a45f8e046bd99d5e7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 12 Jan 2024 14:30:04 -0800 Subject: [PATCH 2/4] [lldb] Store SupportFile as shared_ptr (NFC) --- lldb/include/lldb/Utility/FileSpecList.h | 9 +++++---- lldb/source/Utility/FileSpecList.cpp | 9 ++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h index 6ddb9d1aa646a2..9edff64a4bd081 100644 --- a/lldb/include/lldb/Utility/FileSpecList.h +++ b/lldb/include/lldb/Utility/FileSpecList.h @@ -25,21 +25,22 @@ class SupportFileList { SupportFileList(const SupportFileList &) = delete; SupportFileList(SupportFileList &&other) = default; - typedef std::vector<std::unique_ptr<SupportFile>> collection; + typedef std::vector<std::shared_ptr<SupportFile>> collection; typedef collection::const_iterator const_iterator; const_iterator begin() const { return m_files.begin(); } const_iterator end() const { return m_files.end(); } void Append(const FileSpec &file) { - return Append(std::make_unique<SupportFile>(file)); + return Append(std::make_shared<SupportFile>(file)); } - void Append(std::unique_ptr<SupportFile> &&file) { + void Append(std::shared_ptr<SupportFile> &&file) { m_files.push_back(std::move(file)); } // FIXME: Only used by SymbolFilePDB. Replace with a DenseSet at call site. bool AppendIfUnique(const FileSpec &file); size_t GetSize() const { return m_files.size(); } const FileSpec &GetFileSpecAtIndex(size_t idx) const; + std::shared_ptr<SupportFile> GetSupportFileAtIndex(size_t idx) const; size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const; /// Find a compatible file index. /// @@ -69,7 +70,7 @@ class SupportFileList { template <class... Args> void EmplaceBack(Args &&...args) { m_files.push_back( - std::make_unique<SupportFile>(std::forward<Args>(args)...)); + std::make_shared<SupportFile>(std::forward<Args>(args)...)); } protected: diff --git a/lldb/source/Utility/FileSpecList.cpp b/lldb/source/Utility/FileSpecList.cpp index 8d2cf81efe5b13..7647e04a820451 100644 --- a/lldb/source/Utility/FileSpecList.cpp +++ b/lldb/source/Utility/FileSpecList.cpp @@ -41,7 +41,7 @@ bool FileSpecList::AppendIfUnique(const FileSpec &file_spec) { bool SupportFileList::AppendIfUnique(const FileSpec &file_spec) { collection::iterator end = m_files.end(); if (find_if(m_files.begin(), end, - [&](const std::unique_ptr<SupportFile> &support_file) { + [&](const std::shared_ptr<SupportFile> &support_file) { return support_file->GetSpecOnly() == file_spec; }) == end) { Append(file_spec); @@ -176,6 +176,13 @@ const FileSpec &SupportFileList::GetFileSpecAtIndex(size_t idx) const { return g_empty_file_spec; } +std::shared_ptr<SupportFile> +SupportFileList::GetSupportFileAtIndex(size_t idx) const { + if (idx < m_files.size()) + return m_files[idx]; + return {}; +} + // Return the size in bytes that this object takes in memory. This returns the // size in bytes of this object's member variables and any FileSpec objects its // member variables contain, the result doesn't not include the string values >From f523a4a64c94026a9226dc40fa0d38de506a91e7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 12 Jan 2024 15:19:45 -0800 Subject: [PATCH 3/4] [lldb] Remove unsued LineEntry ctor (NFC) --- lldb/include/lldb/Symbol/LineEntry.h | 6 ------ lldb/source/Symbol/LineEntry.cpp | 13 ------------- 2 files changed, 19 deletions(-) diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h index 698d89974dc634..1c7a9030a97932 100644 --- a/lldb/include/lldb/Symbol/LineEntry.h +++ b/lldb/include/lldb/Symbol/LineEntry.h @@ -23,12 +23,6 @@ struct LineEntry { /// Initialize all member variables to invalid values. LineEntry(); - LineEntry(const lldb::SectionSP §ion_sp, lldb::addr_t section_offset, - lldb::addr_t byte_size, const FileSpec &file, uint32_t _line, - uint16_t _column, bool _is_start_of_statement, - bool _is_start_of_basic_block, bool _is_prologue_end, - bool _is_epilogue_begin, bool _is_terminal_entry); - /// Clear the object's state. /// /// Clears all member variables to invalid values. diff --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp index 1b2801cd036835..e89d1fd1f479bc 100644 --- a/lldb/source/Symbol/LineEntry.cpp +++ b/lldb/source/Symbol/LineEntry.cpp @@ -17,19 +17,6 @@ LineEntry::LineEntry() : range(), file(), is_start_of_statement(0), is_start_of_basic_block(0), is_prologue_end(0), is_epilogue_begin(0), is_terminal_entry(0) {} -LineEntry::LineEntry(const lldb::SectionSP §ion_sp, - lldb::addr_t section_offset, lldb::addr_t byte_size, - const FileSpec &_file, uint32_t _line, uint16_t _column, - bool _is_start_of_statement, bool _is_start_of_basic_block, - bool _is_prologue_end, bool _is_epilogue_begin, - bool _is_terminal_entry) - : range(section_sp, section_offset, byte_size), file(_file), - original_file(_file), line(_line), column(_column), - is_start_of_statement(_is_start_of_statement), - is_start_of_basic_block(_is_start_of_basic_block), - is_prologue_end(_is_prologue_end), is_epilogue_begin(_is_epilogue_begin), - is_terminal_entry(_is_terminal_entry) {} - void LineEntry::Clear() { range.Clear(); file.Clear(); >From d5d1bb523600ef2cb04b9fff33dc40f080f932e5 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Fri, 12 Jan 2024 15:20:12 -0800 Subject: [PATCH 4/4] [lldb] Store SupportFile in LineEntry (NFC) --- lldb/include/lldb/Symbol/LineEntry.h | 4 +++- lldb/include/lldb/Utility/FileSpecList.h | 3 ++- lldb/include/lldb/Utility/SupportFile.h | 1 + lldb/include/lldb/lldb-forward.h | 2 ++ lldb/source/Core/Disassembler.cpp | 5 +++-- lldb/source/Symbol/LineEntry.cpp | 8 ++++---- lldb/source/Symbol/LineTable.cpp | 4 ++-- lldb/source/Symbol/SymbolContext.cpp | 4 ++-- 8 files changed, 19 insertions(+), 12 deletions(-) diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h index 1c7a9030a97932..a6508fe831ca84 100644 --- a/lldb/include/lldb/Symbol/LineEntry.h +++ b/lldb/include/lldb/Symbol/LineEntry.h @@ -11,6 +11,7 @@ #include "lldb/Core/AddressRange.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/SupportFile.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -133,7 +134,8 @@ struct LineEntry { AddressRange range; ///< The section offset address range for this line entry. FileSpec file; ///< The source file, possibly mapped by the target.source-map ///setting - FileSpec original_file; ///< The original source file, from debug info. + lldb::SupportFileSP + original_file; ///< The original source file, from debug info. uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or zero ///< if there is no line number /// information. diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h index 9edff64a4bd081..49edc667ddd5b6 100644 --- a/lldb/include/lldb/Utility/FileSpecList.h +++ b/lldb/include/lldb/Utility/FileSpecList.h @@ -11,6 +11,7 @@ #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/SupportFile.h" +#include "lldb/lldb-forward.h" #include <cstddef> #include <vector> @@ -40,7 +41,7 @@ class SupportFileList { bool AppendIfUnique(const FileSpec &file); size_t GetSize() const { return m_files.size(); } const FileSpec &GetFileSpecAtIndex(size_t idx) const; - std::shared_ptr<SupportFile> GetSupportFileAtIndex(size_t idx) const; + lldb::SupportFileSP GetSupportFileAtIndex(size_t idx) const; size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const; /// Find a compatible file index. /// diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h index 5156b3e72b32d4..0ae0e111cc3f85 100644 --- a/lldb/include/lldb/Utility/SupportFile.h +++ b/lldb/include/lldb/Utility/SupportFile.h @@ -20,6 +20,7 @@ namespace lldb_private { /// file yet. This also stores an optional checksum of the on-disk content. class SupportFile { public: + SupportFile() : m_file_spec(), m_checksum() {} SupportFile(const FileSpec &spec) : m_file_spec(spec), m_checksum() {} SupportFile(const FileSpec &spec, const Checksum &checksum) : m_file_spec(spec), m_checksum(checksum) {} diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index 4e0c62fa26cae4..d89ad21512215f 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -212,6 +212,7 @@ class StringList; class StringTableReader; class StructuredDataImpl; class StructuredDataPlugin; +class SupportFile; class Symbol; class SymbolContext; class SymbolContextList; @@ -462,6 +463,7 @@ typedef std::shared_ptr<lldb_private::TypeSummaryImpl> TypeSummaryImplSP; typedef std::shared_ptr<lldb_private::TypeSummaryOptions> TypeSummaryOptionsSP; typedef std::shared_ptr<lldb_private::ScriptedSyntheticChildren> ScriptedSyntheticChildrenSP; +typedef std::shared_ptr<lldb_private::SupportFile> SupportFileSP; typedef std::shared_ptr<lldb_private::UnixSignals> UnixSignalsSP; typedef std::weak_ptr<lldb_private::UnixSignals> UnixSignalsWP; typedef std::shared_ptr<lldb_private::UnwindAssembly> UnwindAssemblySP; diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 166b5fdf22f0b4..260a4f485bff02 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -202,7 +202,7 @@ Disassembler::GetFunctionDeclLineEntry(const SymbolContext &sc) { sc.function->GetStartLineSourceInfo(func_decl_file, func_decl_line); if (func_decl_file != prologue_end_line.file && - func_decl_file != prologue_end_line.original_file) + func_decl_file != prologue_end_line.original_file->GetSpecOnly()) return {}; SourceLine decl_line; @@ -407,7 +407,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, sc.function->GetStartLineSourceInfo(func_decl_file, func_decl_line); if (func_decl_file == prologue_end_line.file || - func_decl_file == prologue_end_line.original_file) { + func_decl_file == + prologue_end_line.original_file->GetSpecOnly()) { // Add all the lines between the function declaration and // the first non-prologue source line to the list of lines // to print. diff --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp index e89d1fd1f479bc..95ea1e0674295a 100644 --- a/lldb/source/Symbol/LineEntry.cpp +++ b/lldb/source/Symbol/LineEntry.cpp @@ -20,7 +20,7 @@ LineEntry::LineEntry() void LineEntry::Clear() { range.Clear(); file.Clear(); - original_file.Clear(); + original_file = std::make_shared<SupportFile>(); line = LLDB_INVALID_LINE_NUMBER; column = 0; is_start_of_statement = 0; @@ -182,7 +182,7 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange( // different file / line number. AddressRange complete_line_range = range; auto symbol_context_scope = lldb::eSymbolContextLineEntry; - Declaration start_call_site(original_file, line); + Declaration start_call_site(original_file->GetSpecOnly(), line); if (include_inlined_functions) symbol_context_scope |= lldb::eSymbolContextBlock; @@ -240,8 +240,8 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange( void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp) { if (target_sp) { // Apply any file remappings to our file. - if (auto new_file_spec = - target_sp->GetSourcePathMap().FindFile(original_file)) + if (auto new_file_spec = target_sp->GetSourcePathMap().FindFile( + original_file->GetSpecOnly())) file = *new_file_spec; } } diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp index abe4c98d592878..f6918171415ad5 100644 --- a/lldb/source/Symbol/LineTable.cpp +++ b/lldb/source/Symbol/LineTable.cpp @@ -291,7 +291,7 @@ bool LineTable::ConvertEntryAtIndexToLineEntry(uint32_t idx, line_entry.file = m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx); line_entry.original_file = - m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx); + m_comp_unit->GetSupportFiles().GetSupportFileAtIndex(entry.file_idx); line_entry.line = entry.line; line_entry.column = entry.column; line_entry.is_start_of_statement = entry.is_start_of_statement; @@ -357,7 +357,7 @@ void LineTable::Dump(Stream *s, Target *target, Address::DumpStyle style, Address::DumpStyle fallback_style, bool show_line_ranges) { const size_t count = m_entries.size(); LineEntry line_entry; - FileSpec prev_file; + SupportFileSP prev_file; for (size_t idx = 0; idx < count; ++idx) { ConvertEntryAtIndexToLineEntry(idx, line_entry); line_entry.Dump(s, target, prev_file != line_entry.original_file, style, diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp index 9fd40b5ca567f8..0a0cab7d407924 100644 --- a/lldb/source/Symbol/SymbolContext.cpp +++ b/lldb/source/Symbol/SymbolContext.cpp @@ -489,8 +489,8 @@ bool SymbolContext::GetParentOfInlinedScope(const Address &curr_frame_pc, next_frame_sc.line_entry.range.GetBaseAddress() = next_frame_pc; next_frame_sc.line_entry.file = curr_inlined_block_inlined_info->GetCallSite().GetFile(); - next_frame_sc.line_entry.original_file = - curr_inlined_block_inlined_info->GetCallSite().GetFile(); + next_frame_sc.line_entry.original_file = std::make_shared<SupportFile>( + curr_inlined_block_inlined_info->GetCallSite().GetFile()); next_frame_sc.line_entry.line = curr_inlined_block_inlined_info->GetCallSite().GetLine(); next_frame_sc.line_entry.column = _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits