[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 FileSP;


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 wi

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 updated this revision to Diff 258509.
n1tram1 added a comment.

Apply clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

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 FileSP;


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

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 updated this revision to Diff 258527.
n1tram1 added a comment.

Check the modification time against the current selected module.

(Previous patch was only checking against the current target's executable/main 
module)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

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
@@ -21,7 +21,10 @@
 #include "lldb/Symbol/LineEntry.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/PathMappingList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
 #include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -532,7 +535,13 @@
   // 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))
+  // 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 (IsNewerThanCurrentModule(curr_mod_time))
+// TODO: maybe issue a:
+//   'warning: Source file is more recent than executable.' ?
 return;
 
   if (curr_mod_time != llvm::sys::TimePoint<>() &&
@@ -541,31 +550,43 @@
   }
 }
 
-bool SourceManager::File::IsNewerThanDebuggedExecutable(
+bool SourceManager::File::IsNewerThanCurrentModule(
 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;
-}
-  }
-}
+  if (!debugger_sp)
+return false;
+
+  lldb::TargetSP target_sp(debugger_sp->GetSelectedTarget());
+  if (!target_sp)
+return false;
+
+  lldb::ProcessSP process_sp(target_sp->CalculateProcess());
+  if (!process_sp)
+return false;
+
+  ThreadList &thread_list(process_sp->GetThreadList());
+
+  lldb::ThreadSP thread_sp(thread_list.GetSelectedThread());
+  if (!thread_sp)
+return false;
+
+  lldb::StackFrameSP stackframe_sp(thread_sp->GetSelectedFrame());
+  if (!stackframe_sp)
+return false;
+
+  const Address pc_addr(stackframe_sp->GetFrameCodeAddress());
+
+  lldb::ModuleSP current_module(pc_addr.GetModule());
+  if (!current_module)
+return false;
+
+  auto current_module_mod_time = current_module->GetModificationTime();
+
+  if (time <= current_module_mod_time) {
+return false;
   }
 
-  return false;
+  return true;
 }
 
 void SourceManager::File::Update(llvm::sys::TimePoint<> modification_time) {
Index: lldb/include/lldb/Core/SourceManager.h
===
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -87,7 +87,7 @@
   private:
 void CommonInitializer(const FileSpec &file_spec, Target *target);
 
-bool IsNewerThanDebuggedExecutable(llvm::sys::TimePoint<> time);
+bool IsNewerThanCurrentModule(llvm::sys::TimePoint<> time);
 void Update(llvm::sys::TimePoint<> mod_time);
   };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 marked 3 inline comments as done.
n1tram1 added a comment.

Fixed in latest patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

https://reviews.llvm.org/D78421



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 updated this revision to Diff 258705.
n1tram1 added a comment.

Fix the latest patch.

(I hope I got it right this time, sorry it's my first time doing this)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

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
@@ -21,7 +21,10 @@
 #include "lldb/Symbol/LineEntry.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/PathMappingList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
 #include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -436,7 +439,8 @@
 sc_list.GetContextAtIndex(0, sc);
 if (sc.comp_unit)
   m_file_spec = sc.comp_unit->GetPrimaryFile();
-m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+m_mod_time =
+FileSystem::Instance().GetModificationTime(m_file_spec);
   }
 }
   }
@@ -531,12 +535,64 @@
   // 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 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 (IsNewerThanCurrentModule(curr_mod_time))
+// TODO: maybe issue a:
+//   'warning: Source file is more recent than executable.' ?
+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::IsNewerThanCurrentModule(
+llvm::sys::TimePoint<> time) {
+  DebuggerSP debugger_sp(m_debugger_wp.lock());
+  if (!debugger_sp)
+return false;
+
+  lldb::TargetSP target_sp(debugger_sp->GetSelectedTarget());
+  if (!target_sp)
+return false;
+
+  lldb::ProcessSP process_sp(target_sp->CalculateProcess());
+  if (!process_sp)
+return false;
+
+  ThreadList &thread_list(process_sp->GetThreadList());
+
+  lldb::ThreadSP thread_sp(thread_list.GetSelectedThread());
+  if (!thread_sp)
+return false;
+
+  lldb::StackFrameSP stackframe_sp(thread_sp->GetSelectedFrame());
+  if (!stackframe_sp)
+return false;
+
+  const Address pc_addr(stackframe_sp->GetFrameCodeAddress());
+
+  lldb::ModuleSP current_module(pc_addr.GetModule());
+  if (!current_module)
+return false;
+
+  auto current_module_mod_time = current_module->GetModificationTime();
+
+  if (time <= current_module_mod_time) {
+return false;
   }
+
+  return true;
+}
+
+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 IsNewerThanCurrentModule(llvm::sys::TimePoint<> time);
+void Update(llvm::sys::TimePoint<> mod_time);
   };
 
   typedef std::shared_ptr FileSP;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 added a comment.

In D78421#1993030 , @jingham wrote:

> So you check the currently selected frame's module - which is libbar.dylib 
> and find that it was built before FileFromNotBar.c and would show me the old 
> version.
>
> Showing me the latest version of the file is not great, but totally 
> explicable.  Whereas this error, when it happens, would be confusing and hard 
> to understand.


So instead I should try to find which module the source file belongs to and 
check that module's modification time (instead of getting the currently 
selected frame) ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

https://reviews.llvm.org/D78421



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 updated this revision to Diff 261742.
n1tram1 added a comment.

