Summary: SetOutputFileHandle doesn't work with IOBase

[lawrence_danna]

Summary

This is a second attempt at this, fixed up so it builds against python 3 and Windows.

original: https://reviews.llvm.org/D38829

[zturner]

Mentioned earlier, but if at all possible I think it would be helpful to instead of trying to force this into the existing Host/File class, we should instead try to keep all of this in the PythonFile class. This seems like trying to drive a square nail into a round hole. Could PythonFile just inherit from IOObjectBase so that we can use it everywhere we would currently use a File?

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

964–966 ↗

(On Diff #119668)

How about just

if (PyFile_Check(py_obj))
  return true;

1041 ↗

(On Diff #119668)

This all seems horribly complicated. Would it be possible to just make PythonFile inherit directly from IOObjectBase and then just provide an implementation of read, write, and close?

1109 ↗

(On Diff #119668)

:-/

1166–1184 ↗

(On Diff #119668)

Can we replace this entire block with:

if (!PythonFile::Check(m_py_obj))
  return false;

#if PY_MAJOR_VERSION < 3
  int fd = PyObject_AsFileDescriptor(m_py_obj);
  assert(fd >= 0);  // assert, since PythonFile::Check() returned true.
    // We don't own the file descriptor returned by this function, make sure the
    // File object knows about that.
    file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false);
    PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
    file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
    return file.IsValid();
#else
  PyObject *r = PyEval_CallMethod(m_py_obj, "readable", "()");
  bool readable = PyObject_IsTrue(r);

  r = PyEval_CallMethod(m_py_obj, "writable", "()");
  bool writable = PyObject_IsTrue(r);

  // Note that if this function returned a std::unique_ptr<IOObjectBase>, then we
  // wouldn't need all these weird read, write, and close functions, because it could
  // just implement them virtually and return an instance of PyNativeFile or whatever.
  Py_XINCREF(m_py_obj);
  file = File(m_py_obj, readable ? readfn : NULL, writable ? writefn : NULL, closefn);
  if (!file.IsValid())
    closefn(m_py_obj);
  return file.isValid();
#endif

1189 ↗

(On Diff #119668)

Isn't the PyEval_CallMethod function undocumented? Can we use the documented version?

[lawrence_danna]

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1041 ↗

(On Diff #119668)

I don't know, what is IOObjectBase? I don't see a class called that in either LLDB or python.

1166–1184 ↗

(On Diff #119668)

I suppose we could skip the subclass check. What's your reasoning for why we should though? I don't understand why that would be better. Seems like we should be checking something to see if it's a valid stream rather than just start calling functions on it right away.

We shouldn't skip the PyObject_AsFileDescriptor() part on python 3 though. If it's an ordinary file. handle then we should just write to it directly rather than going though funopen/fopencookie.

especially because there's no funopen/fopencookie equivalent on windows so only the file descriptor path can work on windows.

1189 ↗

(On Diff #119668)

It's marked with PyAPI_FUNC, so it's a public function. I think it's OK.

What's the name of the documented version that you're referring to?

[zturner]

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1041 ↗

(On Diff #119668)

Sorry, I meant IOObject. It's the base class of File, So instead of passing the File class pointers to functions that read, write and close, you just inherit PythonFile from IOObject and implement Read, Write, and Close as overridden virtual methods.

1166–1184 ↗

(On Diff #119668)

It's actually not skipping the sanity check though. That's the reason for calling PythonFile::Check() first, which contains all of that logic. This function contained code that appeared to have been copied out of that function, so i was hoping for a way to avoid code duplication.

1189 ↗

(On Diff #119668)

I can't find documentation for it anywhere on the python.org website. In other parts of LLDB where we want to call functions through the C API, we have been using PyObject_CallFunction or PyObject_CallMethod. Is there a functional difference that you know of?

[lawrence_danna]

I'll update this.

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1041 ↗

(On Diff #119668)

I'll try it and see if it works.

1166–1184 ↗

(On Diff #119668)

ah, yea good idea.

1189 ↗

(On Diff #119668)

ah, ok

[lawrence_danna]

source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

1041 ↗

(On Diff #119668)

OK, I investigated this a bit further, and it looks to me like this would require changing the APIs in SBDebugger.h to take IOObject instead of FILE*

Isn't SBDebugger.h the public API that isn't supposed to be changed like that?

[lawrence_danna]

• cleanup according to code review suggestions

[jasonmolenda]

I'll check this in for Larry. I'm going to change the Flush methods to FlushDebuggerOutputHandles to make it clearer what they are doing.

lldb/trunk/include/lldb/API/SBDebugger.h

public:
 
   void SkipAppInitFiles(bool b);
 
   void SetInputFileHandle(FILE *f, bool transfer_ownership);
 
   void SetOutputFileHandle(FILE *f, bool transfer_ownership);
 
   void SetErrorFileHandle(FILE *f, bool transfer_ownership);
   
+  void FlushDebuggerOutputHandles();
+
   FILE *GetInputFileHandle();
 
   FILE *GetOutputFileHandle();
 
   FILE *GetErrorFileHandle();
 
   void SaveInputTerminalState();
 

lldb/trunk/include/lldb/Core/Debugger.h

public:
 
   lldb::StreamFileSP GetErrorFile() { return m_error_file_sp; }
 
   void SetInputFileHandle(FILE *fh, bool tranfer_ownership);
 
   void SetOutputFileHandle(FILE *fh, bool tranfer_ownership);
 
   void SetErrorFileHandle(FILE *fh, bool tranfer_ownership);
                      
+  void FlushDebuggerOutputHandles();
+
   void SaveInputTerminalState();
 
   void RestoreInputTerminalState();
 
   lldb::StreamSP GetAsyncOutputStream();
 
   lldb::StreamSP GetAsyncErrorStream();
 

lldb/trunk/include/lldb/Host/File.h

File()
         m_is_real_terminal(eLazyBoolCalculate) {}
 
   File(FILE *fh, bool transfer_ownership)
       : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
         m_stream(fh), m_options(0), m_own_stream(transfer_ownership),
         m_is_interactive(eLazyBoolCalculate),
         m_is_real_terminal(eLazyBoolCalculate) {}
 
+  File(File &&rhs);
+
+  File& operator= (File &&rhs);
+
+  void Swap(File &other);
+
+  File(void *cookie,
+       int (*readfn)(void *, char *, int),
+       int (*writefn)(void *, const char *, int),
+       int (*closefn)(void *));
+
   //------------------------------------------------------------------
   /// Constructor with path.
   ///
   /// Takes a path to a file which can be just a filename, or a full
   /// path. If \a path is not nullptr or empty, this function will call
   /// File::Open (const char *path, uint32_t options, uint32_t permissions).
   ///
   /// @param[in] path
protected:
   //------------------------------------------------------------------
   int m_descriptor;
   FILE *m_stream;
   uint32_t m_options;
   bool m_own_stream;
   LazyBool m_is_interactive;
   LazyBool m_is_real_terminal;
   LazyBool m_supports_colors;
-
-private:
-  DISALLOW_COPY_AND_ASSIGN(File);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_File_h_

lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

+"""
+Test lldb Python API for setting output and error file handles 
+"""
+
+from __future__ import print_function
+
+
+import contextlib
+import os
+import io
+import re
+import platform
+import unittest
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class StringIO(io.TextIOBase):
+
+    def __init__(self, buf=''):
+        self.buf = buf
+
+    def writable(self):
+        return True
+
+    def write(self, s):
+        self.buf += s
+        return len(s)
+
+
+class BadIO(io.TextIOBase):
+
+    def writable(self):
+        return True
+
+    def write(self, s):
+        raise Exception('OH NOE')
+    
+
[e-mail]
+def replace_stdout(new):
+    old = sys.stdout
+    sys.stdout = new
+    try:
+        yield
+    finally:
+        sys.stdout = old
+
+
+def handle_command(debugger, cmd, raise_on_fail=True, collect_result=True):
+
+    ret = lldb.SBCommandReturnObject()
+
+    if collect_result:
+        interpreter = debugger.GetCommandInterpreter()    
+        interpreter.HandleCommand(cmd, ret)
+    else:
+        debugger.HandleCommand(cmd)
+        
+    if hasattr(debugger, 'FlushDebuggerOutputHandles'):
+        debugger.FlushDebuggerOutputHandles()
+        
+    if collect_result and raise_on_fail and not ret.Succeeded():
+        raise Exception
+    
+    return ret.GetOutput()
+
+
+        
+class FileHandleTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def comment(self, *args):
+        if self.session is not None:
+            print(*args, file=self.session)
+
+    def skip_windows(self):
+        if platform.system() == 'Windows':
+            self.skipTest('windows')
+        
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_file_out(self):
+
+        debugger = lldb.SBDebugger.Create()
+        try:
+            with open('output', 'w') as f:
+                debugger.SetOutputFileHandle(f, False)
+                handle_command(debugger, 'script print("foobar")')
+
+            with open('output', 'r') as f:
+                self.assertEqual(f.read().strip(), "foobar")
+
+        finally:
+            self.RemoveTempFile('output')
+            lldb.SBDebugger.Destroy(debugger)
+            
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_file_error(self):
+
+        debugger = lldb.SBDebugger.Create()
+        try:
+            with open('output', 'w') as f:
+                debugger.SetErrorFileHandle(f, False)
+                handle_command(debugger, 'lolwut', raise_on_fail=False, collect_result=False)
+                
+            with open('output', 'r') as f:
+                errors = f.read()
+                self.assertTrue(re.search(r'error:.*lolwut', errors))
+
+        finally:
+            self.RemoveTempFile('output')
+            lldb.SBDebugger.Destroy(debugger)
+            
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_string_out(self):
+
+        self.skip_windows()
+
+        io = StringIO()
+        debugger = lldb.SBDebugger.Create()
+        try:
+            debugger.SetOutputFileHandle(io, False)
+            handle_command(debugger, 'script print("foobar")')
+
+            self.assertEqual(io.buf.strip(), "foobar")
+            
+        finally:
+            lldb.SBDebugger.Destroy(debugger)
+                    
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_string_error(self):
+
+        self.skip_windows()
+
+        io = StringIO()
+        debugger = lldb.SBDebugger.Create()
+        try:
+            debugger.SetErrorFileHandle(io, False)
+            handle_command(debugger, 'lolwut', raise_on_fail=False, collect_result=False)
+
+            errors = io.buf
+            self.assertTrue(re.search(r'error:.*lolwut', errors))
+
+        finally:
+            lldb.SBDebugger.Destroy(debugger)
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_replace_stdout(self):
+
+        self.skip_windows()
+
+        io = StringIO()
+        debugger = lldb.SBDebugger.Create()
+        try:
+
+            with replace_stdout(io):
+                handle_command(debugger, 'script print("lol, crash")', collect_result=False)
+
+        finally:
+            lldb.SBDebugger.Destroy(debugger)
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_replace_stdout_with_nonfile(self):
+
+        self.skip_windows()
+
+        io = StringIO()
+
+        with replace_stdout(io):
+
+            class Nothing():
+                pass
+
+            debugger = lldb.SBDebugger.Create()
+            try:
+                with replace_stdout(Nothing):
+                    self.assertEqual(sys.stdout, Nothing)
+                    handle_command(debugger, 'script print("lol, crash")', collect_result=False)
+                    self.assertEqual(sys.stdout, Nothing)
+            finally:
+                lldb.SBDebugger.Destroy(debugger)
+
+            sys.stdout.write("FOO")
+
+        self.assertEqual(io.buf, "FOO")
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_stream_error(self):
+
+        self.skip_windows()
+
+        messages = list()
+
+        io = BadIO()
+        debugger = lldb.SBDebugger.Create()
+        try:
+            debugger.SetOutputFileHandle(io, False)
+            debugger.SetLoggingCallback(messages.append)
+            handle_command(debugger, 'log enable lldb script')
+            handle_command(debugger, 'script print "foobar"')
+            
+        finally:
+            lldb.SBDebugger.Destroy(debugger)
+
+        for message in messages:
+            self.comment("GOT: " + message.strip())
+
+        self.assertTrue(any('OH NOE' in msg for msg in messages))
+        self.assertTrue(any('BadIO' in msg for msg in messages))
+
+

lldb/trunk/scripts/interface/SBDebugger.i

public:
     SetInputFileHandle (FILE *f, bool transfer_ownership);
 
     void
     SetOutputFileHandle (FILE *f, bool transfer_ownership);
 
     void
     SetErrorFileHandle (FILE *f, bool transfer_ownership);
 
+    %feature("docstring",
+    "Flush the Debugger's Output/Error file handles.
+For instance, this is needed by a repl implementation on top of 
+the SB API, where fine grained control of output timing was needed."
+    ) FlushDebuggerOutputHandles;
+    void
+    FlushDebuggerOutputHandles ();
+
     FILE *
     GetInputFileHandle ();
 
     FILE *
     GetOutputFileHandle ();
 
     FILE *
     GetErrorFileHandle ();

lldb/trunk/source/API/SBDebugger.cpp

log->Printf(
         "SBDebugger(%p)::SetInputFileHandle (fh=%p, transfer_ownership=%i)",
         static_cast<void *>(m_opaque_sp.get()), static_cast<void *>(fh),
         transfer_ownership);
 
   if (m_opaque_sp)
     m_opaque_sp->SetInputFileHandle(fh, transfer_ownership);
 }
 
+void SBDebugger::FlushDebuggerOutputHandles() {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+  
+  if (log)
+    log->Printf(
+                "SBDebugger(%p)::FlushDebuggerOutputHandles ()",
+                static_cast<void *>(m_opaque_sp.get()));
+
+  if (m_opaque_sp)
+    m_opaque_sp->FlushDebuggerOutputHandles();
+}
+
 void SBDebugger::SetOutputFileHandle(FILE *fh, bool transfer_ownership) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   if (log)
     log->Printf(
         "SBDebugger(%p)::SetOutputFileHandle (fh=%p, transfer_ownership=%i)",
         static_cast<void *>(m_opaque_sp.get()), static_cast<void *>(fh),
         transfer_ownership);

lldb/trunk/source/Core/Debugger.cpp

DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
   debugger_sp->InstanceInitialize();
   return debugger_sp;
 }
 
 void Debugger::Destroy(DebuggerSP &debugger_sp) {
   if (!debugger_sp)
     return;
 
+  /*
+   * FILE* get flushed on process exit.  If those FILEs need to call into python
+   * to flush, we can't have them flushing after python is already torn down.
+   * That would result in a segfault.  We are still relying on the python script
+   * to tear down the debugger before it exits.
+   */
+  debugger_sp->m_output_file_sp->Flush();
+  debugger_sp->m_error_file_sp->Flush();
+
   debugger_sp->Clear();
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
     std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
     DebuggerList::iterator pos, end = g_debugger_list_ptr->end();
     for (pos = g_debugger_list_ptr->begin(); pos != end; ++pos) {
       if ((*pos).get() == debugger_sp.get()) {
         g_debugger_list_ptr->erase(pos);
void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) {
   else
     m_error_file_sp = std::make_shared<StreamFile>(fh, tranfer_ownership);
 
   File &err_file = m_error_file_sp->GetFile();
   if (!err_file.IsValid())
     err_file.SetStream(stderr, false);
 }
 
+void Debugger::FlushDebuggerOutputHandles() {
+  m_output_file_sp->Flush();
+  m_error_file_sp->Flush();
+}
+
 void Debugger::SaveInputTerminalState() {
   if (m_input_file_sp) {
     File &in_file = m_input_file_sp->GetFile();
     if (in_file.GetDescriptor() != File::kInvalidDescriptor)
       m_terminal_state.Save(in_file.GetDescriptor(), true);
   }
 }
 

lldb/trunk/source/Host/common/File.cpp

 //===-- File.cpp ------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/File.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <stdarg.h>
 #include <stdio.h>
+#include <assert.h>
 
 #ifdef _WIN32
 #include "lldb/Host/windows/windows.h"
 #else
 #include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <termios.h>
 #include <unistd.h>
if (options & File::eOpenOptionAppend) {
     return "w";
   }
   return NULL;
 }
 
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = NULL;
 
+File::File(File &&rhs)
+  : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
+  m_stream(kInvalidStream), m_options(), m_own_stream(false),
+  m_is_interactive(eLazyBoolCalculate),
+  m_is_real_terminal(eLazyBoolCalculate),
+  m_supports_colors(eLazyBoolCalculate)
+{
+  Swap(rhs);
+}
+
+File& File::operator= (File &&rhs)
+{
+  Close();
+  Swap(rhs);
+  return *this;
+}
+
+void File::Swap(File &rhs)
+{
+    std::swap(m_descriptor, rhs.m_descriptor);
+    std::swap(m_stream, rhs.m_stream);
+    std::swap(m_own_stream, rhs.m_own_stream);
+    std::swap(m_options, rhs.m_options);
+    std::swap(m_is_interactive, rhs.m_is_interactive);
+    std::swap(m_is_real_terminal, rhs.m_is_real_terminal);
+    std::swap(m_supports_colors, rhs.m_supports_colors);
+}
+
+#if defined(__linux__)
+
+struct context {
+  void *cookie;
+  int (*readfn)(void *, char *, int);
+  int (*writefn)(void *, const char *, int);
+  int (*closefn)(void *);
+};
+
+static ssize_t
+write_wrapper(void *c, const char *buf, size_t size)
+{
+  auto ctx = (struct context *)c;
+  if (size > INT_MAX) {
+    size = INT_MAX;
+  }
+  ssize_t wrote = ctx->writefn(ctx->cookie, buf, (int)size);
+  assert(wrote < 0 || (size_t)wrote <= size);
+  if (wrote < 0) {
+    return -1;
+  } else {
+    return (int)wrote;
+  }
+}
+
+static ssize_t
+read_wrapper(void *c, char *buf, size_t size)
+{
+  auto ctx = (struct context *)c;
+  if (size > INT_MAX) {
+    size = INT_MAX;
+  }
+  ssize_t read = ctx->writefn(ctx->cookie, buf, (int)size);
+  assert(read < 0 || (size_t)read <= size);
+  if (read < 0) {
+    return -1;
+  } else {
+    return (int)read;
+  }
+}
+
+static int
+close_wrapper(void *c)
+{
+  auto ctx = (struct context *)c;
+  int ret = ctx->closefn(ctx->cookie);
+  delete ctx;
+  return ret;
+}
+
+#endif
+
+File::File(void *cookie,
+           int (*readfn)(void *, char *, int),
+           int (*writefn)(void *, const char *, int),
+           int (*closefn)(void *))
+    : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
+    m_stream(kInvalidStream), m_options(), m_own_stream(false),
+    m_is_interactive(eLazyBoolCalculate),
+    m_is_real_terminal(eLazyBoolCalculate),
+    m_supports_colors(eLazyBoolCalculate)
+{
+#if defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__)
+    m_stream = funopen(cookie, readfn, writefn, NULL, closefn);
+#elif defined(__linux__)
+    cookie_io_functions_t io_funcs = {};
+    io_funcs.read = read_wrapper;
+    io_funcs.write = write_wrapper;
+    io_funcs.close = close_wrapper;
+    const char *mode = NULL;
+    if (readfn && writefn) {
+      mode = "r+";
+    } else if (readfn) {
+      mode = "r";
+    } else if (writefn) {
+      mode = "w";
+    }
+    if (mode) {
+      struct context *ctx = new context;
+      ctx->readfn = readfn;
+      ctx->writefn = writefn;
+      ctx->closefn = closefn;
+      ctx->cookie = cookie;
+      m_stream = fopencookie(ctx, mode, io_funcs);
+      if (!m_stream) {
+        delete ctx;
+      }
+    }
+#endif
+    if (m_stream) {
+        m_own_stream = true;
+    }
+}
+
+
 File::File(const char *path, uint32_t options, uint32_t permissions)
     : IOObject(eFDTypeFile, false), m_descriptor(kInvalidDescriptor),
       m_stream(kInvalidStream), m_options(), m_own_stream(false),
       m_is_interactive(eLazyBoolCalculate),
       m_is_real_terminal(eLazyBoolCalculate) {
   Open(path, options, permissions);
 }
 

lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

void Reset(const PythonObject &rhs) {
     // Avoid calling the virtual method if it's not necessary
     // to actually validate the type of the PyObject.
     if (!rhs.IsValid())
       Reset();
     else
       Reset(PyRefType::Borrowed, rhs.m_py_obj);
   }
 
+  operator PyObject*() const { return m_py_obj; }
+
   // PythonObject is implicitly convertible to PyObject *, which will call the
   // wrong overload.  We want to explicitly disallow this, since a PyObject
   // *always* owns its reference.  Therefore the overload which takes a
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;
 
   virtual void Reset(PyRefType type, PyObject *py_obj) {
     if (py_obj == m_py_obj)

lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

 
 // Python is disabled in this build
 
 #else
 
 #include "PythonDataObjects.h"
 #include "ScriptInterpreterPython.h"
 
+#include "lldb/Utility/Log.h"
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/Support/ConvertUTF.h"
 
 #include <stdio.h>

 }
 
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
 
 bool PythonFile::Check(PyObject *py_obj) {
 #if PY_MAJOR_VERSION < 3
-  return PyFile_Check(py_obj);
-#else
+  if (PyFile_Check(py_obj)) {
+    return true;
+  }
+#endif
   // In Python 3, there is no `PyFile_Check`, and in fact PyFile is not even a
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`.
   // As a result, the only way to detect a file in Python 3 is to check whether
   // it inherits from `io.IOBase`.  Since it is possible for non-files to also
   // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
   // attribute, which should guarantee that it is backed by the file system.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
                            PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
   if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
     return false;
+
   if (!object_type.HasAttribute("fileno"))
     return false;
 
   return true;
-#endif
 }
 
 void PythonFile::Reset(PyRefType type, PyObject *py_obj) {
   // Grab the desired reference type so that if we end up rejecting
   // `py_obj` it still gets decremented if necessary.
   PythonObject result(type, py_obj);
 
-  if (!PythonFile::Check(py_obj)) {
+  if (py_obj == NULL || !PythonFile::Check(py_obj)) {
     PythonObject::Reset();
     return;
   }
 
   // Calling PythonObject::Reset(const PythonObject&) will lead to stack
   // overflow since it calls back into the virtual implementation.
   PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
return llvm::StringSwitch<uint32_t>(mode.str())
       .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
       .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
                       File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
       .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
                       File::eOpenOptionAppend | File::eOpenOptionCanCreate)
       .Default(0);
 }
 
+static const char *
+str(PyObject *o, const char *defaultt, PyObject **cleanup)
+{
+  *cleanup = NULL;
+  if (o == NULL) {
+    return defaultt;
+  }
+  PyObject *string = PyObject_Str(o);
+  *cleanup = string;
+  if (string == NULL) {
+    return defaultt;
+  }
+  if (PyUnicode_Check(string)) {
+    PyObject *bytes = PyUnicode_AsEncodedString(string, "utf-8", "Error ~");
+    if (bytes == NULL) {
+      return defaultt;
+    }
+    Py_XDECREF(string);
+    *cleanup = bytes;
+    return PyBytes_AS_STRING(bytes);
+  } else {
+    return PyBytes_AS_STRING(string);
+  }
+}
+
+
+static void
+log_exception(const char *fmt, PyObject *obj)
+{
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT);
+  if (!log) {
+    return;
+  }
+  PyObject *pyclass = PyObject_Type(obj);
+  PyObject *pyclassname = NULL;
+  if (pyclass) {
+    pyclassname = PyObject_GetAttrString(pyclass, "__name__");
+  }
+  PyObject *exception = NULL, *v = NULL, *tb = NULL;
+  PyErr_Fetch(&exception, &v, &tb);
+  if (exception) {
+    PyErr_NormalizeException(&exception, &v, &tb);
+  }
+  PyObject *cleanup1 = NULL, *cleanup2 = NULL;
+  log->Printf(fmt,
+              str(pyclassname, "UknownClass", &cleanup1),
+              str(v, "unknown error", &cleanup2));
+  Py_XDECREF(cleanup1);
+  Py_XDECREF(cleanup2);
+  Py_XDECREF(pyclass);
+  Py_XDECREF(pyclassname);
+  Py_XDECREF(exception);
+  Py_XDECREF(v);
+  Py_XDECREF(tb);
+
+}
+
+class GIL
+{
+private:
+  PyGILState_STATE m_state;
+public:
+  GIL() {
+    m_state = PyGILState_Ensure();
+  }
+  ~GIL() {
+    PyGILState_Release(m_state);
+  }
+};
+
+#define callmethod(obj, name, fmt, ...) \
+  PythonObject(PyRefType::Owned, PyObject_CallMethod(obj, (char*)name, (char*)fmt, __VA_ARGS__))
+
+#define callmethod0(obj, name, fmt) \
+  PythonObject(PyRefType::Owned, PyObject_CallMethod(obj, (char*)name, (char*)fmt))
+
+static int readfn(void *ctx, char *buffer, int n)
+{
+  GIL gil;
+  auto *file = (PyObject *) ctx;
+
+  PythonObject pyresult = callmethod(file, "read", "(i)", n);
+  if (!pyresult.IsValid() || PyErr_Occurred()) {
+    log_exception("read from python %s failed: %s", file);
+    return -1;
+  }
+  if (!PyBytes_Check(pyresult)) {
+    PyErr_SetString(PyExc_TypeError, "read didn't return bytes");
+    log_exception("read from python %s failed: %s", file);
+    return -1;
+  }
+
+  int r = 0;
+  char *data = NULL;
+  ssize_t length = 0;
+  r = PyBytes_AsStringAndSize(pyresult, &data, &length);
+  if (r || length < 0 || PyErr_Occurred()) {
+    log_exception("read from python %s failed: %s", file);
+    return -1;
+  }
+
+  memcpy(buffer, data, length);
+  return length;
+}
+
+static int writefn(void *ctx, const char *buffer, int n)
+{
+  GIL gil;
+  auto *file = (PyObject *) ctx;
+
+  PythonObject pyresult = callmethod(file, "write", "(s#)", buffer, n);
+  if (!pyresult.IsValid() || PyErr_Occurred()) {
+    log_exception("write to python %s failed: %s", file);
+    return -1;
+  }
+
+  int result = PyLong_AsLong(pyresult);
+  if (PyErr_Occurred()) {
+    log_exception("read from python %s failed: %s", file);
+    return -1;
+  }
+
+  return result;
+}
+
+static int closefn(void *ctx) {
+  GIL gil;
+  auto *file = (PyObject *) ctx;
+  Py_XDECREF(file);
+  return 0;
+}
+
 bool PythonFile::GetUnderlyingFile(File &file) const {
   if (!IsValid())
     return false;
 
+  if (!PythonFile::Check(m_py_obj))
+    return false;
+  
   file.Close();
+  
+  int fd = PyObject_AsFileDescriptor(m_py_obj);
+  if (fd >= 0) {
     // We don't own the file descriptor returned by this function, make sure the
     // File object knows about that.
     file.SetDescriptor(PyObject_AsFileDescriptor(m_py_obj), false);
     PythonString py_mode = GetAttributeValue("mode").AsType<PythonString>();
     file.SetOptions(PythonFile::GetOptionsFromMode(py_mode.GetString()));
     return file.IsValid();
   }
   
+  bool readable = PyObject_IsTrue(callmethod0(m_py_obj, "readable", "()"));
+  bool writable = PyObject_IsTrue(callmethod0(m_py_obj, "writable", "()"));
+  
+  Py_XINCREF(m_py_obj);
+  file = File(m_py_obj, readable ? readfn : NULL, writable ? writefn : NULL, closefn);
+  if (!file.IsValid()) {
+    closefn(m_py_obj);
+    return false;
+  } else {
+    return true;
+  }
+}
+
 #endif

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

protected:
   PythonObject &GetMainModule();
 
   PythonDictionary &GetSessionDictionary();
 
   PythonDictionary &GetSysModuleDictionary();
 
   bool GetEmbeddedInterpreterModuleObjects();
 
-  bool SetStdHandle(File &file, const char *py_name, PythonFile &save_file,
+  bool SetStdHandle(File &file, const char *py_name, PythonObject &save_file,
                     const char *mode);
 
-  PythonFile m_saved_stdin;
-  PythonFile m_saved_stdout;
-  PythonFile m_saved_stderr;
+  PythonObject m_saved_stdin;
+  PythonObject m_saved_stdout;
+  PythonObject m_saved_stderr;
   PythonObject m_main_module;
   PythonObject m_lldb_module;
   PythonDictionary m_session_dict;
   PythonDictionary m_sys_module_dict;
   PythonObject m_run_one_line_function;
   PythonObject m_run_one_line_str_global;
   std::string m_dictionary_name;
   TerminalState m_terminal_state;

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

if (sys_module_dict.IsValid()) {
       }
     }
   }
 
   m_session_is_active = false;
 }
 
 bool ScriptInterpreterPython::SetStdHandle(File &file, const char *py_name,
-                                           PythonFile &save_file,
+                                           PythonObject &save_file,
                                            const char *mode) {
   if (file.IsValid()) {
     // Flush the file before giving it to python to avoid interleaved output.
     file.Flush();
 
     PythonDictionary &sys_module_dict = GetSysModuleDictionary();
 
-    save_file = sys_module_dict.GetItemForKey(PythonString(py_name))
-                    .AsType<PythonFile>();
+    save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
 
     PythonFile new_file(file, mode);
     sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
     return true;
   } else
     save_file.Reset();
   return false;
 }