Use c2rust implementation when targeting Wasm

[paulyoung]

I'm working in an environment that only allows targeting wasm32-unknown-unknown.

I recently added a dependency that depended on ring (or openssl) and despite other people saying it worked for them, I wasn't able to get either of these to compile to Wasm because of the non-Rust dependencies.

ring seemed the more likely to work, but then I learned that for most Wasm targets (including the one I'm targeting) the ABI isn't compatible with the C ABI, so while linking with C can appear to work it can result in hidden memory corruption.

rust-lang/rust#83788 and rustwasm/team#291 go into some detail on that.

In hoping to avoid all of that while still using a well-vetted implementation I came across Fully Oxidizing ring and it seemed like the perfect solution. I'm at least going to try using it until a better solution comes along.

After some back and forth I'm pleased to have landed on this minimal set of changes that enables me to use ring when targeting wasm32-unknown-unknown.

I'd be very grateful if you could take a look and let me know what you think.

[paulyoung]

I should add that in doing this I discovered some type "mismatches" that are hidden by (or perhaps rely on) the use of extern "C".

Initially I had some trouble with Wasm interpreting extern "C" as importing host functionality so I manually removed extern "C" in various places while debugging.

Once things were regular Rust functions I got some type errors.

One example of this is as follows:

ring-xous/src/limb.rs

Line 358 in 0920704

fn LIMBS_are_zero(a: *const Limb, num_limbs: c::size_t) -> LimbMask;

ring-xous/src/c2rust/limbs.rs

Line 217 in 0920704

pub unsafe extern "C" fn LIMBS_are_zero(a: *const Limb, num_limbs: size_t) -> Limb {

For the above example I ended up with a diff like this:

diff --git a/src/c2rust/limbs.rs b/src/c2rust/limbs.rs
index e44679e45..a5a472f33 100644
--- a/src/c2rust/limbs.rs
+++ b/src/c2rust/limbs.rs
@@ -11,7 +11,7 @@ extern "C" {
         __function: *const std::os::raw::c_char,
     ) -> !;
 }
-pub type size_t = std::os::raw::c_uint;
+pub type size_t = crate::c::size_t;
 pub type __uint32_t = std::os::raw::c_uint;
 pub type __uint64_t = u64;
 pub type uint32_t = __uint32_t;

diff --git a/src/limb.rs b/src/limb.rs
index 0b8144a57..1e7f6675c 100644
--- a/src/limb.rs
+++ b/src/limb.rs
@@ -19,6 +19,7 @@
//! limbs use the native endianness.

use crate::{c, error};
+use core::convert::{TryFrom, TryInto};

#[cfg(feature = "alloc")]
use crate::bits;
@@ -44,6 +45,18 @@ pub enum LimbMask {
    False = 0,
}

+impl TryFrom<Limb> for LimbMask {
+    type Error = error::Unspecified;
+
+    fn try_from(value: Limb) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(LimbMask::True),
+            1 => Ok(LimbMask::False),
+            _ => Err(error::Unspecified),
+        }
+    }
+}
+
#[cfg(target_pointer_width = "32")]
#[derive(Debug, PartialEq)]
#[repr(u32)]
@@ -83,7 +96,7 @@ pub fn limbs_less_than_limb_constant_time(a: &[Limb], b: Limb) -> LimbMask {

#[inline]
pub fn limbs_are_zero_constant_time(limbs: &[Limb]) -> LimbMask {
-    unsafe { LIMBS_are_zero(limbs.as_ptr(), limbs.len()) }
+    unsafe { LIMBS_are_zero(limbs.as_ptr(), limbs.len()).try_into().expect("Limb should be convertible to LimbMask") }
}

#[cfg(feature = "alloc")]
@@ -355,7 +368,7 @@ extern "C" {

    #[cfg(feature = "alloc")]
    fn LIMBS_are_even(a: *const Limb, num_limbs: c::size_t) -> LimbMask;
-    fn LIMBS_are_zero(a: *const Limb, num_limbs: c::size_t) -> LimbMask;
+    fn LIMBS_are_zero(a: *const Limb, num_limbs: c::size_t) -> Limb;
    #[cfg(feature = "alloc")]
    fn LIMBS_equal_limb(a: *const Limb, b: Limb, num_limbs: c::size_t) -> LimbMask;
    fn LIMBS_less_than(a: *const Limb, b: *const Limb, num_limbs: c::size_t) -> LimbMask;

I made some modifications to the Python script to address this but it had a cascading effect. I ultimately didn't need to get into this so I reverted those changes.

I'm not sure how important it is to address these but I at least wanted to bring it up.

[bunnie]

Hi @paulyoung, thanks for the wasm32 changes. They look good to me, I can merge them in if you think that's good enough for now.

As for the extern C issue, I don't quite follow the effect it's having. The reason those are stuck everywhere, however, is because we still want ring to build for e.g. x86_64. The problem is that our xtask module can auto-fetch libraries from the internet, which requires https, and thus ring. The xtask module runs on the local host (x86_64), and is in our workspace, so we can't simply strip out the extern C without also forcing the x86_64 to use the generated Rust files. The reason we can't just have the x86_64 just use the Rust files isn't a performance problem, it's actually that it's a 64-bit machine and the Rust is transpiled from a 32-bit configuration of the code. We'd have to go back and transpile a parallel set of files to get it working for 64-bit mode.

It's something I was contemplating but actually I think if any rework was to be done, what I should do is to figure out how to use the files directly off of fiat-rust, which come directly from the formally verified code generation system, instead of taking a bounce off a transpiler.

There is also some weirdness about the FFI types still stabilizing in Rust, which is still quite a work in progress: rust-lang/rust#94501 I think once those are stabilized the rube goldberg machine starts to look less clunky.

Anyways, if you can live with the warnings and the code is working for you, i'm happy to absorb your PR. thank you!

[paulyoung]

To clarify; this works as-is for me targeting Wasm and doesn’t produce any warnings (at least as a dependency of the project I’m using it in)

If you’re seeing any warnings perhaps I overlooked them when building the project directly. I can try to address those if there are any.

[bunnie]

Ah, I misunderstood your comment then. I'll go ahead and merge this. Thank you!!