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

Using a class with a std::unique_ptr member #1389

Closed
Neurrone opened this issue Sep 18, 2018 · 10 comments
Closed

Using a class with a std::unique_ptr member #1389

Neurrone opened this issue Sep 18, 2018 · 10 comments

Comments

@Neurrone
Copy link

Hi,

I'm attempting to create an instance of a class with a unique ptr, among other members in it, and invoke some methods. Its crashing probably because the struct layout doesn't match.

I've seen several instances where std::unique_ptr members in classes are being represented as a u64 by bindgen and even once, returned as a u8 by a function that returns a unique_ptr.

I've set everything under the std namespace to be treated as an opaque type, so I'm wondering if I need to specifically whitelist these to have them represented properly.

@emilio
Copy link
Contributor

emilio commented Sep 18, 2018

I've seen several instances where std::unique_ptr members in classes are being represented as a u64 by bindgen and even once, returned as a u8 by a function that returns a unique_ptr.

That's the correct layout for it, right? Assuming you're in a 64-bit system. If you see a unique_ptr returned as a u8, that's clearly a bug, and I'd appreciate a test-case for it. Thanks!

@emilio
Copy link
Contributor

emilio commented Sep 18, 2018

If cargo test passes it should be the right layout... Does that class contain any interior pointer or something like that that?

@Neurrone
Copy link
Author

Neurrone commented Sep 19, 2018

@emilio there are 2 issues here, I'll talk about the std::unique_ptr one and file another for the second.

Yes, I'm on 64 bit.

I discovered the std::unique_ptr being represented by a u8 while working with V8, on my repo for the previous issue.

The function signiture in C++ is:

// line 37 of v8-sys/src/include/libplatform/libplatform.h
V8_PLATFORM_EXPORT std::unique_ptr<v8::Platform> NewDefaultPlatform(
    int thread_pool_size = 0,
    IdleTaskSupport idle_task_support = IdleTaskSupport::kDisabled,
    InProcessStackDumping in_process_stack_dumping =
        InProcessStackDumping::kDisabled,
    std::unique_ptr<v8::TracingController> tracing_controller = {});

Which has the corresponding binding:

// line 18531 in v8-sys/pregenerated_bindings.rs
                #[link_name = "\u{1}?NewDefaultPlatform@platform@v8@@YA?AV?$unique_ptr@VPlatform@v8@@U?$default_delete@VPlatform@v8@@@std@@@std@@HW4IdleTaskSupport@12@W4InProcessStackDumping@12@V?$unique_ptr@VTracingController@v8@@U?$default_delete@VTracingController@v8@@@std@@@4@@Z"]
                pub fn NewDefaultPlatform(
                    thread_pool_size: ::std::os::raw::c_int,
                    idle_task_support: root::v8::platform::IdleTaskSupport,
                    in_process_stack_dumping: root::v8::platform::InProcessStackDumping,
                    tracing_controller: u64,
                ) -> u8;
            }

@emilio
Copy link
Contributor

emilio commented Sep 19, 2018

Hmm, I don't think getting and returning unique_ptr's is going to work, at least on MSVC, where we can't convince rustc to use the same ABI. In particular, MSVC will return the pointer on a reserved slot on the stack, while rustc will expect it in a register.

@emilio
Copy link
Contributor

emilio commented Sep 19, 2018

See rust-lang/rust#38258 and #778.

@Neurrone
Copy link
Author

Ah.

I wonder if you've thought about automatically generating C++ code for the constructs that can't easily cross the Rust/C++ boundary like this one, sort of what Swig has done for scripting languages. Unfortunately, it doesn't support a C backend, which would make using C++ a lot easier.

@emilio
Copy link
Contributor

emilio commented Sep 19, 2018

Yeah, that's one of the solutions proposed in #778.

I haven't thought of a great design for that, but if somebody had the cycles to prototype it I'd be happy to mentor as needed :)

@Neurrone
Copy link
Author

Hm, I have no background in ASTs, compilers and the like, so I'm not even sure how to approach this problem. This is definitely a motivating enough project that I could see myself attempting to learn it, especially if I had some help, though I'm not sure how big this project would be.

Does Clang expose enough information about the parsed C++ files to make this feasible?

@emilio
Copy link
Contributor

emilio commented Sep 19, 2018

Does Clang expose enough information about the parsed C++ files to make this feasible?

Well, sort of, anything that bindgen can generate we already read... There's an idea to make this even better (because libclang isn't that good at templates) which is #297.

You can reproduce that bug without much template magic:

template<typename T>
class SmartPtr
{
    T* m_raw;
    ~SmartPtr();
};

SmartPtr<int> do_something();

So making that solution work should definitely be doable without #297.

Making it work for the C++ standard library and the like might be a bit harder? But in any case I'd treat both issues as different projects.

@emilio
Copy link
Contributor

emilio commented Feb 13, 2019

Closing as everything that's actionable on bindgen's side is tracked in other issues.

@emilio emilio closed this as completed Feb 13, 2019
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