[lldb] Store SupportFiles in SourceManager::File (NFC) (#106639)

JDevlieghere · web-flow · commit ab40ae8ff9f8 · 2024-08-30T07:18:55.000-07:00
To support detecting MD5 checksum mismatches, store a SupportFile rather
than a plain FileSpec in SourceManager::File.
diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h
@@ -37,8 +37,8 @@ class SourceManager {
                            const SourceManager::File &rhs);
 
   public:
-    File(const FileSpec &file_spec, lldb::TargetSP target_sp);
-    File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp);
+    File(lldb::SupportFileSP support_file_sp, lldb::TargetSP target_sp);
+    File(lldb::SupportFileSP support_file_sp, lldb::DebuggerSP debugger_sp);
 
     bool ModificationTimeIsStale() const;
     bool PathRemappingIsStale() const;
@@ -56,7 +56,10 @@ class SourceManager {
 
     bool LineIsValid(uint32_t line);
 
-    const FileSpec &GetFileSpec() { return m_file_spec; }
+    lldb::SupportFileSP GetSupportFile() const {
+      assert(m_support_file_sp && "SupportFileSP must always be valid");
+      return m_support_file_sp;
+    }
 
     uint32_t GetSourceMapModificationID() const { return m_source_map_mod_id; }
 
@@ -70,15 +73,13 @@ class SourceManager {
 
   protected:
     /// Set file and update modification time.
-    void SetFileSpec(FileSpec file_spec);
+    void SetSupportFile(lldb::SupportFileSP support_file_sp);
 
     bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
 
-    FileSpec m_file_spec_orig; // The original file spec that was used (can be
-                               // different from m_file_spec)
-    FileSpec m_file_spec; // The actually file spec being used (if the target
-                          // has source mappings, this might be different from
-                          // m_file_spec_orig)
+    /// The support file. If the target has source mappings, this might be
+    /// different from the original support file passed to the constructor.
+    lldb::SupportFileSP m_support_file_sp;
 
     // Keep the modification time that this file data is valid for
     llvm::sys::TimePoint<> m_mod_time;
@@ -93,7 +94,8 @@ class SourceManager {
     lldb::TargetWP m_target_wp;
 
   private:
-    void CommonInitializer(const FileSpec &file_spec, lldb::TargetSP target_sp);
+    void CommonInitializer(lldb::SupportFileSP support_file_sp,
+                           lldb::TargetSP target_sp);
   };
 
   typedef std::shared_ptr<File> FileSP;
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
@@ -1076,8 +1076,8 @@ class CommandObjectSourceList : public CommandObjectParsed {
               target.GetSourceManager().GetLastFile());
           if (last_file_sp) {
             const bool show_inlines = true;
-            m_breakpoint_locations.Reset(last_file_sp->GetFileSpec(), 0,
-                                         show_inlines);
+            m_breakpoint_locations.Reset(
+                last_file_sp->GetSupportFile()->GetSpecOnly(), 0, show_inlines);
             SearchFilterForUnconstrainedSearches target_search_filter(
                 target.shared_from_this());
             target_search_filter.Search(m_breakpoint_locations);
diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6894,8 +6894,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
           if (context_changed)
             m_selected_line = m_pc_line;
 
-          if (m_file_sp &&
-              m_file_sp->GetFileSpec() == m_sc.line_entry.GetFile()) {
+          if (m_file_sp && m_file_sp->GetSupportFile()->GetSpecOnly() ==
+                               m_sc.line_entry.GetFile()) {
             // Same file, nothing to do, we should either have the lines or
             // not (source file missing)
             if (m_selected_line >= static_cast<size_t>(m_first_visible_line)) {
@@ -7001,7 +7001,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
             LineEntry bp_loc_line_entry;
             if (bp_loc_sp->GetAddress().CalculateSymbolContextLineEntry(
                     bp_loc_line_entry)) {
-              if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile()) {
+              if (m_file_sp->GetSupportFile()->GetSpecOnly() ==
+                  bp_loc_line_entry.GetFile()) {
                 bp_lines.insert(bp_loc_line_entry.line);
               }
             }
@@ -7332,7 +7333,7 @@ class SourceFileWindowDelegate : public WindowDelegate {
         if (exe_ctx.HasProcessScope() && exe_ctx.GetProcessRef().IsAlive()) {
           BreakpointSP bp_sp = exe_ctx.GetTargetRef().CreateBreakpoint(
               nullptr, // Don't limit the breakpoint to certain modules
-              m_file_sp->GetFileSpec(), // Source file
+              m_file_sp->GetSupportFile()->GetSpecOnly(), // Source file
               m_selected_line +
                   1, // Source line number (m_selected_line is zero based)
               0,     // Unspecified column.
@@ -7478,7 +7479,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
           LineEntry bp_loc_line_entry;
           if (bp_loc_sp->GetAddress().CalculateSymbolContextLineEntry(
                   bp_loc_line_entry)) {
-            if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile() &&
+            if (m_file_sp->GetSupportFile()->GetSpecOnly() ==
+                    bp_loc_line_entry.GetFile() &&
                 m_selected_line + 1 == bp_loc_line_entry.line) {
               bool removed =
                   exe_ctx.GetTargetRef().RemoveBreakpointByID(bp_sp->GetID());
@@ -7492,7 +7494,7 @@ class SourceFileWindowDelegate : public WindowDelegate {
       // No breakpoint found on the location, add it.
       BreakpointSP bp_sp = exe_ctx.GetTargetRef().CreateBreakpoint(
           nullptr, // Don't limit the breakpoint to certain modules
-          m_file_sp->GetFileSpec(), // Source file
+          m_file_sp->GetSupportFile()->GetSpecOnly(), // Source file
           m_selected_line +
               1, // Source line number (m_selected_line is zero based)
           0,     // No column specified.
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
@@ -87,8 +87,10 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
     LLDB_LOG(log, "Source file caching disabled: creating new source file: {0}",
              file_spec);
     if (target_sp)
-      return std::make_shared<File>(file_spec, target_sp);
-    return std::make_shared<File>(file_spec, debugger_sp);
+      return std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+                                    target_sp);
+    return std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+                                  debugger_sp);
   }
 
   ProcessSP process_sp = target_sp ? target_sp->GetProcessSP() : ProcessSP();
@@ -136,7 +138,8 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
   }
 
   // Check if the file exists on disk.
-  if (file_sp && !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
+  if (file_sp && !FileSystem::Instance().Exists(
+                     file_sp->GetSupportFile()->GetSpecOnly())) {
     LLDB_LOG(log, "File doesn't exist on disk: {0}", file_spec);
     file_sp.reset();
   }
@@ -148,9 +151,11 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
 
     // (Re)create the file.
     if (target_sp)
-      file_sp = std::make_shared<File>(file_spec, target_sp);
+      file_sp = std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+                                       target_sp);
     else
-      file_sp = std::make_shared<File>(file_spec, debugger_sp);
+      file_sp = std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+                                       debugger_sp);
 
     // Add the file to the debugger and process cache. If the file was
     // invalidated, this will overwrite it.
@@ -444,25 +449,25 @@ void SourceManager::FindLinesMatchingRegex(FileSpec &file_spec,
                                          match_lines);
 }
 
-SourceManager::File::File(const FileSpec &file_spec,
+SourceManager::File::File(SupportFileSP support_file_sp,
                           lldb::DebuggerSP debugger_sp)
-    : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
+    : m_support_file_sp(std::make_shared<SupportFile>()), m_mod_time(),
       m_debugger_wp(debugger_sp), m_target_wp(TargetSP()) {
-  CommonInitializer(file_spec, {});
+  CommonInitializer(support_file_sp, {});
 }
 
-SourceManager::File::File(const FileSpec &file_spec, TargetSP target_sp)
-    : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
+SourceManager::File::File(SupportFileSP support_file_sp, TargetSP target_sp)
+    : m_support_file_sp(std::make_shared<SupportFile>()), m_mod_time(),
       m_debugger_wp(target_sp ? target_sp->GetDebugger().shared_from_this()
                               : DebuggerSP()),
       m_target_wp(target_sp) {
-  CommonInitializer(file_spec, target_sp);
+  CommonInitializer(support_file_sp, target_sp);
 }
 
-void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
+void SourceManager::File::CommonInitializer(SupportFileSP support_file_sp,
                                             TargetSP target_sp) {
   // Set the file and update the modification time.
-  SetFileSpec(file_spec);
+  SetSupportFile(support_file_sp);
 
   // Always update the source map modification ID if we have a target.
   if (target_sp)
@@ -472,65 +477,76 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
   if (m_mod_time == llvm::sys::TimePoint<>()) {
     if (target_sp) {
       // If this is just a file name, try finding it in the target.
-      if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
-        bool check_inlines = false;
-        SymbolContextList sc_list;
-        size_t num_matches =
-            target_sp->GetImages().ResolveSymbolContextForFilePath(
-                file_spec.GetFilename().AsCString(), 0, check_inlines,
-                SymbolContextItem(eSymbolContextModule |
-                                  eSymbolContextCompUnit),
-                sc_list);
-        bool got_multiple = false;
-        if (num_matches != 0) {
-          if (num_matches > 1) {
-            CompileUnit *test_cu = nullptr;
-            for (const SymbolContext &sc : sc_list) {
-              if (sc.comp_unit) {
-                if (test_cu) {
-                  if (test_cu != sc.comp_unit)
-                    got_multiple = true;
-                  break;
-                } else
-                  test_cu = sc.comp_unit;
+      {
+        FileSpec file_spec = support_file_sp->GetSpecOnly();
+        if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
+          bool check_inlines = false;
+          SymbolContextList sc_list;
+          size_t num_matches =
+              target_sp->GetImages().ResolveSymbolContextForFilePath(
+                  file_spec.GetFilename().AsCString(), 0, check_inlines,
+                  SymbolContextItem(eSymbolContextModule |
+                                    eSymbolContextCompUnit),
+                  sc_list);
+          bool got_multiple = false;
+          if (num_matches != 0) {
+            if (num_matches > 1) {
+              CompileUnit *test_cu = nullptr;
+              for (const SymbolContext &sc : sc_list) {
+                if (sc.comp_unit) {
+                  if (test_cu) {
+                    if (test_cu != sc.comp_unit)
+                      got_multiple = true;
+                    break;
+                  } else
+                    test_cu = sc.comp_unit;
+                }
               }
             }
-          }
-          if (!got_multiple) {
-            SymbolContext sc;
-            sc_list.GetContextAtIndex(0, sc);
-            if (sc.comp_unit)
-              SetFileSpec(sc.comp_unit->GetPrimaryFile());
+            if (!got_multiple) {
+              SymbolContext sc;
+              sc_list.GetContextAtIndex(0, sc);
+              if (sc.comp_unit)
+                SetSupportFile(std::make_shared<SupportFile>(
+                    sc.comp_unit->GetPrimaryFile()));
+            }
           }
         }
       }
 
       // Try remapping the file if it doesn't exist.
-      if (!FileSystem::Instance().Exists(m_file_spec)) {
-        // Check target specific source remappings (i.e., the
-        // target.source-map setting), then fall back to the module
-        // specific remapping (i.e., the .dSYM remapping dictionary).
-        auto remapped = target_sp->GetSourcePathMap().FindFile(m_file_spec);
-        if (!remapped) {
-          FileSpec new_spec;
-          if (target_sp->GetImages().FindSourceFile(m_file_spec, new_spec))
-            remapped = new_spec;
+      {
+        FileSpec file_spec = support_file_sp->GetSpecOnly();
+        if (!FileSystem::Instance().Exists(file_spec)) {
+          // Check target specific source remappings (i.e., the
+          // target.source-map setting), then fall back to the module
+          // specific remapping (i.e., the .dSYM remapping dictionary).
+          auto remapped = target_sp->GetSourcePathMap().FindFile(file_spec);
+          if (!remapped) {
+            FileSpec new_spec;
+            if (target_sp->GetImages().FindSourceFile(file_spec, new_spec))
+              remapped = new_spec;
+          }
+          if (remapped)
+            SetSupportFile(std::make_shared<SupportFile>(
+                *remapped, support_file_sp->GetChecksum()));
         }
-        if (remapped)
-          SetFileSpec(*remapped);
       }
     }
   }
 
   // If the file exists, read in the data.
   if (m_mod_time != llvm::sys::TimePoint<>())
-    m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
+    m_data_sp = FileSystem::Instance().CreateDataBuffer(
+        m_support_file_sp->GetSpecOnly());
 }
 
-void SourceManager::File::SetFileSpec(FileSpec file_spec) {
+void SourceManager::File::SetSupportFile(lldb::SupportFileSP support_file_sp) {
+  FileSpec file_spec = support_file_sp->GetSpecOnly();
   resolve_tilde(file_spec);
-  m_file_spec = std::move(file_spec);
-  m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+  m_support_file_sp =
+      std::make_shared<SupportFile>(file_spec, support_file_sp->GetChecksum());
+  m_mod_time = FileSystem::Instance().GetModificationTime(file_spec);
 }
 
 uint32_t SourceManager::File::GetLineOffset(uint32_t line) {
@@ -603,7 +619,8 @@ bool SourceManager::File::ModificationTimeIsStale() const {
   // TODO: use host API to sign up for file modifications to anything in our
   // source cache and only update when we determine a file has been updated.
   // For now we check each time we want to display info for the file.
-  auto curr_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+  auto curr_mod_time = FileSystem::Instance().GetModificationTime(
+      m_support_file_sp->GetSpecOnly());
   return curr_mod_time != llvm::sys::TimePoint<>() &&
          m_mod_time != curr_mod_time;
 }
@@ -644,7 +661,8 @@ size_t SourceManager::File::DisplaySourceLines(uint32_t line,
                        debugger_sp->GetStopShowColumnAnsiSuffix());
 
   HighlighterManager mgr;
-  std::string path = GetFileSpec().GetPath(/*denormalize*/ false);
+  std::string path =
+      GetSupportFile()->GetSpecOnly().GetPath(/*denormalize*/ false);
   // FIXME: Find a way to get the definitive language this file was written in
   // and pass it to the highlighter.
   const auto &h = mgr.getHighlighterFor(lldb::eLanguageTypeUnknown, path);
@@ -698,7 +716,8 @@ void SourceManager::File::FindLinesMatchingRegex(
 
 bool lldb_private::operator==(const SourceManager::File &lhs,
                               const SourceManager::File &rhs) {
-  if (lhs.m_file_spec != rhs.m_file_spec)
+  if (!lhs.GetSupportFile()->Equal(*rhs.GetSupportFile(),
+                                   SupportFile::eEqualChecksumIfSet))
     return false;
   return lhs.m_mod_time == rhs.m_mod_time;
 }
@@ -778,9 +797,9 @@ void SourceManager::SourceFileCache::AddSourceFile(const FileSpec &file_spec,
   assert(file_sp && "invalid FileSP");
 
   AddSourceFileImpl(file_spec, file_sp);
-  const FileSpec &resolved_file_spec = file_sp->GetFileSpec();
+  const FileSpec &resolved_file_spec = file_sp->GetSupportFile()->GetSpecOnly();
   if (file_spec != resolved_file_spec)
-    AddSourceFileImpl(file_sp->GetFileSpec(), file_sp);
+    AddSourceFileImpl(file_sp->GetSupportFile()->GetSpecOnly(), file_sp);
 }
 
 void SourceManager::SourceFileCache::RemoveSourceFile(const FileSP &file_sp) {
diff --git a/lldb/unittests/Core/SourceManagerTest.cpp b/lldb/unittests/Core/SourceManagerTest.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Core/SourceManager.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/SupportFile.h"
 #include "gtest/gtest.h"
 
 #include "TestingSupport/MockTildeExpressionResolver.h"
@@ -29,8 +30,8 @@ TEST_F(SourceFileCache, FindSourceFileFound) {
 
   // Insert: foo
   FileSpec foo_file_spec("foo");
-  auto foo_file_sp =
-      std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
+  auto foo_file_sp = std::make_shared<SourceManager::File>(
+      std::make_shared<SupportFile>(foo_file_spec), lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: foo, expect found.
@@ -43,8 +44,8 @@ TEST_F(SourceFileCache, FindSourceFileNotFound) {
 
   // Insert: foo
   FileSpec foo_file_spec("foo");
-  auto foo_file_sp =
-      std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
+  auto foo_file_sp = std::make_shared<SourceManager::File>(
+      std::make_shared<SupportFile>(foo_file_spec), lldb::DebuggerSP());
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
 
   // Query: bar, expect not found.
@@ -63,7 +64,8 @@ TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) {
 
   // Create the file with the resolved file spec.
   auto foo_file_sp = std::make_shared<SourceManager::File>(
-      resolved_foo_file_spec, lldb::DebuggerSP());
+      std::make_shared<SupportFile>(resolved_foo_file_spec),
+      lldb::DebuggerSP());
 
   // Cache the result with the unresolved file spec.
   cache.AddSourceFile(foo_file_spec, foo_file_sp);
