Skip to content

Commit

Permalink
Temporarily revert [lldb] e81268d - [lldb/Reproducers] Support multip…
Browse files Browse the repository at this point in the history 
…le GDB remotes

This was causing a crash in opt+assert builds on linux and a follow-up
message was posted.

This reverts commit e81268d
  • Loading branch information
@echristo
echristo committed Dec 10, 2019
1 parent a2602bd commit c9e0b35
Show file tree
Hide file tree
Showing 13 changed files with 147 additions and 265 deletions.
43 changes: 1 addition & 42 deletions lldb/include/lldb/Utility/GDBRemote.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#ifndef liblldb_GDBRemote_h_
#define liblldb_GDBRemote_h_

#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Reproducer.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-public.h"
Expand Down Expand Up @@ -71,6 +69,7 @@ struct GDBRemotePacket {
std::string data;
};

void Serialize(llvm::raw_ostream &strm) const;
void Dump(Stream &strm) const;

BinaryData packet;
Expand All @@ -83,46 +82,6 @@ struct GDBRemotePacket {
llvm::StringRef GetTypeStr() const;
};

namespace repro {
class PacketRecorder : public AbstractRecorder {
public:
PacketRecorder(const FileSpec &filename, std::error_code &ec)
: AbstractRecorder(filename, ec) {}

static llvm::Expected<std::unique_ptr<PacketRecorder>>
Create(const FileSpec &filename);

void Record(const GDBRemotePacket &packet);
};

class GDBRemoteProvider : public repro::Provider<GDBRemoteProvider> {
public:
struct Info {
static const char *name;
static const char *file;
};

GDBRemoteProvider(const FileSpec &directory) : Provider(directory) {}

llvm::raw_ostream *GetHistoryStream();
PacketRecorder *GetNewPacketRecorder();

void SetCallback(std::function<void()> callback) {
m_callback = std::move(callback);
}

void Keep() override;
void Discard() override;

static char ID;

private:
std::function<void()> m_callback;
std::unique_ptr<llvm::raw_fd_ostream> m_stream_up;
std::vector<std::unique_ptr<PacketRecorder>> m_packet_recorders;
};

} // namespace repro
} // namespace lldb_private

LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(lldb_private::GDBRemotePacket)
Expand Down
105 changes: 46 additions & 59 deletions lldb/include/lldb/Utility/Reproducer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,32 +153,11 @@ class WorkingDirectoryProvider : public Provider<WorkingDirectoryProvider> {
static char ID;
};

class AbstractRecorder {
protected:
AbstractRecorder(const FileSpec &filename, std::error_code &ec)
: m_filename(filename.GetFilename().GetStringRef()),
m_os(filename.GetPath(), ec, llvm::sys::fs::OF_Text), m_record(true) {}

public:
const FileSpec &GetFilename() { return m_filename; }

void Stop() {
assert(m_record);
m_record = false;
}

private:
FileSpec m_filename;

protected:
llvm::raw_fd_ostream m_os;
bool m_record;
};

class DataRecorder : public AbstractRecorder {
class DataRecorder {
public:
DataRecorder(const FileSpec &filename, std::error_code &ec)
: AbstractRecorder(filename, ec) {}
: m_filename(filename.GetFilename().GetStringRef()),
m_os(filename.GetPath(), ec, llvm::sys::fs::OF_Text), m_record(true) {}

static llvm::Expected<std::unique_ptr<DataRecorder>>
Create(const FileSpec &filename);
Expand All @@ -191,6 +170,18 @@ class DataRecorder : public AbstractRecorder {
m_os << '\n';
m_os.flush();
}

const FileSpec &GetFilename() { return m_filename; }

void Stop() {
assert(m_record);
m_record = false;
}

private:
FileSpec m_filename;
llvm::raw_fd_ostream m_os;
bool m_record;
};

class CommandProvider : public Provider<CommandProvider> {
Expand All @@ -213,6 +204,32 @@ class CommandProvider : public Provider<CommandProvider> {
std::vector<std::unique_ptr<DataRecorder>> m_data_recorders;
};

class ProcessGDBRemoteProvider
: public repro::Provider<ProcessGDBRemoteProvider> {
public:
struct Info {
static const char *name;
static const char *file;
};

ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) {}

llvm::raw_ostream *GetHistoryStream();

void SetCallback(std::function<void()> callback) {
m_callback = std::move(callback);
}

void Keep() override { m_callback(); }
void Discard() override { m_callback(); }

static char ID;

private:
std::function<void()> m_callback;
std::unique_ptr<llvm::raw_fd_ostream> m_stream_up;
};

/// The generator is responsible for the logic needed to generate a
/// reproducer. For doing so it relies on providers, who serialize data that
/// is necessary for reproducing a failure.
Expand Down Expand Up @@ -342,43 +359,13 @@ class Reproducer {
mutable std::mutex m_mutex;
};

template <typename T> class MultiLoader {
/// Helper class for replaying commands through the reproducer.
class CommandLoader {
public:
MultiLoader(std::vector<std::string> files) : m_files(files) {}

static std::unique_ptr<MultiLoader> Create(Loader *loader) {
if (!loader)
return {};

FileSpec file = loader->GetFile<typename T::Info>();
if (!file)
return {};

auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
if (auto err = error_or_file.getError())
return {};
CommandLoader(std::vector<std::string> files) : m_files(files) {}

std::vector<std::string> files;
llvm::yaml::Input yin((*error_or_file)->getBuffer());
yin >> files;

if (auto err = yin.error())
return {};

for (auto &file : files) {
FileSpec absolute_path =
loader->GetRoot().CopyByAppendingPathComponent(file);
file = absolute_path.GetPath();
}

return std::make_unique<MultiLoader<T>>(std::move(files));
}

llvm::Optional<std::string> GetNextFile() {
if (m_index >= m_files.size())
return {};
return m_files[m_index++];
}
static std::unique_ptr<CommandLoader> Create(Loader *loader);
llvm::Optional<std::string> GetNextFile();

private:
std::vector<std::string> m_files;
Expand Down
5 changes: 2 additions & 3 deletions lldb/source/API/SBDebugger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,8 @@ SBError SBDebugger::SetInputFile(SBFile file) {

FileSP file_sp = file.m_opaque_sp;

static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
repro::MultiLoader<repro::CommandProvider>::Create(
repro::Reproducer::Instance().GetLoader());
static std::unique_ptr<repro::CommandLoader> loader =
repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
if (loader) {
llvm::Optional<std::string> nextfile = loader->GetNextFile();
FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
Expand Down
47 changes: 22 additions & 25 deletions lldb/source/Commands/CommandObjectReproducer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,18 +407,20 @@ class CommandObjectReproducerDump : public CommandObjectParsed {
return true;
}
case eReproducerProviderCommands: {
std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> multi_loader =
repro::MultiLoader<repro::CommandProvider>::Create(loader);
if (!multi_loader) {
// Create a new command loader.
std::unique_ptr<repro::CommandLoader> command_loader =
repro::CommandLoader::Create(loader);
if (!command_loader) {
SetError(result,
make_error<StringError>(llvm::inconvertibleErrorCode(),
"Unable to create command loader."));
return false;
}

// Iterate over the command files and dump them.
llvm::Optional<std::string> command_file;
while ((command_file = multi_loader->GetNextFile())) {
while (true) {
llvm::Optional<std::string> command_file =
command_loader->GetNextFile();
if (!command_file)
break;

Expand All @@ -434,29 +436,24 @@ class CommandObjectReproducerDump : public CommandObjectParsed {
return true;
}
case eReproducerProviderGDB: {
std::unique_ptr<repro::MultiLoader<repro::GDBRemoteProvider>>
multi_loader =
repro::MultiLoader<repro::GDBRemoteProvider>::Create(loader);
llvm::Optional<std::string> gdb_file;
while ((gdb_file = multi_loader->GetNextFile())) {
auto error_or_file = MemoryBuffer::getFile(*gdb_file);
if (auto err = error_or_file.getError()) {
SetError(result, errorCodeToError(err));
return false;
}
FileSpec gdb_file = loader->GetFile<ProcessGDBRemoteProvider::Info>();
auto error_or_file = MemoryBuffer::getFile(gdb_file.GetPath());
if (auto err = error_or_file.getError()) {
SetError(result, errorCodeToError(err));
return false;
}

std::vector<GDBRemotePacket> packets;
yaml::Input yin((*error_or_file)->getBuffer());
yin >> packets;
std::vector<GDBRemotePacket> packets;
yaml::Input yin((*error_or_file)->getBuffer());
yin >> packets;

if (auto err = yin.error()) {
SetError(result, errorCodeToError(err));
return false;
}
if (auto err = yin.error()) {
SetError(result, errorCodeToError(err));
return false;
}

for (GDBRemotePacket &packet : packets) {
packet.Dump(result.GetOutputStream());
}
for (GDBRemotePacket &packet : packets) {
packet.Dump(result.GetOutputStream());
}

result.SetStatus(eReturnStatusSuccessFinishResult);
Expand Down
6 changes: 2 additions & 4 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Reproducer.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/ScopedPrinter.h"
Expand Down Expand Up @@ -1244,9 +1243,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess(

void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); }

void GDBRemoteCommunication::SetPacketRecorder(
repro::PacketRecorder *recorder) {
m_history.SetRecorder(recorder);
void GDBRemoteCommunication::SetHistoryStream(llvm::raw_ostream *strm) {
m_history.SetStream(strm);
}

llvm::Error
Expand Down
6 changes: 1 addition & 5 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@
#include "lldb/lldb-public.h"

namespace lldb_private {
namespace repro {
class PacketRecorder;
}
namespace process_gdb_remote {

enum GDBStoppointType {
Expand Down Expand Up @@ -136,8 +133,7 @@ class GDBRemoteCommunication : public Communication {
// fork/exec to avoid having to connect/accept

void DumpHistory(Stream &strm);

void SetPacketRecorder(repro::PacketRecorder *recorder);
void SetHistoryStream(llvm::raw_ostream *strm);

static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
GDBRemoteCommunication &server);
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ void GDBRemoteCommunicationHistory::AddPacket(char packet_char,
m_packets[idx].bytes_transmitted = bytes_transmitted;
m_packets[idx].packet_idx = m_total_packet_count;
m_packets[idx].tid = llvm::get_threadid();
if (m_recorder)
m_recorder->Record(m_packets[idx]);
if (m_stream)
m_packets[idx].Serialize(*m_stream);
}

void GDBRemoteCommunicationHistory::AddPacket(const std::string &src,
Expand All @@ -58,8 +58,8 @@ void GDBRemoteCommunicationHistory::AddPacket(const std::string &src,
m_packets[idx].bytes_transmitted = bytes_transmitted;
m_packets[idx].packet_idx = m_total_packet_count;
m_packets[idx].tid = llvm::get_threadid();
if (m_recorder)
m_recorder->Record(m_packets[idx]);
if (m_stream)
m_packets[idx].Serialize(*m_stream);
}

void GDBRemoteCommunicationHistory::Dump(Stream &strm) const {
Expand Down
8 changes: 2 additions & 6 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,11 @@
#include <vector>

#include "lldb/Utility/GDBRemote.h"
#include "lldb/Utility/Reproducer.h"
#include "lldb/lldb-public.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"

namespace lldb_private {
namespace repro {
class PacketRecorder;
}
namespace process_gdb_remote {

/// The history keeps a circular buffer of GDB remote packets. The history is
Expand All @@ -45,7 +41,7 @@ class GDBRemoteCommunicationHistory {
void Dump(Log *log) const;
bool DidDumpToLog() const { return m_dumped_to_log; }

void SetRecorder(repro::PacketRecorder *recorder) { m_recorder = recorder; }
void SetStream(llvm::raw_ostream *strm) { m_stream = strm; }

private:
uint32_t GetFirstSavedPacketIndex() const {
Expand Down Expand Up @@ -77,7 +73,7 @@ class GDBRemoteCommunicationHistory {
uint32_t m_curr_idx;
uint32_t m_total_packet_count;
mutable bool m_dumped_to_log;
repro::PacketRecorder *m_recorder = nullptr;
llvm::raw_ostream *m_stream = nullptr;
};

} // namespace process_gdb_remote
Expand Down
Loading

0 comments on commit c9e0b35

Please sign in to comment.