[lldb] Print a warning on checksum mismatch

[JDevlieghere]

Print a warning when the debugger detects a mismatch between the MD5 checksum in the DWARF 5 line table and the file on disk. The warning is printed only once per file.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Print a warning when the debugger detects a mismatch between the MD5 checksum in the DWARF 5 line table and the file on disk. The warning is printed only once per file.

---

Full diff: https://github.com/llvm/llvm-project/pull/107968.diff

4 Files Affected:

• (modified) lldb/include/lldb/Core/SourceManager.h (+7)
• (modified) lldb/source/Core/SourceManager.cpp (+18-6)
• (added) lldb/test/Shell/SymbolFile/Inputs/main.c (+4)
• (added) lldb/test/Shell/SymbolFile/checksum-mismatch.test (+7)

diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h
index 172824dc78a6bc..e38627182a9690 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -74,6 +74,10 @@ class SourceManager {
 
     const Checksum &GetChecksum() const { return m_checksum; }
 
+    llvm::once_flag &GetChecksumWarningOnceFlag() {
+      return m_checksum_warning_once_flag;
+    }
+
   protected:
     /// Set file and update modification time.
     void SetSupportFile(lldb::SupportFileSP support_file_sp);
@@ -87,6 +91,9 @@ class SourceManager {
     /// Keep track of the on-disk checksum.
     Checksum m_checksum;
 
+    /// Once flag for emitting a checksum mismatch warning.
+    llvm::once_flag m_checksum_warning_once_flag;
+
     // Keep the modification time that this file data is valid for
     llvm::sys::TimePoint<> m_mod_time;
 
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index f97d86ad79f6ab..fd5b49946c6a92 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -61,6 +61,12 @@ static void resolve_tilde(FileSpec &file_spec) {
   }
 }
 
+static std::string toString(const Checksum &checksum) {
+  if (!checksum)
+    return "";
+  return std::string(llvm::formatv("{0}", checksum.digest()));
+}
+
 // SourceManager constructor
 SourceManager::SourceManager(const TargetSP &target_sp)
     : m_last_support_file_sp(std::make_shared<SupportFile>()), m_last_line(0),
@@ -302,6 +308,18 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
         break;
       }
     }
+
+    Checksum line_table_checksum =
+        last_file_sp->GetSupportFile()->GetChecksum();
+    Checksum on_disk_checksum = last_file_sp->GetChecksum();
+    if (line_table_checksum && line_table_checksum != on_disk_checksum)
+      Debugger::ReportWarning(
+          llvm::formatv(
+              "{0}: source file checksum mismatch between line table "
+              "({1}) and file on disk ({2})",
+              last_file_sp->GetSupportFile()->GetSpecOnly().GetFilename(),
+              toString(line_table_checksum), toString(on_disk_checksum)),
+          std::nullopt, &last_file_sp->GetChecksumWarningOnceFlag());
   }
   return *delta;
 }
@@ -837,12 +855,6 @@ SourceManager::FileSP SourceManager::SourceFileCache::FindSourceFile(
   return {};
 }
 
-static std::string toString(const Checksum &checksum) {
-  if (!checksum)
-    return "";
-  return std::string(llvm::formatv("{0}", checksum.digest()));
-}
-
 void SourceManager::SourceFileCache::Dump(Stream &stream) const {
   // clang-format off
   stream << "Modification time   MD5 Checksum (on-disk)           MD5 Checksum (line table)        Lines    Path\n";
diff --git a/lldb/test/Shell/SymbolFile/Inputs/main.c b/lldb/test/Shell/SymbolFile/Inputs/main.c
new file mode 100644
index 00000000000000..341417f970d7fe
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/Inputs/main.c
@@ -0,0 +1,4 @@
+int main(int argc, char **argv) {
+  // Break on main.
+  return 1;
+}
diff --git a/lldb/test/Shell/SymbolFile/checksum-mismatch.test b/lldb/test/Shell/SymbolFile/checksum-mismatch.test
new file mode 100644
index 00000000000000..5db97647c9aa02
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/checksum-mismatch.test
@@ -0,0 +1,7 @@
+RUN: mkdir -p %t
+RUN: cp %S/Inputs/main.c %t/main.c
+RUN: %clang_host %t/main.c -std=c99 -gdwarf-5 -o %t/main.out
+RUN: echo "// Modify source file hash" >> %t/main.c
+RUN: %lldb -b %t/main.out -o 'b main' -o 'r' 2>&1 | FileCheck %s
+
+CHECK: warning: main.c: source file checksum mismatch between line table ({{.*}}) and file on disk ({{.*}})

[adrian-prantl]

This getter seems rather pointless, isn't the only user in SourceManager itself?

[adrian-prantl]

Yes, but this isn't SourceManager.

[medismailben]

LGTM

[mstorsjo]

This broke building in mingw configurations, and possibly a few others as well.

The ReportWarning function takes a pointer to a std::once_flag, while this is passing it with a llvm::once_flag. In many configurations, these are the same type, but in a couple ones, they're not.

LLDB seems to be using a mixture of std::once_flag and llvm::once_flag throughout, with nontrivial numbers of both, so there's no one obvious preferred way to go here... Or should ReportWarning be extended to accept either kinds of flags, until the implementation is settled on either of them?

[mstorsjo]

This broke building in mingw configurations, and possibly a few others as well.

The ReportWarning function takes a pointer to a std::once_flag, while this is passing it with a llvm::once_flag. In many configurations, these are the same type, but in a couple ones, they're not.

LLDB seems to be using a mixture of std::once_flag and llvm::once_flag throughout, with nontrivial numbers of both, so there's no one obvious preferred way to go here... Or should ReportWarning be extended to accept either kinds of flags, until the implementation is settled on either of them?

If there are no suggestions on a way to go here, I'll push a fix within a couple hours, that fixes compilation - probably switching the newly added code to std::once_flag as that's what the existing function takes.

[mstorsjo]

I pushed a fix in a81a4b2.