Revert "[lldb] Add reproducer verifier"

This reverts commit 297f69afac. It broke
the Fedora 33 x86-64 bot. See the review for more info.
This commit is contained in:
Raphael Isemann 2020-09-01 12:21:44 +02:00
parent a787a4ed16
commit 7c80f2da81
13 changed files with 32 additions and 388 deletions

View File

@ -11,32 +11,8 @@
#include "lldb/API/SBDefines.h"
namespace lldb_private {
namespace repro {
struct ReplayOptions;
}
} // namespace lldb_private
namespace lldb {
class LLDB_API SBReplayOptions {
public:
SBReplayOptions();
SBReplayOptions(const SBReplayOptions &rhs);
~SBReplayOptions();
SBReplayOptions &operator=(const SBReplayOptions &rhs);
void SetVerify(bool verify);
bool GetVerify() const;
void SetCheckVersion(bool check);
bool GetCheckVersion() const;
private:
std::unique_ptr<lldb_private::repro::ReplayOptions> m_opaque_up;
};
/// The SBReproducer class is special because it bootstraps the capture and
/// replay of SB API calls. As a result we cannot rely on any other SB objects
/// in the interface or implementation of this class.
@ -46,7 +22,6 @@ public:
static const char *Capture(const char *path);
static const char *Replay(const char *path);
static const char *Replay(const char *path, bool skip_version_check);
static const char *Replay(const char *path, const SBReplayOptions &options);
static const char *PassiveReplay(const char *path);
static const char *GetPath();
static bool SetAutoGenerate(bool b);

View File

@ -227,22 +227,6 @@ private:
mutable std::mutex m_mutex;
};
class Verifier {
public:
Verifier(Loader *loader) : m_loader(loader) {}
void Verify(llvm::function_ref<void(llvm::StringRef)> error_callback,
llvm::function_ref<void(llvm::StringRef)> warning_callback,
llvm::function_ref<void(llvm::StringRef)> note_callback) const;
private:
Loader *m_loader;
};
struct ReplayOptions {
bool verify = true;
bool check_version = true;
};
} // namespace repro
} // namespace lldb_private

View File

@ -30,33 +30,6 @@ using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::repro;
SBReplayOptions::SBReplayOptions()
: m_opaque_up(std::make_unique<ReplayOptions>()){};
SBReplayOptions::SBReplayOptions(const SBReplayOptions &rhs)
: m_opaque_up(std::make_unique<ReplayOptions>(*rhs.m_opaque_up)) {}
SBReplayOptions::~SBReplayOptions() = default;
SBReplayOptions &SBReplayOptions::operator=(const SBReplayOptions &rhs) {
if (this == &rhs)
return *this;
*m_opaque_up = *rhs.m_opaque_up;
return *this;
}
void SBReplayOptions::SetVerify(bool verify) { m_opaque_up->verify = verify; }
bool SBReplayOptions::GetVerify() const { return m_opaque_up->verify; }
void SBReplayOptions::SetCheckVersion(bool check) {
m_opaque_up->check_version = check;
}
bool SBReplayOptions::GetCheckVersion() const {
return m_opaque_up->check_version;
}
SBRegistry::SBRegistry() {
Registry &R = *this;
@ -190,18 +163,10 @@ const char *SBReproducer::PassiveReplay(const char *path) {
}
const char *SBReproducer::Replay(const char *path) {
SBReplayOptions options;
return SBReproducer::Replay(path, options);
return SBReproducer::Replay(path, false);
}
const char *SBReproducer::Replay(const char *path, bool skip_version_check) {
SBReplayOptions options;
options.SetCheckVersion(!skip_version_check);
return SBReproducer::Replay(path, options);
}
const char *SBReproducer::Replay(const char *path,
const SBReplayOptions &options) {
static std::string error;
if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
error = llvm::toString(std::move(e));
@ -214,7 +179,7 @@ const char *SBReproducer::Replay(const char *path,
return error.c_str();
}
if (options.GetCheckVersion()) {
if (!skip_version_check) {
llvm::Expected<std::string> version = loader->LoadBuffer<VersionProvider>();
if (!version) {
error = llvm::toString(version.takeError());
@ -230,30 +195,6 @@ const char *SBReproducer::Replay(const char *path,
}
}
if (options.GetVerify()) {
bool verification_failed = false;
llvm::raw_string_ostream os(error);
auto error_callback = [&](llvm::StringRef error) {
verification_failed = true;
os << "\nerror: " << error;
};
auto warning_callback = [&](llvm::StringRef warning) {
verification_failed = true;
os << "\nwarning: " << warning;
};
auto note_callback = [&](llvm::StringRef warning) {};
Verifier verifier(loader);
verifier.Verify(error_callback, warning_callback, note_callback);
if (verification_failed) {
os.flush();
return error.c_str();
}
}
FileSpec file = loader->GetFile<SBProvider::Info>();
if (!file) {
error = "unable to get replay data from reproducer.";

View File

@ -116,9 +116,6 @@ static constexpr OptionEnumValues ReproducerSignalType() {
#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);
@ -137,38 +134,6 @@ llvm::Expected<T> static ReadFromYAML(StringRef filename) {
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 from 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 to 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)
@ -347,6 +312,12 @@ protected:
}
};
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)
@ -411,11 +382,29 @@ protected:
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 =
GetLoaderFromPathOrCurrent(loader_storage, result, m_options.file);
if (!loader)
return false;
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);
switch (m_options.provider) {
case eReproducerProviderFiles: {
@ -594,101 +583,6 @@ private:
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 "
"verifies 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);
};
auto note_callback = [&](llvm::StringRef warning) {
result.AppendMessage(warning);
};
Verifier verifier(loader);
verifier.Verify(error_callback, warning_callback, note_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(
@ -711,8 +605,6 @@ CommandObjectReproducer::CommandObjectReproducer(
new CommandObjectReproducerStatus(interpreter)));
LoadSubCommand("dump",
CommandObjectSP(new CommandObjectReproducerDump(interpreter)));
LoadSubCommand("verify", CommandObjectSP(
new CommandObjectReproducerVerify(interpreter)));
LoadSubCommand("xcrash", CommandObjectSP(
new CommandObjectReproducerXCrash(interpreter)));
}

View File

@ -451,12 +451,6 @@ let Command = "reproducer dump" in {
"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()">,

View File

@ -268,94 +268,3 @@ bool Loader::HasFile(StringRef file) {
auto it = std::lower_bound(m_files.begin(), m_files.end(), file.str());
return (it != m_files.end()) && (*it == file);
}
void Verifier::Verify(
llvm::function_ref<void(llvm::StringRef)> error_callback,
llvm::function_ref<void(llvm::StringRef)> warning_callback,
llvm::function_ref<void(llvm::StringRef)> note_callack) 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(*home_dir))
warning_callback("home directory '" + *home_dir + "' not in VFS");
} else {
warning_callback("no home directory in reproducer: " +
toString(home_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());
}
}
// Missing files in the VFS are notes rather than warnings. Because the VFS
// is a snapshot, temporary files could have been removed between when they
// were recorded and when the reproducer was generated.
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)
note_callack("'" + iter->path().str() +
"': " + status.getError().message());
}
}
}

