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

@@ -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();

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

lldb/include/lldb/Core/Debugger.h

@@ -132,12 +132,11 @@ public:
 
   repro::DataRecorder *GetInputRecorder();
 
-  void SetInputFileHandle(FILE *fh, bool tranfer_ownership,
+  void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = nullptr);
-                          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();
 

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)

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 ();
 

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<File>(fh, transfer_ownership));
+}
 
-  if (!m_opaque_sp)
+// Shouldn't really be settable after initialization as this could cause lots
-    return;
+// 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<repro::CommandProvider>().GetNewDataRecorder();
 
+  FileSP file_sp = file.m_opaque_sp;
+
   static std::unique_ptr<repro::CommandLoader> loader =
       repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
   if (loader) {
-    llvm::Optional<std::string> file = loader->GetNextFile();
+    llvm::Optional<std::string> nextfile = loader->GetNextFile();
-    fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr;
+    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<File>(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<File>(fh, transfer_ownership));
+}
 
-  if (m_opaque_sp)
+SBError SBDebugger::SetOutputFile(SBFile file) {
-    m_opaque_sp->SetOutputFileHandle(fh, transfer_ownership);
+  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<File>(fh, transfer_ownership));
+}
 
-  if (m_opaque_sp)
+SBError SBDebugger::SetErrorFile(SBFile file) {
-    m_opaque_sp->SetErrorFileHandle(fh, transfer_ownership);
+  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<SBDebugger>(Registry &R) {
                                        &SBDebugger::GetDefaultArchitecture),
                                    &GetDefaultArchitectureRedirect);
 
+  R.Register(&invoke<SBError (SBDebugger::*)(
+                 SBFile)>::method<&SBDebugger::SetInputFile>::doit,
+             &SetFileRedirect);
+  R.Register(&invoke<SBError (SBDebugger::*)(
+                 SBFile)>::method<&SBDebugger::SetOutputFile>::doit,
+             &SetFileRedirect);
+  R.Register(&invoke<SBError (SBDebugger::*)(
+                 SBFile)>::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<SBDebugger>(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,

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) {

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,
+void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
-                                  repro::DataRecorder *recorder) {
+  assert(file_sp && file_sp->IsValid());
   m_input_recorder = recorder;
-
+  m_input_file_sp = file_sp;
-  m_input_file_sp = std::make_shared<File>(fh, tranfer_ownership);
-  if (!m_input_file_sp->IsValid())
-    m_input_file_sp = std::make_shared<File>(stdin, false);
-
   // 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) {
+void Debugger::SetOutputFile(FileSP file_sp) {
-  FileSP file_sp = std::make_shared<File>(fh, tranfer_ownership);
+  assert(file_sp && file_sp->IsValid());
-  if (!file_sp->IsValid())
-    file_sp = std::make_shared<File>(stdout, false);
   m_output_stream_sp = std::make_shared<StreamFile>(file_sp);
-
 }
 
-void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) {
+void Debugger::SetErrorFile(FileSP file_sp) {
-  FileSP file_sp = std::make_shared<File>(fh, tranfer_ownership);
+  assert(file_sp && file_sp->IsValid());
-  if (!file_sp->IsValid())
-    file_sp = std::make_shared<File>(stderr, false);
   m_error_stream_sp = std::make_shared<StreamFile>(file_sp);
 }
 

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