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;
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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
