Skip to content

Commit 417fa02

Browse files
authored
Fix soundness of simd_dispatch using target-features 1.1 (#69)
Previously, you would be able to write an unsafe function, and then call it using `simd_dispatch` without needing the `unsafe` token at the call site. E.g. ```rust /// # Safety /// /// `value` must be `Some` #[inline(always)] unsafe fn evil<S: Simd>(simd: S, value: Option<u32>) -> u32 { value.unwrap_unchecked() } simd_dispatch!(disguised(level, value: Option<u32>) -> u32); fn unwitting(){ disguised(None); } ``` This is a real soundness hole, if `fearless_simd` expands to wider use, as it would effectively mean that external crates are unable to write unsafe "simd-compatible" functions. The fix is to require the `unsafe` token in the macro definition in these cases; we detect a true unsafe case without this unsafe block by using target feature 1.1. In adding support for `unsafe`, I've also added a requirement that each function definition created in `simd_dispatch` is prefixed with the `fn` token. This isn't fundamental, but seems worth requiring. This does bump up our MSRV to 1.86, for target feature 1.1
1 parent 58e7838 commit 417fa02

File tree

10 files changed

+84
-30
lines changed

10 files changed

+84
-30
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ env:
88
# If the compilation fails, then the version specified here needs to be bumped up to reality.
99
# Be sure to also update the rust-version property in the workspace Cargo.toml file,
1010
# plus all the README.md files of the affected packages.
11-
RUST_MIN_VER: "1.85"
11+
RUST_MIN_VER: "1.86"
1212
# List of packages that will be checked with the minimum supported Rust version.
1313
# This should be limited to packages that are intended for publishing.
1414
RUST_MIN_VER_PKGS: "-p fearless_simd"

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ You can find its changes [documented below](#011-2018-11-05).
1313

1414
## [Unreleased]
1515

16-
This release has an [MSRV][] of 1.85.
16+
This release has an [MSRV][] of 1.86.
1717

1818
There has been a complete rewrite of Fearless SIMD.
1919
For some details of the ideas used, see our blog post [*Towards fearless SIMD, 7 years later*](https://linebender.org/blog/towards-fearless-simd/).

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ license = "Apache-2.0 OR MIT"
1313
repository = "https://github.com/linebender/fearless_simd"
1414
# Keep in sync with RUST_MIN_VER in .github/workflows/ci.yml, with the relevant README.md files
1515
# and with the MSRV in the `Unreleased` section of CHANGELOG.md.
16-
rust-version = "1.85"
16+
rust-version = "1.86"
1717

1818
[workspace.lints]
1919

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ It benefited from conversations with Luca Versari, though he is not responsible
5959

6060
## Minimum supported Rust Version (MSRV)
6161

62-
This version of Fearless SIMD has been verified to compile with **Rust 1.85** and later.
62+
This version of Fearless SIMD has been verified to compile with **Rust 1.86** and later.
6363

6464
Future versions of Fearless SIMD might increase the Rust version requirement.
6565
It will not be treated as a breaking change and as such can even happen with small patch releases.

fearless_simd/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ use fearless_simd::{Simd, simd_dispatch};
6363
#[inline(always)]
6464
fn sigmoid_impl<S: Simd>(simd: S, x: &[f32], out: &mut [f32]) { /* ... */ }
6565

66-
simd_dispatch!(sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
66+
simd_dispatch!(fn sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
6767
```
6868

6969
A few things to note:
@@ -129,7 +129,7 @@ At least one of `std` and `libm` is required; `std` overrides `libm`.
129129

130130
## Minimum supported Rust Version (MSRV)
131131

132-
This version of Fearless SIMD has been verified to compile with **Rust 1.85** and later.
132+
This version of Fearless SIMD has been verified to compile with **Rust 1.86** and later.
133133

134134
Future versions of Fearless SIMD might increase the Rust version requirement.
135135
It will not be treated as a breaking change and as such can even happen with small patch releases.

fearless_simd/examples/play.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ fn foo_inner<S: Simd>(simd: S, x: f32) -> f32 {
3030
simd.splat_f32x4(x).sqrt()[0]
3131
}
3232

33-
simd_dispatch!(foo(level, x: f32) -> f32 = foo_inner);
33+
simd_dispatch!(fn foo(level, x: f32) -> f32 = foo_inner);
3434

3535
// currently requires `safe_wrappers` feature
3636
fn do_something_on_neon(_level: Level) -> f32 {

fearless_simd/examples/sigmoid.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn sigmoid_impl<S: Simd>(simd: S, x: &[f32], out: &mut [f32]) {
1818
}
1919
}
2020

21-
simd_dispatch!(sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
21+
simd_dispatch!(fn sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
2222

2323
fn main() {
2424
let level = Level::new();

fearless_simd/examples/srgb.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ fn to_srgb_impl<S: Simd>(simd: S, rgba: [f32; 4]) -> [f32; 4] {
6767
result.into()
6868
}
6969

70-
simd_dispatch!(to_srgb(level, rgba: [f32; 4]) -> [f32; 4] = to_srgb_impl);
70+
simd_dispatch!(fn to_srgb(level, rgba: [f32; 4]) -> [f32; 4] = to_srgb_impl);
7171

7272
fn main() {
7373
let level = Level::new();

fearless_simd/src/lib.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
//! #[inline(always)]
2626
//! fn sigmoid_impl<S: Simd>(simd: S, x: &[f32], out: &mut [f32]) { /* ... */ }
2727
//!
28-
//! simd_dispatch!(sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
28+
//! simd_dispatch!(fn sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
2929
//! ```
3030
//!
3131
//! A few things to note:
@@ -337,8 +337,7 @@ impl Level {
337337
#[cfg(target_arch = "aarch64")]
338338
#[target_feature(enable = "neon")]
339339
#[inline]
340-
// unsafe not needed here with tf11, but can be justified
341-
unsafe fn dispatch_neon<W: WithSimd>(f: W, neon: Neon) -> W::Output {
340+
fn dispatch_neon<W: WithSimd>(f: W, neon: Neon) -> W::Output {
342341
f.with_simd(neon)
343342
}
344343

@@ -351,14 +350,14 @@ impl Level {
351350
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
352351
#[target_feature(enable = "sse4.2")]
353352
#[inline]
354-
unsafe fn dispatch_sse4_2<W: WithSimd>(f: W, sse4_2: Sse4_2) -> W::Output {
353+
fn dispatch_sse4_2<W: WithSimd>(f: W, sse4_2: Sse4_2) -> W::Output {
355354
f.with_simd(sse4_2)
356355
}
357356

358357
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
359358
#[target_feature(enable = "avx2,fma")]
360359
#[inline]
361-
unsafe fn dispatch_avx2<W: WithSimd>(f: W, avx2: Avx2) -> W::Output {
360+
fn dispatch_avx2<W: WithSimd>(f: W, avx2: Avx2) -> W::Output {
362361
f.with_simd(avx2)
363362
}
364363

fearless_simd/src/macros.rs

Lines changed: 71 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,46 +3,101 @@
33

44
//! Macros publicly exported
55
6-
#![expect(
7-
missing_docs,
8-
reason = "TODO: https://github.com/linebender/fearless_simd/issues/40"
9-
)]
10-
6+
/// Defines a new function which dispatches to a SIMD-generic function, enabling the correct
7+
/// target features.
8+
///
9+
/// The `fn` token in the definition can be prefixed with a visibility (e.g. `pub`),
10+
/// to set the visibility of the outer function.
11+
/// We recommend that the implementation function remains private, and
12+
/// should only be called through the dispatch function.
13+
/// (The exact patterns for SIMD functions using Fearleess SIMD have not
14+
/// yet been designed/enumerated).
15+
///
16+
/// The implementation function (which is outside of this macro) *should* have the
17+
/// `#[inline(always)]` attribute.
18+
/// There are likely to be severe performance consequences if this is not the case, as
19+
/// Rust will be unable to inline SIMD intrinsics in that case.
20+
///
21+
/// The `fn` token in the definition can be prefixed with `unsafe`, to allow an unsafe inner function.
22+
/// The safety comment added by you in the call to `simd_dispatch` the function must have
23+
/// the preconditions required to call the inner function.
24+
///
25+
/// # Examples
26+
///
27+
/// ```rust
28+
/// use fearless_simd::{Simd, simd_dispatch};
29+
///
30+
/// #[inline(always)]
31+
/// fn sigmoid_impl<S: Simd>(simd: S, x: &[f32], out: &mut [f32]) { /* ... */ }
32+
///
33+
/// simd_dispatch!(fn sigmoid(level, x: &[f32], out: &mut [f32]) = sigmoid_impl);
34+
/// ```
35+
///
36+
/// The signature of the generated function will be:
37+
///
38+
/// ```rust
39+
/// use fearless_simd::Level;
40+
/// fn sigmoid(level: Level, x: &[f32], out: &mut [f32]) { /* ... */ }
41+
/// ```
1142
#[macro_export]
1243
macro_rules! simd_dispatch {
1344
(
1445
$( #[$meta:meta] )* $vis:vis
15-
$func:ident ( level $( , $arg:ident : $ty:ty $(,)? )* ) $( -> $ret:ty )?
46+
unsafe fn $func:ident ( level $( , $arg:ident : $ty:ty $(,)? )* ) $( -> $ret:ty )?
47+
= $inner:ident
48+
) => {
49+
simd_dispatch!{@impl => $(#[$meta])* $vis (unsafe) fn $func (level, $(,$arg:$ty,)*) $(->$ret)? = $inner}
50+
};
51+
(
52+
$( #[$meta:meta] )* $vis:vis
53+
fn $func:ident ( level $( , $arg:ident : $ty:ty $(,)? )* ) $( -> $ret:ty )?
54+
= $inner:ident
55+
) => {
56+
simd_dispatch!{@impl => $(#[$meta])* $vis () fn $func (level $(,$arg:$ty)*) $(->$ret)? = $inner}
57+
};
58+
(
59+
@impl => $( #[$meta:meta] )* $vis:vis
60+
($($unsafe: ident)?) fn $func:ident ( level $( , $arg:ident : $ty:ty $(,)? )* ) $( -> $ret:ty )?
1661
= $inner:ident
1762
) => {
1863
$( #[$meta] )* $vis
19-
fn $func(level: $crate::Level $(, $arg: $ty )*) $( -> $ret )? {
64+
$($unsafe)? fn $func(level: $crate::Level $(, $arg: $ty )*) $( -> $ret )? {
2065
#[cfg(target_arch = "aarch64")]
2166
#[target_feature(enable = "neon")]
2267
#[inline]
23-
unsafe fn inner_neon(neon: $crate::aarch64::Neon $( , $arg: $ty )* ) $( -> $ret )? {
24-
$inner( neon $( , $arg )* )
68+
$($unsafe)? fn inner_neon(neon: $crate::aarch64::Neon $( , $arg: $ty )* ) $( -> $ret )? {
69+
$($unsafe)? {
70+
$inner( neon $( , $arg )* )
71+
}
2572
}
2673
#[cfg(all(target_arch = "wasm32", target_feature = "simd128"))]
2774
#[inline]
28-
unsafe fn inner_wasm_simd128(simd128: $crate::wasm32::WasmSimd128 $( , $arg: $ty )* ) $( -> $ret )? {
29-
$inner( simd128 $( , $arg )* )
75+
$($unsafe)? fn inner_wasm_simd128(simd128: $crate::wasm32::WasmSimd128 $( , $arg: $ty )* ) $( -> $ret )? {
76+
$($unsafe)? {
77+
$inner( simd128 $( , $arg )* )
78+
}
3079
}
3180
#[cfg(all(feature = "std", any(target_arch = "x86", target_arch = "x86_64")))]
3281
#[target_feature(enable = "sse4.2")]
3382
#[inline]
34-
unsafe fn inner_sse4_2(sse4_2: $crate::x86::Sse4_2 $( , $arg: $ty )* ) $( -> $ret )? {
35-
$inner( sse4_2 $( , $arg )* )
83+
$($unsafe)? fn inner_sse4_2(sse4_2: $crate::x86::Sse4_2 $( , $arg: $ty )* ) $( -> $ret )? {
84+
$($unsafe)? {
85+
$inner( sse4_2 $( , $arg )* )
86+
}
3687
}
3788
#[cfg(all(feature = "std", any(target_arch = "x86", target_arch = "x86_64")))]
3889
#[target_feature(enable = "avx2,fma")]
3990
#[inline]
40-
unsafe fn inner_avx2(avx2: $crate::x86::Avx2 $( , $arg: $ty )* ) $( -> $ret )? {
41-
$inner( avx2 $( , $arg )* )
91+
$($unsafe)? fn inner_avx2(avx2: $crate::x86::Avx2 $( , $arg: $ty )* ) $( -> $ret )? {
92+
$($unsafe)? {
93+
$inner( avx2 $( , $arg )* )
94+
}
4295
}
4396
match level {
4497
$crate::Level::Fallback(fb) => {
45-
$inner(fb $( , $arg )* )
98+
$($unsafe)? {
99+
$inner(fb $( , $arg )* )
100+
}
46101
},
47102
#[cfg(target_arch = "aarch64")]
48103
$crate::Level::Neon(neon) => unsafe { inner_neon (neon $( , $arg )* ) }

0 commit comments

Comments
 (0)