allow arbitrary python streams to be converted to SBFile

[lawrence_danna]

Summary

This patch adds SWIG typemaps that can convert arbitrary python
file objects into lldb_private::File.

A SBFile may be initialized from a python file using the
constructor. There are also alternate, tagged constructors
that allow python files to be borrowed, and for the caller
to control whether or not the python I/O methods will be
called even when a file descriptor is available.I

[labath]

I haven't looked at the patch in detail, but I have one high-level question. It seems to be that instead of subclassing a fully-functional File class, it would be better to create an abstract class, that both File and the new python thingy could implement. That would also create a natural place to write down the FILE* conversion semantics and other stuff we talked about at lldb-dev. I don't have any opinion as to whether the name File should denote the new abstract class, or the real thing, but it should most likely be done as a separate patch.

What do you think about that?

lldb/include/lldb/API/SBFile.h

16–21

I don't think we have anything similar to this in any of our SB classes. It might be better to avoid it. Could the same thing be achieved e.g. with a static factory function (static SBFile SBFile::Create(FileSP file_sp, bool borrow, bool force_scripting_io)) ?

[lawrence_danna]

lldb/include/lldb/API/SBFile.h

16–21

I don't think it can be done that way, because we want those bools to control how the python object is translated into a C++ object, so we need a different swig wrapper for each possibility, which means we need a different c++ function to wrap for each possibility.

One way around this would be to have SWIG translate the python object into an intermediate object that just takes a reference, and then perform further translation inside the SB layer. But we can't do that either, because scripting support is not part of the base LLDB library, it's a plugin. And in fact Xcode ships two versions of the plugin, one for python2 and one for python3. The SB needs to be agnostic about different versions of python, so it can't do the translation itself.

[lawrence_danna]

In D68188#1687532, @labath wrote:

It seems to be that instead of subclassing a fully-functional File class, it would be better to create an abstract class, that both File and the new python thingy could implement. That would also create a natural place to write down the FILE* conversion semantics and other stuff we talked about at lldb-dev. I don't have any opinion as to whether the name File should denote the new abstract class, or the real thing, but it should most likely be done as a separate patch.

What do you think about that?

I could do it that way, but I have some reservations about it. In this version the class hierarchy looks like this:

          File
           |
 SimplePythonFile --------+
  |                       |
TextPythonFile     BinaryPythonFile

If we added an abstract base class, I guess it would look like this:

          AbstractFile
           |        |                
          File      PythonFile
           |         |     | |
         SimplePythonFile  | |
                           | |
  + -----------------------+ |  
  |                          |
TextPythonFile     BinaryPythonFile

Does a more complicated class hierarchy like that really solve a problem or make the code easier to understand?

[labath]

In D68188#1689104, @lawrence_danna wrote:

In D68188#1687532, @labath wrote:

It seems to be that instead of subclassing a fully-functional File class, it would be better to create an abstract class, that both File and the new python thingy could implement. That would also create a natural place to write down the FILE* conversion semantics and other stuff we talked about at lldb-dev. I don't have any opinion as to whether the name File should denote the new abstract class, or the real thing, but it should most likely be done as a separate patch.

What do you think about that?

I could do it that way, but I have some reservations about it. In this version the class hierarchy looks like this:

          File
           |
 SimplePythonFile --------+
  |                       |
TextPythonFile     BinaryPythonFile

If we added an abstract base class, I guess it would look like this:

          AbstractFile
           |        |                
          File      PythonFile
           |         |     | |
         SimplePythonFile  | |
                           | |
  + -----------------------+ |  
  |                          |
TextPythonFile     BinaryPythonFile

Does a more complicated class hierarchy like that really solve a problem or make the code easier to understand?

No that wouldn't make things more understandable, but I think it has helped me understand what are you doing here. The usual OO answer to attempts to introduce complex class hierarchies is to try to use object composition instead. Trying to apply that here gives me the following set of classes.

• File (or AbstractFile ?) - defines just the abstract interface, documents what the interface is
• NativeFile (or File ?) - implements the File interface via the usual FILE* and file descriptor routines (one day these may be split into two classes too)
• TextPythonFile - implements the File interface via Python TextIOBase interface. Does not handle ownership/closing.
• BinaryPythonFile - implements the File interface via Python RawIOBase interface. Does not handle ownership/closing.
• OwnedPythonFile - implements the File interface by delegating to another File object. Has special handling for the close method.

This way the decision whether to "own" something is decoupled from the decision what api do you use to actually write to the file, and I _think_ it would make things clearer. What do you make of that? The thing I don't like about the current solution is that it leaves you with unused FILE* members in the Text/BinaryPythonFile classes, and forces the introduction of weird-looking OverridesIO methods...

