PatriosTheGreat updated this revision to Diff 353875.
PatriosTheGreat added a comment.
Hi everyone.
Thanks for review.
In order to move new method code to Debugger class I also had to move there
SetInputFile(lldb::FileSP file) method.
After I did that -- the reproducer tests started to fail.
If I understand correctly there is a problem to deserialize SBStream object
during the reply session and in previous patch it the replay tests was working
since I was calling SetInputFile API method from SetStreamInput.
To fix that in this patch I replaced SetStreamInput method with SetInputData
(const char* data, size_t size); which takes a raw chat array instead of
SBStream.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104413/new/
https://reviews.llvm.org/D104413
Files:
lldb/bindings/interface/SBDebugger.i
lldb/include/lldb/API/SBDebugger.h
lldb/include/lldb/Core/Debugger.h
lldb/source/API/SBDebugger.cpp
lldb/source/Core/Debugger.cpp
lldb/test/API/python_api/default-constructor/sb_debugger.py
lldb/test/API/python_api/file_handle/TestFileHandle.py
lldb/tools/driver/Driver.cpp
Index: lldb/tools/driver/Driver.cpp
===================================================================
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -23,7 +23,6 @@
#include "llvm/Support/Format.h"
#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
@@ -42,14 +41,6 @@
#include <cstring>
#include <fcntl.h>
-// Includes for pipe()
-#if defined(_WIN32)
-#include <fcntl.h>
-#include <io.h>
-#else
-#include <unistd.h>
-#endif
-
#if !defined(__APPLE__)
#include "llvm/Support/DataTypes.h"
#endif
@@ -400,60 +391,6 @@
return error;
}
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
- return _pipe(fds, size, O_BINARY);
-#else
- (void)size;
- return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
- size_t commands_size) {
- enum PIPES { READ, WRITE }; // Indexes for the read and write fds
- int fds[2] = {-1, -1};
-
- if (OpenPipe(fds, commands_size) != 0) {
- WithColor::error()
- << "can't create pipe file descriptors for LLDB commands\n";
- return nullptr;
- }
-
- ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
- if (size_t(nrwr) != commands_size) {
- WithColor::error()
- << format(
- "write(%i, %p, %" PRIu64
- ") failed (errno = %i) when trying to open LLDB commands pipe",
- fds[WRITE], static_cast<const void *>(commands_data),
- static_cast<uint64_t>(commands_size), errno)
- << '\n';
- llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
- llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
- return nullptr;
- }
-
- // Close the write end of the pipe, so that the command interpreter will exit
- // when it consumes all the data.
- llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
- // Open the read file descriptor as a FILE * that we can return as an input
- // handle.
- ::FILE *commands_file = fdopen(fds[READ], "rb");
- if (commands_file == nullptr) {
- WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
- << '\n';
- llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
- return nullptr;
- }
-
- // 'commands_file' now owns the read descriptor.
- return commands_file;
-}
-
std::string EscapeString(std::string arg) {
std::string::size_type pos = 0;
while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -583,21 +520,16 @@
// Check if we have any data in the commands stream, and if so, save it to a
// temp file
// so we can then run the command interpreter using the file contents.
- const char *commands_data = commands_stream.GetData();
- const size_t commands_size = commands_stream.GetSize();
-
bool go_interactive = true;
- if ((commands_data != nullptr) && (commands_size != 0u)) {
- FILE *commands_file =
- PrepareCommandsForSourcing(commands_data, commands_size);
-
- if (commands_file == nullptr) {
- // We should have already printed an error in PrepareCommandsForSourcing.
+ if ((commands_stream.GetData() != nullptr) &&
+ (commands_stream.GetSize() != 0u)) {
+ SBError error = m_debugger.SetInputData(commands_stream.GetData(),
+ commands_stream.GetSize());
+ if (error.Fail()) {
+ WithColor::error() << error.GetCString() << '\n';
return 1;
}
- m_debugger.SetInputFileHandle(commands_file, true);
-
// Set the debugger into Sync mode when running the command file. Otherwise
// command files that run the target won't run in a sensible way.
bool old_async = m_debugger.GetAsync();
@@ -629,12 +561,9 @@
SBStream crash_commands_stream;
WriteCommandsForSourcing(eCommandPlacementAfterCrash,
crash_commands_stream);
- const char *crash_commands_data = crash_commands_stream.GetData();
- const size_t crash_commands_size = crash_commands_stream.GetSize();
- commands_file =
- PrepareCommandsForSourcing(crash_commands_data, crash_commands_size);
- if (commands_file != nullptr) {
- m_debugger.SetInputFileHandle(commands_file, true);
+ SBError error = m_debugger.SetInputData(crash_commands_stream.GetData(),
+ crash_commands_stream.GetSize());
+ if (error.Success()) {
SBCommandInterpreterRunResult local_results =
m_debugger.RunCommandInterpreter(options);
if (local_results.GetResult() ==
Index: lldb/test/API/python_api/file_handle/TestFileHandle.py
===================================================================
--- lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -901,3 +901,23 @@
self.assertTrue(f.closed)
with open(self.out_filename, 'r') as f:
self.assertEqual(f.read().strip(), "Frobozz")
+
+ @skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+ def test_set_sbstream(self):
+ with open(self.out_filename, 'w') as outf:
+ outsbf = lldb.SBFile(outf.fileno(), "w", False)
+ status = self.dbg.SetOutputFile(outsbf)
+ self.assertTrue(status.Success())
+
+ stream = lldb.SBStream()
+ stream.Print("version\nhelp\n")
+ self.dbg.SetInputData(stream.GetData(), stream.GetSize())
+
+ opts = lldb.SBCommandInterpreterRunOptions()
+ self.dbg.RunCommandInterpreter(True, False, opts, 0, False, False)
+ self.dbg.GetOutputFile().Flush()
+
+ with open(self.out_filename, 'r') as f:
+ output = f.read()
+ self.assertTrue(re.search(r'Show a list of all debugger commands', output))
+ self.assertTrue(re.search(r'llvm revision', output))
Index: lldb/test/API/python_api/default-constructor/sb_debugger.py
===================================================================
--- lldb/test/API/python_api/default-constructor/sb_debugger.py
+++ lldb/test/API/python_api/default-constructor/sb_debugger.py
@@ -13,6 +13,7 @@
obj.SetInputFileHandle(None, True)
obj.SetOutputFileHandle(None, True)
obj.SetErrorFileHandle(None, True)
+ obj.SetInputData("", 0)
obj.GetInputFileHandle()
obj.GetOutputFileHandle()
obj.GetErrorFileHandle()
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,6 +44,7 @@
#include "lldb/Utility/Listener.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/ReproducerProvider.h"
#include "lldb/Utility/State.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StreamCallback.h"
@@ -74,6 +75,14 @@
#include <string>
#include <system_error>
+// Includes for pipe()
+#if defined(_WIN32)
+#include <fcntl.h>
+#include <io.h>
+#else
+#include <unistd.h>
+#endif
+
namespace lldb_private {
class Address;
}
@@ -801,6 +810,87 @@
repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; }
+static inline int OpenPipe(int fds[2], std::size_t size) {
+#ifdef _WIN32
+ return _pipe(fds, size, O_BINARY);
+#else
+ (void)size;
+ return pipe(fds);
+#endif
+}
+
+Status Debugger::SetInputData(const char *data, size_t size) {
+ Status result;
+ enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+ int fds[2] = {-1, -1};
+
+ if (OpenPipe(fds, size) != 0) {
+ result.SetErrorString(
+ "can't create pipe file descriptors for LLDB commands");
+ return result;
+ }
+
+ ssize_t nrwr = write(fds[WRITE], data, size);
+ if (size_t(nrwr) != size) {
+ result.SetErrorStringWithFormat(
+ "write(%i, %p, %" PRIu64
+ ") failed (errno = %i) when trying to open LLDB commands pipe",
+ fds[WRITE], static_cast<const void *>(data),
+ static_cast<uint64_t>(size), errno);
+
+ llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
+ llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+ return result;
+ }
+
+ // Close the write end of the pipe, so that the command interpreter will exit
+ // when it consumes all the data.
+ llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+
+ // Open the read file descriptor as a FILE * that we can return as an input
+ // handle.
+ FILE *commands_file = fdopen(fds[READ], "rb");
+ if (commands_file == nullptr) {
+ result.SetErrorStringWithFormat("fdopen(%i, \"rb\") failed (errno = %i) "
+ "when trying to open LLDB commands pipe",
+ fds[READ], errno);
+ llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
+ return result;
+ }
+
+ return SetInputFile(
+ (FileSP)std::make_shared<NativeFile>(commands_file, true));
+}
+
+Status Debugger::SetInputFile(FileSP file_sp) {
+ Status error;
+ repro::DataRecorder *recorder = nullptr;
+ if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+ recorder = g->GetOrCreate<repro::CommandProvider>().GetNewRecorder();
+
+ static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
+ repro::MultiLoader<repro::CommandProvider>::Create(
+ repro::Reproducer::Instance().GetLoader());
+ if (loader) {
+ llvm::Optional<std::string> nextfile = loader->GetNextFile();
+ FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
+ : nullptr;
+ // FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
+ // reproducer somehow if fh is NULL?
+ if (fh) {
+ file_sp = std::make_shared<NativeFile>(fh, true);
+ }
+ }
+
+ if (!file_sp || !file_sp->IsValid()) {
+ error.SetErrorString("invalid file");
+ return error;
+ }
+
+ SetInputFile(file_sp, recorder);
+ return error;
+}
+
void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
assert(file_sp && file_sp->IsValid());
m_input_recorder = recorder;
Index: lldb/source/API/SBDebugger.cpp
===================================================================
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -327,12 +327,27 @@
void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) {
LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh,
transfer_ownership);
- SetInputFile((FileSP)std::make_shared<NativeFile>(fh, transfer_ownership));
+ if (m_opaque_sp)
+ m_opaque_sp->SetInputFile(
+ (FileSP)std::make_shared<NativeFile>(fh, transfer_ownership));
}
-SBError SBDebugger::SetInputFile(FileSP file_sp) {
- LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (FileSP), file_sp);
- return LLDB_RECORD_RESULT(SetInputFile(SBFile(file_sp)));
+SBError SBDebugger::SetInputData(const char *data, size_t size) {
+ LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputData, (const char *, size_t),
+ data, size);
+ SBError sb_error;
+ if (size == 0) {
+ sb_error.SetErrorString("empty data");
+ return LLDB_RECORD_RESULT(sb_error);
+ }
+
+ if (!m_opaque_sp) {
+ sb_error.SetErrorString("invalid debugger");
+ return LLDB_RECORD_RESULT(sb_error);
+ }
+
+ sb_error.SetError(m_opaque_sp->SetInputData(data, size));
+ return LLDB_RECORD_RESULT(sb_error);
}
// Shouldn't really be settable after initialization as this could cause lots
@@ -346,36 +361,16 @@
error.ref().SetErrorString("invalid debugger");
return LLDB_RECORD_RESULT(error);
}
+ error.SetError(m_opaque_sp->SetInputFile(file.m_opaque_sp));
- repro::DataRecorder *recorder = nullptr;
- if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
- recorder = g->GetOrCreate<repro::CommandProvider>().GetNewRecorder();
-
- 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());
- if (loader) {
- llvm::Optional<std::string> nextfile = loader->GetNextFile();
- FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
- : nullptr;
- // FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
- // reproducer somehow if fh is NULL?
- if (fh) {
- file_sp = std::make_shared<NativeFile>(fh, true);
- }
- }
-
- if (!file_sp || !file_sp->IsValid()) {
- error.ref().SetErrorString("invalid file");
- return LLDB_RECORD_RESULT(error);
- }
-
- m_opaque_sp->SetInputFile(file_sp, recorder);
return LLDB_RECORD_RESULT(error);
}
+SBError SBDebugger::SetInputFile(FileSP file_sp) {
+ LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (FileSP), file_sp);
+ return LLDB_RECORD_RESULT(SetInputFile(SBFile(file_sp)));
+}
+
SBError SBDebugger::SetOutputFile(FileSP file_sp) {
LLDB_RECORD_METHOD(SBError, SBDebugger, SetOutputFile, (FileSP), file_sp);
return LLDB_RECORD_RESULT(SetOutputFile(SBFile(file_sp)));
@@ -1760,6 +1755,8 @@
LLDB_REGISTER_METHOD(bool, SBDebugger, GetAsync, ());
LLDB_REGISTER_METHOD(void, SBDebugger, SkipLLDBInitFiles, (bool));
LLDB_REGISTER_METHOD(void, SBDebugger, SkipAppInitFiles, (bool));
+ LLDB_REGISTER_METHOD(SBError, SBDebugger, SetInputData,
+ (const char *, size_t));
LLDB_REGISTER_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool));
LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetInputFileHandle, ());
LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetOutputFileHandle, ());
Index: lldb/include/lldb/Core/Debugger.h
===================================================================
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -176,7 +176,11 @@
repro::DataRecorder *GetInputRecorder();
- void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = nullptr);
+ Status SetInputData(const char *data, size_t size);
+
+ Status SetInputFile(lldb::FileSP file);
+
+ void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder);
void SetOutputFile(lldb::FileSP file);
Index: lldb/include/lldb/API/SBDebugger.h
===================================================================
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -126,6 +126,8 @@
FILE *GetErrorFileHandle();
+ SBError SetInputData(const char *data, size_t size);
+
SBError SetInputFile(SBFile file);
SBError SetOutputFile(SBFile file);
Index: lldb/bindings/interface/SBDebugger.i
===================================================================
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -206,6 +206,9 @@
}
}
+ SBError
+ SetInputData (const char* data, size_t size);
+
SBError
SetInputFile (SBFile file);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
