JDevlieghere updated this revision to Diff 287710.
JDevlieghere added a comment.
Herald added a subscriber: ormris.
- Add tests
- Extract common code from `CommandObjectReproducerDump` and
`CommandObjectReproducerVerify` into `GetLoaderFromPathOrCurrent`
- Address code review feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86497/new/
https://reviews.llvm.org/D86497
Files:
lldb/include/lldb/Utility/ReproducerProvider.h
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Commands/Options.td
lldb/source/Utility/ReproducerProvider.cpp
lldb/test/Shell/Reproducer/TestDebugSymbols.test
lldb/test/Shell/Reproducer/TestVerify.test
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1159,6 +1159,17 @@
return ExternalContentsPrefixDir;
}
+void RedirectingFileSystem::setFallthrough(bool Fallthrough) {
+ IsFallthrough = Fallthrough;
+}
+
+std::vector<StringRef> RedirectingFileSystem::getRoots() const {
+ std::vector<StringRef> R;
+ for (const auto &Root : Roots)
+ R.push_back(Root->getName());
+ return R;
+}
+
void RedirectingFileSystem::dump(raw_ostream &OS) const {
for (const auto &Root : Roots)
dumpEntry(OS, Root.get());
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -749,6 +749,10 @@
StringRef getExternalContentsPrefixDir() const;
+ void setFallthrough(bool Fallthrough);
+
+ std::vector<llvm::StringRef> getRoots() const;
+
void dump(raw_ostream &OS) const;
void dumpEntry(raw_ostream &OS, Entry *E, int NumSpaces = 0) const;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
Index: lldb/test/Shell/Reproducer/TestVerify.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestVerify.test
@@ -0,0 +1,15 @@
+# RUN: rm -rf %t.repro
+# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
+# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture --capture-path %t.repro %t.out
+# RUN: %lldb --replay %t.repro
+
+# RUN: echo "/bogus/home/dir" > %t.repro/home.txt
+# RUN: echo "/bogus/current/working/dir" > %t.repro/cwd.txt
+# RUN: rm %t.repro/root/%S/Inputs/GDBRemoteCapture.in
+
+# RUN: not %lldb -b -o 'reproducer verify -f %t.repro' 2>&1 | FileCheck %s
+# CHECK: warning: working directory '/bogus/current/working/dir' not in VFS
+# CHECK: warning: home directory '/bogus/current/working/dir' not in VFS
+
+# RUN: echo "CHECK: %S/Inputs/GDBRemoteCapture.in" > %t.check
+# RUN: not %lldb -b -o 'reproducer verify -f %t.repro' 2>&1 | FileCheck %t.check
Index: lldb/test/Shell/Reproducer/TestDebugSymbols.test
===================================================================
--- lldb/test/Shell/Reproducer/TestDebugSymbols.test
+++ lldb/test/Shell/Reproducer/TestDebugSymbols.test
@@ -12,3 +12,7 @@
# DUMP: uuid: AD52358C-94F8-3796-ADD6-B20FFAC00E5C
# DUMP-NEXT: module path: /path/to/unstripped/executable
# DUMP-NEXT: symbol path: /path/to/foo.dSYM/Contents/Resources/DWARF/foo
+
+# RUN: not %lldb -b -o 'reproducer verify -f %t.repro' 2>&1 | FileCheck %s --check-prefix VERIFY
+# VERIFY: warning: '/path/to/unstripped/executable': module path for AD52358C-94F8-3796-ADD6-B20FFAC00E5C not in VFS
+# VERIFY: warning: '/path/to/foo.dSYM/Contents/Resources/DWARF/foo': symbol path for AD52358C-94F8-3796-ADD6-B20FFAC00E5C not in VFS
Index: lldb/source/Utility/ReproducerProvider.cpp
===================================================================
--- lldb/source/Utility/ReproducerProvider.cpp
+++ lldb/source/Utility/ReproducerProvider.cpp
@@ -9,6 +9,7 @@
#include "lldb/Utility/ReproducerProvider.h"
#include "lldb/Utility/ProcessInfo.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
using namespace lldb_private;
@@ -160,6 +161,87 @@
FileSpec(it->symbol_path));
}
+void Verifier::Verify(
+ std::function<void(llvm::StringRef)> error_callback,
+ std::function<void(llvm::StringRef)> warning_callback) const {
+ if (!m_loader) {
+ error_callback("invalid loader");
+ return;
+ }
+
+ FileSpec vfs_mapping = m_loader->GetFile<FileProvider::Info>();
+ ErrorOr<std::unique_ptr<MemoryBuffer>> buffer =
+ vfs::getRealFileSystem()->getBufferForFile(vfs_mapping.GetPath());
+ if (!buffer) {
+ error_callback("unable to read files: " + buffer.getError().message());
+ return;
+ }
+
+ IntrusiveRefCntPtr<vfs::FileSystem> vfs = vfs::getVFSFromYAML(
+ std::move(buffer.get()), nullptr, vfs_mapping.GetPath());
+ if (!vfs) {
+ error_callback("unable to initialize the virtual file system");
+ return;
+ }
+
+ auto &redirecting_vfs = static_cast<vfs::RedirectingFileSystem &>(*vfs);
+ redirecting_vfs.setFallthrough(false);
+
+ llvm::Expected<std::string> working_dir =
+ GetDirectoryFrom<WorkingDirectoryProvider>(m_loader);
+ if (working_dir) {
+ if (!vfs->exists(*working_dir))
+ warning_callback("working directory '" + *working_dir + "' not in VFS");
+ vfs->setCurrentWorkingDirectory(*working_dir);
+ } else {
+ warning_callback("no working directory in reproducer: " +
+ toString(working_dir.takeError()));
+ }
+
+ llvm::Expected<std::string> home_dir =
+ GetDirectoryFrom<HomeDirectoryProvider>(m_loader);
+ if (home_dir) {
+ if (!vfs->exists(*working_dir))
+ warning_callback("home directory '" + *working_dir + "' not in VFS");
+ } else {
+ warning_callback("no home directory in reproducer: " +
+ toString(working_dir.takeError()));
+ }
+
+ Expected<std::string> symbol_files =
+ m_loader->LoadBuffer<SymbolFileProvider>();
+ if (symbol_files) {
+ std::vector<SymbolFileProvider::Entry> entries;
+ llvm::yaml::Input yin(*symbol_files);
+ yin >> entries;
+ for (const auto &entry : entries) {
+ if (!entry.module_path.empty() && !vfs->exists(entry.module_path)) {
+ warning_callback("'" + entry.module_path + "': module path for " +
+ entry.uuid + " not in VFS");
+ }
+ if (!entry.symbol_path.empty() && !vfs->exists(entry.symbol_path)) {
+ warning_callback("'" + entry.symbol_path + "': symbol path for " +
+ entry.uuid + " not in VFS");
+ }
+ }
+ } else {
+ llvm::consumeError(symbol_files.takeError());
+ }
+
+ std::vector<llvm::StringRef> roots = redirecting_vfs.getRoots();
+ for (llvm::StringRef root : roots) {
+ std::error_code ec;
+ vfs::recursive_directory_iterator iter(*vfs, root, ec);
+ vfs::recursive_directory_iterator end;
+ for (; iter != end && !ec; iter.increment(ec)) {
+ ErrorOr<vfs::Status> status = vfs->status(iter->path());
+ if (!status)
+ warning_callback("'" + iter->path().str() +
+ "': " + status.getError().message());
+ }
+ }
+}
+
void ProviderBase::anchor() {}
char CommandProvider::ID = 0;
char FileProvider::ID = 0;
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -202,8 +202,8 @@
Desc<"Move breakpoints to nearest code. If not set the "
"target.move-to-nearest-codesetting is used.">;
def breakpoint_set_file_colon_line : Option<"joint-specifier", "y">, Group<12>, Arg<"FileLineColumn">,
- Required, Completion<"SourceFile">,
- Desc<"A specifier in the form filename:line[:column] for setting file & line breakpoints.">;
+ Required, Completion<"SourceFile">,
+ Desc<"A specifier in the form filename:line[:column] for setting file & line breakpoints.">;
/* Don't add this option till it actually does something useful...
def breakpoint_set_exception_typename : Option<"exception-typename", "O">,
Arg<"TypeName">, Desc<"The breakpoint will only stop if an "
@@ -451,6 +451,12 @@
"provided, that reproducer is dumped.">;
}
+let Command = "reproducer verify" in {
+ def reproducer_verify_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
+ Desc<"The reproducer path. If a reproducer is replayed and no path is "
+ "provided, that reproducer is dumped.">;
+}
+
let Command = "reproducer xcrash" in {
def reproducer_signal : Option<"signal", "s">, Group<1>,
EnumArg<"None", "ReproducerSignalType()">,
@@ -751,7 +757,7 @@
def source_list_reverse : Option<"reverse", "r">, Group<4>, Desc<"Reverse the"
" listing to look backwards from the last displayed block of source.">;
def source_list_file_colon_line : Option<"joint-specifier", "y">, Group<5>,
- Arg<"FileLineColumn">, Completion<"SourceFile">,
+ Arg<"FileLineColumn">, Completion<"SourceFile">,
Desc<"A specifier in the form filename:line[:column] from which to display"
" source.">;
}
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===================================================================
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -116,6 +116,9 @@
#define LLDB_OPTIONS_reproducer_xcrash
#include "CommandOptions.inc"
+#define LLDB_OPTIONS_reproducer_verify
+#include "CommandOptions.inc"
+
template <typename T>
llvm::Expected<T> static ReadFromYAML(StringRef filename) {
auto error_or_file = MemoryBuffer::getFile(filename);
@@ -134,6 +137,38 @@
return t;
}
+static void SetError(CommandReturnObject &result, Error err) {
+ result.GetErrorStream().Printf("error: %s\n",
+ toString(std::move(err)).c_str());
+ result.SetStatus(eReturnStatusFailed);
+}
+
+/// Create a loader form the given path if specified. Otherwise use the current
+/// loader used for replay.
+static Loader *
+GetLoaderFromPathOrCurrent(llvm::Optional<Loader> &loader_storage,
+ CommandReturnObject &result,
+ FileSpec reproducer_path) {
+ if (reproducer_path) {
+ loader_storage.emplace(reproducer_path);
+ Loader *loader = &(*loader_storage);
+ if (Error err = loader->LoadIndex()) {
+ // This is a hard error and will set the result tot eReturnStatusFailed.
+ SetError(result, std::move(err));
+ return nullptr;
+ }
+ return loader;
+ }
+
+ if (Loader *loader = Reproducer::Instance().GetLoader())
+ return loader;
+
+ // This is a soft error because this is expected to fail during capture.
+ result.SetError("Not specifying a reproducer is only support during replay.");
+ result.SetStatus(eReturnStatusSuccessFinishNoResult);
+ return nullptr;
+}
+
class CommandObjectReproducerGenerate : public CommandObjectParsed {
public:
CommandObjectReproducerGenerate(CommandInterpreter &interpreter)
@@ -312,12 +347,6 @@
}
};
-static void SetError(CommandReturnObject &result, Error err) {
- result.GetErrorStream().Printf("error: %s\n",
- toString(std::move(err)).c_str());
- result.SetStatus(eReturnStatusFailed);
-}
-
class CommandObjectReproducerDump : public CommandObjectParsed {
public:
CommandObjectReproducerDump(CommandInterpreter &interpreter)
@@ -382,29 +411,11 @@
return false;
}
- // If no reproducer path is specified, use the loader currently used for
- // replay. Otherwise create a new loader just for dumping.
llvm::Optional<Loader> loader_storage;
- Loader *loader = nullptr;
- if (!m_options.file) {
- loader = Reproducer::Instance().GetLoader();
- if (loader == nullptr) {
- result.SetError(
- "Not specifying a reproducer is only support during replay.");
- result.SetStatus(eReturnStatusSuccessFinishNoResult);
- return false;
- }
- } else {
- loader_storage.emplace(m_options.file);
- loader = &(*loader_storage);
- if (Error err = loader->LoadIndex()) {
- SetError(result, std::move(err));
- return false;
- }
- }
-
- // If we get here we should have a valid loader.
- assert(loader);
+ Loader *loader =
+ GetLoaderFromPathOrCurrent(loader_storage, result, m_options.file);
+ if (!loader)
+ return false;
switch (m_options.provider) {
case eReproducerProviderFiles: {
@@ -583,6 +594,97 @@
CommandOptions m_options;
};
+class CommandObjectReproducerVerify : public CommandObjectParsed {
+public:
+ CommandObjectReproducerVerify(CommandInterpreter &interpreter)
+ : CommandObjectParsed(interpreter, "reproducer verify",
+ "Verify the contents of a reproducer. "
+ "If no reproducer is specified during replay, it "
+ "dumps the content of the current reproducer.",
+ nullptr) {}
+
+ ~CommandObjectReproducerVerify() override = default;
+
+ Options *GetOptions() override { return &m_options; }
+
+ class CommandOptions : public Options {
+ public:
+ CommandOptions() : Options(), file() {}
+
+ ~CommandOptions() override = default;
+
+ Status SetOptionValue(uint32_t option_idx, StringRef option_arg,
+ ExecutionContext *execution_context) override {
+ Status error;
+ const int short_option = m_getopt_table[option_idx].val;
+
+ switch (short_option) {
+ case 'f':
+ file.SetFile(option_arg, FileSpec::Style::native);
+ FileSystem::Instance().Resolve(file);
+ break;
+ default:
+ llvm_unreachable("Unimplemented option");
+ }
+
+ return error;
+ }
+
+ void OptionParsingStarting(ExecutionContext *execution_context) override {
+ file.Clear();
+ }
+
+ ArrayRef<OptionDefinition> GetDefinitions() override {
+ return makeArrayRef(g_reproducer_verify_options);
+ }
+
+ FileSpec file;
+ };
+
+protected:
+ bool DoExecute(Args &command, CommandReturnObject &result) override {
+ if (!command.empty()) {
+ result.AppendErrorWithFormat("'%s' takes no arguments",
+ m_cmd_name.c_str());
+ return false;
+ }
+
+ llvm::Optional<Loader> loader_storage;
+ Loader *loader =
+ GetLoaderFromPathOrCurrent(loader_storage, result, m_options.file);
+ if (!loader)
+ return false;
+
+ bool errors = false;
+ auto error_callback = [&](llvm::StringRef error) {
+ errors = true;
+ result.AppendError(error);
+ };
+
+ bool warnings = false;
+ auto warning_callback = [&](llvm::StringRef warning) {
+ warnings = true;
+ result.AppendWarning(warning);
+ };
+
+ Verifier verifier(loader);
+ verifier.Verify(error_callback, warning_callback);
+
+ if (warnings || errors) {
+ result.AppendMessage("reproducer verification failed");
+ result.SetStatus(eReturnStatusFailed);
+ } else {
+ result.AppendMessage("reproducer verification succeeded");
+ result.SetStatus(eReturnStatusSuccessFinishResult);
+ }
+
+ return result.Succeeded();
+ }
+
+private:
+ CommandOptions m_options;
+};
+
CommandObjectReproducer::CommandObjectReproducer(
CommandInterpreter &interpreter)
: CommandObjectMultiword(
@@ -605,6 +707,8 @@
new CommandObjectReproducerStatus(interpreter)));
LoadSubCommand("dump",
CommandObjectSP(new CommandObjectReproducerDump(interpreter)));
+ LoadSubCommand("verify", CommandObjectSP(
+ new CommandObjectReproducerVerify(interpreter)));
LoadSubCommand("xcrash", CommandObjectSP(
new CommandObjectReproducerXCrash(interpreter)));
}
Index: lldb/include/lldb/Utility/ReproducerProvider.h
===================================================================
--- lldb/include/lldb/Utility/ReproducerProvider.h
+++ lldb/include/lldb/Utility/ReproducerProvider.h
@@ -400,6 +400,16 @@
return std::string(llvm::StringRef(*dir).rtrim());
}
+class Verifier {
+public:
+ Verifier(Loader *loader) : m_loader(loader) {}
+ void Verify(std::function<void(llvm::StringRef)> error_callback,
+ std::function<void(llvm::StringRef)> warning_callback) const;
+
+private:
+ Loader *m_loader;
+};
+
} // namespace repro
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
