mozilla / ffi-support

A crate to help expose Rust functions over the FFI.
Apache License 2.0
41 stars 5 forks source link

Empty ByteBuffer from Golang leads to "invalid pointer found on stack" #7

Closed appaquet closed 3 years ago

appaquet commented 3 years ago

First, thanks for the great library! It was very helpful in quickly integrating a new Rust component into our existing Go project.

Unfortunately, we stumbled on a bug when a Rust method returns an empty ByteBuffer. It may lead to a Golang runtime panic with a "invalid pointer found on stack" error (thrown here).

After analysis, from what I gather, when Go migrates a goroutine stack to another thread, it validates pointers that are stored on the stack. In Rust, an empty Vec has a non-zero pointer with a value set to std::ptr::Unique::dangling(), which fails to validate once in Go.

I have an easy fix here, but I decided to open this bug first to gather your feedback on if this kind of language specific code can be merged. If so, I'll open a PR.

mhammond commented 3 years ago

This looks reasonable to me and given the patch is small I think we'd take it. For your own future reference, we will probably push back on too many changes to support Go because Mozilla considers this repo soft-deprecated and is investing more time and effort into uniffi - (and a quick look shows it probably has the same problem here :)

(As a nit on the patch, where you wrote std::ptr::Unique::dangling() did you mean std::ptr::Unique::empty()? I can't see a method named dangling() while empty() documents itself as "Creates a new Unique that is dangling, but well-aligned.")

appaquet commented 3 years ago

Oh, didn't know about uniffi. I'll have a look at it, but I don't think it would have been of help since there is no generator for Golang.

As for std::ptr::Unique::dangling(), it's not exposed in the doc, but it's referenced in RawVec here and here and defined here.

I'll open the PR.