-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb] Print a warning on checksum mismatch #71459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,13 +428,17 @@ 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); | ||
|
||
/// 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; | ||
|
||
Comment on lines
+458
to
+460
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will increase the byte size of every FileSpec object by 16 bytes which is not good. Can we store the checksum somewhere else? Or make a lldb_private::ChecksumFile which has a lldb_private::FileSpec + Checksum?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The DWARF 5 line table should have a file list where can we easily compute this and warn if needed. It would be fine to store an extra bit in lldb_private::FileSpec that specifies if the checksum has been verified or not, but we would like to keep lldb_private::FileSpec to be 24 bytes or less. |
||
/// True if this path has been resolved. | ||
mutable bool m_is_resolved = false; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Wherever we hardcode the string "error:" or "warning:" we should probably use a more high-level API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CC: @PortalPete There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could use what I'm working on in my other PR, but we'd have to change this to pass the |
||
"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()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that necessary ?