Merge pull request #111 from TheBlueMatt/2023-07-0.0.115-aliasing-fix v0.0.115.3
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 28 Jul 2023 23:00:25 +0000 (23:00 +0000)
committerGitHub <noreply@github.com>
Fri, 28 Jul 2023 23:00:25 +0000 (23:00 +0000)
[0.0.115] Fix strict-aliasing violation on traits holding inner fields

.github/workflows/build.yml
c-bindings-gen/src/main.rs
genbindings.sh
lightning-c-bindings/include/lightning.h
lightning-c-bindings/src/lightning/chain/keysinterface.rs

index 1d1b9b57583e29b5126aadf03285e0607004be2b..25b7a603a5de04a1b2928df4a7e447489f8d6e65 100644 (file)
@@ -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
index 71064008755ea07d7cee59e945e267c81108cd7c..1b54d16de5a8c830220e408e13ae7d1d477a37fa 100644 (file)
@@ -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: std::io::Write>(w: &mut W, w_uses: &mut HashSet<String, NonRa
                                                                        if let syn::Type::Reference(r) = &**rtype {
                                                                                write!(w, "\n\t\t{}{}: ", $indent, $m.sig.ident).unwrap();
                                                                                types.write_empty_rust_val(Some(&gen_types), w, &*r.elem);
-                                                                               writeln!(w, ",\n{}\t\tset_{}: Some({}_{}_set_{}),", $indent, $m.sig.ident, ident, $trait.ident, $m.sig.ident).unwrap();
+                                                                               writeln!(w, ".into(),\n{}\t\tset_{}: Some({}_{}_set_{}),", $indent, $m.sig.ident, ident, $trait.ident, $m.sig.ident).unwrap();
                                                                                printed = true;
                                                                        }
                                                                }
@@ -1198,9 +1198,9 @@ fn writeln_impl<W: std::io::Write>(w: &mut W, w_uses: &mut HashSet<String, NonRa
                                                                                writeln!(w, "\t// This is a bit race-y in the general case, but for our specific use-cases today, we're safe").unwrap();
                                                                                writeln!(w, "\t// Specifically, we must ensure that the first time we're called it can never be in parallel").unwrap();
                                                                                write!(w, "\tif ").unwrap();
-                                                                               $types.write_empty_rust_val_check(Some(&meth_gen_types), w, &*r.elem, &format!("trait_self_arg.{}", $m.sig.ident));
+                                                                               $types.write_empty_rust_val_check(Some(&meth_gen_types), w, &*r.elem, &format!("unsafe {{ &*trait_self_arg.{}.get() }}", $m.sig.ident));
                                                                                writeln!(w, " {{").unwrap();
-                                                                               writeln!(w, "\t\tunsafe {{ &mut *(trait_self_arg as *const {}  as *mut {}) }}.{} = {}_{}_{}(trait_self_arg.this_arg);", $trait.ident, $trait.ident, $m.sig.ident, ident, $trait.ident, $m.sig.ident).unwrap();
+                                                                               writeln!(w, "\t\t*unsafe {{ &mut *(&*(trait_self_arg as *const {})).{}.get() }} = {}_{}_{}(trait_self_arg.this_arg).into();", $trait.ident, $m.sig.ident, ident, $trait.ident, $m.sig.ident).unwrap();
                                                                                writeln!(w, "\t}}").unwrap();
                                                                                writeln!(w, "}}").unwrap();
                                                                        }
index 1d53b96ca1804077661b7603e9974c1f75615350..c5025982dc36a15704da88bbd69580ee1bcc2d49 100755 (executable)
@@ -231,6 +231,9 @@ cbindgen -v --config cbindgen.toml -o include/lightning.h >/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 <stdlib.h>/#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 <stdlib.h>/#include "ldk_rust_types.h"/g' include/lightning.h
index da7fa8e179ea2095980a17bc876325e53b7d06a8..cbbe93e216602072c47bb29557a7753071f6132b 100644 (file)
@@ -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
index eaf36ac24c1096ab119b7a6ad8e7e759089a4fb2..acfb4407ff3de0720543c1bb32c5b1bfc2a89b9f 100644 (file)
@@ -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<crate::lightning::ln::chan_utils::ChannelPublicKeys>,
        /// 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,