Skip to content

Pybind11 causes clang 3.3 and clang 3.4 to segfault #1349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bstaletic opened this issue Apr 5, 2018 · 6 comments
Closed

Pybind11 causes clang 3.3 and clang 3.4 to segfault #1349

bstaletic opened this issue Apr 5, 2018 · 6 comments

Comments

@bstaletic
Copy link
Collaborator

Issue description

As it stands now, compiling with clang 3.4 (assuming the same goes for clang 3.3) makes clang segfault. To work around this, ycm-core/ycmd#925 had to drop support for clang 3.3 and clang 3.4.

Looking at the current ycm_core.cpp and the last year's segfaults (today's segfaults don't look much different to me), @jagerman thought it had something to do with the static variable used in exception binding and told me to try the following in pybind11.h.

NAMESPACE_BEGIN(detail)
template <typename CppException>
exception<CppException> &get_exception_object() {
    static exception<CppException> ex;
    return ex;
}
NAMESPACE_END(detail)

/**
 * Registers a Python exception in `m` of the given `name` and installs an exception translator to
 * translate the C++ exception to the created Python exception using the exceptions what() method.
 * This is intended for simple exception translations; for more complex translation, register the
 * exception object and translator directly.
 */
template <typename CppException>
exception<CppException> &register_exception(handle scope,
                                            const char *name,
                                            PyObject *base = PyExc_Exception) {
    auto &ex = detail::get_exception_object<CppException>();
    ex = exception<CppException>(scope, name, base);
    register_exception_translator([](std::exception_ptr p) {
        if (!p) return;
        try {
            std::rethrow_exception(p);
        } catch (const CppException &e) {
            detail::get_exception_object<CppException>()(e.what());
        }
    });
    return ex;
}

This did solve the segmentation fault (at least on clang 3.4), but is now causing compilation errors.

Reproducible example code

Unfortunately, I can't make this minimal. Something, somewhere in ycmd libclang completer bindings is making clang 3.3 segfault, but I am willing to run tests on travis.

Since travis only supports clang 3.4, I'll try to package clang 3.3 for my distro so that we can test it. I still can't promise I'll manage to get clang 3.3 ready for testing. I will be using https://github.com/bstaletic/ycmd/tree/pybind11_segfault to test whatever we come up with.

@jagerman
Copy link
Member

jagerman commented Apr 5, 2018

Add: exception() = default; to the 'class exception' just above that code; that should fix the compilation error.

If this solves the problem I'll clean up the patch to #ifdef this approach for clang < 3.5. (Edit: it's not particularly intrusive, and cleaner to have just one code path so I didn't #ifdef it.)

@jagerman
Copy link
Member

jagerman commented Apr 5, 2018

If this solves the problem I'll clean up the patch to #ifdef this approach for clang < 3.5.

... which is horrific thanks to Apple deciding that clang's macros should reflect Apple's Xcode marketing versions. Blech.

@jagerman
Copy link
Member

jagerman commented Apr 5, 2018

The issue here is almost certainly https://bugs.llvm.org/show_bug.cgi?id=19052, indeed present in 3.4 and fixed in 3.5.

@bstaletic
Copy link
Collaborator Author

Yup, that fixed it.
https://travis-ci.org/bstaletic/ycmd/builds/362630239

Thanks for looking into this.

@bstaletic
Copy link
Collaborator Author

For completeness, here's the diff that fixed it:

diff --git a/cpp/pybind11/pybind11/pybind11.h b/cpp/pybind11/pybind11/pybind11.h
index 977045d6..c685e06c 100644
--- a/cpp/pybind11/pybind11/pybind11.h
+++ b/cpp/pybind11/pybind11/pybind11.h
@@ -1650,6 +1650,7 @@ void register_exception_translator(ExceptionTranslator&& translator) {
 template <typename type>
 class exception : public object {
 public:
+    exception() = default;
     exception(handle scope, const char *name, PyObject *base = PyExc_Exception) {
         std::string full_name = scope.attr("__name__").cast<std::string>() +
                                 std::string(".") + name;
@@ -1666,6 +1667,14 @@ public:
     }
 };

+NAMESPACE_BEGIN(detail)
+template <typename CppException>
+exception<CppException> &get_exception_object() {
+    static exception<CppException> ex;
+    return ex;
+}
+NAMESPACE_END(detail)
+
 /**
  * Registers a Python exception in `m` of the given `name` and installs an exception translator to
  * translate the C++ exception to the created Python exception using the exceptions what() method.
@@ -1676,13 +1685,14 @@ template <typename CppException>
 exception<CppException> &register_exception(handle scope,
                                             const char *name,
                                             PyObject *base = PyExc_Exception) {
-    static exception<CppException> ex(scope, name, base);
+    auto &ex = detail::get_exception_object<CppException>();
+    ex = exception<CppException>(scope, name, base);
     register_exception_translator([](std::exception_ptr p) {
         if (!p) return;
         try {
             std::rethrow_exception(p);
         } catch (const CppException &e) {
-            ex(e.what());
+            detail::get_exception_object<CppException>()(e.what());
         }
     });
     return ex;

jagerman added a commit to jagerman/pybind11 that referenced this issue Apr 5, 2018
As reported in pybind#1349, clang before 3.5 can segfault on a function-local
variable referenced inside a lambda.  This moves the function-local
static into a separate function that the lambda can invoke to avoid the
issue.

Fixes pybind#1349
@jagerman
Copy link
Member

jagerman commented Apr 5, 2018

I cleaned it up a bit for PR #1350; once that passes the CI tests I'll merge.

jagerman added a commit that referenced this issue Apr 5, 2018
As reported in #1349, clang before 3.5 can segfault on a function-local
variable referenced inside a lambda.  This moves the function-local
static into a separate function that the lambda can invoke to avoid the
issue.

Fixes #1349
wjakob pushed a commit to wjakob/pybind11 that referenced this issue Apr 29, 2018
As reported in pybind#1349, clang before 3.5 can segfault on a function-local
variable referenced inside a lambda.  This moves the function-local
static into a separate function that the lambda can invoke to avoid the
issue.

Fixes pybind#1349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants