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

Target-specific bindings #62

Closed
fpagliughi opened this issue Jan 7, 2020 · 3 comments
Closed

Target-specific bindings #62

fpagliughi opened this issue Jan 7, 2020 · 3 comments

Comments

@fpagliughi
Copy link
Contributor

The pre-generated bindings were created on an x86_64 Linux machine. These are in paho-mqtt-sys/bindings/bindings_paho_mqtt_c_<version>.rs.

There seems to be problems using these bindings with other targets, particularly 32-bit ones, such as ARM v7 (RPi, BeagleBone, etc). The different word size causes mis-alignment of the C structs passed back and forth from the Paho C lib, and in the case of structs containing pointers, these get junk values causing segfaults.

The quick-fix, for now, is for client applications to use the "build_bindgen" feature of the library to always regenerate the bindings on every build.

A better fix, going forward might be to use target-specific bindings like *bindings_paho_mqtt_v_<version>-<target>.rs, like:

bindings_paho_mqtt_c_1.3.1-armv7-unknown-linux-gnueabihf.rs

The problem with this approach is that bindings need to be produced for every possible target in order for the default build to always succeed.

@fpagliughi
Copy link
Contributor Author

Done (with some problems) in v0.7, then (hopefully) fixed in v0.8

@ryankurte
Copy link
Contributor

@fpagliughi did you ever file or find a related bindgen issue for this? starting some discussion about smoothing the *-sys crate edges (see the above linked issue) and a lot of the open issues on bindgen look similar but, not quite the same to me.

@fpagliughi
Copy link
Contributor Author

The problem was definitely mixing up the bindings between 32-bit and 64-bit platforms. When that happened, structs would be misaligned and one side or the other (C or Rust) would load a pointer from a struct using the wrong offset in the struct for the pointer, get a bad value into the pointer, and then get a segfault when trying to dereference and use the pointer.

I did not find a problem if different platforms got mixed, but can't be sure that's universal - meaning if you generate 64-bit bindings on x86_64, it seems to work OK on aarch64.

So it's not a problem with bindgen per se, but me misusing it. If we were to always re-generate the bindings on every build, then everything would be fine. But that significantly increases the build time, which is especially troubling for people doing native builds on underpowered hardware like RPi's and such.

But the thing is... once you have the bindings for your target, you never need to regenerate them; at least not until a -sys library upgrade. So how can we generate once and use forever?

The way it's in the repo right now is that I pre-generated a number of binding files for the most common platforms to which I have access, and labelled the files with the target name. The build.rs looks for the proper target and reports an error if it can't find the proper one. The problem with this method is that the error message is buried in a large failure dump, and really difficult to spot.

The only thing I can think of at the moment is to flip around the optimization. Make using bindgen on every build the default. And then hiding the use of pre-built bindings behind a feature flag, so (hopefully) only people who really need it would try it out and know what to expect on failure.

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