From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri, 28 Jul 2023 23:00:25 +0000 (+0000) Subject: Merge pull request #111 from TheBlueMatt/2023-07-0.0.115-aliasing-fix X-Git-Tag: v0.0.115.3^0 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=dabf0dc9c652443b439ef987f3c890198704573a;hp=316ba2b5b764a1595f0993bc457714efdaf424ad;p=ldk-c-bindings Merge pull request #111 from TheBlueMatt/2023-07-0.0.115-aliasing-fix [0.0.115] Fix strict-aliasing violation on traits holding inner fields --- diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1d1b9b5..25b7a60 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -40,6 +40,11 @@ jobs: git checkout 0.0.115-bindings - name: Fix Github Actions to not be broken run: git config --global --add safe.directory /__w/ldk-c-bindings/ldk-c-bindings + - name: Pin proc-macro and quote to meet MSRV + run: | + cd c-bindings-gen + cargo update -p quote --precise "1.0.30" --verbose + cargo update -p proc-macro2 --precise "1.0.65" --verbose - name: Rebuild bindings without std, and check the sample app builds + links run: ./genbindings.sh ./rust-lightning false - name: Rebuild bindings, and check the sample app builds + links diff --git a/c-bindings-gen/src/main.rs b/c-bindings-gen/src/main.rs index 7106400..1b54d16 100644 --- a/c-bindings-gen/src/main.rs +++ b/c-bindings-gen/src/main.rs @@ -351,10 +351,10 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty // the Rust type and a flag to indicate whether deallocation needs to // happen) as well as provide an Option<>al function pointer which is // called when the trait method is called which allows updating on the fly. - write!(w, "\tpub {}: ", m.sig.ident).unwrap(); - generated_fields.push((format!("{}", m.sig.ident), None, None)); + write!(w, "\tpub {}: core::cell::UnsafeCell<", m.sig.ident).unwrap(); + generated_fields.push((format!("{}", m.sig.ident), Some(("Clone::clone(unsafe { &*core::cell::UnsafeCell::get(".to_owned(), ")}).into()")), None)); types.write_c_type(w, &*r.elem, Some(&meth_gen_types), false); - writeln!(w, ",").unwrap(); + writeln!(w, ">,").unwrap(); writeln!(w, "\t/// Fill in the {} field as a reference to it will be given to Rust after this returns", m.sig.ident).unwrap(); writeln!(w, "\t/// Note that this takes a pointer to this object, not the this_ptr like other methods do").unwrap(); writeln!(w, "\t/// This function pointer may be NULL if {} is filled in when this object is created and never needs updating.", m.sig.ident).unwrap(); @@ -430,7 +430,7 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty let is_clonable = types.is_clonable(s); writeln!(w, "\tpub {}: crate::{},", i, s).unwrap(); (format!("{}", i), if !is_clonable { - Some(format!("crate::{}_clone_fields", s)) + Some((format!("crate::{}_clone_fields(", s), ")")) } else { None }, None) }); } @@ -513,7 +513,7 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty writeln!(w, "\t\t\t(f)(&self{});", $impl_accessor).unwrap(); write!(w, "\t\t}}\n\t\t").unwrap(); $type_resolver.write_from_c_conversion_to_ref_prefix(w, &*r.elem, Some(&meth_gen_types)); - write!(w, "self{}.{}", $impl_accessor, m.sig.ident).unwrap(); + write!(w, "unsafe {{ &*self{}.{}.get() }}", $impl_accessor, m.sig.ident).unwrap(); $type_resolver.write_from_c_conversion_to_ref_suffix(w, &*r.elem, Some(&meth_gen_types)); writeln!(w, "\n\t}}").unwrap(); continue; @@ -557,9 +557,9 @@ fn writeln_trait<'a, 'b, W: std::io::Write>(w: &mut W, t: &'a syn::ItemTrait, ty writeln!(w, "\t{} {{", trait_name).unwrap(); writeln!(w, "\t\tthis_arg: orig.this_arg,").unwrap(); for (field, clone_fn, _) in generated_fields.iter() { - if let Some(f) = clone_fn { + if let Some((pfx, sfx)) = clone_fn { // If the field isn't clonable, blindly assume its a trait and hope for the best. - writeln!(w, "\t\t{}: {}(&orig.{}),", field, f, field).unwrap(); + writeln!(w, "\t\t{}: {}&orig.{}{},", field, pfx, field, sfx).unwrap(); } else { writeln!(w, "\t\t{}: Clone::clone(&orig.{}),", field, field).unwrap(); } @@ -1046,7 +1046,7 @@ fn writeln_impl(w: &mut W, w_uses: &mut HashSet(w: &mut W, w_uses: &mut HashSet/dev/null 2>&1 if is_gnu_sed; then sed -i 's/typedef LDKnative.*Import.*LDKnative.*;//g' include/lightning.h + # UnsafeCell is `repr(transparent)` so should be ignored here + sed -i 's/LDKUnsafeCell<\(.*\)> /\1 /g' include/lightning.h + # stdlib.h doesn't exist in clang's wasm sysroot, and cbindgen # doesn't actually use it anyway, so drop the import. sed -i 's/#include /#include "ldk_rust_types.h"/g' include/lightning.h @@ -238,6 +241,9 @@ else # OSX sed is for some reason not compatible with GNU sed sed -i '' 's/typedef LDKnative.*Import.*LDKnative.*;//g' include/lightning.h + # UnsafeCell is `repr(transparent)` so should be ignored by cbindgen + sed -i '' 's/LDKUnsafeCell<\(.*\)> /\1 /g' include/lightning.h + # stdlib.h doesn't exist in clang's wasm sysroot, and cbindgen # doesn't actually use it anyway, so drop the import. sed -i '' 's/#include /#include "ldk_rust_types.h"/g' include/lightning.h diff --git a/lightning-c-bindings/include/lightning.h b/lightning-c-bindings/include/lightning.h index da7fa8e..cbbe93e 100644 --- a/lightning-c-bindings/include/lightning.h +++ b/lightning-c-bindings/include/lightning.h @@ -7884,7 +7884,7 @@ typedef struct LDKChannelSigner { /** * Returns the holder's channel public keys and basepoints. */ - struct LDKChannelPublicKeys pubkeys; + LDKChannelPublicKeys pubkeys; /** * Fill in the pubkeys field as a reference to it will be given to Rust after this returns * Note that this takes a pointer to this object, not the this_ptr like other methods do diff --git a/lightning-c-bindings/src/lightning/chain/keysinterface.rs b/lightning-c-bindings/src/lightning/chain/keysinterface.rs index eaf36ac..acfb440 100644 --- a/lightning-c-bindings/src/lightning/chain/keysinterface.rs +++ b/lightning-c-bindings/src/lightning/chain/keysinterface.rs @@ -629,7 +629,7 @@ pub struct ChannelSigner { #[must_use] pub validate_holder_commitment: extern "C" fn (this_arg: *const c_void, holder_tx: &crate::lightning::ln::chan_utils::HolderCommitmentTransaction, preimages: crate::c_types::derived::CVec_PaymentPreimageZ) -> crate::c_types::derived::CResult_NoneNoneZ, /// Returns the holder's channel public keys and basepoints. - pub pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys, + pub pubkeys: core::cell::UnsafeCell, /// Fill in the pubkeys field as a reference to it will be given to Rust after this returns /// Note that this takes a pointer to this object, not the this_ptr like other methods do /// This function pointer may be NULL if pubkeys is filled in when this object is created and never needs updating. @@ -662,7 +662,7 @@ pub(crate) extern "C" fn ChannelSigner_clone_fields(orig: &ChannelSigner) -> Cha get_per_commitment_point: Clone::clone(&orig.get_per_commitment_point), release_commitment_secret: Clone::clone(&orig.release_commitment_secret), validate_holder_commitment: Clone::clone(&orig.validate_holder_commitment), - pubkeys: Clone::clone(&orig.pubkeys), + pubkeys: Clone::clone(unsafe { &*core::cell::UnsafeCell::get(&orig.pubkeys)}).into(), set_pubkeys: Clone::clone(&orig.set_pubkeys), channel_keys_id: Clone::clone(&orig.channel_keys_id), provide_channel_parameters: Clone::clone(&orig.provide_channel_parameters), @@ -690,7 +690,7 @@ impl rustChannelSigner for ChannelSigner { if let Some(f) = self.set_pubkeys { (f)(&self); } - self.pubkeys.get_native_ref() + unsafe { &*self.pubkeys.get() }.get_native_ref() } fn channel_keys_id(&self) -> [u8; 32] { let mut ret = (self.channel_keys_id)(self.this_arg); @@ -889,7 +889,7 @@ impl lightning::chain::keysinterface::ChannelSigner for EcdsaChannelSigner { if let Some(f) = self.ChannelSigner.set_pubkeys { (f)(&self.ChannelSigner); } - self.ChannelSigner.pubkeys.get_native_ref() + unsafe { &*self.ChannelSigner.pubkeys.get() }.get_native_ref() } fn channel_keys_id(&self) -> [u8; 32] { let mut ret = (self.ChannelSigner.channel_keys_id)(self.ChannelSigner.this_arg); @@ -1066,7 +1066,7 @@ impl lightning::chain::keysinterface::ChannelSigner for WriteableEcdsaChannelSig if let Some(f) = self.EcdsaChannelSigner.ChannelSigner.set_pubkeys { (f)(&self.EcdsaChannelSigner.ChannelSigner); } - self.EcdsaChannelSigner.ChannelSigner.pubkeys.get_native_ref() + unsafe { &*self.EcdsaChannelSigner.ChannelSigner.pubkeys.get() }.get_native_ref() } fn channel_keys_id(&self) -> [u8; 32] { let mut ret = (self.EcdsaChannelSigner.ChannelSigner.channel_keys_id)(self.EcdsaChannelSigner.ChannelSigner.this_arg); @@ -1761,7 +1761,7 @@ pub extern "C" fn InMemorySigner_as_ChannelSigner(this_arg: &InMemorySigner) -> release_commitment_secret: InMemorySigner_ChannelSigner_release_commitment_secret, validate_holder_commitment: InMemorySigner_ChannelSigner_validate_holder_commitment, - pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }, + pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }.into(), set_pubkeys: Some(InMemorySigner_ChannelSigner_set_pubkeys), channel_keys_id: InMemorySigner_ChannelSigner_channel_keys_id, provide_channel_parameters: InMemorySigner_ChannelSigner_provide_channel_parameters, @@ -1793,8 +1793,8 @@ extern "C" fn InMemorySigner_ChannelSigner_pubkeys(this_arg: *const c_void) -> c extern "C" fn InMemorySigner_ChannelSigner_set_pubkeys(trait_self_arg: &ChannelSigner) { // This is a bit race-y in the general case, but for our specific use-cases today, we're safe // Specifically, we must ensure that the first time we're called it can never be in parallel - if trait_self_arg.pubkeys.inner.is_null() { - unsafe { &mut *(trait_self_arg as *const ChannelSigner as *mut ChannelSigner) }.pubkeys = InMemorySigner_ChannelSigner_pubkeys(trait_self_arg.this_arg); + if unsafe { &*trait_self_arg.pubkeys.get() }.inner.is_null() { + *unsafe { &mut *(&*(trait_self_arg as *const ChannelSigner)).pubkeys.get() } = InMemorySigner_ChannelSigner_pubkeys(trait_self_arg.this_arg).into(); } } #[must_use] @@ -1839,7 +1839,7 @@ pub extern "C" fn InMemorySigner_as_EcdsaChannelSigner(this_arg: &InMemorySigner release_commitment_secret: InMemorySigner_ChannelSigner_release_commitment_secret, validate_holder_commitment: InMemorySigner_ChannelSigner_validate_holder_commitment, - pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }, + pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }.into(), set_pubkeys: Some(InMemorySigner_ChannelSigner_set_pubkeys), channel_keys_id: InMemorySigner_ChannelSigner_channel_keys_id, provide_channel_parameters: InMemorySigner_ChannelSigner_provide_channel_parameters, @@ -1939,7 +1939,7 @@ pub extern "C" fn InMemorySigner_as_WriteableEcdsaChannelSigner(this_arg: &InMem release_commitment_secret: InMemorySigner_ChannelSigner_release_commitment_secret, validate_holder_commitment: InMemorySigner_ChannelSigner_validate_holder_commitment, - pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }, + pubkeys: crate::lightning::ln::chan_utils::ChannelPublicKeys { inner: core::ptr::null_mut(), is_owned: true }.into(), set_pubkeys: Some(InMemorySigner_ChannelSigner_set_pubkeys), channel_keys_id: InMemorySigner_ChannelSigner_channel_keys_id, provide_channel_parameters: InMemorySigner_ChannelSigner_provide_channel_parameters,