test+fix: Add asm testing and fix core_hint_black_box

.github/workflows/test.yml

@@ -62,3 +62,6 @@ jobs:
         with:
           command: build
           args: --no-default-features --target thumbv7em-none-eabi
+      - name: test assembly
+        run: ./test.sh
+        working-directory: ./asm_test

asm_test/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "asm_test"
+version = "0.1.0"
+edition = "2024"
+
+[dependencies]
+subtle = { path = "../", default-features = false }
+
+[features]
+core_hint_black_box = ["subtle/core_hint_black_box"]

asm_test/src/lib.rs

@@ -0,0 +1,8 @@
+#![no_std]
+
+use subtle::{Choice, ConditionallySelectable};
+
+#[inline(never)]
+pub fn select_byte(a: u8, b: u8) -> u8 {
+    ConditionallySelectable::conditional_select(&a, &b, Choice::from(1))
+}

asm_test/test.sh

@@ -0,0 +1,98 @@
+#!/bin/bash
+
+# Track failures globally
+FAILED=0
+
+# Allow some wiggle room for optimizer changes
+BAD_BUFFER=2
+GOOD_BUFFER=2
+
+# Test different feature sets
+function test_all() {
+	cargo --quiet install [email protected] 
+	test_all_targets
+	test_all_targets core_hint_black_box
+
+	exit "${FAILED}"
+}
+
+# Test all the common architectures with their known good and bad values
+function test_all_targets() {
+	test "aarch64-unknown-linux-gnu"     2 14 ${@}
+	test "i686-unknown-linux-gnu"        2 18 ${@}
+	test "x86_64-unknown-linux-gnu"      2 15 ${@}
+	test "arm-unknown-linux-gnueabi"     2 9  ${@}
+	test "loongarch64-unknown-linux-gnu" 2 19 ${@}
+	test "powerpc-unknown-linux-gnu"     2 27 ${@}
+	test "powerpc64-unknown-linux-gnu"   3 22 ${@}
+	test "powerpc64le-unknown-linux-gnu" 2 25 ${@}
+	test "riscv64gc-unknown-linux-gnu"   2 17 ${@}
+	test "s390x-unknown-linux-gnu"       2 13 ${@}
+	test "wasm32-unknown-unknown"        2 11 ${@}
+	test "thumbv7em-none-eabi"           2 10 ${@}
+}
+
+# Args:
+# * The target name
+# * The expected # of instructions for the bad case (optimizer defeated us)
+# * The expected # of instructions for the good case (we beat the optimizer)
+# * Any feature flags
+#
+# We take in both bad and good just to ensure there's no overlap.
+function test() {
+	local TARGET=$1
+	local BAD=$(($2 + ${BAD_BUFFER}))
+	local GOOD=$(($3 - ${GOOD_BUFFER}))
+	shift 3
+	local FEATURES=""
+
+	# Build up the feature flags
+	for F in "${@}"; do
+		FEATURES+="--features=${F} "
+	done
+
+	# Make sure the good and bad ranges don't overlap
+	if [ "${GOOD}" -le "${BAD}" ]; then
+		echo "${TARGET} invalid params"
+		exit 1
+	fi
+
+	# Ensure the target is available
+	rustup -q target add "${TARGET}"
+
+	# Build and capture the assembly
+	catch INST INST_ERR env CARGO_TERM_QUIET=true cargo --quiet asm ${FEATURES} --target "${TARGET}" --simplify select_byte
+
+	# Emit any cargo errors
+	local STATUS=${?}
+	if [ ${STATUS} -ne 0 ]; then 
+		FAILED=1
+		>&2 echo "${INST_ERR}"
+		return
+	fi
+
+	# Check the instruction count
+	local INST_COUNT="$(echo -n "${INST}" | wc -l)"
+	if [ "${INST_COUNT}" -lt "${BAD}" ]; then
+		FAILED=1
+		echo "${TARGET}: Less than ${BAD} instructions (${INST_COUNT})"
+		echo "${INST}"
+	elif [ "${INST_COUNT}" -lt "${GOOD}" ]; then
+		FAILED=1
+		echo "${TARGET}: WARNING Less than ${GOOD} instructions (${INST_COUNT})"
+		echo "Consider adjusting values"
+		echo "${INST}"
+	fi
+}
+
+# Capture both stderr and stdout
+# https://stackoverflow.com/questions/11027679/capture-stdout-and-stderr-into-different-variables
+function catch() {
+    {
+        IFS=$'\n' read -r -d '' "${1}";
+        IFS=$'\n' read -r -d '' "${2}";
+        (IFS=$'\n' read -r -d '' _ERRNO_; return ${_ERRNO_});
+    } < <((printf '\0%s\0%d\0' "$(((({ shift 2; "${@}"; echo "${?}" 1>&3-; } | tr -d '\0' 1>&4-) 4>&2- 2>&1- | tr -d '\0' 1>&4-) 3>&1- | exit "$(cat)") 4>&1-)" "${?}" 1>&2) 2>&1)
+}
+
+test_all

src/lib.rs

@@ -96,9 +96,6 @@ use core::cmp;
 use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Neg, Not};
 use core::option::Option;
 
-#[cfg(feature = "core_hint_black_box")]
-use core::hint::black_box;
-
 /// The `Choice` struct represents a choice for use in conditional assignment.
 ///
 /// It is a wrapper around a `u8`, which should have the value either `1` (true)
@@ -220,6 +217,7 @@ impl Not for Choice {
 /// code may break in a non-destructive way in the future, “constant-time” code
 /// is a continually moving target, and this is better than doing nothing.
 #[inline(never)]
+#[cfg(not(feature = "core_hint_black_box"))]
 fn black_box<T: Copy>(input: T) -> T {
     unsafe {
         // Optimization barrier
@@ -232,6 +230,12 @@ fn black_box<T: Copy>(input: T) -> T {
     }
 }
 
+#[cfg(feature = "core_hint_black_box")]
+#[inline(never)]
+const fn black_box<T>(input: T) -> T {
+    core::hint::black_box(input)
+}
+
 impl From<u8> for Choice {
     #[inline]
     fn from(input: u8) -> Choice {