From 96898eb6a935533aaebbfbd085150fbf705c0ffc Mon Sep 17 00:00:00 2001 From: Lawrence D'Anna Date: Thu, 3 Oct 2019 04:04:48 +0000 Subject: [PATCH] SBDebugger::SetInputFile, SetOutputFile, etc. Summary: Add new methods to SBDebugger to set IO files as SBFiles instead of as FILE* streams. In future commits, the FILE* methods will be deprecated and these will become the primary way to set the debugger I/O streams. Reviewers: JDevlieghere, jasonmolenda, labath Reviewed By: labath Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D68181 llvm-svn: 373563 --- lldb/include/lldb/API/SBDebugger.h | 12 ++ lldb/include/lldb/API/SBFile.h | 3 + lldb/include/lldb/Core/Debugger.h | 7 +- .../python_api/file_handle/TestFileHandle.py | 126 +++++++++++++++++- lldb/scripts/interface/SBDebugger.i | 18 +++ lldb/source/API/SBDebugger.cpp | 117 ++++++++++++++-- lldb/source/API/SBFile.cpp | 2 + lldb/source/Core/Debugger.cpp | 23 +--- .../Python/PythonDataObjects.cpp | 2 + 9 files changed, 275 insertions(+), 35 deletions(-) diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h index 417cead24a8c..1f36999c3c47 100644 --- a/lldb/include/lldb/API/SBDebugger.h +++ b/lldb/include/lldb/API/SBDebugger.h @@ -88,6 +88,18 @@ public: FILE *GetErrorFileHandle(); + SBError SetInputFile(SBFile file); + + SBError SetOutputFile(SBFile file); + + SBError SetErrorFile(SBFile file); + + SBFile GetInputFile(); + + SBFile GetOutputFile(); + + SBFile GetErrorFile(); + void SaveInputTerminalState(); void RestoreInputTerminalState(); diff --git a/lldb/include/lldb/API/SBFile.h b/lldb/include/lldb/API/SBFile.h index da330440a991..a5fc2cd41210 100644 --- a/lldb/include/lldb/API/SBFile.h +++ b/lldb/include/lldb/API/SBFile.h @@ -14,6 +14,8 @@ namespace lldb { class LLDB_API SBFile { + friend class SBDebugger; + public: SBFile(); SBFile(FILE *file, bool transfer_ownership); @@ -31,6 +33,7 @@ public: private: FileSP m_opaque_sp; + SBFile(FileSP file_sp); }; } // namespace lldb diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 90eefe115244..b2f696c22834 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -132,12 +132,11 @@ public: repro::DataRecorder *GetInputRecorder(); - void SetInputFileHandle(FILE *fh, bool tranfer_ownership, - repro::DataRecorder *recorder = nullptr); + void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = nullptr); - void SetOutputFileHandle(FILE *fh, bool tranfer_ownership); + void SetOutputFile(lldb::FileSP file); - void SetErrorFileHandle(FILE *fh, bool tranfer_ownership); + void SetErrorFile(lldb::FileSP file); void SaveInputTerminalState(); diff --git a/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py b/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py index fe75521e1b1d..3231fdfe6887 100644 --- a/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py +++ b/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py @@ -12,9 +12,19 @@ from contextlib import contextmanager import lldb from lldbsuite.test import lldbtest -from lldbsuite.test.decorators import add_test_categories, no_debug_info_test +from lldbsuite.test.decorators import ( + add_test_categories, no_debug_info_test, skipIf) +@contextmanager +def replace_stdout(new): + old = sys.stdout + sys.stdout = new + try: + yield + finally: + sys.stdout = old + def readStrippedLines(f): def i(): for line in f: @@ -66,6 +76,8 @@ class FileHandleTestCase(lldbtest.TestBase): interpreter.HandleCommand(cmd, ret) else: self.debugger.HandleCommand(cmd) + self.debugger.GetOutputFile().Flush() + self.debugger.GetErrorFile().Flush() if collect_result and check: self.assertTrue(ret.Succeeded()) return ret.GetOutput() @@ -97,6 +109,19 @@ class FileHandleTestCase(lldbtest.TestBase): with open(self.out_filename, 'r') as f: self.assertIn('deadbeef', f.read()) + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_legacy_file_err_with_get(self): + with open(self.out_filename, 'w') as f: + self.debugger.SetErrorFileHandle(f, False) + self.handleCmd('lolwut', check=False, collect_result=False) + self.debugger.GetErrorFileHandle().write('FOOBAR\n') + lldb.SBDebugger.Destroy(self.debugger) + with open(self.out_filename, 'r') as f: + errors = f.read() + self.assertTrue(re.search(r'error:.*lolwut', errors)) + self.assertTrue(re.search(r'FOOBAR', errors)) + @add_test_categories(['pyapi']) @no_debug_info_test @@ -148,3 +173,102 @@ class FileHandleTestCase(lldbtest.TestBase): self.assertTrue(e.Success()) self.assertEqual(buffer[:n], b'FOO') + + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_fileno_out(self): + with open(self.out_filename, 'w') as f: + sbf = lldb.SBFile(f.fileno(), "w", False) + status = self.debugger.SetOutputFile(sbf) + self.assertTrue(status.Success()) + self.handleCmd('script 1+2') + self.debugger.GetOutputFile().Write(b'quux') + + with open(self.out_filename, 'r') as f: + self.assertEqual(readStrippedLines(f), ['3', 'quux']) + + + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_fileno_help(self): + with open(self.out_filename, 'w') as f: + sbf = lldb.SBFile(f.fileno(), "w", False) + status = self.debugger.SetOutputFile(sbf) + self.assertTrue(status.Success()) + self.handleCmd("help help", collect_result=False, check=False) + with open(self.out_filename, 'r') as f: + self.assertTrue(re.search(r'Show a list of all debugger commands', f.read())) + + + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_immediate(self): + with open(self.out_filename, 'w') as f: + ret = lldb.SBCommandReturnObject() + ret.SetImmediateOutputFile(f) + interpreter = self.debugger.GetCommandInterpreter() + interpreter.HandleCommand("help help", ret) + # make sure the file wasn't closed early. + f.write("\nQUUX\n") + + ret = None # call destructor and flush streams + + 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'QUUX', output)) + + + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_fileno_inout(self): + with open(self.in_filename, 'w') as f: + f.write("help help\n") + + with open(self.out_filename, 'w') as outf, open(self.in_filename, 'r') as inf: + + outsbf = lldb.SBFile(outf.fileno(), "w", False) + status = self.debugger.SetOutputFile(outsbf) + self.assertTrue(status.Success()) + + insbf = lldb.SBFile(inf.fileno(), "r", False) + status = self.debugger.SetInputFile(insbf) + self.assertTrue(status.Success()) + + opts = lldb.SBCommandInterpreterRunOptions() + self.debugger.RunCommandInterpreter(True, False, opts, 0, False, False) + self.debugger.GetOutputFile().Flush() + + with open(self.out_filename, 'r') as f: + self.assertTrue(re.search(r'Show a list of all debugger commands', f.read())) + + + @add_test_categories(['pyapi']) + @no_debug_info_test + def test_fileno_error(self): + with open(self.out_filename, 'w') as f: + + sbf = lldb.SBFile(f.fileno(), 'w', False) + status = self.debugger.SetErrorFile(sbf) + self.assertTrue(status.Success()) + + self.handleCmd('lolwut', check=False, collect_result=False) + + self.debugger.GetErrorFile().Write(b'\nzork\n') + + with open(self.out_filename, 'r') as f: + errors = f.read() + self.assertTrue(re.search(r'error:.*lolwut', errors)) + self.assertTrue(re.search(r'zork', errors)) + + #FIXME This shouldn't fail for python2 either. + @add_test_categories(['pyapi']) + @no_debug_info_test + @skipIf(py_version=['<', (3,)]) + def test_replace_stdout(self): + f = io.StringIO() + with replace_stdout(f): + self.assertEqual(sys.stdout, f) + self.handleCmd('script sys.stdout.write("lol")', + collect_result=False, check=False) + self.assertEqual(sys.stdout, f) diff --git a/lldb/scripts/interface/SBDebugger.i b/lldb/scripts/interface/SBDebugger.i index f3cee14b61af..c4eb11ea3567 100644 --- a/lldb/scripts/interface/SBDebugger.i +++ b/lldb/scripts/interface/SBDebugger.i @@ -183,6 +183,24 @@ public: FILE * GetErrorFileHandle (); + SBError + SetInputFile (SBFile file); + + SBError + SetOutputFile (SBFile file); + + SBError + SetErrorFile (SBFile file); + + SBFile + GetInputFile (); + + SBFile + GetOutputFile (); + + SBFile + GetErrorFile (); + lldb::SBCommandInterpreter GetCommandInterpreter (); diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 893ede5e4149..97bc743ae872 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -18,6 +18,7 @@ #include "lldb/API/SBCommandReturnObject.h" #include "lldb/API/SBError.h" #include "lldb/API/SBEvent.h" +#include "lldb/API/SBFile.h" #include "lldb/API/SBFrame.h" #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" @@ -285,49 +286,96 @@ void SBDebugger::SkipAppInitFiles(bool b) { m_opaque_sp->GetCommandInterpreter().SkipAppInitFiles(b); } -// Shouldn't really be settable after initialization as this could cause lots -// of problems; don't want users trying to switch modes in the middle of a -// debugging session. void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) { LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh, transfer_ownership); + SetInputFile(std::make_shared(fh, transfer_ownership)); +} - if (!m_opaque_sp) - return; +// Shouldn't really be settable after initialization as this could cause lots +// of problems; don't want users trying to switch modes in the middle of a +// debugging session. +SBError SBDebugger::SetInputFile(SBFile file) { + LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (SBFile), file); + + SBError error; + if (!m_opaque_sp) { + error.ref().SetErrorString("invalid debugger"); + return error; + } repro::DataRecorder *recorder = nullptr; if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) recorder = g->GetOrCreate().GetNewDataRecorder(); + FileSP file_sp = file.m_opaque_sp; + static std::unique_ptr loader = repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader()); if (loader) { - llvm::Optional file = loader->GetNextFile(); - fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr; + llvm::Optional 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(fh, true); + } } - m_opaque_sp->SetInputFileHandle(fh, transfer_ownership, recorder); + if (!file_sp || !file_sp->IsValid()) { + error.ref().SetErrorString("invalid file"); + return error; + } + + m_opaque_sp->SetInputFile(file_sp, recorder); + return error; } void SBDebugger::SetOutputFileHandle(FILE *fh, bool transfer_ownership) { LLDB_RECORD_METHOD(void, SBDebugger, SetOutputFileHandle, (FILE *, bool), fh, transfer_ownership); + SetOutputFile(std::make_shared(fh, transfer_ownership)); +} - if (m_opaque_sp) - m_opaque_sp->SetOutputFileHandle(fh, transfer_ownership); +SBError SBDebugger::SetOutputFile(SBFile file) { + LLDB_RECORD_METHOD(SBError, SBDebugger, SetOutputFile, (SBFile file), file); + SBError error; + if (!m_opaque_sp) { + error.ref().SetErrorString("invalid debugger"); + return error; + } + if (!file) { + error.ref().SetErrorString("invalid file"); + return error; + } + m_opaque_sp->SetOutputFile(file.m_opaque_sp); + return error; } void SBDebugger::SetErrorFileHandle(FILE *fh, bool transfer_ownership) { LLDB_RECORD_METHOD(void, SBDebugger, SetErrorFileHandle, (FILE *, bool), fh, transfer_ownership); + SetErrorFile(std::make_shared(fh, transfer_ownership)); +} - if (m_opaque_sp) - m_opaque_sp->SetErrorFileHandle(fh, transfer_ownership); +SBError SBDebugger::SetErrorFile(SBFile file) { + LLDB_RECORD_METHOD(SBError, SBDebugger, SetErrorFile, (SBFile file), file); + SBError error; + if (!m_opaque_sp) { + error.ref().SetErrorString("invalid debugger"); + return error; + } + if (!file) { + error.ref().SetErrorString("invalid file"); + return error; + } + m_opaque_sp->SetErrorFile(file.m_opaque_sp); + return error; } FILE *SBDebugger::GetInputFileHandle() { LLDB_RECORD_METHOD_NO_ARGS(FILE *, SBDebugger, GetInputFileHandle); - if (m_opaque_sp) { File &file_sp = m_opaque_sp->GetInputFile(); return LLDB_RECORD_RESULT(file_sp.GetStream()); @@ -335,9 +383,16 @@ FILE *SBDebugger::GetInputFileHandle() { return nullptr; } +SBFile SBDebugger::GetInputFile() { + LLDB_RECORD_METHOD_NO_ARGS(SBFile, SBDebugger, GetInputFile); + if (m_opaque_sp) { + return LLDB_RECORD_RESULT(SBFile(m_opaque_sp->GetInputFileSP())); + } + return LLDB_RECORD_RESULT(SBFile()); +} + FILE *SBDebugger::GetOutputFileHandle() { LLDB_RECORD_METHOD_NO_ARGS(FILE *, SBDebugger, GetOutputFileHandle); - if (m_opaque_sp) { StreamFile &stream_file = m_opaque_sp->GetOutputStream(); return LLDB_RECORD_RESULT(stream_file.GetFile().GetStream()); @@ -345,6 +400,15 @@ FILE *SBDebugger::GetOutputFileHandle() { return nullptr; } +SBFile SBDebugger::GetOutputFile() { + LLDB_RECORD_METHOD_NO_ARGS(SBFile, SBDebugger, GetOutputFile); + if (m_opaque_sp) { + SBFile file(m_opaque_sp->GetOutputStream().GetFileSP()); + return LLDB_RECORD_RESULT(file); + } + return LLDB_RECORD_RESULT(SBFile()); +} + FILE *SBDebugger::GetErrorFileHandle() { LLDB_RECORD_METHOD_NO_ARGS(FILE *, SBDebugger, GetErrorFileHandle); @@ -355,6 +419,16 @@ FILE *SBDebugger::GetErrorFileHandle() { return nullptr; } +SBFile SBDebugger::GetErrorFile() { + LLDB_RECORD_METHOD_NO_ARGS(SBFile, SBDebugger, GetErrorFile); + SBFile file; + if (m_opaque_sp) { + SBFile file(m_opaque_sp->GetErrorStream().GetFileSP()); + return LLDB_RECORD_RESULT(file); + } + return LLDB_RECORD_RESULT(SBFile()); +} + void SBDebugger::SaveInputTerminalState() { LLDB_RECORD_METHOD_NO_ARGS(void, SBDebugger, SaveInputTerminalState); @@ -1503,6 +1577,8 @@ static void SetFileHandleRedirect(SBDebugger *, FILE *, bool) { // Do nothing. } +static SBError SetFileRedirect(SBDebugger *, SBFile file) { return SBError(); } + static bool GetDefaultArchitectureRedirect(char *arch_name, size_t arch_name_len) { // The function is writing to its argument. Without the redirect it would @@ -1523,6 +1599,16 @@ template <> void RegisterMethods(Registry &R) { &SBDebugger::GetDefaultArchitecture), &GetDefaultArchitectureRedirect); + R.Register(&invoke::method<&SBDebugger::SetInputFile>::doit, + &SetFileRedirect); + R.Register(&invoke::method<&SBDebugger::SetOutputFile>::doit, + &SetFileRedirect); + R.Register(&invoke::method<&SBDebugger::SetErrorFile>::doit, + &SetFileRedirect); + LLDB_REGISTER_CONSTRUCTOR(SBDebugger, ()); LLDB_REGISTER_CONSTRUCTOR(SBDebugger, (const lldb::DebuggerSP &)); LLDB_REGISTER_CONSTRUCTOR(SBDebugger, (const lldb::SBDebugger &)); @@ -1547,6 +1633,9 @@ template <> void RegisterMethods(Registry &R) { LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetInputFileHandle, ()); LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetOutputFileHandle, ()); LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetErrorFileHandle, ()); + LLDB_REGISTER_METHOD(SBFile, SBDebugger, GetInputFile, ()); + LLDB_REGISTER_METHOD(SBFile, SBDebugger, GetOutputFile, ()); + LLDB_REGISTER_METHOD(SBFile, SBDebugger, GetErrorFile, ()); LLDB_REGISTER_METHOD(void, SBDebugger, SaveInputTerminalState, ()); LLDB_REGISTER_METHOD(void, SBDebugger, RestoreInputTerminalState, ()); LLDB_REGISTER_METHOD(lldb::SBCommandInterpreter, SBDebugger, diff --git a/lldb/source/API/SBFile.cpp b/lldb/source/API/SBFile.cpp index fc4f5572f010..b36dedb628f4 100644 --- a/lldb/source/API/SBFile.cpp +++ b/lldb/source/API/SBFile.cpp @@ -16,6 +16,8 @@ using namespace lldb_private; SBFile::~SBFile() {} +SBFile::SBFile(FileSP file_sp) : m_opaque_sp(file_sp) {} + SBFile::SBFile() { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBFile); } SBFile::SBFile(FILE *file, bool transfer_ownership) { diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index fe2815029ca4..3774d75cc7e5 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -821,31 +821,22 @@ void Debugger::SetAsyncExecution(bool async_execution) { repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; } -void Debugger::SetInputFileHandle(FILE *fh, bool tranfer_ownership, - repro::DataRecorder *recorder) { +void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) { + assert(file_sp && file_sp->IsValid()); m_input_recorder = recorder; - - m_input_file_sp = std::make_shared(fh, tranfer_ownership); - if (!m_input_file_sp->IsValid()) - m_input_file_sp = std::make_shared(stdin, false); - + m_input_file_sp = file_sp; // Save away the terminal state if that is relevant, so that we can restore // it in RestoreInputState. SaveInputTerminalState(); } -void Debugger::SetOutputFileHandle(FILE *fh, bool tranfer_ownership) { - FileSP file_sp = std::make_shared(fh, tranfer_ownership); - if (!file_sp->IsValid()) - file_sp = std::make_shared(stdout, false); +void Debugger::SetOutputFile(FileSP file_sp) { + assert(file_sp && file_sp->IsValid()); m_output_stream_sp = std::make_shared(file_sp); - } -void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) { - FileSP file_sp = std::make_shared(fh, tranfer_ownership); - if (!file_sp->IsValid()) - file_sp = std::make_shared(stderr, false); +void Debugger::SetErrorFile(FileSP file_sp) { + assert(file_sp && file_sp->IsValid()); m_error_stream_sp = std::make_shared(file_sp); } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 20745d42e7a9..58431447f4f5 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -954,6 +954,8 @@ PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); } PythonFile::~PythonFile() {} bool PythonFile::Check(PyObject *py_obj) { + if (!py_obj) + return false; #if PY_MAJOR_VERSION < 3 return PyFile_Check(py_obj); #else