exception handling in PythonDataObjects.

[lawrence_danna]

Summary

Python APIs nearly all can return an exception. They do this
by returning NULL, or -1, or some such value and setting
the exception state with PyErr_Set*(). Exceptions must be
handled before further python API functions are called. Failure
to do so will result in asserts on debug builds of python.
It will also sometimes, but not usually result in crashes of
release builds.

Nearly everything in PythonDataObjects.h needs to be updated
to account for this. This patch doesn't fix everything,
but it does introduce some new methods using Expected<>
return types that are safe to use.

split off from https://reviews.llvm.org/D68188

[labath]

I like the direction this is going in. Some questions about the implementation/interface inline..

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

44–45

Please remove this. It kind of looks like this using declarations are local to this block, but they are not, and you are exposing these two new identifiers to anyone who happens to include this file. I am actually a proponent of importing these identifiers (and several others) into lldb_private, but that should be discussed separately. I started a discussion about that several months ago http://lists.llvm.org/pipermail/lldb-dev/2019-April/014978.html, but there wasn't universal consensus, so it kind of fizzled out.

Feel free to to restart that thread and/or to put these directives in a cpp file though.

101–107

What should be the behavior of these methods if obj is null? And what if obj is not of type T ? Right now, they will return an "empty" object, but this is the old behavior of the data objects classes (which was implemented back when we did not have Expected<T>), and I am not sure it is the right thing to do here. For the nullptr case, maybe we could handle that via an assertion (or even making these function take a PyObject& argument)? And a type mismatch seems like it could be best handled by returning an error instead of an empty object.