(BTW, as this is C++, it allows you to "cheat" and instead of creating another File object the delegation can be incorporated into the type system by making OwnedPythonFile a template parameterized on the base class. I am fine with both solutions though I think I'd prefer the regular class variant.)

lldb/include/lldb/API/SBFile.h

16–21

Are you sure about that? Swig allows you to define typemaps for sequences of values. This example is taken from the swig documentation:

%typemap(in) (char *str, int len) {
    $1 = PyString_AsString($input);   /* char *str */
    $2 = PyString_Size($input);       /* int len   */
}

It seems to me that your typemap could switch on the value of the flag at runtime rather than having compile time switching by swig. This way you wouldn't even need a static function, you could just have an additional constructor with two extra bool arguments.

However, if this is not the preferred syntax for some reason, then we could also have a set of static functions, one for each flag combination, do the conversion that way.

(BTW, the main reason I am trying to avoid the tag structs is because they are wading into uncharted territory in terms of our SB coding guidelines https://lldb.llvm.org/resources/sbapi.html)

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

484

no else after return.

[lawrence_danna]

updated based on an abstract base clase

[lawrence_danna]

In D68188#1689544, @labath wrote:

This way the decision whether to "own" something is decoupled from the decision what api do you use to actually write to the file, and I _think_ it would make things clearer. What do you make of that?

Yea I think that works. Updated. I decided to go with the template thingy because there's kind of a lot of virtual methods to delegate and it's nice to have a list of them in the header.

[lawrence_danna]

rename LLDBPythonFile -> OwnedPythonFile

[lawrence_danna]

style fix

[lawrence_danna]

use a python helper instead of funny tag structs

[lawrence_danna]

lldb/include/lldb/API/SBFile.h

16–21

I really do think it can't be done with swig. You can group multiple arguments together on the C++ side, but not on the python side.

I think I did find a reasonable way to deal with it though without uglying up the SB headers with those tags. I added some statics with %extend, and then wrapped them with a python-friendly classmethod called Create, with both flags as keyword args.

How's that look?

[labath]

Hmm... I like your solution of the typemap problem, but this CRTP class seems to be way more complicated than needed. There shouldn't be any need for CRTP as we already have regular dynamic dispatch via virtual methods. Also, I don't think the forwarding should be needed if you're going the template route. How about something like this instead?

template<typename Base>
class OwningPythonFile : public Base {
  Status Close() { /* close magic here*/; }
  // no need to forward anything -- inheritance handles that
};

class TextPythonFile: public OwningPythonFile<File> {
  // override methods as usual
};
// same for BinaryPythonFile

Then depending on what you need, you create either a NativeFile, OwningPythonFile<NativeFile>, or a Text/BinaryPythonFile. How does that sound?

[lawrence_danna]

In D68188#1691129, @labath wrote:

Hmm... I like your solution of the typemap problem, but this CRTP class seems to be way more complicated than needed. There shouldn't be any need for CRTP as we already have regular dynamic dispatch via virtual methods. Also, I don't think the forwarding should be needed if you're going the template route. How about something like this instead?

template<typename Base>
class OwningPythonFile : public Base {
  Status Close() { /* close magic here*/; }
  // no need to forward anything -- inheritance handles that
};

class TextPythonFile: public OwningPythonFile<File> {
  // override methods as usual
};
// same for BinaryPythonFile

Then depending on what you need, you create either a NativeFile, OwningPythonFile<NativeFile>, or a Text/BinaryPythonFile. How does that sound?

Next step is add type maps to convert these things back to python. Without the CRTP it can’t just check for a single OwnedPythonfile base class — it’ll have to check for all three. Is that worse than using CRTP?

[labath]

In D68188#1691337, @lawrence_danna wrote:

In D68188#1691129, @labath wrote:

Then depending on what you need, you create either a NativeFile, OwningPythonFile<NativeFile>, or a Text/BinaryPythonFile. How does that sound?

Next step is add type maps to convert these things back to python. Without the CRTP it can’t just check for a single OwnedPythonfile base class — it’ll have to check for all three. Is that worse than using CRTP?

Hmm... hard to say without knowing more details, but I would say that being worse than CRTP is quite hard. :) Also, Text and Binary python file will have OwningPythonFile<File> as their base, so you can check for those together. I'd say we go with this first, and then revisit if needed.

[lawrence_danna]

rebased

[lawrence_danna]

get rid of CRTP gobbledygook

[lawrence_danna]

rebased

[labath]

Ok, I think that the major issues are out of the way now. I've done a more careful pass over the file, and I think this is looking pretty good. I am particularly happy with how you've implemented (and tested) the interaction with python exceptions. A bunch of additional comments inline, but none really that important.

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

335–336

Actually, I'm pretty sure that this category annotation is unnecessary, as the python_api contains a .categories file specifying this category. Also, the @no_debug_info_test thingy might be better achieved by setting NO_DEBUG_INFO_TESTCASE = True class property.

465–466

self.assertTrue(status.Succes()). If you're doing this because you want a better error message or something, feel free to implement something like assertSuccess or the like. I'd be happy to use that, as I want better error messages too.

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

1039

All of these file-local classes (unless you choose to move them to a separate file) should go into an anonymous namespace. See http://llvm.org/docs/CodingStandards.html#anonymous-namespaces on the recommended way to do that.

1069–1071

We're using LLDB_LOG or LLDB_LOGF these days. The first one is prefereed if you have complex values that can't be displayed by printf natively (e.g. StringRef).

1176

s/Simply/Simple

1180–1182

This reordering of arguments is confusing. Can we make the two orders match?

1238–1245

How about if OwnedPythonFile defines IsValid() as IsPythonObjectValid() || Base::IsValid(). Then PythonIOFile could redefine it to be simply IsPythonObjectValid() without the need for the extra class?

1251

no semicolon

1255–1264

Hmm... how exactly does this differ from the implementation in OwnedPythonFile? Is it only that it does not chain into the base class? Could we use the same trick as for IsValid to reduce the duplication?

1278

What is this needed for? I don't see the PythonFile class doing anything funny (and it probably shouldn't).

1397

I was struggling to find this method, as I was looking it next to the other PythonFile methods. I get why it's here, but I think it's confusing to have PythonFile be split this way. Could you move the other classes so that they are declared before all of PythonFile methods. Or, given that this file is starting to become largish, maybe move some of this stuff into a separate file?

[lawrence_danna]

We need to obsessively check PyErr_Occurred() before calling
into the next python API, even when we'd just catch it a
little later. Python APIs will generally work if you don't,
but it's undefined behavior to do it and debug builds of python
will assert.

[lawrence_danna]

no_debug_info_test -> NO_DEBUG_INFO_TESTCASE

[lawrence_danna]

anonymous namespace, and a windows fix

[lawrence_danna]

assert instead of raise

[lawrence_danna]

use LLDB_LOGF and add a logging test

[lawrence_danna]

cleanup

[lawrence_danna]

cleanup

[lawrence_danna]

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

1238–1245

If I did that then a SimplePythonFile would still be valid if the file was closed on the python side... seems like the wrong behavior.

1251

huh?

1278

python API functions expect you to check for an exception with PyErr_Occured() and clear it before calling into them again. If you don't things usually work out OK, except on a debug build in which an assert will trip.

[lawrence_danna]

group all the PythonFile support together in the source file

[labath]

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

1238–1245

Sorry, I meant &&: IsPythonObjectValid() && Base::IsValid(). Basically, I'm trying to see if there's a reasonable way to reduce the number of classes floating around, and this PresumptivelyValidFile seems like it could be avoided...

1251

There's no need to put a semicolon after the body of the constructor.

1278

Are you sure you're looking at the right place (the comments tend to move around as you reorganize code)? I was asking why is PythonFile a friend of the BinaryPythonFile class. That doesn't seem relevant to the PyErr_Occured checks...

[lawrence_danna]

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

1255–1264

they actually have different behavior. The OwnedPythonFile version chains into the base class, but the PythonIOFile version also flushes the stream on the python side if the file is borrowed. The idea is that closing any file should flush the buffer you've been writing too even if the file is borrowed. That way you don't get any weird surprises where you "close" a file and nothing gets written.

[lawrence_danna]

remove unnecessary friends

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

1238–1245

If i did it that way, with && and no PresumptivelyValidFile then the python IO files would chain into the base File class and say they're invalid.

That's how I coded it up at first, I didn't notice I needed PresumptivelyValidFile until I saw the python IO files coming up as invalid.

1278

oh, yea it got moved around. fixed.

[lawrence_danna]

semicolons

[lawrence_danna]

fixed

[labath]

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

1238–1245

Yeah, but that's where the second part of this idea comes in. The python IO files can re-override IsValid() so that it does *not* chain into the base class, and just calls IsPythonObjectValid() from OwnedPythonFile. That arrangement seems to make sense to me, though I am willing to be convinced otherwise.

[lawrence_danna]

rm class PresumptivelyValidFile

[lawrence_danna]

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

1238–1245

oh, yea that works.

[labath]

Thanks. I was just about to hit approve, but then I noticed one other thing... :/ It seems that somebody (I guess it was @zturner) spent a lot of time in creating the whole PythonObject hierarchy, and it's (worthwhile) goal seems to be to enable the rest of the to code to avoid dealing with the python APIs directly. However, here you're ignoring all of that, and jumping to the C python interface directly.

I'd like to know what's your take on that? Have you considered basing these file classes on the PythonDataObject stuff?

