[Lldb-commits] [lldb] [lldb] Store SupportFile in LineEntry (NFC) (PR #77999)

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 12 15:22:57 PST 2024


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 <jonas at 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 <jonas at 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 <jonas at 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 &section_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 &section_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 <jonas at 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 =



More information about the lldb-commits mailing list