Skip to content

Commit

Permalink
PCC: x64: insertlane instructions read only scalar-sized values. (byt…
Browse files Browse the repository at this point in the history
…ecodealliance#8207)

* PCC: x64: insertlane instructions read only scalar-sized values.

Also fix `clamp_range` on greater-than-64-bit values: no range fact is
possible in this case (propagate `Option` a bit deeper to represent
this).

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=67538.

* Rebase to latest main with leaf-function changes and update test expectations.
  • Loading branch information
cfallin authored Mar 21, 2024
1 parent d69ba34 commit a79cf76
Show file tree
Hide file tree
Showing 6 changed files with 618 additions and 16 deletions.
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/aarch64/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ pub(crate) fn check(
check_constant(ctx, vcode, rd, 64, constant)
} else {
check_output(ctx, vcode, rd, &[], |_vcode| {
Ok(Fact::max_range_for_width(64))
Ok(Some(Fact::max_range_for_width(64)))
})
}
}
Expand Down Expand Up @@ -426,7 +426,7 @@ fn check_addr<'a>(
trace!(
"checking a load: loaded_fact = {loaded_fact:?} result_fact = {result_fact:?}"
);
if ctx.subsumes_fact_optionals(Some(&loaded_fact), result_fact) {
if ctx.subsumes_fact_optionals(loaded_fact.as_ref(), result_fact) {
Ok(())
} else {
Err(PccError::UnsupportedFact)
Expand Down
20 changes: 20 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,26 @@ impl SseOpcode {
_ => 8,
}
}

/// Is `src2` with this opcode a scalar, as for lane insertions?
pub(crate) fn has_scalar_src2(self) -> bool {
match self {
SseOpcode::Pinsrb | SseOpcode::Pinsrw | SseOpcode::Pinsrd => true,
SseOpcode::Pmovsxbw
| SseOpcode::Pmovsxbd
| SseOpcode::Pmovsxbq
| SseOpcode::Pmovsxwd
| SseOpcode::Pmovsxwq
| SseOpcode::Pmovsxdq => true,
SseOpcode::Pmovzxbw
| SseOpcode::Pmovzxbd
| SseOpcode::Pmovzxbq
| SseOpcode::Pmovzxwd
| SseOpcode::Pmovzxwq
| SseOpcode::Pmovzxdq => true,
_ => false,
}
}
}

impl fmt::Debug for SseOpcode {
Expand Down
35 changes: 31 additions & 4 deletions cranelift/codegen/src/isa/x64/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub(crate) fn check(
dst,
..
} => check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| {
Ok(Fact::constant(64, 0))
Ok(Some(Fact::constant(64, 0)))
}),

