Skip to content
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

Typeid information not synchronized across shared library boundaries #912

Closed
wjakob opened this issue Jun 21, 2017 · 6 comments · Fixed by #915
Closed

Typeid information not synchronized across shared library boundaries #912

wjakob opened this issue Jun 21, 2017 · 6 comments · Fixed by #915

Comments

@wjakob
Copy link
Member

wjakob commented Jun 21, 2017

Dear all,

I've been running into strange inconsistencies where type information is not synchronized across shared library boundaries. This happens when using Clang & libc++ on Linux/x86_64 machines.

Before escalating to Clang/LLVM, I wanted to gauge if anyone here has previously run into this. To reproduce:

issue.py

import mod1, mod2

mod2.test(mod1.Test())

# Expected output
# ================
# $ python issue.py
# $


# Actual output:
# ================
# $ python issue.py
# Traceback (most recent call last):
#   File "issue.py", line 3, in <module>
#     mod2.test(mod1.Test())
# TypeError: test(): incompatible function arguments. The following argument types are supported:
#     1. (arg0: Test) -> None
#
# Invoked with: <mod1.Test object at 0x7f12861f5df8>

CMakeLists.txt:

cmake_minimum_required(VERSION 2.8.12)
project(example)

# Issue is triggered in a release build
set(CMAKE_BUILD_TYPE Release CACHE STRING "" FORCE)

# Issue is triggered when using libc++
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++")

add_subdirectory(pybind11)

add_library(shared SHARED shared.cpp)

pybind11_add_module(mod1 mod1.cpp)
pybind11_add_module(mod2 mod2.cpp)

target_link_libraries(mod1 shared)
target_link_libraries(mod2 shared)

shared.h

#pragma once

#define EXPORT __attribute__ ((visibility("default")))

class EXPORT Test {
public:
    Test();
};

shared.cpp

#include "shared.h"

Test::Test() { }

mod1.cpp

#include <pybind11/pybind11.h>
#include "shared.h"

namespace py = pybind11;

PYBIND11_MODULE(mod1, m) {
    py::class_<Test>(m, "Test")
        .def(py::init<>());
}

mod2.cpp

#include <pybind11/pybind11.h>
#include "shared.h"

namespace py = pybind11;

void test(Test b) { }

PYBIND11_MODULE(mod2, m) {
    py::module::import("mod1");
    m.def("test", &test);
}

Best,
Wenzel

@aldanor
Copy link
Member

aldanor commented Jun 21, 2017

Hm, is typeid(Test) not the same in mod1/mod2, can you check?

@wjakob
Copy link
Member Author

wjakob commented Jun 21, 2017

Yes, indeed -- the typeids differ in these two modules despite the __attribute__ ((visibility("default"))) annotation. Interestingly, the problem disappears when the -stdlib=libc++ flag is removed from the compilation flags.

@jagerman
Copy link
Member

We do have a few places that do a pointer equality comparison, which seems slightly wrong, but fixing it (PR #914) doesn't fix the issue.

I think, however, given the response in https://bugs.llvm.org/show_bug.cgi?id=33542:

I think our stance has been that if you dlopen with RTLD_LOCAL (the default),
then the "Test" type in mod1.so and mod2.so are different types. They live in
different symbol namespaces, all the symbols in mod1.so are not visible from
mod2.so, etc.

libstdc++ has taken a different stance by moving to string equality for
std::type_info::operator==.


Unfortunately, the C++ standard gives us very few tools to reason about how C++
should behave in the presence of dynamic libraries. The best mental model we
have is either a DSO is part of the "program" (a collection of translation
units) or they represent separate "programs" with some implementation-defined
semantics for interoperating.

we might have to switch to using the type.name() field to identify types, at least under libc++.

I'm curious, though, why this doesn't seem to affect macOS.

@jagerman
Copy link
Member

As a workaround, you can call this before loading the pybind modules:

import os, sys
sys.setdlopenflags(os.RTLD_NOW | os.RTLD_GLOBAL)

jagerman added a commit to jagerman/pybind11 that referenced this issue Jun 21, 2017
Using `std::type_info::operator==` fails under libc++ because the .so
is loaded with RTLD_LOCAL.  libc++ considers types under such .sos
distinct, and so comparing typeid() values directly isn't going to work.

This adds a custom hasher and equality class for the type lookup maps
when not under stdlibc++, and adds a `detail::same_type` function to
perform the equality test.  It also converts a few pointer arguments to
const lvalue references, particularly since doing the pointer
comparison wasn't technically valid to being with (though in practice,
appeared to work everywhere).

This fixes pybind#912.
@jagerman
Copy link
Member

@wjakob - give #915 a try in your project: it switches to name-based type lookups outside libstdc++ (which already does name-based equality/hashing). It does fixes the code you posted in the initial report under clang/libc++ for me (tested with clang/libc++ 4.0).

@wjakob
Copy link
Member Author

wjakob commented Jun 22, 2017

I just did, and it works with that patch -- thank you! Regarding the OSX inconsistency: it's likely that the dlopen semantics are a bit different on Mach-O binaries.

jagerman added a commit that referenced this issue Jun 24, 2017
Using `std::type_info::operator==` fails under libc++ because the .so
is loaded with RTLD_LOCAL.  libc++ considers types under such .sos
distinct, and so comparing typeid() values directly isn't going to work.

This adds a custom hasher and equality class for the type lookup maps
when not under stdlibc++, and adds a `detail::same_type` function to
perform the equality test.  It also converts a few pointer arguments to
const lvalue references, particularly since doing the pointer
comparison wasn't technically valid to being with (though in practice,
appeared to work everywhere).

This fixes #912.
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

Successfully merging a pull request may close this issue.

3 participants