llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> Print a warning when the debugger detects a mismatch between the MD5 checksum in the DWARF 5 line table and the file on disk. --- Full diff: https://github.com/llvm/llvm-project/pull/71459.diff 11 Files Affected: - (modified) lldb/include/lldb/Core/SourceManager.h (+9) - (added) lldb/include/lldb/Utility/Checksum.h (+36) - (modified) lldb/include/lldb/Utility/FileSpec.h (+17-2) - (modified) lldb/source/Core/SourceManager.cpp (+22-6) - (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+8-1) - (modified) lldb/source/Utility/CMakeLists.txt (+1) - (added) lldb/source/Utility/Checksum.cpp (+46) - (modified) lldb/source/Utility/FileSpec.cpp (+6-3) - (modified) lldb/unittests/Utility/CMakeLists.txt (+1) - (added) lldb/unittests/Utility/ChecksumTest.cpp (+57) - (modified) lldb/unittests/Utility/FileSpecTest.cpp (+21-37) ``````````diff diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h index 5239ac6f4055f5d..0296657451c1b8c 100644 --- a/lldb/include/lldb/Core/SourceManager.h +++ b/lldb/include/lldb/Core/SourceManager.h @@ -9,11 +9,13 @@ #ifndef LLDB_CORE_SOURCEMANAGER_H #define LLDB_CORE_SOURCEMANAGER_H +#include "lldb/Utility/Checksum.h" #include "lldb/Utility/FileSpec.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-forward.h" #include "llvm/Support/Chrono.h" +#include "llvm/Support/MD5.h" #include "llvm/Support/RWMutex.h" #include <cstddef> @@ -68,6 +70,10 @@ class SourceManager { llvm::sys::TimePoint<> GetTimestamp() const { return m_mod_time; } + const Checksum &GetChecksum() const { return m_checksum; } + + llvm::once_flag &GetChecksumOnceFlag() { return m_checksum_once_flag; } + protected: /// Set file and update modification time. void SetFileSpec(FileSpec file_spec); @@ -83,6 +89,9 @@ class SourceManager { // Keep the modification time that this file data is valid for llvm::sys::TimePoint<> m_mod_time; + Checksum m_checksum; + llvm::once_flag m_checksum_once_flag; + // If the target uses path remappings, be sure to clear our notion of a // source file if the path modification ID changes uint32_t m_source_map_mod_id = 0; diff --git a/lldb/include/lldb/Utility/Checksum.h b/lldb/include/lldb/Utility/Checksum.h new file mode 100644 index 000000000000000..90a579b247636ac --- /dev/null +++ b/lldb/include/lldb/Utility/Checksum.h @@ -0,0 +1,36 @@ +//===-- Checksum.h ----------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_UTILITY_CHECKSUM_H +#define LLDB_UTILITY_CHECKSUM_H + +#include "llvm/Support/MD5.h" + +namespace lldb_private { +class Checksum { +public: + static llvm::MD5::MD5Result sentinel; + + Checksum(llvm::MD5::MD5Result md5 = sentinel); + Checksum(const Checksum &checksum); + Checksum &operator=(const Checksum &checksum); + + explicit operator bool() const; + bool operator==(const Checksum &checksum) const; + bool operator!=(const Checksum &checksum) const; + + std::string digest() const; + +private: + void SetMD5(llvm::MD5::MD5Result); + + llvm::MD5::MD5Result m_checksum; +}; +} // namespace lldb_private + +#endif // LLDB_UTILITY_CHECKSUM_H diff --git a/lldb/include/lldb/Utility/FileSpec.h b/lldb/include/lldb/Utility/FileSpec.h index f06a8543a056e87..29506b01c56d40b 100644 --- a/lldb/include/lldb/Utility/FileSpec.h +++ b/lldb/include/lldb/Utility/FileSpec.h @@ -13,15 +13,18 @@ #include <optional> #include <string> +#include "lldb/Utility/Checksum.h" #include "lldb/Utility/ConstString.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/MD5.h" #include "llvm/Support/Path.h" #include <cstddef> #include <cstdint> +#include <optional> namespace lldb_private { class Stream; @@ -72,7 +75,8 @@ class FileSpec { /// The style of the path /// /// \see FileSpec::SetFile (const char *path) - explicit FileSpec(llvm::StringRef path, Style style = Style::native); + explicit FileSpec(llvm::StringRef path, Style style = Style::native, + const Checksum &checksum = {}); explicit FileSpec(llvm::StringRef path, const llvm::Triple &triple); @@ -362,7 +366,11 @@ class FileSpec { /// /// \param[in] style /// The style for the given path. - void SetFile(llvm::StringRef path, Style style); + /// + /// \param[in] checksum + /// The checksum for the given path. + void SetFile(llvm::StringRef path, Style style, + const Checksum &checksum = {}); /// Change the file specified with a new path. /// @@ -420,6 +428,9 @@ class FileSpec { /// The lifetime of the StringRefs is tied to the lifetime of the FileSpec. std::vector<llvm::StringRef> GetComponents() const; + /// Return the checksum for this FileSpec or all zeros if there is none. + const Checksum &GetChecksum() const { return m_checksum; }; + protected: // Convenience method for setting the file without changing the style. void SetFile(llvm::StringRef path); @@ -427,6 +438,7 @@ class FileSpec { /// Called anytime m_directory or m_filename is changed to clear any cached /// state in this object. void PathWasModified() { + m_checksum = Checksum(); m_is_resolved = false; m_absolute = Absolute::Calculate; } @@ -443,6 +455,9 @@ class FileSpec { /// The unique'd filename path. ConstString m_filename; + /// The optional MD5 checksum of the file. + Checksum m_checksum; + /// True if this path has been resolved. mutable bool m_is_resolved = false; diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp index 517a4b0268d2a00..d1946d34dfe34fe 100644 --- a/lldb/source/Core/SourceManager.cpp +++ b/lldb/source/Core/SourceManager.cpp @@ -300,6 +300,16 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile( break; } } + + Checksum checksum = last_file_sp->GetFileSpec().GetChecksum(); + if (checksum && checksum != last_file_sp->GetChecksum()) { + llvm::call_once(last_file_sp->GetChecksumOnceFlag(), [&]() { + s->Printf("warning: source file checksum mismatch between the debug " + "info (%s) and the file on disk (%s).\n", + checksum.digest().c_str(), + last_file_sp->GetChecksum().digest().c_str()); + }); + } } return *delta; } @@ -446,13 +456,13 @@ void SourceManager::FindLinesMatchingRegex(FileSpec &file_spec, SourceManager::File::File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp) - : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), + : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), m_checksum(), m_debugger_wp(debugger_sp), m_target_wp(TargetSP()) { CommonInitializer(file_spec, {}); } SourceManager::File::File(const FileSpec &file_spec, TargetSP target_sp) - : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), + : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), m_checksum(), m_debugger_wp(target_sp ? target_sp->GetDebugger().shared_from_this() : DebuggerSP()), m_target_wp(target_sp) { @@ -523,14 +533,17 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec, } // If the file exists, read in the data. - if (m_mod_time != llvm::sys::TimePoint<>()) + if (m_mod_time != llvm::sys::TimePoint<>()) { m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec); + m_checksum = llvm::MD5::hash(m_data_sp->GetData()); + } } void SourceManager::File::SetFileSpec(FileSpec file_spec) { resolve_tilde(file_spec); m_file_spec = std::move(file_spec); m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec); + m_checksum = file_spec.GetChecksum(); } uint32_t SourceManager::File::GetLineOffset(uint32_t line) { @@ -821,13 +834,16 @@ SourceManager::FileSP SourceManager::SourceFileCache::FindSourceFile( } void SourceManager::SourceFileCache::Dump(Stream &stream) const { - stream << "Modification time Lines Path\n"; - stream << "------------------- -------- --------------------------------\n"; + stream + << "Modification time MD5 Checksum Lines Path\n"; + stream << "------------------- -------------------------------- -------- " + "--------------------------------\n"; for (auto &entry : m_file_cache) { if (!entry.second) continue; FileSP file = entry.second; - stream.Format("{0:%Y-%m-%d %H:%M:%S} {1,8:d} {2}\n", file->GetTimestamp(), + stream.Format("{0:%Y-%m-%d %H:%M:%S} {1} {2,8:d} {3}\n", + file->GetTimestamp(), file->GetChecksum().digest(), file->GetNumLines(), entry.first.GetPath()); } } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index aabd83a194932ff..79d44bce3d663b6 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -229,8 +229,15 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP &module, remapped_file = std::move(*file_path); } + Checksum checksum; + if (prologue.ContentTypes.HasMD5) { + const llvm::DWARFDebugLine::FileNameEntry &file_name_entry = + prologue.getFileNameEntry(idx); + checksum = file_name_entry.Checksum; + } + // Unconditionally add an entry, so the indices match up. - support_files.EmplaceBack(remapped_file, style); + support_files.EmplaceBack(remapped_file, style, checksum); } return support_files; diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt index 16afab1113a970c..a3b0a405b4133f6 100644 --- a/lldb/source/Utility/CMakeLists.txt +++ b/lldb/source/Utility/CMakeLists.txt @@ -29,6 +29,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES Args.cpp Baton.cpp Broadcaster.cpp + Checksum.cpp CompletionRequest.cpp Connection.cpp ConstString.cpp diff --git a/lldb/source/Utility/Checksum.cpp b/lldb/source/Utility/Checksum.cpp new file mode 100644 index 000000000000000..70167e497a526c4 --- /dev/null +++ b/lldb/source/Utility/Checksum.cpp @@ -0,0 +1,46 @@ +//===-- Checksum.cpp ------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/Checksum.h" +#include "llvm/ADT/SmallString.h" + +using namespace lldb_private; + +Checksum::Checksum(llvm::MD5::MD5Result md5) { SetMD5(md5); } + +Checksum::Checksum(const Checksum &checksum) { SetMD5(checksum.m_checksum); } + +Checksum &Checksum::operator=(const Checksum &checksum) { + SetMD5(checksum.m_checksum); + return *this; +} + +void Checksum::SetMD5(llvm::MD5::MD5Result md5) { + std::uninitialized_copy_n(md5.begin(), 16, m_checksum.begin()); +} + +Checksum::operator bool() const { + return !std::equal(m_checksum.begin(), m_checksum.end(), sentinel.begin()); +} + +bool Checksum::operator==(const Checksum &checksum) const { + return std::equal(m_checksum.begin(), m_checksum.end(), + checksum.m_checksum.begin()); +} + +bool Checksum::operator!=(const Checksum &checksum) const { + return !std::equal(m_checksum.begin(), m_checksum.end(), + checksum.m_checksum.begin()); +} + +std::string Checksum::digest() const { + return std::string(m_checksum.digest().str()); +} + +llvm::MD5::MD5Result Checksum::sentinel = {0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0}; diff --git a/lldb/source/Utility/FileSpec.cpp b/lldb/source/Utility/FileSpec.cpp index eb34ef97cea0b2f..4bbfbb7c1fab5e6 100644 --- a/lldb/source/Utility/FileSpec.cpp +++ b/lldb/source/Utility/FileSpec.cpp @@ -68,8 +68,9 @@ void Denormalize(llvm::SmallVectorImpl<char> &path, FileSpec::Style style) { FileSpec::FileSpec() : m_style(GetNativeStyle()) {} // Default constructor that can take an optional full path to a file on disk. -FileSpec::FileSpec(llvm::StringRef path, Style style) : m_style(style) { - SetFile(path, style); +FileSpec::FileSpec(llvm::StringRef path, Style style, const Checksum &checksum) + : m_checksum(checksum), m_style(style) { + SetFile(path, style, checksum); } FileSpec::FileSpec(llvm::StringRef path, const llvm::Triple &triple) @@ -171,9 +172,11 @@ void FileSpec::SetFile(llvm::StringRef pathname) { SetFile(pathname, m_style); } // Update the contents of this object with a new path. The path will be split // up into a directory and filename and stored as uniqued string values for // quick comparison and efficient memory usage. -void FileSpec::SetFile(llvm::StringRef pathname, Style style) { +void FileSpec::SetFile(llvm::StringRef pathname, Style style, + const Checksum &checksum) { Clear(); m_style = (style == Style::native) ? GetNativeStyle() : style; + m_checksum = checksum; if (pathname.empty()) return; diff --git a/lldb/unittests/Utility/CMakeLists.txt b/lldb/unittests/Utility/CMakeLists.txt index 5c7003a156813dc..097dae860b15911 100644 --- a/lldb/unittests/Utility/CMakeLists.txt +++ b/lldb/unittests/Utility/CMakeLists.txt @@ -4,6 +4,7 @@ add_lldb_unittest(UtilityTests OptionsWithRawTest.cpp ArchSpecTest.cpp BroadcasterTest.cpp + ChecksumTest.cpp ConstStringTest.cpp CompletionRequestTest.cpp DataBufferTest.cpp diff --git a/lldb/unittests/Utility/ChecksumTest.cpp b/lldb/unittests/Utility/ChecksumTest.cpp new file mode 100644 index 000000000000000..7537d30b5ff5b84 --- /dev/null +++ b/lldb/unittests/Utility/ChecksumTest.cpp @@ -0,0 +1,57 @@ +//===-- ChecksumTest.cpp --------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "gtest/gtest.h" + +#include "lldb/Utility/Checksum.h" + +using namespace lldb_private; + +static llvm::MD5::MD5Result hash1 = {0, 1, 2, 3, 4, 5, 6, 7, + 8, 9, 10, 11, 12, 13, 14, 15}; + +static llvm::MD5::MD5Result hash2 = {0, 1, 2, 3, 4, 5, 6, 7, + 8, 9, 10, 11, 12, 13, 14, 15}; + +static llvm::MD5::MD5Result hash3 = {8, 9, 10, 11, 12, 13, 14, 15, + 0, 1, 2, 3, 4, 5, 6, 7}; + +TEST(ChecksumTest, TestConstructor) { + Checksum checksum1; + EXPECT_FALSE(static_cast<bool>(checksum1)); + EXPECT_EQ(checksum1, Checksum()); + + Checksum checksum2 = Checksum(hash1); + EXPECT_EQ(checksum2, Checksum(hash1)); + + Checksum checksum3(checksum2); + EXPECT_EQ(checksum3, Checksum(hash1)); +} + +TEST(ChecksumTest, TestCopyConstructor) { + Checksum checksum1; + EXPECT_FALSE(static_cast<bool>(checksum1)); + EXPECT_EQ(checksum1, Checksum()); + + Checksum checksum2 = checksum1; + EXPECT_EQ(checksum2, checksum1); + + Checksum checksum3(checksum1); + EXPECT_EQ(checksum3, checksum1); +} + +TEST(ChecksumTest, TestMD5) { + Checksum checksum1(hash1); + EXPECT_TRUE(static_cast<bool>(checksum1)); + + // Make sure two checksums with the same underlying hashes are the same. + EXPECT_EQ(Checksum(hash1), Checksum(hash2)); + + // Make sure two checksums with different underlying hashes are different. + EXPECT_NE(Checksum(hash1), Checksum(hash3)); +} diff --git a/lldb/unittests/Utility/FileSpecTest.cpp b/lldb/unittests/Utility/FileSpecTest.cpp index 2a62f6b1e76120f..ebe7bde7d9c2169 100644 --- a/lldb/unittests/Utility/FileSpecTest.cpp +++ b/lldb/unittests/Utility/FileSpecTest.cpp @@ -28,8 +28,8 @@ TEST(FileSpecTest, FileAndDirectoryComponents) { FileSpec fs_windows("F:\\bar", FileSpec::Style::windows); EXPECT_STREQ("F:\\bar", fs_windows.GetPath().c_str()); - // EXPECT_STREQ("F:\\", fs_windows.GetDirectory().GetPath().c_str()); // It returns - // "F:/" + // EXPECT_STREQ("F:\\", fs_windows.GetDirectory().GetPath().c_str()); // It + // returns "F:/" EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString()); FileSpec fs_posix_root("/", FileSpec::Style::posix); @@ -297,45 +297,18 @@ TEST(FileSpecTest, FormatFileSpec) { TEST(FileSpecTest, IsRelative) { llvm::StringRef not_relative[] = { - "/", - "/a", - "/a/", - "/a/b", - "/a/b/", - "//", - "//a/", - "//a/b", - "//a/b/", - "~", - "~/", - "~/a", - "~/a/", - "~/a/b", - "~/a/b/", - "/foo/.", - "/foo/..", - "/foo/../", - "/foo/../.", + "/", "/a", "/a/", "/a/b", "/a/b/", "//", "//a/", + "//a/b", "//a/b/", "~", "~/", "~/a", "~/a/", "~/a/b", + "~/a/b/", "/foo/.", "/foo/..", "/foo/../", "/foo/../.", }; - for (const auto &path: not_relative) { + for (const auto &path : not_relative) { SCOPED_TRACE(path); EXPECT_FALSE(PosixSpec(path).IsRelative()); } llvm::StringRef is_relative[] = { - ".", - "./", - ".///", - "a", - "./a", - "./a/", - "./a/", - "./a/b", - "./a/b/", - "../foo", - "foo/bar.c", - "./foo/bar.c" - }; - for (const auto &path: is_relative) { + ".", "./", ".///", "a", "./a", "./a/", + "./a/", "./a/b", "./a/b/", "../foo", "foo/bar.c", "./foo/bar.c"}; + for (const auto &path : is_relative) { SCOPED_TRACE(path); EXPECT_TRUE(PosixSpec(path).IsRelative()); } @@ -421,7 +394,6 @@ TEST(FileSpecTest, Match) { EXPECT_TRUE(Match("", "/foo/bar")); EXPECT_TRUE(Match("", "")); - } TEST(FileSpecTest, TestAbsoluteCaching) { @@ -534,3 +506,15 @@ TEST(FileSpecTest, TestGetComponents) { EXPECT_EQ(file_spec.GetComponents(), pair.second); } } + +TEST(FileSpecTest, TestChecksum) { + Checksum checksum({0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}); + FileSpec file_spec("/foo/bar", FileSpec::Style::posix, checksum); + EXPECT_TRUE(static_cast<bool>(file_spec.GetChecksum())); + EXPECT_EQ(file_spec.GetChecksum(), checksum); + + FileSpec copy = file_spec; + + EXPECT_TRUE(static_cast<bool>(copy.GetChecksum())); + EXPECT_EQ(file_spec.GetChecksum(), copy.GetChecksum()); +} `````````` </details> https://github.com/llvm/llvm-project/pull/71459 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
