Python: SetOutputFileHandle doesn't work with IOBase

[lawrence_danna]

Summary

SetOutputFileHandle only works with legit file objects with file descriptors. It ought to work with subclasses of IOBase as well.

This patch adds support for passing an arbitrary python stream (anything inheriting from IOBase) to SetOutputFileHandle or SetErrorFileHandle.

It also fixes some crashes associated with PythonFile.

Supposedly it also works for SetInputFileHandle, but as far as I can tell nothing actually uses the input file handle so I coluldn’t figure out how to test that part at all.

[krytarowski]

lldb/source/Host/common/File.cpp

103 ↗

(On Diff #118717)

I propose __linux__ for consistency.

[lawrence_danna]

code review fix

[lawrence_danna]

oops, that last diff I added replaced the whole diff instead of stacking on top.

[jasonmolenda]

Looks good Larry, I'll commit it later tonight.

[labath]

(Please add lldb-commits to future reviews, so that people are aware of what's going on.)

So, the reason why this failed to compile is that android does not have the fopencookie function (neither does windows as far as I am aware). However, looking at the change, it's not clear to me whether we actually need the fopencookie functionality to implement this. Couldn't we just change the File::Read/Write functions to call these directly. Right now they do if (am_i_backed_by_FILE) fwrite() else write(). We could add a option for them to be backed by a set of callbacks. An even better option would be to just use virtual functions for this (that's the c++ way of doing cookie callbacks). I think that's what the IOObject hierarchy (which File is a part of) was meant to be used for, although I am not sure it will work out of the box for this case.

And it would be great to see some tests for this.

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

65

Why are you changing the File to be movable? I don't see the connection between this and the fopencookie part.

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

124

cast seems wrong

140

same here

[zturner]

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

1075–1096

I am still pretty unhappy about these functions, and passing function pointers into the File class.

I think another approach would be this:

1. Make the File class contain a member std::unique_ptr<IOObject> LowLevelIo;

2. In File.cpp, define something called class DefaultLowLevelIo : public IOObject that implements the virtual methods against an fd.

3. In PythonDataObjects, define PythonFileIo : public IOObject and implement the virtual methods against a PyObject.

4. Add an additional constructor to File which takes a std::unique_ptr<IOObject> LowLevelIo, which we can use when creating one of these from a python file.

One advantage of this method is that it allows the PythonFileIo class to be easily tested.

(Also, sorry for not getting back to reviewing this several weeks ago)

[lawrence_danna]

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

1075–1096

I don't see how this approach gets around the problem that the interfaces in SBDebugger use FILE *, not IOObject

The only way I can see to do it the way you are saying is to also add a SBIOObject, with swig wrappers to that, and variants of the SBDebugger
interfaces that take IOObject instead of FILE *

Do you want to do it that way?

[labath]

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

1075–1096

What's the final use case here. In the patch itself I don't see anything that would necessitate a FILE * conversion, but I don't know what do you actually intend to use this for. We can always return a null FILE * if the File object is backed by a a python file (we do the same for file descriptors, as there is no way to convert those into FILE*, not without going the fopencookie way).

[zturner]

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

1075–1096

Alright, I re-read this more closely. This is actually something I wanted to fix for a long time. Specifically, I don't believe LLDB should be using FILE* anywhere. I would prefer to mark this method dangerous in big letters in the SB API, and add new versions that take an fd. A FILE* doesn't even mean anything in Python. It's specifically a native construct.

I see it being used in two places currently. 1) Setting to None, and 2) setting to the result of open("/dev/null"). The open method documentation says "Open a file, returning an object of the file type described in section File Objects".

So when this method is being called from Python, it is not even a real FILE*, it's a pointer to some python object. I think this is just a bug in the design of the SB API, and we should fix it there.

I don't often propose adding new SB APIs, but I think we need an entirely different API here. There should be methods:

SetOutputFileHandle(SBFile F);
SetInputFileHandle(SBFile F);
SetErrorFileHandle(SBFile F);

And we should be passing those. This will in turn necessitate a lot of trickle down changes in the native side too. We can mark the older functions deprecated to indicate that people should no longer be using them.

Thoughts?

1075–1096

Sorry, s/add a new version that takes an fd/add a new version that takes an SBFile/

[labath]

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

1075–1096

I don't often propose adding new SB APIs, but I think we need an entirely different API here. There should be methods:

SetOutputFileHandle(SBFile F);
SetInputFileHandle(SBFile F);
SetErrorFileHandle(SBFile F);
And we should be passing those. This will in turn necessitate a lot of trickle down changes in the native side too. We can mark the older functions deprecated to indicate that people should no longer be using them.

Note that these file handles eventually trickle down into libedit, which expects to see a FILE *. These could probably be synthesized(*) for libedit's usage alone, and leave the rest of the world oblivious to FILE*, but it will need to be done very carefully.

(*) Windows does not have a fopencookie/funopen equivalent afaik, but then again, it also does not have libedit...

[clayborg]

Another option would be to add a URL version of these functions:

class SBDebugger {
  void SetInputURL(const char *url);
  void SetOutputURL(const char *url);
  void SetErrorURL(const char *url);
};

Then we allow this to trickle down to the IOBase in that way. This allows us to innovate by adding new support for new URLs.

"FILE*" is unfortunately what we will need for libedit for now, but we can probably make sure the IOBase call can provide this somehow, or convert a file descriptor into one, and fall back on a simple non-libedit based command interpreter if not. The current IOHandler stack will check the FILE's abilities and do the right thing already.

[zturner]

Ok, I wasn't aware of the libedit problem.

I still don't like this approach, because it causes the design of the File class to be influenced by a limitation of a 3rd party library.

What about adding a method to IOObject called FILE *GetFileStream(). Then the method I proposed earlier would just involve PythonFileIo implementing this in the proper way. Then we can pass SBFile or lldb_private::File through all layers of the codebase, and once we're in libedit we just call file.GetFileStream() and pass it to libedit?

[lawrence_danna]

@labath

(Please add lldb-commits to future reviews, so that people are aware of what's going on.)

ok

Couldn't we just change the File::Read/Write functions to call these directly

Like I said to @zturner , this is possible, but it can't work with the existing APIs in SBDebugger.h. We'd need to add new APIs that take SBIOBase instead of FILE *. Do you agree with that plan?

And it would be great to see some tests for this

yup, there's tests in TestFileHandle.py

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

65

I use the move constructor here:

file = File(m_py_obj, readable ? readfn : NULL, writable ? writefn : NULL, closefn);

[clayborg]

In D38829#914300, @zturner wrote:

Ok, I wasn't aware of the libedit problem.

I still don't like this approach, because it causes the design of the File class to be influenced by a limitation of a 3rd party library.

What about adding a method to IOObject called FILE *GetFileStream(). Then the method I proposed earlier would just involve PythonFileIo implementing this in the proper way. Then we can pass SBFile or lldb_private::File through all layers of the codebase, and once we're in libedit we just call file.GetFileStream() and pass it to libedit?

That could work. If we don't get a "FILE *" back from IOObject::GetFileStream() we need to fall back onto a simple implementation. The current IOHandler will fall back to fgets() on a FILE*, but we can easily make it just call a method on IOObject. Probably would be best to add a fgets() type call to IOObject so the underlying implementation can do this as efficiently as possible.

I mainly care that the current API continues to work with "FILE *" at least on all Unix based platforms. We can add new APIs, but we must leave the existing ones there.

[labath]

In D38829#914300, @zturner wrote:

Ok, I wasn't aware of the libedit problem.

I still don't like this approach, because it causes the design of the File class to be influenced by a limitation of a 3rd party library.

What about adding a method to IOObject called FILE *GetFileStream(). Then the method I proposed earlier would just involve PythonFileIo implementing this in the proper way. Then we can pass SBFile or lldb_private::File through all layers of the codebase, and once we're in libedit we just call file.GetFileStream() and pass it to libedit?

That's how I'd do it.

(*) libedit needs both a FILE* and a raw file descriptor in this case (I think it's calling select, and some terminal functions).

[lawrence_danna]

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

1075–1096

What's the final use case here

LLDB integration with iPython. I want to be able to redirect LLDB's output to a python callback so I can interact with LLDB from inside an iPython notebook.

but I think we need an entirely different API here

OK I will prepare a new version of the patch that introduces a SBFile API

[labath]

In D38829#914305, @lawrence_danna wrote:

@labath

Couldn't we just change the File::Read/Write functions to call these directly

Like I said to @zturner , this is possible, but it can't work with the existing APIs in SBDebugger.h.

Ok, I am starting to understand what you want to do here.

And it would be great to see some tests for this

yup, there's tests in TestFileHandle.py

These may prove that you don't break existing functionality, but here you are introducing new one, which should be tested as well. (But don't go writing these yet until we figure out how exactly to implement this)

[zturner]

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

1075–1096

Thanks, and apologies that the scope is kind of expanding here.

If possible, can you try to do this incrementally in several patches?

For example, a first pass you might just try having SBFile that wraps a lldb_private::File and provides a couple of simple methods. This will allow us to focus on the API bridge without having to worry about implementation details.

Next add SetOutputFile etc, and try to update existing uses of SetOutputFileHandle etc to use this new api. It should continue to work as today, with the same bugs.

Then, implement the dynamic dispatch inside of lldb_private::File so that python files and native files go to a different underlying implementation.

[lawrence_danna]

I've reworked this -- finally -- as zturner suggested. I'll be posting it as a bunch of individual commits, so i'm abandoning this one.

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 Flush();
+
   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 Flush();
+
   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/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);
 
+    void
+    Flush ();
+
     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::Flush() {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+  
+  if (log)
+    log->Printf(
+                "SBDebugger(%p)::Flush ()",
+                static_cast<void *>(m_opaque_sp.get()));
+
+  if (m_opaque_sp)
+    m_opaque_sp->Flush();
+}
+
 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::Flush() {
+  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.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
+  bool is_ordinary_file = PyFile_Check(py_obj);
+  if (is_ordinary_file) {
+    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 void
+log_exception(const char *fmt, PyObject *obj)
+{
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT);
+  if (!log) {
+    return;
+  }
+  const char *classname = "unknown class";
+  PyObject *pyclass = PyObject_Type(obj);
+  if (pyclass) {
+    PyObject *s = PyObject_GetAttrString(pyclass, "__name__");
+    if (s) {
+      classname = PyString_AsString(s);
+      Py_XDECREF(s);
+    }
+    Py_XDECREF(pyclass);
+  }
+  const char *error = "unknown error";
+  PyObject *exception, *v, *tb;
+  PyErr_Fetch(&exception, &v, &tb);
+  if (exception) {
+    PyErr_NormalizeException(&exception, &v, &tb);
+  }
+  PyObject *string = NULL;
+  if (v) {
+    string = PyObject_Str(v);
+  }
+  error = PyString_AsString(string);
+  log->Printf(fmt, classname, error);
+  Py_XDECREF(exception);
+  Py_XDECREF(v);
+  Py_XDECREF(tb);
+  Py_XDECREF(string);
+}
+
+static int readfn(void *ctx, char *buffer, int n)
+{
+  auto state = PyGILState_Ensure();
+  auto *file = (PyObject *) ctx;
+  int result = -1;
+  auto pybuffer = PyBuffer_FromMemory(buffer, n);
+  PyObject *pyresult = NULL;
+  if (!pybuffer) {
+    goto fail;
+  }
+  pyresult = PyEval_CallMethod(file, "read", "(i)", n);
+  if (pyresult == NULL || !PyInt_Check(pyresult)) {
+    log_exception("read from python %s failed: %s", file);
+    goto fail;
+  }
+  result =  _PyInt_AsInt(pyresult);
+fail:
+  Py_XDECREF(pybuffer);
+  Py_XDECREF(pyresult);
+  PyGILState_Release(state);
+  return result;
+}
+
+static int writefn(void *ctx, const char *buffer, int n)
+{
+  auto state = PyGILState_Ensure();
+  auto *file = (PyObject *) ctx;
+  int result = -1;
+  auto pyresult = PyEval_CallMethod(file, "write", "(s#)", buffer, n);
+  if (pyresult == NULL || !PyInt_Check(pyresult)) {
+    log_exception("write to python %s failed: %s", file);
+    goto fail;
+  }
+  result =  _PyInt_AsInt(pyresult);
+fail:
+  Py_XDECREF(pyresult);
+  PyGILState_Release(state);
+  return result;
+}
+
+
+static int closefn(void *ctx) {
+  auto *file = (PyObject *) ctx;
+  auto state = PyGILState_Ensure();
+  Py_XDECREF(file);
+  PyGILState_Release(state);
+  return 0;
+}
+
+
 bool PythonFile::GetUnderlyingFile(File &file) const {
   if (!IsValid())
     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();
   }
   
+  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(m_py_obj));
+  
+  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
+    return false;
+  
+  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);
+
+  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;
 }