expressvpn / wolfssl-sys

A low level Rust binding for WolfSSL
GNU General Public License v2.0
9 stars 5 forks source link

Tests fail under macOS: reference to packed field is unaligned #6

Closed KizzyCode closed 2 years ago

KizzyCode commented 2 years ago

The tests fail under macOS: reference to packed field is unaligned

Expected Behavior

The tests should not fail :D

Current Behavior

The tests fail with (example):

...

error: reference to packed field is unaligned
     --> wolfssl-sys/target/debug/build/wolfssl-sys-ea177715975f0b68/out/bindings.rs:16207:18
      |
16207 |         unsafe { &(*(::std::ptr::null::<__msfilterreq>())).msfr_group as *const _ as usize },
      |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = 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)
      = help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)

...

Possible Solution

Update bindgen-dependency to 0.60.1 – at least on my system, this fixes the error.

Steps to Reproduce (for bugs)

  1. Clone the repo
  2. Run cargo test

Context

I want to use this crate :)

Your Environment

expressvpn-pete-m commented 2 years ago

Hi @KizzyCode, apologies for the delay in responding! For some reason I didn't get an email notification that an issue had been created. I'll look into what happened.

I am updating the crate to use the 5.4 release of WolfSSL so I'll fix this at the same time :)

expressvpn-pete-m commented 2 years ago

I just tested this on an M1 with version 12.4 and cargo test completes okay. I'm happy to upgrade the dependency anyway, but I'd prefer to be able to replicate the issue first :smile:

KizzyCode commented 2 years ago

Ok thats interesting – I've just tested this again and can reproduce πŸ˜…

For me it's pretty straight forward:

If you're interested, I've attached a short video of all the steps above: https://user-images.githubusercontent.com/13999009/179969866-5e909d9e-4378-46e2-8d68-ab50b1ffa884.mp4 (apparently in some browsers it can not be viewed inline and must be downloaded manually πŸ˜‘)

Edit: Maybe you're using an older Rust toolchain? I'm using stable-aarch64-apple-darwin unchanged - rustc 1.62.1 (e092d0b6b 2022-07-16)

expressvpn-pete-m commented 2 years ago

Yup, after a quick rustup upgrade I now get the same error. I can confirm that upgrading bindgen also resolved the error for me. The upgrade doesn't break on x86_64 either which is a plus :smile:

The fix will go out tomorrow once I have my signing key handy. Thanks for reporting this! :smile:

expressvpn-pete-m commented 2 years ago

Sorry for the delay - ended up sorting out a new signing key :smile:

This is live now on https://crates.io/crates/wolfssl-sys as 0.1.5

KizzyCode commented 2 years ago

Cool thank you 😊