Skip to content
New issue

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

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

Already on GitHub? # to your account

Autogenerated tests contains UB: reference to packed field is unaligned #2053

Closed
Apostoln opened this issue May 13, 2021 · 2 comments
Closed

Comments

@Apostoln
Copy link

Input C/C++ Header

struct Foo {
    unsigned int a;
} __attribute__((__packed__));

Bindgen Invocation

bindgen::Builder::default()
        .header("wrapper.h")
        .generate()
        .unwrap();

Actual Results

cargo +nightly test --no-run emits a warning which causes UB and will be considered as error in the future rust releases

warning: reference to packed field is unaligned
   |
21 |         unsafe { &(*(::std::ptr::null::<Foo>())).a as *const _ as usize },
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unaligned_references)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
   = note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)

Expected Results

No warnings, errors, and UB.
We can use the workaround mentioned in #82523 rust-lang/rust#82523 to avoid UB.

@Apostoln
Copy link
Author

Up:
Also, there is another warning on nightly which is also reports an UB:

warning: dereferencing a null pointer
--> /home/nikbond/CLionProjects/untitled2/target/debug/build/untitled2-a0b2d2dcec1e02fa/out/bindings.rs:21:19
|
21 | unsafe { &(*(::std::ptr::null::())).a as *const _ as usize },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
|
= note: #[warn(deref_nullptr)] on by default

I'm not really experienced in low-level unsafe magic, but this way to get a field offset looks so tricky. Can't we use something out of the box like offset_of! ? If so, I can prepare a PR.

@emilio
Copy link
Contributor

emilio commented May 13, 2021

Yeah, this is a dupe of #1651, see the discussion there. If we determine that what memoffset is doing (which is just doing the same but with a MaybeUninit<> or a dangling pointer (https://docs.rs/memoffset/0.6.3/src/memoffset/offset_of.rs.html#91-100), rather than null) is fine, I'd take a PR to do that, but I'm not 100% sure that's the case. Dereferencing an uninitialized or dangling pointer seems equally undefined to dereferencing null.

@emilio emilio closed this as completed May 13, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants