Skip to content

[lldb] Deal with SupportFiles in SourceManager (NFC) #106740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

JDevlieghere
Copy link
Member

To support detecting MD5 checksum mismatches, deal with SupportFiles rather than a plain FileSpecs in the SourceManager.

To support detecting MD5 checksum mismatches, deal with SupportFiles
rather than a plain FileSpecs in the SourceManager.
@llvmbot
Copy link
Member

llvmbot commented Aug 30, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

To support detecting MD5 checksum mismatches, deal with SupportFiles rather than a plain FileSpecs in the SourceManager.


Patch is 21.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/106740.diff

11 Files Affected:

  • (modified) lldb/include/lldb/Core/SourceManager.h (+22-14)
  • (modified) lldb/source/API/SBSourceManager.cpp (+5-5)
  • (modified) lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp (+2-1)
  • (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+10-6)
  • (modified) lldb/source/Commands/CommandObjectSource.cpp (+15-11)
  • (modified) lldb/source/Core/Disassembler.cpp (+2-1)
  • (modified) lldb/source/Core/IOHandlerCursesGUI.cpp (+2-2)
  • (modified) lldb/source/Core/SourceManager.cpp (+40-44)
  • (modified) lldb/source/Expression/REPL.cpp (+7-8)
  • (modified) lldb/source/Target/StackFrame.cpp (+1-1)
  • (modified) lldb/source/Target/StackFrameList.cpp (+1-1)
diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h
index 8feeb4347dd52e..b1c9b3d054ef9f 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -141,14 +141,13 @@ class SourceManager {
 
   ~SourceManager();
 
-  FileSP GetLastFile() { return GetFile(m_last_file_spec); }
+  FileSP GetLastFile() { return GetFile(m_last_support_file_sp); }
 
-  size_t
-  DisplaySourceLinesWithLineNumbers(const FileSpec &file, uint32_t line,
-                                    uint32_t column, uint32_t context_before,
-                                    uint32_t context_after,
-                                    const char *current_line_cstr, Stream *s,
-                                    const SymbolContextList *bp_locs = nullptr);
+  size_t DisplaySourceLinesWithLineNumbers(
+      lldb::SupportFileSP support_file_sp, uint32_t line, uint32_t column,
+      uint32_t context_before, uint32_t context_after,
+      const char *current_line_cstr, Stream *s,
+      const SymbolContextList *bp_locs = nullptr);
 
   // This variant uses the last file we visited.
   size_t DisplaySourceLinesWithLineNumbersUsingLastFile(
@@ -159,22 +158,31 @@ class SourceManager {
   size_t DisplayMoreWithLineNumbers(Stream *s, uint32_t count, bool reverse,
                                     const SymbolContextList *bp_locs = nullptr);
 
-  bool SetDefaultFileAndLine(const FileSpec &file_spec, uint32_t line);
+  bool SetDefaultFileAndLine(lldb::SupportFileSP support_file_sp,
+                             uint32_t line);
+
+  struct SupportFileAndLine {
+    lldb::SupportFileSP support_file_sp;
+    uint32_t line;
+    SupportFileAndLine(lldb::SupportFileSP support_file_sp, uint32_t line)
+        : support_file_sp(support_file_sp), line(line){};
+  };
 
-  bool GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line);
+  std::optional<SupportFileAndLine> GetDefaultFileAndLine();
 
   bool DefaultFileAndLineSet() {
-    return (GetFile(m_last_file_spec).get() != nullptr);
+    return (GetFile(m_last_support_file_sp).get() != nullptr);
   }
 
-  void FindLinesMatchingRegex(FileSpec &file_spec, RegularExpression &regex,
-                              uint32_t start_line, uint32_t end_line,
+  void FindLinesMatchingRegex(lldb::SupportFileSP support_file_sp,
+                              RegularExpression &regex, uint32_t start_line,
+                              uint32_t end_line,
                               std::vector<uint32_t> &match_lines);
 
-  FileSP GetFile(const FileSpec &file_spec);
+  FileSP GetFile(lldb::SupportFileSP support_file_sp);
 
 protected:
-  FileSpec m_last_file_spec;
+  lldb::SupportFileSP m_last_support_file_sp;
   uint32_t m_last_line;
   uint32_t m_last_count;
   bool m_default_set;
diff --git a/lldb/source/API/SBSourceManager.cpp b/lldb/source/API/SBSourceManager.cpp
index e46f990698d826..4b96f1222bc88f 100644
--- a/lldb/source/API/SBSourceManager.cpp
+++ b/lldb/source/API/SBSourceManager.cpp
@@ -46,15 +46,15 @@ class SourceManagerImpl {
     lldb::TargetSP target_sp(m_target_wp.lock());
     if (target_sp) {
       return target_sp->GetSourceManager().DisplaySourceLinesWithLineNumbers(
-          file, line, column, context_before, context_after, current_line_cstr,
-          s);
+          std::make_shared<SupportFile>(file), line, column, context_before,
+          context_after, current_line_cstr, s);
     } else {
       lldb::DebuggerSP debugger_sp(m_debugger_wp.lock());
       if (debugger_sp) {
         return debugger_sp->GetSourceManager()
-            .DisplaySourceLinesWithLineNumbers(file, line, column,
-                                               context_before, context_after,
-                                               current_line_cstr, s);
+            .DisplaySourceLinesWithLineNumbers(
+                std::make_shared<SupportFile>(file), line, column,
+                context_before, context_after, current_line_cstr, s);
       }
     }
     return 0;
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
index 0509924e6300be..05fa7b93096889 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
@@ -102,7 +102,8 @@ Searcher::CallbackReturn BreakpointResolverFileRegex::SearchCallback(
   FileSpec cu_file_spec = cu->GetPrimaryFile();
   std::vector<uint32_t> line_matches;
   context.target_sp->GetSourceManager().FindLinesMatchingRegex(
-      cu_file_spec, m_regex, 1, UINT32_MAX, line_matches);
+      std::make_shared<SupportFile>(cu_file_spec), m_regex, 1, UINT32_MAX,
+      line_matches);
 
   uint32_t num_matches = line_matches.size();
   for (uint32_t i = 0; i < num_matches; i++) {
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index abde27b2b53ad8..1399867feae1b8 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -769,20 +769,26 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
 private:
   bool GetDefaultFile(Target &target, FileSpec &file,
                       CommandReturnObject &result) {
-    uint32_t default_line;
     // First use the Source Manager's default file. Then use the current stack
     // frame's file.
-    if (!target.GetSourceManager().GetDefaultFileAndLine(file, default_line)) {
+    auto file_and_line = target.GetSourceManager().GetDefaultFileAndLine();
+    if (file_and_line) {
+      file = file_and_line->support_file_sp->GetSpecOnly();
+      return true;
+    }
+
       StackFrame *cur_frame = m_exe_ctx.GetFramePtr();
       if (cur_frame == nullptr) {
         result.AppendError(
             "No selected frame to use to find the default file.");
         return false;
-      } else if (!cur_frame->HasDebugInformation()) {
+      }
+      if (!cur_frame->HasDebugInformation()) {
         result.AppendError("Cannot use the selected frame to find the default "
                            "file, it has no debug info.");
         return false;
-      } else {
+      }
+
         const SymbolContext &sc =
             cur_frame->GetSymbolContext(eSymbolContextLineEntry);
         if (sc.line_entry.GetFile()) {
@@ -791,8 +797,6 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
           result.AppendError("Can't find the file for the selected frame to "
                              "use as the default file.");
           return false;
-        }
-      }
     }
     return true;
   }
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index 1a0629c6765d41..1fc122420388d8 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -777,14 +777,16 @@ class CommandObjectSourceList : public CommandObjectParsed {
     if (sc.function) {
       Target &target = GetTarget();
 
-      FileSpec start_file;
+      SupportFileSP start_file = std::make_shared<SupportFile>();
       uint32_t start_line;
       uint32_t end_line;
       FileSpec end_file;
 
       if (sc.block == nullptr) {
         // Not an inlined function
-        sc.function->GetStartLineSourceInfo(start_file, start_line);
+        FileSpec function_file_spec;
+        sc.function->GetStartLineSourceInfo(function_file_spec, start_line);
+        start_file = std::make_shared<SupportFile>(function_file_spec);
         if (start_line == 0) {
           result.AppendErrorWithFormat("Could not find line information for "
                                        "start of function: \"%s\".\n",
@@ -794,7 +796,7 @@ class CommandObjectSourceList : public CommandObjectParsed {
         sc.function->GetEndLineSourceInfo(end_file, end_line);
       } else {
         // We have an inlined function
-        start_file = source_info.line_entry.GetFile();
+        start_file = source_info.line_entry.file_sp;
         start_line = source_info.line_entry.line;
         end_line = start_line + m_options.num_lines;
       }
@@ -825,14 +827,15 @@ class CommandObjectSourceList : public CommandObjectParsed {
 
       if (m_options.show_bp_locs) {
         const bool show_inlines = true;
-        m_breakpoint_locations.Reset(start_file, 0, show_inlines);
+        m_breakpoint_locations.Reset(start_file->GetSpecOnly(), 0,
+                                     show_inlines);
         SearchFilterForUnconstrainedSearches target_search_filter(
             m_exe_ctx.GetTargetSP());
         target_search_filter.Search(m_breakpoint_locations);
       }
 
-      result.AppendMessageWithFormat("File: %s\n",
-                                     start_file.GetPath().c_str());
+      result.AppendMessageWithFormat(
+          "File: %s\n", start_file->GetSpecOnly().GetPath().c_str());
       // We don't care about the column here.
       const uint32_t column = 0;
       return target.GetSourceManager().DisplaySourceLinesWithLineNumbers(
@@ -1050,8 +1053,9 @@ class CommandObjectSourceList : public CommandObjectParsed {
                   ? sc.line_entry.column
                   : 0;
           target.GetSourceManager().DisplaySourceLinesWithLineNumbers(
-              sc.comp_unit->GetPrimaryFile(), sc.line_entry.line, column,
-              lines_to_back_up, m_options.num_lines - lines_to_back_up, "->",
+              std::make_shared<SupportFile>(sc.comp_unit->GetPrimaryFile()),
+              sc.line_entry.line, column, lines_to_back_up,
+              m_options.num_lines - lines_to_back_up, "->",
               &result.GetOutputStream(), GetBreakpointLocations());
           result.SetStatus(eReturnStatusSuccessFinishResult);
         }
@@ -1170,9 +1174,9 @@ class CommandObjectSourceList : public CommandObjectParsed {
             m_options.num_lines = 10;
           const uint32_t column = 0;
           target.GetSourceManager().DisplaySourceLinesWithLineNumbers(
-              sc.comp_unit->GetPrimaryFile(), m_options.start_line, column, 0,
-              m_options.num_lines, "", &result.GetOutputStream(),
-              GetBreakpointLocations());
+              std::make_shared<SupportFile>(sc.comp_unit->GetPrimaryFile()),
+              m_options.start_line, column, 0, m_options.num_lines, "",
+              &result.GetOutputStream(), GetBreakpointLocations());
 
           result.SetStatus(eReturnStatusSuccessFinishResult);
         } else {
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 9286f62058bc8d..d071e3bfe4f77d 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -517,7 +517,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
             line_highlight = "**";
           }
           source_manager.DisplaySourceLinesWithLineNumbers(
-              ln.file, ln.line, ln.column, 0, 0, line_highlight, &strm);
+              std::make_shared<SupportFile>(ln.file), ln.line, ln.column, 0, 0,
+              line_highlight, &strm);
         }
         if (source_lines_to_display.print_source_context_end_eol)
           strm.EOL();
diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 8f44e3d0cd016b..3d69aedb6b13ee 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6910,8 +6910,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
           } else {
             // File changed, set selected line to the line with the PC
             m_selected_line = m_pc_line;
-            m_file_sp = m_debugger.GetSourceManager().GetFile(
-                m_sc.line_entry.GetFile());
+            m_file_sp =
+                m_debugger.GetSourceManager().GetFile(m_sc.line_entry.file_sp);
             if (m_file_sp) {
               const size_t num_lines = m_file_sp->GetNumLines();
               m_line_width = 1;
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index cd0011a25f1c39..c427bb91f4643a 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -63,18 +63,22 @@ static void resolve_tilde(FileSpec &file_spec) {
 
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
-    : m_last_line(0), m_last_count(0), m_default_set(false),
-      m_target_wp(target_sp),
+    : m_last_support_file_sp(std::make_shared<SupportFile>()), m_last_line(0),
+      m_last_count(0), m_default_set(false), m_target_wp(target_sp),
       m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
-    : m_last_line(0), m_last_count(0), m_default_set(false), m_target_wp(),
+    : m_last_support_file_sp(std::make_shared<SupportFile>()), m_last_line(0),
+      m_last_count(0), m_default_set(false), m_target_wp(),
       m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() = default;
 
-SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
+SourceManager::FileSP SourceManager::GetFile(SupportFileSP support_file_sp) {
+  assert(support_file_sp && "SupportFileSP must be valid");
+
+  FileSpec file_spec = support_file_sp->GetSpecOnly();
   if (!file_spec)
     return {};
 
@@ -87,10 +91,8 @@ 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>(std::make_shared<SupportFile>(file_spec),
-                                    target_sp);
-    return std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
-                                  debugger_sp);
+      return std::make_shared<File>(support_file_sp, target_sp);
+    return std::make_shared<File>(support_file_sp, debugger_sp);
   }
 
   ProcessSP process_sp = target_sp ? target_sp->GetProcessSP() : ProcessSP();
@@ -151,11 +153,9 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
 
     // (Re)create the file.
     if (target_sp)
-      file_sp = std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
-                                       target_sp);
+      file_sp = std::make_shared<File>(support_file_sp, target_sp);
     else
-      file_sp = std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
-                                       debugger_sp);
+      file_sp = std::make_shared<File>(support_file_sp, debugger_sp);
 
     // Add the file to the debugger and process cache. If the file was
     // invalidated, this will overwrite it.
@@ -235,11 +235,8 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
       start_line = 1;
   }
 
-  if (!m_default_set) {
-    FileSpec tmp_spec;
-    uint32_t tmp_line;
-    GetDefaultFileAndLine(tmp_spec, tmp_line);
-  }
+  if (!m_default_set)
+    GetDefaultFileAndLine();
 
   m_last_line = start_line;
   m_last_count = count;
@@ -310,11 +307,12 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
 }
 
 size_t SourceManager::DisplaySourceLinesWithLineNumbers(
-    const FileSpec &file_spec, uint32_t line, uint32_t column,
+    lldb::SupportFileSP support_file_sp, uint32_t line, uint32_t column,
     uint32_t context_before, uint32_t context_after,
     const char *current_line_cstr, Stream *s,
     const SymbolContextList *bp_locs) {
-  FileSP file_sp(GetFile(file_spec));
+  assert(support_file_sp && "SupportFile must be valid");
+  FileSP file_sp(GetFile(support_file_sp));
 
   uint32_t start_line;
   uint32_t count = context_before + context_after + 1;
@@ -327,8 +325,9 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbers(
   if (last_file_sp.get() != file_sp.get()) {
     if (line == 0)
       m_last_line = 0;
-    m_last_file_spec = file_spec;
+    m_last_support_file_sp = support_file_sp;
   }
+
   return DisplaySourceLinesWithLineNumbersUsingLastFile(
       start_line, count, line, column, current_line_cstr, s, bp_locs);
 }
@@ -339,11 +338,8 @@ size_t SourceManager::DisplayMoreWithLineNumbers(
   // to figure it out here.
   FileSP last_file_sp(GetLastFile());
   const bool have_default_file_line = last_file_sp && m_last_line > 0;
-  if (!m_default_set) {
-    FileSpec tmp_spec;
-    uint32_t tmp_line;
-    GetDefaultFileAndLine(tmp_spec, tmp_line);
-  }
+  if (!m_default_set)
+    GetDefaultFileAndLine();
 
   if (last_file_sp) {
     if (m_last_line == UINT32_MAX)
@@ -378,26 +374,27 @@ size_t SourceManager::DisplayMoreWithLineNumbers(
   return 0;
 }
 
-bool SourceManager::SetDefaultFileAndLine(const FileSpec &file_spec,
+bool SourceManager::SetDefaultFileAndLine(lldb::SupportFileSP support_file_sp,
                                           uint32_t line) {
+  assert(support_file_sp && "SupportFile must be valid");
+
   m_default_set = true;
-  FileSP file_sp(GetFile(file_spec));
 
-  if (file_sp) {
+  if (FileSP file_sp = GetFile(support_file_sp)) {
     m_last_line = line;
-    m_last_file_spec = file_spec;
+    m_last_support_file_sp = support_file_sp;
     return true;
-  } else {
-    return false;
   }
+
+  return false;
 }
 
-bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
-  if (FileSP last_file_sp = GetLastFile()) {
-    file_spec = m_last_file_spec;
-    line = m_last_line;
-    return true;
-  } else if (!m_default_set) {
+std::optional<SourceManager::SupportFileAndLine>
+SourceManager::GetDefaultFileAndLine() {
+  if (FileSP last_file_sp = GetLastFile())
+    return SupportFileAndLine(m_last_support_file_sp, m_last_line);
+
+  if (!m_default_set) {
     TargetSP target_sp(m_target_wp.lock());
 
     if (target_sp) {
@@ -423,26 +420,25 @@ bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
             if (sc.function->GetAddressRange()
                     .GetBaseAddress()
                     .CalculateSymbolContextLineEntry(line_entry)) {
-              SetDefaultFileAndLine(line_entry.GetFile(), line_entry.line);
-              file_spec = m_last_file_spec;
-              line = m_last_line;
-              return true;
+              SetDefaultFileAndLine(line_entry.file_sp, line_entry.line);
+              return SupportFileAndLine(line_entry.file_sp, m_last_line);
             }
           }
         }
       }
     }
   }
-  return false;
+
+  return std::nullopt;
 }
 
-void SourceManager::FindLinesMatchingRegex(FileSpec &file_spec,
+void SourceManager::FindLinesMatchingRegex(SupportFileSP support_file_sp,
                                            RegularExpression &regex,
                                            uint32_t start_line,
                                            uint32_t end_line,
                                            std::vector<uint32_t> &match_lines) {
   match_lines.clear();
-  FileSP file_sp = GetFile(file_spec);
+  FileSP file_sp = GetFile(support_file_sp);
   if (!file_sp)
     return;
   return file_sp->FindLinesMatchingRegex(regex, start_line, end_line,
diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp
index a6a4ffb5e0af9e..56c50e346b39b8 100644
--- a/lldb/source/Expression/REPL.cpp
+++ b/lldb/source/Expression/REPL.cpp
@@ -473,7 +473,8 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) {
 
             // Now set the default file and line to the REPL source file
             m_target.GetSourceManager().SetDefaultFileAndLine(
-                FileSpec(m_repl_source_path), new_default_line);
+                std::make_shared<SupportFile>(FileSpec(m_repl_source_path)),
+                new_default_line);
           }
           static_cast<IOHandlerEditline &>(io_handler)
               .SetBaseLineNumber(m_code.GetSize() + 1);
@@ -570,13 +571,11 @@ Status REPL::RunLoop() {
 
   lldb::IOHandlerSP io_handler_sp(GetIOHandler());
 
-  FileSpec save_default_file;
-  uint32_t save_default_line = 0;
+  std::optional<SourceManager::SupportFileAndLine> default_file_line;
 
   if (!m_repl_source_path.empty()) {
     // Save the current default file and line
-    m_target.GetSourceManager().GetDefaultFileAndLine(save_default_file,
-                                                      save_default_line);
+    default_file_line = m_target.GetSourceManager().GetDefaultFileAndLine();
   }
 ...
[truncated]

Copy link

github-actions bot commented Aug 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, I looked at the previous PR to get some context.

@JDevlieghere JDevlieghere merged commit 130eddf into llvm:main Aug 30, 2024
5 of 6 checks passed
@JDevlieghere JDevlieghere deleted the source-manager branch August 30, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants