Skip to content

cranelift: Optimize select_spectre_guard, carefully #8139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jameysharp merged 2 commits into from
Mar 20, 2024
Merged

cranelift: Optimize select_spectre_guard, carefully #8139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e70e51b
cranelift: Optimize select_spectre_guard, carefully
jameysharp Mar 14, 2024
3beb9c2
Add filetests
jameysharp Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cranelift/codegen/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
49 changes: 31 additions & 18 deletions cranelift/codegen/meta/src/shared/instructions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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(
Expand Down
14 changes: 14 additions & 0 deletions cranelift/codegen/src/opts/spectre.isle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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))
26 changes: 26 additions & 0 deletions cranelift/filetests/filetests/egraph/spectre.clif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
48 changes: 18 additions & 30 deletions tests/disas/typed-funcrefs.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down