Inst::AluConstOp { dst, .. } => undefined_result(ctx, vcode, dst, 64, 64),
Expand Down Expand Up @@ -319,7 +319,7 @@ pub(crate) fn check(

Inst::Imm { simm64, dst, .. } => {
check_output(ctx, vcode, dst.to_writable_reg(), &[], |_vcode| {
Ok(Fact::constant(64, simm64))
Ok(Some(Fact::constant(64, simm64)))
})
}

Expand Down Expand Up @@ -629,6 +629,10 @@ pub(crate) fn check(
let (ty, size) = match op {
AvxOpcode::Vmovss => (F32, 32),
AvxOpcode::Vmovsd => (F64, 64),
AvxOpcode::Vpinsrb => (I8, 8),
AvxOpcode::Vpinsrw => (I16, 16),
AvxOpcode::Vpinsrd => (I32, 32),
AvxOpcode::Vpinsrq => (I64, 64),

// We assume all other operations happen on 128-bit values.
_ => (I8X16, 128),
Expand Down Expand Up @@ -767,6 +771,29 @@ pub(crate) fn check(
RegMem::Reg { .. } => Ok(()),
},

Inst::XmmRmRImm {
dst,
ref src2,
size,
op,
..
} if op.has_scalar_src2() => {
match <&RegMem>::from(src2) {
RegMem::Mem { ref addr } => {
check_load(
ctx,
None,
addr,
vcode,
size.to_type(),
size.to_bits().into(),
)?;
}
RegMem::Reg { .. } => {}
}
ensure_no_fact(vcode, dst.to_reg())
}

Inst::XmmRmRImm { dst, ref src2, .. } => {
match <&RegMem>::from(src2) {
RegMem::Mem { ref addr } => {
Expand Down Expand Up @@ -917,8 +944,8 @@ fn check_mem<'a>(
loaded_fact,
result_fact
);
if ctx.subsumes_fact_optionals(Some(&loaded_fact), result_fact) {
Ok(Some(loaded_fact.clone()))
if ctx.subsumes_fact_optionals(loaded_fact.as_ref(), result_fact) {
Ok(loaded_fact.clone())
} else {
Err(PccError::UnsupportedFact)
}
Expand Down
30 changes: 20 additions & 10 deletions cranelift/codegen/src/machinst/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ pub(crate) fn clamp_range(
to_bits: u16,
from_bits: u16,
fact: Option<Fact>,
) -> PccResult<Fact> {
let max = if from_bits == 64 {
) -> PccResult<Option<Fact>> {
let max = if from_bits > 64 {
return Ok(None);
} else if from_bits == 64 {
u64::MAX
} else {
(1u64 << from_bits) - 1
Expand All @@ -42,32 +44,40 @@ pub(crate) fn clamp_range(
);
Ok(fact
.and_then(|f| ctx.uextend(&f, from_bits, to_bits))
.unwrap_or_else(|| {
.or_else(|| {
let result = Fact::Range {
bit_width: to_bits,
min: 0,
max,
};
trace!(" -> clamping to {:?}", result);
result
Some(result)
}))
}

pub(crate) fn check_subsumes(ctx: &FactContext, subsumer: &Fact, subsumee: &Fact) -> PccResult<()> {
check_subsumes_optionals(ctx, Some(subsumer), Some(subsumee))
}

pub(crate) fn check_subsumes_optionals(
ctx: &FactContext,
subsumer: Option<&Fact>,
subsumee: Option<&Fact>,
) -> PccResult<()> {
trace!(
"checking if derived fact {:?} subsumes stated fact {:?}",
subsumer,
subsumee
);

if ctx.subsumes(subsumer, subsumee) {
if ctx.subsumes_fact_optionals(subsumer, subsumee) {
Ok(())
} else {
Err(PccError::UnsupportedFact)
}
}

pub(crate) fn check_output<I: VCodeInst, F: FnOnce(&VCode<I>) -> PccResult<Fact>>(
pub(crate) fn check_output<I: VCodeInst, F: FnOnce(&VCode<I>) -> PccResult<Option<Fact>>>(
ctx: &FactContext,
vcode: &mut VCode<I>,
out: Writable<Reg>,
Expand All @@ -76,14 +86,14 @@ pub(crate) fn check_output<I: VCodeInst, F: FnOnce(&VCode<I>) -> PccResult<Fact>
) -> PccResult<()> {
if let Some(fact) = vcode.vreg_fact(out.to_reg().into()) {
let result = f(vcode)?;
check_subsumes(ctx, &result, fact)
check_subsumes_optionals(ctx, result.as_ref(), Some(fact))
} else if ins.iter().any(|r| {
vcode
.vreg_fact(r.into())
.map(|fact| fact.propagates())
.unwrap_or(false)
}) {
if let Ok(fact) = f(vcode) {
if let Ok(Some(fact)) = f(vcode) {
trace!("setting vreg {:?} to {:?}", out, fact);
vcode.set_vreg_fact(out.to_reg().into(), fact);
}
Expand All @@ -93,7 +103,7 @@ pub(crate) fn check_output<I: VCodeInst, F: FnOnce(&VCode<I>) -> PccResult<Fact>
}
}

pub(crate) fn check_unop<I: VCodeInst, F: FnOnce(&Fact) -> PccResult<Fact>>(
pub(crate) fn check_unop<I: VCodeInst, F: FnOnce(&Fact) -> PccResult<Option<Fact>>>(
ctx: &FactContext,
vcode: &mut VCode<I>,
reg_width: u16,
Expand All @@ -107,7 +117,7 @@ pub(crate) fn check_unop<I: VCodeInst, F: FnOnce(&Fact) -> PccResult<Fact>>(
})
}

pub(crate) fn check_binop<I: VCodeInst, F: FnOnce(&Fact, &Fact) -> PccResult<Fact>>(
pub(crate) fn check_binop<I: VCodeInst, F: FnOnce(&Fact, &Fact) -> PccResult<Option<Fact>>>(
ctx: &FactContext,
vcode: &mut VCode<I>,
reg_width: u16,
Expand Down
Loading

0 comments on commit a79cf76

Please sign in to comment.