Skip to content

Commit dfbe997

Browse files
authored
x64: Add bitwidth-specific newtypes for registers. (#13184)
This change addresses the kind of lowering mismatch that we saw in CVE-2026-24116 ([advisory]): a case where an instruction lowering reasons about a narrower value (e.g. a scalar `f64`) in a wider register (e.g. a 128-bit XMM), and implicit conversions cause load-sinking to happen in a way that leads to a larger-than-expected load from memory. Background: in Cranelift in general, we have an "upper bits undefined" invariant. That is, a 32/64-bit FP value can live in a 128-bit XMM register, or 8/16/32-bit integer can live in a 64-bit GPR, with no problem. The instruction lowering sequences generally deal fine with this: e.g., an `add` instruction doesn't care what is in the upper bits (garbage in, garbage out, and carry only flows from lower to upper, not the other way). However, load-sinking, designed to be as "automatic" as possible to apply in all of the places where it is applicable, throws a wrench into this: because we don't make a distinction about how large the actual operand is, and many layers of the ISLE deal only in register-related operand types (`RegMem`, `GprMem` or `XmmMem`), we can get an implicit load of the *entire* register width when we had started with a narrowly-typed value. This is a miscompile that can become serious if paired with a user of Cranelift (e.g. Wasmtime) that relies on certain characteristics of out-of-bound accesses to maintain a sandbox. In this PR, we take the approach discussed in #12477: we add newtypes for all *specific* bit-widths of operand, to make the distinction solely at metacopmilation time (i.e., when the ISLE DSL itself is typechecked) in the lowering patterns. The expected widths flow upward from the auto-generated instruction constructors in the assembler layer. We use specific types almost universally; in a few cases where helpers do a dynamic dispatch on type, we have specific generic-to-specific-width converters that *also* take the `ty` and assert that it has the correct width. Then, finally and most importantly, the auto-conversions for load sinking will only work (and will release-assert otherwise) if the type-width of the corresponding `Value` matches. As a result of doing this large refactor, three separate bugs in sunk-load width were found; see the new filetests. They are: - `bitselect` on scalar f32/f64 args could sink a load, and would use a SIMD instruction that could fold a load and do a 128-bit load. (Wasm lowering only uses `bitselect` with 128-bit args.) - `swiden_low` and `uwiden_low` could fold a 128-bit load and do a 64-bit load (i.e., could *narrow* the memory op) because the SSE4.1 instruction takes a 64-bit source operand. (Wasm lowering inserts bitcasts that prevent load-sinking in this case.) - `fcvt_from_sint` could likewise turn a 128-bit load into a 64-bit load. (Wasm lowering likewise foils the bug with bitcasts.) I verified that none of them are reachable from Wasm, fortunately, so we don't have a repeat of the above CVE. Fixes #12477. [advisory]: GHSA-vc8c-j3xm-xj73
1 parent 599d821 commit dfbe997

9 files changed

Lines changed: 2712 additions & 533 deletions

File tree

cranelift/codegen/meta/src/gen_asm.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ fn include_inst(inst: &Inst) -> bool {
1919
}
2020

2121
/// Returns the Rust type used for the `IsleConstructorRaw` variants.
22+
///
23+
/// RegMem operand types are width-tagged based on the DSL operand width
24+
/// (e.g. `rm8` → `GprMem8`, `xmm_m128` with `align` → `XmmMemAligned128`).
2225
fn rust_param_raw(op: &Operand) -> String {
2326
match op.location.kind() {
2427
OperandKind::Imm(loc) => {
@@ -32,7 +35,8 @@ fn rust_param_raw(op: &Operand) -> String {
3235
OperandKind::RegMem(rm) => {
3336
let reg = rm.reg_class().unwrap();
3437
let aligned = if op.align { "Aligned" } else { "" };
35-
format!("&{reg}Mem{aligned}")
38+
let bits = rm.bits();
39+
format!("&{reg}Mem{aligned}{bits}")
3640
}
3741
OperandKind::Mem(_) => {
3842
format!("&SyntheticAmode")
@@ -243,7 +247,8 @@ pub fn generate_rust_macro(f: &mut Formatter, insts: &[Inst]) {
243247
}
244248

245249
/// Returns the type of this operand in ISLE as a part of the ISLE "raw"
246-
/// constructors.
250+
/// constructors. RegMem operand types are width-tagged based on the DSL
251+
/// operand width; see [`rust_param_raw`].
247252
fn isle_param_raw(op: &Operand) -> String {
248253
match op.location.kind() {
249254
OperandKind::Imm(loc) => {
@@ -265,7 +270,8 @@ fn isle_param_raw(op: &Operand) -> String {
265270
OperandKind::RegMem(rm) => {
266271
let reg = rm.reg_class().unwrap();
267272
let aligned = if op.align { "Aligned" } else { "" };
268-
format!("{reg}Mem{aligned}")
273+
let bits = rm.bits();
274+
format!("{reg}Mem{aligned}{bits}")
269275
}
270276
}
271277
}

cranelift/codegen/src/isa/x64/inst.isle

Lines changed: 1138 additions & 397 deletions
Large diffs are not rendered by default.

cranelift/codegen/src/isa/x64/inst/args.rs

Lines changed: 70 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,25 @@ pub trait FromWritableReg: Sized {
2424

2525
/// A macro for defining a newtype of `Reg` that enforces some invariant about
2626
/// the wrapped `Reg` (such as that it is of a particular register class).
27+
///
28+
/// Each `reg_mem` / `reg_mem_imm` entry can optionally carry:
29+
/// * `aligned:BOOL` -- require aligned memory operands (used for SSE
30+
/// non-VEX encodings).
31+
/// * `size:BITS` -- the bit width of the value the operand holds. This
32+
/// is purely a type-level distinction (a sized newtype is a different
33+
/// Rust type than an unsized one of the same "family"), used to prevent
34+
/// cross-width operand mix-ups at compile time.
2735
macro_rules! newtype_of_reg {
2836
(
2937
$newtype_reg:ident,
3038
$newtype_writable_reg:ident,
3139
$newtype_option_writable_reg:ident,
32-
reg_mem: ($($newtype_reg_mem:ident $(aligned:$aligned:ident)?),*),
33-
reg_mem_imm: ($($newtype_reg_mem_imm:ident $(aligned:$aligned_imm:ident)?),*),
40+
reg_mem: ($($newtype_reg_mem:ident
41+
$(size:$size:literal)?
42+
$(aligned:$aligned:ident)?),*),
43+
reg_mem_imm: ($($newtype_reg_mem_imm:ident
44+
$(size:$size_imm:literal)?
45+
$(aligned:$aligned_imm:ident)?),*),
3446
|$check_reg:ident| $check:expr
3547
) => {
3648
/// A newtype wrapper around `Reg`.
@@ -146,6 +158,13 @@ macro_rules! newtype_of_reg {
146158
}
147159

148160
impl $newtype_reg_mem {
161+
$(
162+
/// The bit width of the value this operand holds. This is
163+
/// a type-level distinction only; the underlying `RegMem`
164+
/// does not record the width.
165+
pub const SIZE_BITS: u32 = $size;
166+
)?
167+
149168
/// Construct a `RegMem` newtype from the given `RegMem`, or return
150169
/// `None` if the `RegMem` is not a valid instance of this `RegMem`
151170
/// newtype.
@@ -227,6 +246,13 @@ macro_rules! newtype_of_reg {
227246
}
228247

229248
impl $newtype_reg_mem_imm {
249+
$(
250+
/// The bit width of the value this operand holds. This is
251+
/// a type-level distinction only; the underlying
252+
/// `RegMemImm` does not record the width.
253+
pub const SIZE_BITS: u32 = $size_imm;
254+
)?
255+
230256
/// Construct this newtype from the given `RegMemImm`, or return
231257
/// `None` if the `RegMemImm` is not a valid instance of this
232258
/// newtype.
@@ -294,12 +320,24 @@ macro_rules! newtype_of_reg {
294320
}
295321

296322
// Define a newtype of `Reg` for general-purpose registers.
323+
//
324+
// The sized `GprMemN` / `GprMemImmN` variants indicate the width of
325+
// the register or memory operand that an instruction operates on; for
326+
// a memory operand, the width of the load or store in particular.
297327
newtype_of_reg!(
298328
Gpr,
299329
WritableGpr,
300330
OptionWritableGpr,
301-
reg_mem: (GprMem),
302-
reg_mem_imm: (GprMemImm),
331+
reg_mem: (GprMem,
332+
GprMem8 size:8,
333+
GprMem16 size:16,
334+
GprMem32 size:32,
335+
GprMem64 size:64),
336+
reg_mem_imm: (GprMemImm,
337+
GprMemImm8 size:8,
338+
GprMemImm16 size:16,
339+
GprMemImm32 size:32,
340+
GprMemImm64 size:64),
303341
|reg| reg.class() == RegClass::Int
304342
);
305343

@@ -324,12 +362,38 @@ impl Gpr {
324362
}
325363

326364
// Define a newtype of `Reg` for XMM registers.
365+
//
366+
// Specific-width newtypes are, as for the GPR case above, used to
367+
// distinguish the width of the operation particularly when a memory
368+
// load or store occurs.
327369
newtype_of_reg!(
328370
Xmm,
329371
WritableXmm,
330372
OptionWritableXmm,
331-
reg_mem: (XmmMem, XmmMemAligned aligned:true),
332-
reg_mem_imm: (XmmMemImm, XmmMemAlignedImm aligned:true),
373+
reg_mem: (XmmMem,
374+
XmmMem8 size:8,
375+
XmmMem16 size:16,
376+
XmmMem32 size:32,
377+
XmmMem64 size:64,
378+
XmmMem128 size:128,
379+
XmmMemAligned aligned:true,
380+
XmmMemAligned8 size:8 aligned:true,
381+
XmmMemAligned16 size:16 aligned:true,
382+
XmmMemAligned32 size:32 aligned:true,
383+
XmmMemAligned64 size:64 aligned:true,
384+
XmmMemAligned128 size:128 aligned:true),
385+
reg_mem_imm: (XmmMemImm,
386+
XmmMemImm8 size:8,
387+
XmmMemImm16 size:16,
388+
XmmMemImm32 size:32,
389+
XmmMemImm64 size:64,
390+
XmmMemImm128 size:128,
391+
XmmMemAlignedImm aligned:true,
392+
XmmMemAlignedImm8 size:8 aligned:true,
393+
XmmMemAlignedImm16 size:16 aligned:true,
394+
XmmMemAlignedImm32 size:32 aligned:true,
395+
XmmMemAlignedImm64 size:64 aligned:true,
396+
XmmMemAlignedImm128 size:128 aligned:true),
333397
|reg| reg.class() == RegClass::Float
334398
);
335399

0 commit comments

Comments
 (0)