View File

@ -9,7 +9,6 @@
#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;

View File

@ -12,7 +12,3 @@
# 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

View File

@ -1,27 +0,0 @@
# RUN: rm -rf %t.repro
# RUN: rm -rf %t.repro2
# 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: not %lldb -b -o 'reproducer verify -f %t.repro' 2>&1 | FileCheck %s
# CHECK: working directory '/bogus/current/working/dir' not in VFS
# CHECK: home directory '/bogus/home/dir' not in VFS
# RUN: rm %t.repro/root/%S/Inputs/GDBRemoteCapture.in
# RUN: echo "CHECK: '%S/Inputs/GDBRemoteCapture.in': No such file or directory" > %t.check
# RUN: not %lldb -b -o 'reproducer verify -f %t.repro' 2>&1 | FileCheck %t.check
# RUN: not %lldb --replay %t.repro 2>&1 | FileCheck %s
# At this point the reproducer is too broken to ignore the verification issues.
# Capture a new reproducer and only change the home directory, which is
# recoverable as far as this test goes.
# RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture --capture-path %t.repro2 %t.out
# RUN: echo "/bogus/home/dir" > %t.repro2/home.txt
# RUN: %lldb --replay %t.repro2 --reproducer-no-verify 2>&1 | FileCheck %s --check-prefix NO-VERIFY
# NO-VERIFY-NOT: home directory '/bogus/home/dir' not in VFS

View File

@ -800,11 +800,9 @@ EXAMPLES:
llvm::Optional<int> InitializeReproducer(llvm::StringRef argv0,
opt::InputArgList &input_args) {
if (auto *replay_path = input_args.getLastArg(OPT_replay)) {
SBReplayOptions replay_options;
replay_options.SetCheckVersion(!input_args.hasArg(OPT_no_version_check));
replay_options.SetVerify(!input_args.hasArg(OPT_no_verification));
const bool no_version_check = input_args.hasArg(OPT_no_version_check);
if (const char *error =
SBReproducer::Replay(replay_path->getValue(), replay_options)) {
SBReproducer::Replay(replay_path->getValue(), no_version_check)) {
WithColor::error() << "reproducer replay failed: " << error << '\n';
return 1;
}

View File

@ -234,8 +234,6 @@ def replay: Separate<["--", "-"], "replay">,
HelpText<"Tells the debugger to replay a reproducer from <filename>.">;
def no_version_check: F<"reproducer-no-version-check">,
HelpText<"Disable the reproducer version check.">;
def no_verification: F<"reproducer-no-verify">,
HelpText<"Disable the reproducer verification.">;
def no_generate_on_signal: F<"reproducer-no-generate-on-signal">,
HelpText<"Don't generate reproducer when a signal is received.">;
def generate_on_exit: F<"reproducer-generate-on-exit">,

View File

@ -749,10 +749,6 @@ public:
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)

View File

@ -1159,17 +1159,6 @@ StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const {
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());