Have the File first check if it is the current stack frame being displayed and 
check if it needs an update that way.
Else, iteratively look for it's belonging module and check if it needs an 
update.

Also fix the SourceCache::AddSourceFile function by using the input file as a 
key (Before the key was always the default constructor of FileSpec).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

https://reviews.llvm.org/D78421

Files:
  lldb/include/lldb/Core/SourceManager.h
  lldb/source/Core/SourceManager.cpp
  lldb/test/API/source-manager/TestSourceManager.py

Index: lldb/test/API/source-manager/TestSourceManager.py
===
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -230,12 +230,14 @@
 "os.path.getmtime() after writing new content:",
 os.path.getmtime(self.file))
 
-# Display the source code again.  We should see the updated line.
+# Display the source code again.
+# We should not see the updated line or there would be an incoherence
+# with the module being ran.
 self.expect(
 "source list -f main-copy.c -l %d" %
 self.line,
 SOURCE_DISPLAYED_CORRECTLY,
-substrs=['Hello lldb'])
+substrs=['Hello world'])
 
 def test_set_breakpoint_with_absolute_path(self):
 self.build()
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -21,7 +21,10 @@
 #include "lldb/Symbol/LineEntry.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/PathMappingList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
 #include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBuffer.h"
@@ -30,6 +33,8 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "lldb/Utility/Log.h"
+
 #include "llvm/ADT/Twine.h"
 
 #include 
@@ -436,7 +441,8 @@
 sc_list.GetContextAtIndex(0, sc);
 if (sc.comp_unit)
   m_file_spec = sc.comp_unit->GetPrimaryFile();
-m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+m_mod_time =
+FileSystem::Instance().GetModificationTime(m_file_spec);
   }
 }
   }
@@ -531,12 +537,107 @@
   // 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 the source file is newer than the module don't update,
+  // otherwise the source file being displayed will be different from
+  // the module being ran.
+  // (We only want to update if the module has been recompiled)
+  if (IsCurrentModuleAndNewer(curr_mod_time) ||
+  IsNewerThanItsModule(curr_mod_time))
+// TODO: maybe issue a:
+// 'warning: Source file is more recent than executable.' ?
+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::IsCurrentModuleAndNewer(llvm::sys::TimePoint<> time) {
+  DebuggerSP debugger_sp(m_debugger_wp.lock());
+  if (!debugger_sp)
+return false;
+
+  lldb::TargetSP target_sp(debugger_sp->GetSelectedTarget());
+  if (!target_sp)
+return false;
+
+  lldb::ProcessSP process_sp(target_sp->CalculateProcess());
+  if (!process_sp)
+return false;
+
+  ThreadList &thread_list(process_sp->GetThreadList());
+
+  lldb::ThreadSP thread_sp(thread_list.GetSelectedThread());
+  if (!thread_sp)
+return false;
+
+  lldb::StackFrameSP stackframe_sp(thread_sp->GetSelectedFrame());
+  if (!stackframe_sp)
+return false;
+
+  const Address pc_addr(stackframe_sp->GetFrameCodeAddress());
+
+  lldb::ModuleSP current_module(pc_addr.GetModule());
+  if (!current_module)
+return false;
+
+  SymbolContextList sc_list;
+  auto num_matches = current_module->ResolveSymbolContextsForFileSpec(
+  m_file_spec, 0, false, eSymbolContextModule | eSymbolContextCompUnit,
+  sc_list);
+
+  if (!num_matches)
+return false;
+
+  // FIXME: We need to get the timestamp from the filesystem because the Module
+  // doesn't properly update its m_mod_time.
+  auto current_module_mod_time =
+  FileSystem::Instance().GetModificationTime(current_module->GetFileSpec());
+
+  return time > current_module_mod_time;
+}
+
+bool SourceManager::File::IsNewerThanItsModule(llvm::sys::TimeP

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 added a comment.

Passing a simple `Module &` sounds like an easy but I would also have to make 
`SourceManager::GetFile` take a `Module &`.

There is one corner case, let's say file.c is being used by //Module A// and 
//Module B//  (which are both shared libraries). If //file.c// is modified and 
//Module B// recompile and reloaded, well will want to see the updated 
//file.c// when stepping through //Module B// but we will want the old 
//file.c// when stepping through //Module B//.
My issue is that the `SourceCacheManager` use only the `FileSpec` as a key to 
the cache, therefore it might fetch the wrong `File` (gets the //file.c// 
corresponding to //Module A// when we wanted the //file.c// corresponding to 
//Module B//).
For the same reason I can't just modify `ModuleList::FindSourceFile` to have it 
return a `ModuleSP` because it might either //Module A// or //Module B//.

I feel like there is a lot to be done for such a simple bug. I don't know it 
it's worth it...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

https://reviews.llvm.org/D78421



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Martin Schmidt via Phabricator via lldb-commits]

n1tram1 added a comment.

In D78421#2060327 , @labath wrote:

> So, I'm not actually convinced that this is an improvement...


This is exactly what is was worried about, I'm not sure either.

The only thing I thought was a bug is the fact the lldb shows you the modified 
file but the running module hasn't changed (GDB for example shows the previous 
file unless the executable is recompiled&reloaded).

But I'm not sure this is really a bug a worth fix fixing (I'm very worried 
about breaking other weird behavior), at least the current behavior is very 
understandable.

I think I'm going to close this issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78421/new/

https://reviews.llvm.org/D78421



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits