[Lldb-commits] [PATCH] D78421: Fix out of sync source code/executable when debugging

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 created this revision.
n1tram1 added reviewers: k8stone, JDevlieghere, clayborg.
n1tram1 added a project: LLDB.
Herald added a subscriber: lldb-commits.
n1tram1 added a reviewer: jasonmolenda.

Currently lldb's SourceManager will check a source file for any updates each time it accesses it.
The issue is that if you are debugging an executable and you modify one of its source files, lldb will show you the updated source file BUT the executable hasn't changed.
So you end up stepping through lines that aren't what is being executed.

This patch prevents this being doing an early return in the case when the source is newer than the executable (thus not allowing the rest of the function to update the source file).

For now I feel like this is a very ugly patch but I can't find any other way to de-reference so many pointers safely.

This breaks test "test_modify_source_file_while_debugging (TestSourceManager.SourceManagerTestCase)" but as I explained above I don't think this is normal behavior. I discussed it with @kwk on IRC and he agreed.
Should I rewrite the test for the new expected behavior or was I wrong this whole time ?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78421

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -531,12 +531,47 @@
   // For now we check each time we want to display info for the file.
   auto curr_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
 
+  if (IsNewerThanDebuggedExecutable(curr_mod_time))
+    return;
+
   if (curr_mod_time != llvm::sys::TimePoint<>() &&
       m_mod_time != curr_mod_time) {
-    m_mod_time = curr_mod_time;
-    m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
-    m_offsets.clear();
+    Update(curr_mod_time);
+  }
+}
+
+bool SourceManager::File::IsNewerThanDebuggedExecutable(llvm::sys::TimePoint<> time)
+{
+  // Check if we are running in a debugger.
+  DebuggerSP debugger_sp(m_debugger_wp.lock());
+  if (debugger_sp) {
+    lldb::TargetSP target_sp(debugger_sp->GetSelectedTarget());
+    if (target_sp) {
+      lldb::ModuleSP exec_module_sp(target_sp->GetExecutableModule());
+      if (exec_module_sp) {
+        auto exec_module_mod_time = exec_module_sp->GetModificationTime();
+
+        // If the source file is newer than the executable don't update,
+	// otherwise the source file being displayed will be different from
+	// the executable being ran.
+	// (We only want to update if the executable has been recompiled)
+        if (time > exec_module_mod_time) {
+	  // TODO: maybe issue a:
+	  //       'warning: Source file is more recent than executable.' ?
+          return true;
+	}
+      }
+    }
   }
+
+  return false;
+}
+
+void SourceManager::File::Update(llvm::sys::TimePoint<> modification_time)
+{
+  m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
+  m_offsets.clear();
+  m_mod_time = modification_time;
 }
 
 size_t SourceManager::File::DisplaySourceLines(uint32_t line,
Index: lldb/include/lldb/Core/SourceManager.h
===================================================================
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -86,6 +86,9 @@
 
   private:
     void CommonInitializer(const FileSpec &file_spec, Target *target);
+
+    bool IsNewerThanDebuggedExecutable(llvm::sys::TimePoint<> time);
+    void Update(llvm::sys::TimePoint<> mod_time);
   };
 
   typedef std::shared_ptr<File> FileSP;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78421.258502.patch
Type: text/x-patch
Size: 2366 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200418/5e5c2619/attachment.bin>