(Also, these functions shouldn't be static.)

101–119

Are there any cases where it is valid to "take" an object even if an exception has occured. Should we maybe move the "assert" into the previous two functions and delete this one? (From the name it does not seem too clear what is this function asserting, so it would be best to remove it altogether).

160

I think we should discourage people from passing these objects into the native APIs, so forcing them to use the existing get() methods seems sufficient to me.

282

explicit

303

Implemented like this, this method still requires one to work with python types directly, and deal with things (signatures) that is better left to the glue code. As this is c++, what do you think of an implementation like:

template<typename T, typename Enable = void> struct PythonFormat;
template<> struct PythonFormat<unsigned long long> {
    static constexpr char format = 'K';
    static auto get(unsigned long long value) { return value; }
};
template<typename T>
struct PythonFormat<T,
        typename std::enable_if<
            std::is_base_of<PythonObject ,T>::value>::type> {
    static constexpr char format = 'O';
    static auto get(const T &value) { return value.get(); }
};
// etc.

template<typename... T>
Expected<PythonObject> CallMethod(const char *name, const T &... t) {
    const char format[] = { '(', PythonEncoding<T>::format..., ')'};
    PyObject *obj = PyObject_CallMethod(m_py_obj, name, format, PythonFormat<T>::get(t)...);
    ...
}

This should make calling a python method as close to calling a native one as possible. The main downside of that is that it is impossible to use fancier formats like s#. If we really wanted to, we could make that work too (at the expense of a fairly large increase in template complexity), but it doesn't look like you need to call any fancy method now anyway.

336–344

Should this maybe be a specialization of AsType for T = long long ? That might reduce your desire for monads...

346

I guess the RHS should by a PythonObject too..

371

Are you sure this std::move actually does anything -- I see no applicable rvalue constructor

433

this is not needed. const char * is implicitly convertible to a StringRef

710

Add a comment about when should this be used. I guess it should be only done as the last thing before returning to python (?)

[lawrence_danna]

review fixes

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

303

oh, yea i like that idea.

371

It doesn't do anything yet, but it's conceptually correct and it will do something when refactor the PythonObject classes to get rid of the virtual methods.

[lawrence_danna]

review fix

[labath]

I am quite happy about how this is turning out to be. Thanks for taking your time to do this.

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

36–53

llvm style is to not put else after a return http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return. Also, for short statements like this, I'd omit braces. And lastly, in line with the "keep namespaces small" philosophy I think you should define these as template<> ... python::As<long long>(...) :)

1100–1104

no else after return, no braces

[lawrence_danna]

style fixes

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

 //===-- PythonDataObjects.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
 //
 //===----------------------------------------------------------------------===//
 
+//
+// !! FIXME FIXME FIXME !!
+//
+// Python APIs nearly all can return an exception.   They do this
+// by returning NULL, or -1, or some such value and setting
+// the exception state with PyErr_Set*().   Exceptions must be
+// handled before further python API functions are called.   Failure
+// to do so will result in asserts on debug builds of python.
+// It will also sometimes, but not usually result in crashes of
+// release builds.
+//
+// Nearly all the code in this header does not handle python exceptions
+// correctly.  It should all be converted to return Expected<> or
+// Error types to capture the exception.
+//
+// Everything in this file except functions that return Error or
+// Expected<> is considered deprecated and should not be
+// used in new code.  If you need to use it, fix it first.
+//
+
 #ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
 #define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_PYTHONDATAOBJECTS_H
 
 #ifndef LLDB_DISABLE_PYTHON
 
 // LLDB Python header must be included first
 #include "lldb-python.h"
 
 #include "lldb/Host/File.h"
 #include "lldb/Utility/StructuredData.h"
 
 #include "llvm/ADT/ArrayRef.h"
 
 namespace lldb_private {
 
+class PythonObject;
 class PythonBytes;
 class PythonString;
 class PythonList;
 class PythonDictionary;
 class PythonInteger;
+class PythonException;
 
 class StructuredPythonObject : public StructuredData::Generic {
 public:
   StructuredPythonObject() : StructuredData::Generic() {}
 
   StructuredPythonObject(void *obj) : StructuredData::Generic(obj) {
     Py_XINCREF(GetValue());
   }

 
 enum class PyRefType {
   Borrowed, // We are not given ownership of the incoming PyObject.
             // We cannot safely hold it without calling Py_INCREF.
   Owned     // We have ownership of the incoming PyObject.  We should
             // not call Py_INCREF.
 };
 
+namespace python {
+
+// Take a reference that you already own, and turn it into
+// a PythonObject.
+//
+// Most python API methods will return a +1 reference
+// if they succeed or NULL if and only if
+// they set an exception.   Use this to collect such return
+// values, after checking for NULL.
+//
+// If T is not just PythonObject, then obj must be already be
+// checked to be of the correct type.
+template <typename T> T Take(PyObject *obj) {
+  assert(obj);
+  assert(!PyErr_Occurred());
+  T thing(PyRefType::Owned, obj);
+  assert(thing.IsValid());
+  return std::move(thing);
+}
+
+// Retain a reference you have borrowed, and turn it into
+// a PythonObject.
+//
+// A minority of python APIs return a borrowed reference
+// instead of a +1.   They will also return NULL if and only
+// if they set an exception.   Use this to collect such return
+// values, after checking for NULL.
+//
+// If T is not just PythonObject, then obj must be already be
+// checked to be of the correct type.
+template <typename T> T Retain(PyObject *obj) {
+  assert(obj);
+  assert(!PyErr_Occurred());
+  T thing(PyRefType::Borrowed, obj);
+  assert(thing.IsValid());
+  return std::move(thing);
+}
+
+} // namespace python
+
 enum class PyInitialValue { Invalid, Empty };
 
+template <typename T, typename Enable = void> struct PythonFormat;
+
+template <> struct PythonFormat<unsigned long long> {
+  static constexpr char format = 'K';
+  static auto get(unsigned long long value) { return value; }
+};
+
+template <> struct PythonFormat<long long> {
+  static constexpr char format = 'L';
+  static auto get(long long value) { return value; }
+};
+
+template <typename T>
+struct PythonFormat<
+    T, typename std::enable_if<std::is_base_of<PythonObject, T>::value>::type> {
+  static constexpr char format = 'O';
+  static auto get(const T &value) { return value.get(); }
+};
+
 class PythonObject {
 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();
     else
       Reset(PyRefType::Borrowed, rhs.m_py_obj);
   }
 
   // PythonObject is implicitly convertible to PyObject *, which will call the
   // wrong overload.  We want to explicitly disallow this, since a PyObject
   // *always* owns its reference.  Therefore the overload which takes a
   // PyRefType doesn't make sense, and the copy constructor should be used.
   void Reset(PyRefType type, const PythonObject &ref) = delete;
 
+  // FIXME We shouldn't have virtual anything.  PythonObject should be a
+  // strictly pass-by-value type.
   virtual void Reset(PyRefType type, PyObject *py_obj) {
     if (py_obj == m_py_obj)
       return;
 
     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");
PyObject *release() {
     return result;
   }
 
   PythonObject &operator=(const PythonObject &other) {
     Reset(PyRefType::Borrowed, other.get());
     return *this;
   }
 
+  void Reset(PythonObject &&other) {
+    Reset();
+    m_py_obj = other.m_py_obj;
+    other.m_py_obj = nullptr;
+  }
+
+  PythonObject &operator=(PythonObject &&other) {
+    Reset(std::move(other));
+    return *this;
+  }
+
   PyObjectType GetObjectType() const;
 
   PythonString Repr() const;
 
   PythonString Str() const;
 
   static PythonObject ResolveNameWithDictionary(llvm::StringRef name,
                                                 const PythonDictionary &dict);
public:
   template <typename T> T ResolveName(llvm::StringRef name) const {
     return ResolveName(name).AsType<T>();
   }
 
   bool HasAttribute(llvm::StringRef attribute) const;
 
   PythonObject GetAttributeValue(llvm::StringRef attribute) const;
 
-  bool IsValid() const;
+  bool IsNone() const { return m_py_obj == Py_None; }
 
-  bool IsAllocated() const;
+  bool IsValid() const { return m_py_obj != nullptr; }
 
-  bool IsNone() const;
+  bool IsAllocated() const { return IsValid() && !IsNone(); }
+
+  explicit operator bool() const { return IsValid() && !IsNone(); }
 
   template <typename T> T AsType() const {
     if (!T::Check(m_py_obj))
       return T();
     return T(PyRefType::Borrowed, m_py_obj);
   }
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
 protected:
+  static llvm::Error nullDeref() {
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "A NULL PyObject* was dereferenced");
+  }
+  static llvm::Error exception(const char *s = nullptr) {
+    return llvm::make_error<PythonException>(s);
+  }
+
+public:
+  template <typename... T>
+  llvm::Expected<PythonObject> CallMethod(const char *name,
+                                          const T &... t) const {
+    const char format[] = {'(', PythonFormat<T>::format..., ')', 0};
+#if PY_MAJOR_VERSION < 3
+    PyObject *obj = PyObject_CallMethod(m_py_obj, const_cast<char *>(name),
+                                        const_cast<char *>(format),
+                                        PythonFormat<T>::get(t)...);
+#else
+    PyObject *obj =
+        PyObject_CallMethod(m_py_obj, name, format, PythonFormat<T>::get(t)...);
+#endif
+    if (!obj)
+      return exception();
+    return python::Take<PythonObject>(obj);
+  }
+
+  llvm::Expected<PythonObject> GetAttribute(const char *name) const {
+    if (!m_py_obj)
+      return nullDeref();
+    PyObject *obj = PyObject_GetAttrString(m_py_obj, name);
+    if (!obj)
+      return exception();
+    return python::Take<PythonObject>(obj);
+  }
+
+  llvm::Expected<bool> IsTrue() {
+    if (!m_py_obj)
+      return nullDeref();
+    int r = PyObject_IsTrue(m_py_obj);
+    if (r < 0)
+      return exception();
+    return !!r;
+  }
+
+  llvm::Expected<long long> AsLongLong() {
+    if (!m_py_obj)
+      return nullDeref();
+    assert(!PyErr_Occurred());
+    long long r = PyLong_AsLongLong(m_py_obj);
+    if (PyErr_Occurred())
+      return exception();
+    return r;
+  }
+
+  llvm::Expected<bool> IsInstance(const PythonObject &cls) {
+    if (!m_py_obj || !cls.IsValid())
+      return nullDeref();
+    int r = PyObject_IsInstance(m_py_obj, cls.get());
+    if (r < 0)
+      return exception();
+    return !!r;
+  }
+
+protected:
   PyObject *m_py_obj;
 };
 
+namespace python {
+
+// This is why C++ needs monads.
+template <typename T> llvm::Expected<T> As(llvm::Expected<PythonObject> &&obj) {
+  if (!obj)
+    return obj.takeError();
+  if (!T::Check(obj.get().get()))
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "type error");
+  return T(PyRefType::Borrowed, std::move(obj.get().get()));
+}
+
+template <> llvm::Expected<bool> As<bool>(llvm::Expected<PythonObject> &&obj);
+
+template <>
+llvm::Expected<long long> As<long long>(llvm::Expected<PythonObject> &&obj);
+
+} // namespace python
+
 class PythonBytes : public PythonObject {
 public:
   PythonBytes();
   explicit PythonBytes(llvm::ArrayRef<uint8_t> bytes);
   PythonBytes(const uint8_t *bytes, size_t length);
   PythonBytes(PyRefType type, PyObject *o);
 
   ~PythonBytes() override;
public:
 
   void SetBytes(llvm::ArrayRef<uint8_t> stringbytes);
 
   StructuredData::StringSP CreateStructuredString() const;
 };
 
 class PythonString : public PythonObject {
 public:
+  static llvm::Expected<PythonString> FromUTF8(llvm::StringRef string);
+
   PythonString();
-  explicit PythonString(llvm::StringRef string);
-  explicit PythonString(const char *string);
+  explicit PythonString(llvm::StringRef string); // safe, null on error
   PythonString(PyRefType type, PyObject *o);
 
   ~PythonString() override;
 
   static bool Check(PyObject *py_obj);
 
   // Bring in the no-argument base class version
   using PythonObject::Reset;
 
   void Reset(PyRefType type, PyObject *py_obj) override;
 
-  llvm::StringRef GetString() const;
+  llvm::StringRef GetString() const; // safe, empty string on error
+
+  llvm::Expected<llvm::StringRef> AsUTF8() const;
 
   size_t GetSize() const;
 
-  void SetString(llvm::StringRef string);
+  void SetString(llvm::StringRef string); // safe, null on error
 
   StructuredData::StringSP CreateStructuredString() const;
 };
 
 class PythonInteger : public PythonObject {
 public:
   PythonInteger();
   explicit PythonInteger(int64_t value);
public:
   static bool Check(PyObject *py_obj);
 
   static PythonModule BuiltinsModule();
 
   static PythonModule MainModule();
 
   static PythonModule AddModule(llvm::StringRef module);
 
-  static PythonModule ImportModule(llvm::StringRef module);
+  // safe, returns invalid on error;
+  static PythonModule ImportModule(llvm::StringRef name) {
+    std::string s = name;
+    auto mod = Import(s.c_str());
+    if (!mod) {
+      llvm::consumeError(mod.takeError());
+      return PythonModule();
+    }
+    return std::move(mod.get());
+  }
+
+  static llvm::Expected<PythonModule> Import(const char *name);
+
+  llvm::Expected<PythonObject> Get(const char *name);
 
   // Bring in the no-argument base class version
   using PythonObject::Reset;
 
   void Reset(PyRefType type, PyObject *py_obj) override;
 
   PythonDictionary GetDictionary() const;
 };
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 = nullptr);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+// This extracts the underlying T out of an Expected<T> and returns it.
+// If the Expected is an Error instead of a T, that error will be converted
+// into a python exception, and this will return a default-constructed T.
+//
+// This is appropriate for use right at the boundary of python calling into
+// C++, such as in a SWIG typemap.   In such a context you should simply
+// check if the returned T is valid, and if it is, return a NULL back
+// to python.   This will result in the Error being raised as an exception
+// from python code's point of view.
+//
+// For example:
+// ```
+// Expected<Foo *> efoop = some_cpp_function();
+// Foo *foop = unwrapOrSetPythonException(efoop);
+// if (!foop)
+//    return NULL;
+// do_something(*foop);
+//
+// If the Error returned was itself created because a python exception was
+// raised when C++ code called into python, then the original exception
+// will be restored.   Otherwise a simple string exception will be raised.
+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;
+using namespace lldb_private::python;
+using llvm::Error;
+using llvm::Expected;
+
+template <> Expected<bool> python::As<bool>(Expected<PythonObject> &&obj) {
+  if (!obj)
+    return obj.takeError();
+  return obj.get().IsTrue();
+}
+
+template <>
+Expected<long long> python::As<long long>(Expected<PythonObject> &&obj) {
+  if (!obj)
+    return obj.takeError();
+  return obj.get().AsLongLong();
+}
 
 void StructuredPythonObject::Serialize(llvm::json::OStream &s) const {
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
 // PythonObject
 
 void PythonObject::Dump(Stream &strm) const {
   if (m_py_obj) {
     FILE *file = llvm::sys::RetryAfterSignal(nullptr, ::tmpfile);
     if (file) {
       ::PyObject_Print(m_py_obj, file, 0);
       const long length = ftell(file);
       if (length) {
PythonObject PythonObject::GetAttributeValue(llvm::StringRef attr) const {
   PythonString py_attr(attr);
   if (!PyObject_HasAttr(m_py_obj, py_attr.get()))
     return PythonObject();
 
   return PythonObject(PyRefType::Owned,
                       PyObject_GetAttr(m_py_obj, py_attr.get()));
 }
 
-bool PythonObject::IsNone() const { return m_py_obj == Py_None; }
-
-bool PythonObject::IsValid() const { return m_py_obj != nullptr; }
-
-bool PythonObject::IsAllocated() const { return IsValid() && !IsNone(); }
-
 StructuredData::ObjectSP PythonObject::CreateStructuredObject() const {
   switch (GetObjectType()) {
   case PyObjectType::Dictionary:
     return PythonDictionary(PyRefType::Borrowed, m_py_obj)
         .CreateStructuredDictionary();
   case PyObjectType::Boolean:
     return PythonBoolean(PyRefType::Borrowed, m_py_obj)
         .CreateStructuredBoolean();
StructuredData::StringSP PythonByteArray::CreateStructuredString() const {
   llvm::ArrayRef<uint8_t> bytes = GetBytes();
   const char *str = reinterpret_cast<const char *>(bytes.data());
   result->SetValue(std::string(str, bytes.size()));
   return result;
 }
 
 // PythonString
 
+Expected<PythonString> PythonString::FromUTF8(llvm::StringRef string) {
+#if PY_MAJOR_VERSION >= 3
+  PyObject *str = PyUnicode_FromStringAndSize(string.data(), string.size());
+#else
+  PyObject *str = PyString_FromStringAndSize(string.data(), string.size());
+#endif
+  if (!str)
+    return llvm::make_error<PythonException>();
+  return Take<PythonString>(str);
+}
+
 PythonString::PythonString(PyRefType type, PyObject *py_obj) : PythonObject() {
   Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string
 }
 
 PythonString::PythonString(llvm::StringRef string) : PythonObject() {
   SetString(string);
 }
 
-PythonString::PythonString(const char *string) : PythonObject() {
-  SetString(llvm::StringRef(string));
-}
-
 PythonString::PythonString() : PythonObject() {}
 
 PythonString::~PythonString() {}
 
 bool PythonString::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
void PythonString::Reset(PyRefType type, PyObject *py_obj) {
   if (!PythonString::Check(py_obj)) {
     PythonObject::Reset();
     return;
   }
 #if PY_MAJOR_VERSION < 3
   // In Python 2, Don't store PyUnicode objects directly, because we need
   // access to their underlying character buffers which Python 2 doesn't
   // provide.
-  if (PyUnicode_Check(py_obj))
-    result.Reset(PyRefType::Owned, PyUnicode_AsUTF8String(result.get()));
+  if (PyUnicode_Check(py_obj)) {
+    PyObject *s = PyUnicode_AsUTF8String(result.get());
+    if (s == NULL)
+      PyErr_Clear();
+    result.Reset(PyRefType::Owned, s);
+  }
 #endif
   // Calling PythonObject::Reset(const PythonObject&) will lead to stack
   // overflow since it calls back into the virtual implementation.
   PythonObject::Reset(PyRefType::Borrowed, result.get());
 }
 
 llvm::StringRef PythonString::GetString() const {
+  auto s = AsUTF8();
+  if (!s) {
+    llvm::consumeError(s.takeError());
+    return llvm::StringRef("");
+  }
+  return s.get();
+}
+
+Expected<llvm::StringRef> PythonString::AsUTF8() const {
   if (!IsValid())
-    return llvm::StringRef();
+    return nullDeref();
 
   Py_ssize_t size;
   const char *data;
 
 #if PY_MAJOR_VERSION >= 3
   data = PyUnicode_AsUTF8AndSize(m_py_obj, &size);
 #else
-  char *c;
-  PyString_AsStringAndSize(m_py_obj, &c, &size);
+  char *c = NULL;
+  int r = PyString_AsStringAndSize(m_py_obj, &c, &size);
+  if (r < 0)
+    c = NULL;
   data = c;
 #endif
+
+  if (!data)
+    return exception();
+
   return llvm::StringRef(data, size);
 }
 
 size_t PythonString::GetSize() const {
   if (IsValid()) {
 #if PY_MAJOR_VERSION >= 3
     return PyUnicode_GetSize(m_py_obj);
 #else
     return PyString_Size(m_py_obj);
 #endif
   }
   return 0;
 }
 
 void PythonString::SetString(llvm::StringRef string) {
-#if PY_MAJOR_VERSION >= 3
-  PyObject *unicode = PyUnicode_FromStringAndSize(string.data(), string.size());
-  PythonObject::Reset(PyRefType::Owned, unicode);
-#else
-  PyObject *str = PyString_FromStringAndSize(string.data(), string.size());
-  PythonObject::Reset(PyRefType::Owned, str);
-#endif
+  auto s = FromUTF8(string);
+  if (!s) {
+    llvm::consumeError(s.takeError());
+    Reset();
+  } else {
+    PythonObject::Reset(std::move(s.get()));
+  }
 }
 
 StructuredData::StringSP PythonString::CreateStructuredString() const {
   StructuredData::StringSP result(new StructuredData::String);
   result->SetValue(GetString());
   return result;
 }
 

 
 PythonModule PythonModule::MainModule() { return AddModule("__main__"); }
 
 PythonModule PythonModule::AddModule(llvm::StringRef module) {
   std::string str = module.str();
   return PythonModule(PyRefType::Borrowed, PyImport_AddModule(str.c_str()));
 }
 
-PythonModule PythonModule::ImportModule(llvm::StringRef module) {
-  std::string str = module.str();
-  return PythonModule(PyRefType::Owned, PyImport_ImportModule(str.c_str()));
+Expected<PythonModule> PythonModule::Import(const char *name) {
+  PyObject *mod = PyImport_ImportModule(name);
+  if (!mod)
+    return exception();
+  return Take<PythonModule>(mod);
+}
+
+Expected<PythonObject> PythonModule::Get(const char *name) {
+  if (!IsValid())
+    return nullDeref();
+  PyObject *dict = PyModule_GetDict(m_py_obj);
+  if (!dict)
+    return exception();
+  PyObject *item = PyDict_GetItemString(dict, name);
+  if (!item)
+    return exception();
+  return Retain<PythonObject>(item);
 }
 
 bool PythonModule::Check(PyObject *py_obj) {
   if (!py_obj)
     return false;
 
   return PyModule_Check(py_obj);
 }
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;
 }
 
+const char *PythonException::toCString() const {
+  if (!m_repr_bytes)
+    return "unknown exception";
+  return PyBytes_AS_STRING(m_repr_bytes);
+}
+
+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);
+  PyErr_Clear();
+  if (m_exception) {
+    PyObject *repr = PyObject_Repr(m_exception);
+    if (repr) {
+      m_repr_bytes = PyUnicode_AsEncodedString(repr, "utf-8", nullptr);
+      if (!m_repr_bytes) {
+        PyErr_Clear();
+      }
+      Py_XDECREF(repr);
+    } else {
+      PyErr_Clear();
+    }
+  }
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT);
+  if (caller)
+    LLDB_LOGF(log, "%s failed with exception: %s", caller, toCString());
+  else
+    LLDB_LOGF(log, "python exception: %s", toCString());
+}
+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;
+
 #endif