rust: update toolchain to 1.65.0

benma · benma · commit 4a5b65cfee7f · 2022-11-08T12:37:58.000+01:00
rust-bindgen CLI is now available under `bindgen-cli`. The CHANGELOG (https://github.com/rust-lang/rust-bindgen/blob/0631a27bee19601e393e31e7dfc2563412b2fe01/CHANGELOG.md#0610) mentions at 0.61.0: > The --size_t-is-usize flag is enabled by default. Since we previously manually set size_t to usize anyway, we can adopt this change and just use usize everywhere. Rust 1.65.0 uses LLVM 15. For the C unit tests, the LLVM version installed in Docker must be at least the version that Rust uses for compatibility, so we update to LLVM-15 (this was the intention in #1001, but updating llvm was missed). New clippy linter results are addressed. rust_eh_personality now seems to be included by Rust in the Rust unit tests and we only need to include it in the C unit tests :S Signed-off-by: Marko Bencun <marko@shiftcrypto.ch>
diff --git a/.ci/run-container-ci b/.ci/run-container-ci
@@ -25,7 +25,7 @@
 set -e
 set -x
 
-CONTAINER=shiftcrypto/firmware_v2:28
+CONTAINER=shiftcrypto/firmware_v2:29
 
 if [ "$1" == "pull" ] ; then
 	docker pull "$CONTAINER"
diff --git a/Dockerfile b/Dockerfile
@@ -37,7 +37,7 @@ RUN mkdir ~/Downloads &&\
 # Tools for building
 RUN apt-get update && apt-get install -y \
     build-essential \
-    llvm \
+    llvm-15 \
     gcc-10 \
     binutils \
     valgrind \
@@ -129,8 +129,8 @@ RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | CARGO_HOME=/opt/
 RUN rustup target add thumbv7em-none-eabi
 RUN rustup component add rustfmt
 RUN rustup component add clippy
-RUN CARGO_HOME=/opt/cargo cargo install cbindgen --version 0.23.0
-RUN CARGO_HOME=/opt/cargo cargo install bindgen --version 0.59.2
+RUN CARGO_HOME=/opt/cargo cargo install cbindgen --version 0.24.3
+RUN CARGO_HOME=/opt/cargo cargo install bindgen-cli --version 0.61.0
 
 COPY tools/prost-build prost-build
 RUN CARGO_HOME=/opt/cargo cargo install --path prost-build --locked
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -453,6 +453,8 @@ if(NOT CMAKE_CROSSCOMPILING)
             -A clippy::manual_range_contains
             -A clippy::match_ref_pats
             -A clippy::enum_variant_names
+            -A clippy::derive_partial_eq_without_eq
+            -A clippy::forget_non_drop
     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/rust/
     )
   add_dependencies(rust-clippy rust-bindgen generate-protobufs)
diff --git a/src/rust/bitbox02-rust-c/Cargo.toml b/src/rust/bitbox02-rust-c/Cargo.toml
@@ -33,10 +33,6 @@ sha3 = { version = "0.9.1", default-features = false, optional = true }
 bs58 = { version = "0.4.0", default-features = false, features = ["alloc", "check"] }
 
 [features]
-# "Negative" feature is a hack because tests are ran with `--all-features`, currently without a way to exclude specific features.
-# This should be enabled for Rust unit tests and Clippy, and not enabled for the firmware targets and the C unit tests.
-dont-export-eh-personality = []
-
 # Only one of the "target-" should be activated, which in turn defines/activates the dependent features.
 target-bootloader = ["bootloader", "platform-bitbox02"]
 target-bootloader-development = ["bootloader", "platform-bitbox02"]
diff --git a/src/rust/bitbox02-rust-c/src/alloc.rs b/src/rust/bitbox02-rust-c/src/alloc.rs
@@ -23,7 +23,7 @@ fn bitbox02_alloc_error_handler(layout: core::alloc::Layout) -> ! {
 struct BB02Allocator;
 
 extern "C" {
-    pub fn malloc(size: util::c_types::size_t) -> *mut util::c_types::c_void;
+    pub fn malloc(size: usize) -> *mut util::c_types::c_void;
     pub fn free(p: *mut util::c_types::c_void);
 }
 
diff --git a/src/rust/bitbox02-rust-c/src/lib.rs b/src/rust/bitbox02-rust-c/src/lib.rs
@@ -51,18 +51,3 @@ fn panic(info: &core::panic::PanicInfo) -> ! {
     bitbox02_rust::print_debug!(0, "Error: {}", info);
     loop {}
 }
-
-// Needed to link the C unit test executables in /test/unit-test.
-//
-// See https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib.
-//
-// One could get rid of this and also considerably shrink the binary size by compiling core instead
-// of using pre-built binaries. See a proof of concept implementation here:
-// https://github.com/digitalbitbox/bitbox02-firmware/tree/build-std-PoC.  We decided against doing
-// this for now as the feature seems immature and because of the warnings against using it in
-// production:
-// https://github.com/rust-lang/wg-cargo-std-aware/tree/81765f0eb744b9c47840c16f43a32c9f61fd7f0c#mvp-implementation
-#[cfg(not(feature = "dont-export-eh-personality"))]
-#[lang = "eh_personality"]
-#[no_mangle]
-pub extern "C" fn rust_eh_personality() {}
diff --git a/src/rust/bitbox02-rust-c/src/sha2.rs b/src/rust/bitbox02-rust-c/src/sha2.rs
@@ -57,7 +57,7 @@ pub unsafe extern "C" fn rust_sha256_finish(ctx: *mut *mut c_void, out: *mut c_u
 pub unsafe extern "C" fn rust_sha256_free(ctx: *mut *mut c_void) {
     if !(*ctx).is_null() {
         #[allow(clippy::cast_ptr_alignment)] // ctx is properly aligned, see `Box::into_raw`.
-        Box::from_raw(*ctx as *mut Sha256);
+        drop(Box::from_raw(*ctx as *mut Sha256));
         *ctx = core::ptr::null_mut();
     }
 }
diff --git a/src/rust/bitbox02-rust/src/backup.rs b/src/rust/bitbox02-rust/src/backup.rs
@@ -86,7 +86,7 @@ fn compute_checksum(
         result[..name.len()].copy_from_slice(name);
         result
     };
-    hasher.update(&padded_name);
+    hasher.update(padded_name);
     hasher.update(data.seed_length.to_le_bytes());
     if data.seed.len() != 32 {
         return Err(());
@@ -102,7 +102,7 @@ fn compute_checksum(
         result[..generator.len()].copy_from_slice(generator);
         result
     };
-    hasher.update(&padded_generator);
+    hasher.update(padded_generator);
     hasher.update(data_length.to_le_bytes());
     Ok(hasher.finalize().to_vec())
 }
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin.rs
@@ -214,9 +214,7 @@ pub async fn process_pub(request: &pb::BtcPubRequest) -> Result<Response, Error>
         Some(coin) => coin,
         None => return Err(Error::InvalidInput),
     };
-    if let Err(err) = coin_enabled(coin) {
-        return Err(err);
-    }
+    coin_enabled(coin)?;
     match request.output {
         None => Err(Error::InvalidInput),
         Some(Output::XpubType(xpub_type)) => {
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/bip341.rs
@@ -38,8 +38,8 @@ pub struct Args {
 pub fn sighash(args: &Args) -> [u8; 32] {
     let tag = Sha256::digest(b"TapSighash");
     let mut ctx = Sha256::new();
-    ctx.update(&tag);
-    ctx.update(&tag);
+    ctx.update(tag);
+    ctx.update(tag);
     // Sighash epoch 0
     ctx.update(0u8.to_le_bytes());
     // Control:
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
@@ -123,7 +123,7 @@ fn format_member_type(typ: &MemberType) -> Result<String, Error> {
 fn encode_type(types: &[StructType], name: &str) -> Result<String, Error> {
     let mut transitive_types = get_transitive_types(types, name)?;
     // First element contains the name of the type to encode - sort the rest of the types.
-    (&mut transitive_types[1..]).sort_unstable();
+    transitive_types[1..].sort_unstable();
     Ok(transitive_types
         .iter()
         .map(|name| -> Result<String, Error> {
@@ -460,7 +460,7 @@ async fn validate_chain_id(request: &pb::EthSignTypedMessageRequest) -> Result<(
 
 async fn eip712_sighash(types: &[StructType], primary_type: &str) -> Result<[u8; 32], Error> {
     let mut hasher = sha3::Keccak256::new();
-    hasher.update(&[0x19u8, 0x01]);
+    hasher.update([0x19u8, 0x01]);
     let domain_separator =
         hash_struct(types, RootObject::Domain, DOMAIN_TYPE_NAME, &[], &[], None).await?;
     hasher.update(&domain_separator);
@@ -633,7 +633,7 @@ mod tests {
             match self {
                 Object::String(s) => s.as_bytes().to_vec(),
                 Object::Bytes(b) => b.to_vec(),
-                Object::Bool(b) => [if *b { 1 } else { 0 }].to_vec(),
+                Object::Bool(b) => [(*b).into()].to_vec(),
                 Object::BigInt(i) => i.to_signed_bytes_be(),
                 Object::BigUint(i) => i.to_bytes_be(),
                 Object::List(l) => (l.len() as u32).to_be_bytes().to_vec(),
diff --git a/src/rust/bitbox02-rust/src/workflow/verify_message.rs b/src/rust/bitbox02-rust/src/workflow/verify_message.rs
@@ -36,7 +36,7 @@ impl core::convert::From<confirm::UserAbort> for Error {
 ///
 /// Otherwise, it is displayed as hex.
 pub async fn verify(msg: &[u8]) -> Result<(), Error> {
-    if ascii::is_printable_ascii(&msg, ascii::Charset::AllNewline) {
+    if ascii::is_printable_ascii(msg, ascii::Charset::AllNewline) {
         // The message is all ascii and printable.
         let msg = core::str::from_utf8(msg).unwrap();
 
diff --git a/src/rust/bitbox02/src/app_btc.rs b/src/rust/bitbox02/src/app_btc.rs
@@ -23,7 +23,7 @@ pub fn pkscript_from_multisig(
     keypath_address: u32,
 ) -> Result<Vec<u8>, ()> {
     let mut out = [0u8; bitbox02_sys::MAX_PK_SCRIPT_SIZE as usize];
-    let mut out_len: bitbox02_sys::size_t = out.len() as _;
+    let mut out_len: usize = out.len() as _;
     match unsafe {
         bitbox02_sys::btc_common_pkscript_from_multisig(
             multisig,
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
@@ -271,15 +271,15 @@ pub fn secp256k1_nonce_commit(
 pub fn bip39_mnemonic_to_seed(mnemonic: &str) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
     let mnemonic = zeroize::Zeroizing::new(crate::util::str_to_cstr_vec(mnemonic)?);
     let mut seed = zeroize::Zeroizing::new([0u8; MAX_SEED_LENGTH]);
-    let mut seed_len: util::c_types::c_uint = 0;
+    let mut seed_len: usize = 0;
     match unsafe {
         bitbox02_sys::keystore_bip39_mnemonic_to_seed(
             mnemonic.as_ptr(),
             seed.as_mut_ptr(),
             &mut seed_len,
         )
     } {
-        true => Ok(zeroize::Zeroizing::new(seed[..seed_len as usize].to_vec())),
+        true => Ok(zeroize::Zeroizing::new(seed[..seed_len].to_vec())),
         false => Err(()),
     }
 }
diff --git a/src/rust/bitbox02/src/sd.rs b/src/rust/bitbox02/src/sd.rs
@@ -55,7 +55,7 @@ pub fn list_subdir(subdir: Option<&str>) -> Result<Vec<String>, ()> {
     } {
         true => (0..list.0.num_files)
             .map(|i| unsafe {
-                let ptr = *list.0.files.offset(i as _) as *const u8;
+                let ptr = *list.0.files.add(i) as *const u8;
                 crate::util::str_from_null_terminated_ptr(ptr).map(String::from)
             })
             .collect(),
@@ -77,7 +77,7 @@ pub fn erase_file_in_subdir(filename: &str, dir: &str) -> Result<(), ()> {
 
 pub fn load_bin(filename: &str, dir: &str) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
     let mut contents = zeroize::Zeroizing::new([0u8; SD_MAX_FILE_SIZE as _]);
-    let mut contents_len: bitbox02_sys::size_t = 0;
+    let mut contents_len: usize = 0;
     match unsafe {
         bitbox02_sys::sd_load_bin(
             str_to_cstr_vec(filename).unwrap().as_ptr(),
diff --git a/src/rust/rust-toolchain b/src/rust/rust-toolchain
@@ -1 +1 @@
-1.61.0
+1.65.0
diff --git a/src/rust/util/src/c_types.rs b/src/rust/util/src/c_types.rs
@@ -42,4 +42,3 @@ pub type c_ulong = u32;
 
 pub type c_longlong = i64;
 pub type c_ulonglong = u64;
-pub type size_t = usize;
diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt
@@ -252,7 +252,7 @@ foreach(I RANGE 0 ${TEST_LIST_LEN} 2)
   list(GET TEST_LIST ${I} TEST_NAME)
   list(GET TEST_LIST ${I2} TEST_LINK_ARGS)
   set(EXE test_${TEST_NAME})
-  add_executable(${EXE} test_${TEST_NAME}.c)
+  add_executable(${EXE} test_${TEST_NAME}.c framework/eh_personality.c)
   # asan must be first library in linking order
   target_link_libraries(${EXE} PRIVATE
     $<$<BOOL:SANITIZE_ADDRESS>:asan>
@@ -279,7 +279,7 @@ foreach(TEST_NAME ${U2F_TESTS})
   set(EXE test_${TEST_NAME})
 
   # This tests link to our code
-  add_executable(${EXE} test_${TEST_NAME}.c framework/mock_hidapi.c)
+  add_executable(${EXE} test_${TEST_NAME}.c framework/mock_hidapi.c framework/eh_personality.c)
   target_link_libraries(${EXE} PRIVATE
     $<$<BOOL:SANITIZE_ADDRESS>:asan>
     $<$<BOOL:SANITIZE_UNDEFINED>:-fsanitize=undefined>
diff --git a/test/unit-test/framework/eh_personality.c b/test/unit-test/framework/eh_personality.c
@@ -0,0 +1,28 @@
+// Copyright 2022 Shift Crypto AG
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Needed to link the C unit test executables in /test/unit-test, which link to bitbox_merged.
+// `rust_eh_personality` is provided by Rust when building the firmware or running Rust unit tests
+//
+// See
+// https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib.
+//
+// One could get rid of this and also considerably shrink the binary size by compiling core instead
+// of using pre-built binaries. See a proof of concept implementation here:
+// https://github.com/digitalbitbox/bitbox02-firmware/tree/build-std-PoC.  We decided against doing
+// this for now as the feature seems immature and because of the warnings against using it in
+// production:
+// https://github.com/rust-lang/wg-cargo-std-aware/tree/81765f0eb744b9c47840c16f43a32c9f61fd7f0c#mvp-implementation
+void rust_eh_personality(void);
+void rust_eh_personality(void) {}

Original file line number	Diff line number	Diff line change
`@@ -453,6 +453,8 @@ if(NOT CMAKE_CROSSCOMPILING)`
`453`	`453`	`-A clippy::manual_range_contains`
`454`	`454`	`-A clippy::match_ref_pats`
`455`	`455`	`-A clippy::enum_variant_names`
	`456`	`+ -A clippy::derive_partial_eq_without_eq`
	`457`	`+ -A clippy::forget_non_drop`
`456`	`458`	`WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/rust/`
`457`	`459`	`)`
`458`	`460`	`add_dependencies(rust-clippy rust-bindgen generate-protobufs)`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ fn bitbox02_alloc_error_handler(layout: core::alloc::Layout) -> ! {`
`23`	`23`	`struct BB02Allocator;`
`24`	`24`
`25`	`25`	`extern "C" {`
`26`		`- pub fn malloc(size: util::c_types::size_t) -> *mut util::c_types::c_void;`
	`26`	`+ pub fn malloc(size: usize) -> *mut util::c_types::c_void;`
`27`	`27`	`pub fn free(p: *mut util::c_types::c_void);`
`28`	`28`	`}`
`29`	`29`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ pub unsafe extern "C" fn rust_sha256_finish(ctx: mut mut c_void, out: *mut c_u`
`57`	`57`	`pub unsafe extern "C" fn rust_sha256_free(ctx: mut mut c_void) {`
`58`	`58`	`if !(*ctx).is_null() {`
`59`	`59`	#[allow(clippy::cast_ptr_alignment)] // ctx is properly aligned, see `Box::into_raw`.
`60`		`- Box::from_raw(ctx as mut Sha256);`
	`60`	`+ drop(Box::from_raw(ctx as mut Sha256));`
`61`	`61`	`*ctx = core::ptr::null_mut();`
`62`	`62`	`}`
`63`	`63`	`}`