The main reason I mention that is your comment about PyErr_Occurred, and the resulting boiler plate it produced. This seems like something that would be nicely solved by some c++-like wrapper, which is exactly what the PythonObject stuff is AFAICT.

Most of your interactions seem to be about calling methods. Would it be possible to add a PythonDataObject wrapper for this (and any other frequently used python API)? I'm hoping that we could have something like:

if (Expected<PythonInteger> i = m_py_obj.CallMethod<PythonInteger>(pybuffer))
  l_bytes_written = i->GetInteger();
else
  return Status(i.takeError());

instead of stuff like

auto bytes_written = Take<PythonObject>(
    PyObject_CallMethod(m_py_obj, "write", "(O)", pybuffer.get()));
if (PyErr_Occurred())
  return Status(llvm::make_error<PythonException>("Write"));
long l_bytes_written = PyLong_AsLong(bytes_written.get());
if (PyErr_Occurred())
  return Status(llvm::make_error<PythonException>("Write"));

It doesn't look like adding this should be too hard (including the template-based computation of the python signature), but it could go a long way towards improving the readability, and it might actually fix other issues too -- it seems that the PythonDataObjects currently completely ignore the PyErr_Occurred stuff, which seems to be wrong, and it could actually be the reason why @amccarth was unable to run the test suite with a debug python on windows. Improving that, even if it's just for the stuff that's needed for your work would be really helpful, as it would provide a pattern on how to fix the remaining issues.

I'm sorry that I'm bringing this up so late in the review, but I am also learning this stuff as we go along -- unfortunately, I don't think we have anyone really knowledgeable about the python stuff still active in the lldb community..

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

1162

Maybe this GIL-talking can be done inside IsPythonSideValid (instead of two IsValid methods)?

1194

extra semicolon here too

1366

This is an example where _I think_ you may be using PyErr_Occurred incorrectly. If python really asserts that PyErr_Occurred is always called, then this is not right, as you will not call it if the result is null. There are a couple of other examples like that in the code too.

[lawrence_danna]

In D68188#1694276, @labath wrote:

Most of your interactions seem to be about calling methods. Would it be possible to add a PythonDataObject wrapper for this (and any other frequently used python API)? I'm hoping that we could have something like...

I think this is a good idea, but there's a lot of places in PythonDataObjects that need to be fixed to check for exceptions and return Expected<>. I really think it ought to be done in a separate patch.

[lawrence_danna]

minor fixes

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

1366

I think it is correct.

Python doesn't assert that PyErr_Occured is called, it asserts that the error is cleared before you call back in to another thing that can cause an error. PythonException will clear the error if there is one, or say "unknown error" if there isn't one (which should never happen if utf8==NULL, but i'm checking out of abundance of caution)

[labath]

I agree about the separate patch stuff, but it seems to be that this should be done before this one. After all, all (most?) of the existing code has already been DataObject-ized and this patch is the thing that's deviating from that practice. I don't think you should rewrite all of the existing code -- hopefully we can find a way to fit this in there. For instance, the CallMethod thingy would be a new api, so we can have that return expected independently of the other APIs. I guess the main part you will need is (and that already existing in the non-Expected form) is the type conversions, but maybe there we can introduce a new kind of conversion, which is more strict in checking for errors. Or possibly have the current form return Expected<T>, but then have a helper function to suppress those errors and return a default-constructed T() for legacy uses (so legacy code would be sth like suppressError(object.AsType<PythonInteger>()))

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

1366

Ok, thanks for the explanation.

[lawrence_danna]

better integration with PythonObject

[lawrence_danna]

In D68188#1695069, @labath wrote:

I agree about the separate patch stuff, but it seems to be that this should be done before this one. After all, all (most?) of the existing code has already been DataObject-ized and this patch is the thing that's deviating from that practice. I don't think you should rewrite all of the existing code -- hopefully we can find a way to fit this in there.

Yea... I tried updating all the PythonDataObjects code to use Errors and Expected<> and it wound up being just way too big of a patch.

I've updated this patch to add a giant warning at the top of PythonDataObjects.h about the dangers, as well adding just enough Expected<> support to do what I need for this patch.

How's it look now?

[lawrence_danna]

spelling

[lawrence_danna]

comment

[lawrence_danna]

I changed my mind about splitting this into another patch. When I first said that I had in mind a much more extensive rewrite of the PythonDataObjects. I think it's fine now as one patch -- though we should still go back later add Expected<> everywhere in PythonDataObjects.

[lawrence_danna]

python2 const fix

[lawrence_danna]

fix typemaps to deal with None correctly

[lawrence_danna]

rebased

[lawrence_danna]

add safe PythonModule functions and use them

[lawrence_danna]

split off generic exception handling stuff into a separate patch

[lawrence_danna]

rebased

[lawrence_danna]

@labath anything else for this one?

[lawrence_danna]

rebased

[lawrence_danna]

rebased

[labath]

I think this is looking pretty good. I like how the read/write methods look now. Just one more question about the PythonBuffer class.

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

1129–1136

Storing the Error as a member is quite unusual. It would be better to have a factory function which returns Expected<PythonBuffer>.

[lawrence_danna]

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

1129–1136

Oh yea I’ll fix that

[lawrence_danna]

fixed pybuffer error handling weirdness

[lawrence_danna]

fixed

[labath]

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

1155

I would slightly prefer if the PyObject_GetBuffer happened in the Create function (and the constructor took an already-constructed Py_buffer). That way, the call is next to the exception check and you avoid creating an "invalid" PythonBuffer object, even as an implementation detail.

[lawrence_danna]

Pavel would slightly prefer if the PyObject_GetBuffer happened in the Create function

[jankratochvil]

Fixed now by rL374322, there was a regression - Linux Fedora 30 x86_64:

======================================================================
ERROR: test_exceptions (TestFileHandle.FileHandleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jkratoch/redhat/llvm-monorepo2/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py", line 483, in test_exceptions
    self.assertRaises(TypeError, lldb.SBFile, None)
  File "/home/jkratoch/redhat/llvm-monorepo2/lldb/third_party/Python/module/unittest2/unittest2/case.py", line 534, in assertRaises
    callableObj(*args, **kwargs)
  File "/home/jkratoch/redhat/llvm-monorepo2-clangassert/lib/python2.7/site-packages/lldb/__init__.py", line 5334, in __init__
    this = _lldb.new_SBFile(*args)
NotImplementedError: Wrong number or type of arguments for overloaded function 'new_SBFile'.
  Possible C/C++ prototypes are:
    lldb::SBFile::SBFile()
    lldb::SBFile::SBFile(int,char const *,bool)
    lldb::SBFile::SBFile(lldb::FileSP)

[labath]

Yeah, as Jan mentions, I've had to make some tweaks (r374331, r374322, r374299) in order to get this passing on all python and swig versions. Let me know if you have questions/issues/etc about any of those.

lldb/include/lldb/API/SBFile.h

 //===-- SBFile.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SBFile_h_
 #define LLDB_SBFile_h_
 
 #include "lldb/API/SBDefines.h"
 
 namespace lldb {
 
+/* These tags make no difference at the c++ level, but
+ * when the constructors are called from python they control
+ * how python files are converted by SWIG into FileSP */
+struct FileBorrow {};
+struct FileForceScriptingIO {};
+struct FileBorrowAndForceScriptingIO {};
+
 class LLDB_API SBFile {
   friend class SBDebugger;
 
 public:
   SBFile();
+  SBFile(FileSP file_sp);
+  SBFile(FileBorrow, FileSP file_sp);
+  SBFile(FileForceScriptingIO, FileSP file_sp);
+  SBFile(FileBorrowAndForceScriptingIO, FileSP file_sp);
   SBFile(FILE *file, bool transfer_ownership);
   SBFile(int fd, const char *mode, bool transfer_ownership);
   ~SBFile();
 
   SBError Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read);
   SBError Write(const uint8_t *buf, size_t num_bytes, size_t *bytes_written);
   SBError Flush();
   bool IsValid() const;
   SBError Close();
 
   operator bool() const;
   bool operator!() const;
 
 private:
   FileSP m_opaque_sp;
-  SBFile(FileSP file_sp);
 };
 
 } // namespace lldb
 
 #endif // LLDB_SBFile_h_

lldb/include/lldb/Host/File.h

protected:
   LazyBool m_supports_colors;
 
   void CalculateInteractiveAndTerminal();
 
 private:
   DISALLOW_COPY_AND_ASSIGN(File);
 };
 
