diff --git a/cranelift/codegen/build.rs b/cranelift/codegen/build.rs index d5c5f7a57469..70b62f580103 100644 --- a/cranelift/codegen/build.rs +++ b/cranelift/codegen/build.rs @@ -231,8 +231,9 @@ fn get_isle_compilations( src_opts.join("icmp.isle"), src_opts.join("remat.isle"), src_opts.join("selects.isle"), - src_opts.join("spaceship.isle"), src_opts.join("shifts.isle"), + src_opts.join("spaceship.isle"), + src_opts.join("spectre.isle"), src_opts.join("vector.isle"), ], untracked_inputs: vec![clif_opt_isle], diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index 5e75ac3a4b20..909f801601a1 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1449,19 +1449,36 @@ pub(crate) fn define( Conditional select intended for Spectre guards. This operation is semantically equivalent to a select instruction. - However, it is guaranteed to not be removed or otherwise altered by any - optimization pass, and is guaranteed to result in a conditional-move - instruction, not a branch-based lowering. As such, it is suitable - for use when producing Spectre guards. For example, a bounds-check - may guard against unsafe speculation past a bounds-check conditional - branch by passing the address or index to be accessed through a - conditional move, also gated on the same condition. Because no - Spectre-vulnerable processors are known to perform speculation on - conditional move instructions, this is guaranteed to pick the - correct input. If the selected input in case of overflow is a "safe" - value, for example a null pointer that causes an exception in the - speculative path, this ensures that no Spectre vulnerability will - exist. + However, this instruction prohibits all speculation on the + controlling value when determining which input to use as the result. + As such, it is suitable for use in Spectre guards. + + For example, on a target which may speculatively execute branches, + the lowering of this instruction is guaranteed to not conditionally + branch. Instead it will typically lower to a conditional move + instruction. (No Spectre-vulnerable processors are known to perform + value speculation on conditional move instructions.) + + Ensure that the instruction you're trying to protect from Spectre + attacks has a data dependency on the result of this instruction. + That prevents an out-of-order CPU from evaluating that instruction + until the result of this one is known, which in turn will be blocked + until the controlling value is known. + + Typical usage is to use a bounds-check as the controlling value, + and select between either a null pointer if the bounds-check + fails, or an in-bounds address otherwise, so that dereferencing + the resulting address with a load or store instruction will trap if + the bounds-check failed. When this instruction is used in this way, + any microarchitectural side effects of the memory access will only + occur after the bounds-check finishes, which ensures that no Spectre + vulnerability will exist. + + Optimization opportunities for this instruction are limited compared + to a normal select instruction, but it is allowed to be replaced + by other values which are functionally equivalent as long as doing + so does not introduce any new opportunities to speculate on the + controlling value. "#, &formats.ternary, ) @@ -1470,11 +1487,7 @@ pub(crate) fn define( Operand::new("x", Any).with_doc("Value to use when `c` is true"), Operand::new("y", Any).with_doc("Value to use when `c` is false"), ]) - .operands_out(vec![Operand::new("a", Any)]) - .other_side_effects() - // We can de-duplicate spectre selects since the side effect is - // idempotent. - .side_effects_idempotent(), + .operands_out(vec![Operand::new("a", Any)]), ); ig.push( diff --git a/cranelift/codegen/src/opts/spectre.isle b/cranelift/codegen/src/opts/spectre.isle new file mode 100644 index 000000000000..c58e29eaa9fe --- /dev/null +++ b/cranelift/codegen/src/opts/spectre.isle @@ -0,0 +1,14 @@ +;; Rewrites for `select_spectre_guard`. Check these rules carefully! This +;; instruction prohibits all speculation on the controlling value when +;; determining which input to use as the result. Rewrites must respect that +;; requirement. + +;; If we statically know which value will be the result, it's safe to just use +;; that value. No speculation on the controlling value is possible if we simply +;; don't depend on that value at all. +(rule (simplify (select_spectre_guard _ _ x x)) + (subsume x)) +(rule (simplify (select_spectre_guard _ (iconst_u _ (u64_nonzero _)) x _)) + (subsume x)) +(rule (simplify (select_spectre_guard _ (iconst_u _ 0) _ y)) + (subsume y)) diff --git a/cranelift/filetests/filetests/egraph/spectre.clif b/cranelift/filetests/filetests/egraph/spectre.clif new file mode 100644 index 000000000000..ca3432f61ffc --- /dev/null +++ b/cranelift/filetests/filetests/egraph/spectre.clif @@ -0,0 +1,26 @@ +test optimize +set opt_level=speed +target x86_64 + +function %same_value(i8, i64) -> i64 { +block0(v0: i8, v1: i64): + v2 = select_spectre_guard v0, v1, v1 + return v2 +} +; check: return v1 + +function %const_true(i64, i64) -> i64 { +block0(v0: i64, v1: i64): + v2 = iconst.i8 42 + v3 = select_spectre_guard v2, v0, v1 + return v3 +} +; check: return v0 + +function %const_false(i64, i64) -> i64 { +block0(v0: i64, v1: i64): + v2 = iconst.i8 0 + v3 = select_spectre_guard v2, v0, v1 + return v3 +} +; check: return v1 diff --git a/tests/disas/typed-funcrefs.wat b/tests/disas/typed-funcrefs.wat index 10ba6a95599e..8bf6beb374cf 100644 --- a/tests/disas/typed-funcrefs.wat +++ b/tests/disas/typed-funcrefs.wat @@ -151,12 +151,9 @@ ;; v32 -> v4 ;; v33 -> v5 ;; @0048 v12 = load.i64 notrap aligned v0+72 -;; v62 = iconst.i8 0 -;; @0048 v15 = iconst.i64 0 ;; v70 = iconst.i64 8 ;; @0048 v14 = iadd v12, v70 ; v70 = 8 -;; @0048 v16 = select_spectre_guard v62, v15, v14 ; v62 = 0, v15 = 0 -;; @0048 v17 = load.i64 table_oob aligned table v16 +;; @0048 v17 = load.i64 table_oob aligned table v14 ;; v58 = iconst.i64 -2 ;; @0048 v18 = band v17, v58 ; v58 = -2 ;; @0048 brif v17, block3(v18), block2 @@ -175,22 +172,19 @@ ;; @004a v26 = load.i64 notrap aligned readonly v19+32 ;; @004a v27 = call_indirect sig1, v25(v26, v0, v2, v3, v4, v5) ;; @005b v38 = load.i64 notrap aligned v0+72 -;; v79 = iconst.i8 0 -;; v80 = iconst.i64 0 ;; v78 = iconst.i64 16 ;; @005b v40 = iadd v38, v78 ; v78 = 16 -;; @005b v42 = select_spectre_guard v79, v80, v40 ; v79 = 0, v80 = 0 -;; @005b v43 = load.i64 table_oob aligned table v42 -;; v81 = iconst.i64 -2 -;; v82 = band v43, v81 ; v81 = -2 -;; @005b brif v43, block5(v82), block4 +;; @005b v43 = load.i64 table_oob aligned table v40 +;; v79 = iconst.i64 -2 +;; v80 = band v43, v79 ; v79 = -2 +;; @005b brif v43, block5(v80), block4 ;; ;; block4 cold: -;; v83 = load.i64 notrap aligned readonly v0+56 -;; v84 = load.i64 notrap aligned readonly v83+72 -;; v85 = iconst.i32 0 +;; v81 = load.i64 notrap aligned readonly v0+56 +;; v82 = load.i64 notrap aligned readonly v81+72 +;; v83 = iconst.i32 0 ;; @0059 v34 = iconst.i32 2 -;; @005b v50 = call_indirect sig0, v84(v0, v85, v34) ; v85 = 0, v34 = 2 +;; @005b v50 = call_indirect sig0, v82(v0, v83, v34) ; v83 = 0, v34 = 2 ;; @005b jump block5(v50) ;; ;; block5(v45: i64): @@ -227,12 +221,9 @@ ;; v32 -> v4 ;; v33 -> v5 ;; @0075 v12 = load.i64 notrap aligned v0+72 -;; v62 = iconst.i8 0 -;; @0075 v15 = iconst.i64 0 ;; v70 = iconst.i64 8 ;; @0075 v14 = iadd v12, v70 ; v70 = 8 -;; @0075 v16 = select_spectre_guard v62, v15, v14 ; v62 = 0, v15 = 0 -;; @0075 v17 = load.i64 table_oob aligned table v16 +;; @0075 v17 = load.i64 table_oob aligned table v14 ;; v58 = iconst.i64 -2 ;; @0075 v18 = band v17, v58 ; v58 = -2 ;; @0075 brif v17, block3(v18), block2 @@ -251,22 +242,19 @@ ;; @0075 v26 = load.i64 notrap aligned readonly v19+32 ;; @0075 v27 = call_indirect sig0, v25(v26, v0, v2, v3, v4, v5) ;; @0087 v38 = load.i64 notrap aligned v0+72 -;; v79 = iconst.i8 0 -;; v80 = iconst.i64 0 ;; v78 = iconst.i64 16 ;; @0087 v40 = iadd v38, v78 ; v78 = 16 -;; @0087 v42 = select_spectre_guard v79, v80, v40 ; v79 = 0, v80 = 0 -;; @0087 v43 = load.i64 table_oob aligned table v42 -;; v81 = iconst.i64 -2 -;; v82 = band v43, v81 ; v81 = -2 -;; @0087 brif v43, block5(v82), block4 +;; @0087 v43 = load.i64 table_oob aligned table v40 +;; v79 = iconst.i64 -2 +;; v80 = band v43, v79 ; v79 = -2 +;; @0087 brif v43, block5(v80), block4 ;; ;; block4 cold: -;; v83 = load.i64 notrap aligned readonly v0+56 -;; v84 = load.i64 notrap aligned readonly v83+72 -;; v85 = iconst.i32 0 +;; v81 = load.i64 notrap aligned readonly v0+56 +;; v82 = load.i64 notrap aligned readonly v81+72 +;; v83 = iconst.i32 0 ;; @0085 v34 = iconst.i32 2 -;; @0087 v50 = call_indirect sig1, v84(v0, v85, v34) ; v85 = 0, v34 = 2 +;; @0087 v50 = call_indirect sig1, v82(v0, v83, v34) ; v83 = 0, v34 = 2 ;; @0087 jump block5(v50) ;; ;; block5(v45: i64):