+// This is a CRTP mixin that just delegates all the virtual functions
+// to another File.
+template <typename T, typename B> class DelegatingFile : public B {
+public:
+  template <typename... Args> DelegatingFile(Args... args) : B(args...){};
+
+  ~DelegatingFile() override { Close(); };
+
+  Status Read(void *buf, size_t &num_bytes) override {
+    return getDelegate().Read(buf, num_bytes);
+  }
+  Status Write(const void *buf, size_t &num_bytes) override {
+    return getDelegate().Write(buf, num_bytes);
+  }
+  bool IsValid() const override { return getDelegate().IsValid(); }
+  Status Close() override { return getDelegate().Close(); }
+  IOObject::WaitableHandle GetWaitableHandle() override {
+    return getDelegate().GetWaitableHandle();
+  }
+  Status GetFileSpec(FileSpec &file_spec) const override {
+    return getDelegate().GetFileSpec(file_spec);
+  }
+  FILE *TakeStreamAndClear() override {
+    return getDelegate().TakeStreamAndClear();
+  }
+  int GetDescriptor() const override { return getDelegate().GetDescriptor(); }
+  FILE *GetStream() override { return getDelegate().GetStream(); }
+  off_t SeekFromStart(off_t offset, Status *error_ptr = nullptr) override {
+    return getDelegate().SeekFromStart(offset, error_ptr);
+  }
+  off_t SeekFromCurrent(off_t offset, Status *error_ptr = nullptr) override {
+    return getDelegate().SeekFromCurrent(offset, error_ptr);
+  }
+  off_t SeekFromEnd(off_t offset, Status *error_ptr = nullptr) override {
+    return getDelegate().SeekFromEnd(offset, error_ptr);
+  }
+  Status Read(void *dst, size_t &num_bytes, off_t &offset) override {
+    return getDelegate().Read(dst, num_bytes, offset);
+  }
+  Status Write(const void *src, size_t &num_bytes, off_t &offset) override {
+    return getDelegate().Write(src, num_bytes, offset);
+  }
+  Status Flush() override { return getDelegate().Flush(); }
+  Status Sync() override { return getDelegate().Sync(); }
+  size_t PrintfVarArg(const char *format, va_list args) override {
+    return getDelegate().PrintfVarArg(format, args);
+  }
+
+protected:
+  DelegatingFile() {}
+  File &getDelegate() { return static_cast<T *>(this)->getDelegate(); }
+  const File &getDelegate() const {
+    return static_cast<const T *>(this)->getDelegate();
+  }
+};
+
 class NativeFile : public File {
 public:
   NativeFile()
       : m_descriptor(kInvalidDescriptor), m_own_descriptor(false),
         m_stream(kInvalidStream), m_options(0), m_own_stream(false) {}
 
   NativeFile(FILE *fh, bool transfer_ownership)
       : m_descriptor(kInvalidDescriptor), m_own_descriptor(false), m_stream(fh),

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

 import sys
 from contextlib import contextmanager
 
 import lldb
 from lldbsuite.test import  lldbtest
 from lldbsuite.test.decorators import (
     add_test_categories, no_debug_info_test, skipIf)
 
+class OhNoe(Exception):
+    pass
+
+class BadIO(io.TextIOBase):
+    def writable(self):
+        return True
+    def readable(self):
+        return True
+    def write(self, s):
+        raise OhNoe('OH NOE')
+    def read(self, n):
+        raise OhNoe("OH NOE")
+
+# This class will raise an exception while it's being
+# converted into a C++ object by swig
+class ReallyBadIO(io.TextIOBase):
+    def fileno(self):
+        return 999
+    def writable(self):
+        raise OhNoe("OH NOE!!!")
+
+class MutableBool():
+    def __init__(self, value):
+        self.value = value
+    def set(self, value):
+        self.value = bool(value)
+    def __bool__(self):
+        return self.value
 
 @contextmanager
 def replace_stdout(new):
     old = sys.stdout
     sys.stdout = new
     try:
         yield
     finally:
def test_sbfile_type_errors(self):
         self.assertRaises(TypeError, sbf.Read, None)
         self.assertRaises(TypeError, sbf.Read, b'this bytes is not mutable')
         self.assertRaises(TypeError, sbf.Write, u"ham sandwich")
         self.assertRaises(TypeError, sbf.Read, u"ham sandwich")
 
 
     @add_test_categories(['pyapi'])
     @no_debug_info_test
-    def test_sbfile_write(self):
+    def test_sbfile_write_fileno(self):
         with open(self.out_filename, 'w') as f:
             sbf = lldb.SBFile(f.fileno(), "w", False)
             self.assertTrue(sbf.IsValid())
             e, n = sbf.Write(b'FOO\nBAR')
             self.assertTrue(e.Success())
             self.assertEqual(n, 7)
             sbf.Close()
             self.assertFalse(sbf.IsValid())
         with open(self.out_filename, 'r') as f:
             self.assertEqual(readStrippedLines(f), ['FOO', 'BAR'])
 
 
     @add_test_categories(['pyapi'])
     @no_debug_info_test
-    def test_sbfile_read(self):
+    def test_sbfile_write(self):
+        with open(self.out_filename, 'w') as f:
+            sbf = lldb.SBFile(f)
+            e, n = sbf.Write(b'FOO\n')
+            self.assertTrue(e.Success())
+            self.assertEqual(n, 4)
+            sbf.Close()
+            self.assertTrue(f.closed)
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(f.read().strip(), 'FOO')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_sbfile_read_fileno(self):
         with open(self.out_filename, 'w') as f:
             f.write('FOO')
         with open(self.out_filename, 'r') as f:
             sbf = lldb.SBFile(f.fileno(), "r", False)
             self.assertTrue(sbf.IsValid())
             buffer = bytearray(100)
             e, n = sbf.Read(buffer)
             self.assertTrue(e.Success())
             self.assertEqual(buffer[:n], b'FOO')
 
 
     @add_test_categories(['pyapi'])
     @no_debug_info_test
+    def test_sbfile_read(self):
+        with open(self.out_filename, 'w') as f:
+            f.write('foo')
+        with open(self.out_filename, 'r') as f:
+            sbf = lldb.SBFile(f)
+            buf = bytearray(100)
+            e, n = sbf.Read(buf)
+            self.assertTrue(e.Success())
+            self.assertEqual(n, 3)
+            self.assertEqual(buf[:n], b'foo')
+            sbf.Close()
+            self.assertTrue(f.closed)
+
+
+    @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')
 
class FileHandleTestCase(lldbtest.TestBase):
     @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)
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_sbfile_write_borrowed(self):
+        with open(self.out_filename, 'w') as f:
+            sbf = lldb.SBFile(lldb.FileBorrow(), f)
+            e, n = sbf.Write(b'FOO')
+            self.assertTrue(e.Success())
+            self.assertEqual(n, 3)
+            sbf.Close()
+            self.assertFalse(f.closed)
+            f.write('BAR\n')
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(f.read().strip(), 'FOOBAR')
+
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_write_forced(self):
+        with open(self.out_filename, 'w') as f:
+            written = MutableBool(False)
+            orig_write = f.write
+            def mywrite(x):
+                written.set(True)
+                return orig_write(x)
+            f.write = mywrite
+            sbf = lldb.SBFile(lldb.FileForceScriptingIO(), f)
+            e, n = sbf.Write(b'FOO')
+            self.assertTrue(written)
+            self.assertTrue(e.Success())
+            self.assertEqual(n, 3)
+            sbf.Close()
+            self.assertTrue(f.closed)
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(f.read().strip(), 'FOO')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_write_forced_borrowed(self):
+        with open(self.out_filename, 'w') as f:
+            written = MutableBool(False)
+            orig_write = f.write
+            def mywrite(x):
+                written.set(True)
+                return orig_write(x)
+            f.write = mywrite
+            sbf = lldb.SBFile(lldb.FileBorrowAndForceScriptingIO(), f)
+            e, n = sbf.Write(b'FOO')
+            self.assertTrue(written)
+            self.assertTrue(e.Success())
+            self.assertEqual(n, 3)
+            sbf.Close()
+            self.assertFalse(f.closed)
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(f.read().strip(), 'FOO')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_write_string(self):
+        f = io.StringIO()
+        sbf = lldb.SBFile(f)
+        e, n = sbf.Write(b'FOO')
+        self.assertEqual(f.getvalue().strip(), "FOO")
+        self.assertTrue(e.Success())
+        self.assertEqual(n, 3)
+        sbf.Close()
+        self.assertTrue(f.closed)
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_write_bytes(self):
+        f = io.BytesIO()
+        sbf = lldb.SBFile(f)
+        e, n = sbf.Write(b'FOO')
+        self.assertEqual(f.getvalue().strip(), b"FOO")
+        self.assertTrue(e.Success())
+        self.assertEqual(n, 3)
+        sbf.Close()
+        self.assertTrue(f.closed)
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_read_string(self):
+        f = io.StringIO('zork')
+        sbf = lldb.SBFile(f)
+        buf = bytearray(100)
+        e, n = sbf.Read(buf)
+        self.assertTrue(e.Success())
+        self.assertEqual(buf[:n], b'zork')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_read_string_one_byte(self):
+        f = io.StringIO('z')
+        sbf = lldb.SBFile(f)
+        buf = bytearray(1)
+        e, n = sbf.Read(buf)
+        self.assertTrue(e.Fail())
+        self.assertEqual(n, 0)
+        self.assertEqual(e.GetCString(), "can't read less than 6 bytes from a utf8 text stream")
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_read_bytes(self):
+        f = io.BytesIO(b'zork')
+        sbf = lldb.SBFile(f)
+        buf = bytearray(100)
+        e, n = sbf.Read(buf)
+        self.assertTrue(e.Success())
+        self.assertEqual(buf[:n], b'zork')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    @skipIf(py_version=['<', (3,)])
+    def test_sbfile_out(self):
+        with open(self.out_filename, 'w') as f:
+            sbf = lldb.SBFile(f)
+            status = self.debugger.SetOutputFile(sbf)
+            if status.Fail():
+                raise Exception(status)
+            self.handleCmd('script 2+2')
+        with open(self.out_filename, 'r') as f:
+            self.assertEqual(f.read().strip(), '4')
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_sbfile_error(self):
+        with open(self.out_filename, 'w') as f:
+            sbf = lldb.SBFile(f)
+            status = self.debugger.SetErrorFile(sbf)
+            if status.Fail():
+                raise Exception(status)
+            self.handleCmd('lolwut', check=False, collect_result=False)
+        with open(self.out_filename, 'r') as f:
+            errors = f.read()
+            self.assertTrue(re.search(r'error:.*lolwut', errors))
+
+
+    @add_test_categories(['pyapi'])
+    @no_debug_info_test
+    def test_exceptions(self):
+        self.assertRaises(TypeError, lldb.SBFile, None)
+        self.assertRaises(TypeError, lldb.SBFile, "ham sandwich")
+        if sys.version_info[0] < 3:
+            self.assertRaises(TypeError, lldb.SBFile, ReallyBadIO())
+        else:
+            self.assertRaises(OhNoe, lldb.SBFile, ReallyBadIO())
+            error, n = lldb.SBFile(BadIO()).Write(b"FOO")
+            self.assertEqual(n, 0)
+            self.assertTrue(error.Fail())
+            self.assertEqual(error.GetCString(), "OhNoe('OH NOE')")
+            error, n = lldb.SBFile(BadIO()).Read(bytearray(100))
+            self.assertEqual(n, 0)
+            self.assertTrue(error.Fail())
+            self.assertEqual(error.GetCString(), "OhNoe('OH NOE')")

lldb/scripts/Python/python-typemaps.swig

%typemap(in) (lldb::LogOutputCallback log_callback, void *baton) {
   $2 = $input;
 }
 
 %typemap(typecheck) (lldb::LogOutputCallback log_callback, void *baton) {
   $1 = $input == Py_None;
   $1 = $1 || PyCallable_Check(reinterpret_cast<PyObject*>($input));
 }
 
+
+%typemap(in) lldb::FileSP {
+  using namespace lldb_private;
+  if (PythonFile::Check($input)) {
+    PythonFile py_file(PyRefType::Borrowed, $input);
+    auto sp = unwrapOrSetPythonException(py_file.ConvertToFile());
+    if (sp)
+      $1 = sp;
+    else
+      return nullptr;
+  }
+}
+
+%typemap(in) lldb::FileSP FORCE_IO_METHODS {
+  using namespace lldb_private;
+  if (PythonFile::Check($input)) {
+    PythonFile py_file(PyRefType::Borrowed, $input);
+    auto sp = unwrapOrSetPythonException(py_file.ConvertToFileForcingUseOfScriptingIOMethods());
+    if (sp)
+      $1 = sp;
+    else
+      return nullptr;
+  }
+}
+
+%typemap(in) lldb::FileSP BORROWED {
+  using namespace lldb_private;
+  if (PythonFile::Check($input)) {
+    PythonFile py_file(PyRefType::Borrowed, $input);
+    auto sp = unwrapOrSetPythonException(py_file.ConvertToFile(/*borrowed=*/true));
+    if (sp)
+      $1 = sp;
+    else
+      return nullptr;
+  }
+}
+
+%typemap(in) lldb::FileSP BORROWED_FORCE_IO_METHODS {
+  using namespace lldb_private;
+  if (PythonFile::Check($input)) {
+    PythonFile py_file(PyRefType::Borrowed, $input);
+    auto sp = unwrapOrSetPythonException(py_file.ConvertToFileForcingUseOfScriptingIOMethods(/*borrowed=*/true));
+    if (sp)
+      $1 = sp;
+    else
+      return nullptr;
+  }
+}
+
+%typecheck(SWIG_TYPECHECK_POINTER) lldb::FileSP {
+  if (lldb_private::PythonFile::Check($input)) {
+    $1 = 1;
+  } else {
+    PyErr_Clear();
+    $1 = 0;
+  }
+}
+
+
 // FIXME both of these paths wind up calling fdopen() with no provision for ever calling
 // fclose() on the result.  SB interfaces that use FILE* should be deprecated for scripting
 // use and this typemap should eventually be removed.
 %typemap(in) FILE * {
    using namespace lldb_private;
    if ($input == Py_None)
       $1 = nullptr;
    else if (!lldb_private::PythonFile::Check($input)) {

lldb/scripts/interface/SBFile.i

 //===-- SWIG Interface for SBFile -----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 namespace lldb {
 
 %feature("docstring",
 "Represents a file."
 ) SBFile;
 
+struct FileBorrow {};
+struct FileForceScriptingIO {};
+struct FileBorrowAndForceScriptingIO {};
+
 class SBFile
 {
 public:
+
+
     SBFile();
+
+    %feature("docstring", "
+    Initialize a SBFile from a file descriptor.  mode is
+    'r', 'r+', or 'w', like fdopen.");
     SBFile(int fd, const char *mode, bool transfer_ownership);
 
+    %feature("docstring", "initialize a SBFile from a python file object");
+    SBFile(FileSP file);
+
+    %feature("docstring", "
+    Like SBFile(f), but the underlying file will
+    not be closed when the SBFile is closed or destroyed.");
+    SBFile(FileBorrow, FileSP BORROWED);
+
+    %feature("docstring" "
+    like SetFile(f), but the python read/write methods will be called even if
+    a file descriptor is available.");
+    SBFile(FileForceScriptingIO, FileSP FORCE_IO_METHODS);
+
+    %feature("docstring" "
+    like SetFile(f), but the python read/write methods will be called even
+    if a file descriptor is available -- and the underlying file will not
+    be closed when the SBFile is closed or destroyed.");
+    SBFile(FileBorrowAndForceScriptingIO, FileSP BORROWED_FORCE_IO_METHODS);
+
     ~SBFile ();
 
     %feature("autodoc", "Read(buffer) -> SBError, bytes_read") Read;
     SBError Read(uint8_t *buf, size_t num_bytes, size_t *OUTPUT);
 
     %feature("autodoc", "Write(buffer) -> SBError, written_read") Write;
     SBError Write(const uint8_t *buf, size_t num_bytes, size_t *OUTPUT);
 

lldb/source/API/SBFile.cpp

 #include "lldb/API/SBError.h"
 #include "lldb/Host/File.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBFile::~SBFile() {}
 
-SBFile::SBFile(FileSP file_sp) : m_opaque_sp(file_sp) {}
+SBFile::SBFile(FileSP file_sp) : m_opaque_sp(file_sp) {
+  LLDB_RECORD_DUMMY(void, SBfile, SBFile, (FileSP), file_sp);
+}
+
+// fixme
+SBFile::SBFile(FileBorrow, FileSP file_sp) : m_opaque_sp(file_sp){};
+SBFile::SBFile(FileForceScriptingIO, FileSP file_sp) : m_opaque_sp(file_sp){};
+SBFile::SBFile(FileBorrowAndForceScriptingIO, FileSP file_sp)
+    : m_opaque_sp(file_sp){};
 
 SBFile::SBFile() { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBFile); }
 
 SBFile::SBFile(FILE *file, bool transfer_ownership) {
   m_opaque_sp = NativeFile::make_shared(file, transfer_ownership);
 }
 
 SBFile::SBFile(int fd, const char *mode, bool transfer_owndership) {

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

public:
   PythonObject() : m_py_obj(nullptr) {}
 
   PythonObject(PyRefType type, PyObject *py_obj) : m_py_obj(nullptr) {
     Reset(type, py_obj);
   }
 
   PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &&rhs) {
+    m_py_obj = rhs.m_py_obj;
+    rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
     // Avoid calling the virtual method since it's not necessary
     // to actually validate the type of the PyObject if we're
     // just setting to null.
-    if (Py_IsInitialized())
-      Py_XDECREF(m_py_obj);
+    if (m_py_obj && Py_IsInitialized())
+      Py_DECREF(m_py_obj);
     m_py_obj = nullptr;
   }
 
   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();
if (Py_IsInitialized())
       Py_XDECREF(m_py_obj);
 
     m_py_obj = py_obj;
 
     // If this is a borrowed reference, we need to convert it to
     // an owned reference by incrementing it.  If it is an owned
     // reference (for example the caller allocated it with PyDict_New()
     // then we must *not* increment it.
-    if (Py_IsInitialized() && type == PyRefType::Borrowed)
+    if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
       Py_XINCREF(m_py_obj);
   }
 
   void Dump() const {
     if (m_py_obj)
       _PyObject_Dump(m_py_obj);
     else
       puts("NULL");
public:
   static bool Check(PyObject *py_obj);
 
   using PythonObject::Reset;
 
   void Reset(PyRefType type, PyObject *py_obj) override;
   void Reset(File &file, const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected<lldb::FileSP> ConvertToFile(bool borrowed = false);
+  llvm::Expected<lldb::FileSP>
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo<PythonException> {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template <typename T> T unwrapOrSetPythonException(llvm::Expected<T> expected) {
+  if (expected)
+    return expected.get();
+  llvm::handleAllErrors(
+      expected.takeError(), [](PythonException &E) { E.Restore(); },
+      [](const llvm::ErrorInfoBase &E) {
+        PyErr_SetString(PyExc_Exception, E.message().c_str());
+      });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
 
 #endif // LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H

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

 #else
 
 #include "PythonDataObjects.h"
 #include "ScriptInterpreterPython.h"
 
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Errno.h"
 
 #include <stdio.h>
 
 using namespace lldb_private;
 using namespace lldb;
 
+template <typename T> static T Take(PyObject *obj) {
+  return T(PyRefType::Owned, obj);
+}
+
+template <typename T> static T Retain(PyObject *obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 void StructuredPythonObject::Dump(Stream &s, bool pretty_print) const {
   s << "Python Obj: 0x" << GetValue();
 }
 
 // PythonObject
 
 void PythonObject::Dump(Stream &strm) const {
   if (m_py_obj) {
if (!py_obj)
     return false;
 #if PY_MAJOR_VERSION < 3
   return PyFile_Check(py_obj);
 #else
   // 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.
+  // inherits from `io.IOBase`.
   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.
FileUP PythonFile::GetUnderlyingFile() const {
   auto options = File::GetOptionsFromMode(py_mode.GetString());
   auto file = std::unique_ptr<File>(
       new NativeFile(PyObject_AsFileDescriptor(m_py_obj), options, false));
   if (!file->IsValid())
     return nullptr;
   return file;
 }
 
+class GIL {
+public:
+  GIL() { m_state = PyGILState_Ensure(); }
+  ~GIL() { PyGILState_Release(m_state); }
+
+protected:
+  PyGILState_STATE m_state;
+};
+
+const char *PythonException::toCString() const {
+  if (m_repr_bytes) {
+    return PyBytes_AS_STRING(m_repr_bytes);
+  } else {
+    return "unknown exception";
+  }
+}
+
+PythonException::PythonException(const char *caller) {
+  assert(PyErr_Occurred());
+  m_exception_type = m_exception = m_traceback = m_repr_bytes = NULL;
+  PyErr_Fetch(&m_exception_type, &m_exception, &m_traceback);
+  PyErr_NormalizeException(&m_exception_type, &m_exception, &m_traceback);
+  if (m_exception) {
+    PyObject *repr = PyObject_Repr(m_exception);
+    if (repr) {
+      m_repr_bytes = PyUnicode_AsEncodedString(repr, "utf-8", nullptr);
+      Py_XDECREF(repr);
+    }
+  }
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT);
+  if (log) {
+    log->Printf("%s failed with exception: %s", caller, toCString());
+  }
+  PyErr_Clear();
+}
+void PythonException::Restore() {
+  if (m_exception_type && m_exception) {
+    PyErr_Restore(m_exception_type, m_exception, m_traceback);
+  } else {
+    PyErr_SetString(PyExc_Exception, toCString());
+  }
+  m_exception_type = m_exception = m_traceback = NULL;
+}
+
+PythonException::~PythonException() {
+  Py_XDECREF(m_exception_type);
+  Py_XDECREF(m_exception);
+  Py_XDECREF(m_traceback);
+  Py_XDECREF(m_repr_bytes);
+}
+
+void PythonException::log(llvm::raw_ostream &OS) const { OS << toCString(); }
+
+std::error_code PythonException::convertToErrorCode() const {
+  return llvm::inconvertibleErrorCode();
+}
+
+char PythonException::ID = 0;
+
+llvm::Expected<uint32_t> GetOptionsForPyObject(PythonObject &obj) {
+  uint32_t options = 0;
+#if PY_MAJOR_VERSION >= 3
+  auto readable =
+      Take<PythonObject>(PyObject_CallMethod(obj.get(), "readable", "()"));
+  auto writable =
+      Take<PythonObject>(PyObject_CallMethod(obj.get(), "writable", "()"));
+  if (PyErr_Occurred()) {
+    return llvm::make_error<PythonException>("ConvertToFile");
+  }
+  if (PyObject_IsTrue(readable.get()))
+    options |= File::eOpenOptionRead;
+  if (PyObject_IsTrue(writable.get()))
+    options |= File::eOpenOptionWrite;
+#else
+  PythonString py_mode = obj.GetAttributeValue("mode").AsType<PythonString>();
+  options = File::GetOptionsFromMode(py_mode.GetString());
+#endif
+  return options;
+}
+
+// Abstract base class for python files.   All it knows how to do
+// is hold a reference to the python object and close or flush it
+// when the File is closed.
+class OwnedPythonFile : public File {
+public:
+  OwnedPythonFile(const PythonFile &file, bool borrowed)
+      : m_py_obj(file.get()), m_borrowed(borrowed) {
+    assert(m_py_obj);
+    Py_INCREF(m_py_obj);
+  }
+
+  ~OwnedPythonFile() override {
+    assert(m_py_obj);
+    GIL takeGIL;
+    Close();
+    Py_DECREF(m_py_obj);
+    m_py_obj = nullptr;
+  }
+
+  PythonFile GetPythonFile() { return Retain<PythonFile>(m_py_obj); }
+
+  bool IsValid() const override {
+    GIL takeGIL;
+    auto closed =
+        Take<PythonObject>(PyObject_GetAttrString(m_py_obj, "closed"));
+    if (!closed.IsValid() || PyErr_Occurred()) {
+      PyErr_Clear();
+      return false;
+    }
+    if (PyObject_IsTrue(closed.get())) {
+      return false;
+    }
+    return true;
+  }
+
+  Status Close() override {
+    assert(m_py_obj);
+    GIL takeGIL;
+    if (!OwnedPythonFile::IsValid())
+      return Status("invalid file");
+    if (m_borrowed)
+      PyObject_CallMethod(m_py_obj, "flush", "()");
+    else
+      PyObject_CallMethod(m_py_obj, "close", "()");
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Close"));
+    return Status();
+  };
+
+protected:
+  PyObject *m_py_obj;
+  bool m_borrowed;
+};
+
+// A SimplyPythonFile is a OwnedPythonFile that just delegates all I/O to a
+// NativeFile.
+class SimplePythonFile
+    : public DelegatingFile<SimplePythonFile, OwnedPythonFile> {
+public:
+  typedef DelegatingFile<SimplePythonFile, OwnedPythonFile> Base;
+
+  SimplePythonFile(int fd, uint32_t options, const PythonFile &file,
+                   bool borrowed)
+      : Base(file, borrowed), m_native_file(fd, options, false) {
+    Py_INCREF(m_py_obj);
+  }
+
+  ~SimplePythonFile() override { Close(); };
+
+  bool IsValid() const override {
+    return m_native_file.IsValid() && OwnedPythonFile::IsValid();
+  }
+
+  Status Close() override {
+    Status err1 = m_native_file.Close();
+    Status err2 = OwnedPythonFile::Close();
+    if (err2.Fail())
+      return err2;
+    return err1;
+  }
+
+  const File &getDelegate() const { return m_native_file; }
+
+  File &getDelegate() { return m_native_file; }
+
+protected:
+  NativeFile m_native_file;
+};
+
+llvm::Expected<FileSP> PythonFile::ConvertToFile(bool borrowed) {
+  if (!IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid PythonFile");
+
+  int fd = PyObject_AsFileDescriptor(m_py_obj);
+  if (fd < 0) {
+    return ConvertToFileForcingUseOfScriptingIOMethods(borrowed);
+  }
+  auto options = GetOptionsForPyObject(*this);
+  if (!options)
+    return options.takeError();
+
+  FileSP file_sp;
+  if (borrowed) {
+    // In this case we we don't need to retain the python
+    // object at all.
+    file_sp = NativeFile::make_shared(fd, options.get(), false);
+  } else {
+    file_sp = std::static_pointer_cast<File>(
+        std::make_shared<SimplePythonFile>(fd, options.get(), *this, borrowed));
+  }
+  if (!file_sp->IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid File");
+
+  return file_sp;
+}
+
+#if PY_MAJOR_VERSION >= 3
+
+class PythonBuffer {
+public:
+  PythonBuffer(PythonObject &obj, int flags = PyBUF_SIMPLE) : m_buffer({}) {
+    PyObject_GetBuffer(obj.get(), &m_buffer, flags);
+  }
+  ~PythonBuffer() {
+    if (m_buffer.obj) {
+      PyBuffer_Release(&m_buffer);
+    }
+  }
+  Py_buffer &get() { return m_buffer; }
+
+protected:
+  Py_buffer m_buffer;
+};
+
+class BinaryPythonFile : public OwnedPythonFile {
+  friend class PythonFile;
+
+protected:
+  int m_descriptor;
+
+public:
+  BinaryPythonFile(int fd, const PythonFile &file, bool borrowed)
+      : OwnedPythonFile(file, borrowed),
+        m_descriptor(File::DescriptorIsValid(fd) ? fd
+                                                 : File::kInvalidDescriptor) {}
+
+  int GetDescriptor() const override { return m_descriptor; }
+
+  Status Write(const void *buf, size_t &num_bytes) override {
+    GIL takeGIL;
+    auto pybuffer = Take<PythonObject>(PyMemoryView_FromMemory(
+        const_cast<char *>((const char *)buf), num_bytes, PyBUF_READ));
+    num_bytes = 0;
+    auto bytes_written = Take<PythonObject>(
+        PyObject_CallMethod(m_py_obj, "write", "(O)", pybuffer.get()));
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Write"));
+    long l_bytes_written = PyLong_AsLong(bytes_written.get());
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Write"));
+    num_bytes = l_bytes_written;
+    if (l_bytes_written < 0 || (unsigned long)l_bytes_written != num_bytes) {
+      return Status("overflow");
+    }
+    return Status();
+  }
+
+  Status Read(void *buf, size_t &num_bytes) override {
+    GIL takeGIL;
+    auto pybuffer_obj = Take<PythonObject>(PyObject_CallMethod(
+        m_py_obj, "read", "(L)", (unsigned long long)num_bytes));
+    num_bytes = 0;
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Read"));
+    if (pybuffer_obj.IsNone()) {
+      // EOF
+      num_bytes = 0;
+      return Status();
+    }
+    PythonBuffer pybuffer(pybuffer_obj);
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Read"));
+    memcpy(buf, pybuffer.get().buf, pybuffer.get().len);
+    num_bytes = pybuffer.get().len;
+    return Status();
+  }
+
+  Status Flush() override {
+    GIL takeGIL;
+    PyErr_Clear();
+    PyObject_CallMethod(m_py_obj, "flush", "()");
+    Status error;
+    if (PyErr_Occurred())
+      error = llvm::make_error<PythonException>("Flush");
+    return error;
+  }
+};
+
+class TextPythonFile : public OwnedPythonFile {
+  friend class PythonFile;
+
+protected:
+  int m_descriptor;
+
+public:
+  TextPythonFile(int fd, const PythonFile &file, bool borrowed)
+      : OwnedPythonFile(file, borrowed),
+        m_descriptor(File::DescriptorIsValid(fd) ? fd
+                                                 : File::kInvalidDescriptor) {}
+
+  int GetDescriptor() const override { return m_descriptor; }
+
+  Status Write(const void *buf, size_t &num_bytes) override {
+    GIL takeGIL;
+    auto pystring = Take<PythonObject>(
+        PyUnicode_FromStringAndSize((const char *)buf, num_bytes));
+    num_bytes = 0;
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Write"));
+    auto bytes_written = Take<PythonObject>(
+        PyObject_CallMethod(m_py_obj, "write", "(O)", pystring.get()));
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Write"));
+    long l_bytes_written = PyLong_AsLong(bytes_written.get());
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Write"));
+    num_bytes = l_bytes_written;
+    if (l_bytes_written < 0 || (unsigned long)l_bytes_written != num_bytes) {
+      return Status("overflow");
+    }
+    return Status();
+  }
+
+  Status Read(void *buf, size_t &num_bytes) override {
+    GIL takeGIL;
+    size_t num_chars = num_bytes / 6;
+    size_t orig_num_bytes = num_bytes;
+    num_bytes = 0;
+    if (orig_num_bytes < 6) {
+      return Status("can't read less than 6 bytes from a utf8 text stream");
+    }
+    auto pystring = Take<PythonObject>(PyObject_CallMethod(
+        m_py_obj, "read", "(L)", (unsigned long long)num_chars));
+    if (pystring.IsNone()) {
+      // EOF
+      return Status();
+    }
+    if (PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Read"));
+    if (!PyUnicode_Check(pystring.get()))
+      return Status("read() didn't return a str");
+    Py_ssize_t size;
+    const char *utf8 = PyUnicode_AsUTF8AndSize(pystring.get(), &size);
+    if (!utf8 || PyErr_Occurred())
+      return Status(llvm::make_error<PythonException>("Read"));
+    assert(size >= 0 && (size_t)size <= orig_num_bytes);
+    memcpy(buf, utf8, size);
+    num_bytes = size;
+    return Status();
+  }
+
+  Status Flush() override {
+    GIL takeGIL;
+    PyErr_Clear();
+    PyObject_CallMethod(m_py_obj, "flush", "()");
+    Status error;
+    if (PyErr_Occurred())
+      error = llvm::make_error<PythonException>("Flush");
+    return error;
+  }
+};
+
+#endif
+
+llvm::Expected<FileSP>
+PythonFile::ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed) {
+  if (!IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid PythonFile");
+
+#if PY_MAJOR_VERSION < 3
+
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "not supported on python 2");
+
+#else
+
+  int fd = PyObject_AsFileDescriptor(m_py_obj);
+  if (fd < 0) {
+    PyErr_Clear();
+    fd = File::kInvalidDescriptor;
+  }
+
+  auto io_module = Take<PythonObject>(PyImport_ImportModule("io"));
+  auto io_dict = Retain<PythonDictionary>(PyModule_GetDict(io_module.get()));
+  auto textIOBase = io_dict.GetItemForKey(PythonString("TextIOBase"));
+  auto rawIOBase = io_dict.GetItemForKey(PythonString("BufferedIOBase"));
+  auto bufferedIOBase = io_dict.GetItemForKey(PythonString("RawIOBase"));
+
+  FileSP file_sp;
+  if (PyObject_IsInstance(m_py_obj, textIOBase.get())) {
+    file_sp = std::static_pointer_cast<File>(
+        std::make_shared<TextPythonFile>(fd, *this, borrowed));
+  } else if (PyObject_IsInstance(m_py_obj, rawIOBase.get()) ||
+             PyObject_IsInstance(m_py_obj, bufferedIOBase.get())) {
+    file_sp = std::static_pointer_cast<File>(
+        std::make_shared<BinaryPythonFile>(fd, *this, borrowed));
+  } else
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "python file is neither text nor binary");
+
+  if (!file_sp->IsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid File");
+
+  return file_sp;
+
+#endif
+}
+
 #endif