Skip to content
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.

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

Move volatile_store and ensure alignment #794

Merged
merged 6 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 43 additions & 19 deletions src/kani-compiler/rustc_codegen_kani/src/codegen/intrinsic.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR MIT
//! this module handles intrinsics
use tracing::{debug, warn};

use crate::GotocCtx;
use cbmc::goto_program::{BuiltinFn, Expr, Location, Stmt, Type};
use rustc_middle::mir::Place;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::Instance;
use rustc_middle::ty::{self, Ty, TyS};
use rustc_span::Span;
use tracing::{debug, warn};

macro_rules! emit_concurrency_warning {
($intrinsic: expr, $loc: expr) => {{
warn!(
"Kani does not support concurrency for now. `{}` in {} treated as a sequential operation.",
$intrinsic,
$loc.short_string()
);
}};
}

struct SizeAlign {
size: Expr,
Expand Down Expand Up @@ -279,8 +288,8 @@ impl<'tcx> GotocCtx<'tcx> {
// Note: Atomic arithmetic operations wrap around on overflow.
macro_rules! codegen_atomic_binop {
($op: ident) => {{
warn!("Kani does not support concurrency for now. {} treated as a sequential operation.", intrinsic);
let loc = self.codegen_span_option(span);
emit_concurrency_warning!(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference();
let tmp = self.gen_temp_variable(var1.typ().clone(), loc.clone()).to_expr();
Expand Down Expand Up @@ -519,6 +528,10 @@ impl<'tcx> GotocCtx<'tcx> {
"volatile_copy_memory" => codegen_intrinsic_copy!(Memmove),
"volatile_copy_nonoverlapping_memory" => codegen_intrinsic_copy!(Memcpy),
"volatile_load" => self.codegen_expr_to_place(p, fargs.remove(0).dereference()),
"volatile_store" => {
assert!(self.place_ty(p).is_unit());
self.codegen_volatile_store(instance, intrinsic, fargs, loc)
}
"wrapping_add" => codegen_wrapping_op!(plus),
"wrapping_mul" => codegen_wrapping_op!(mul),
"wrapping_sub" => codegen_wrapping_op!(sub),
Expand Down Expand Up @@ -657,10 +670,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
warn!(
"Kani does not support concurrency for now. {} treated as a sequential operation.",
intrinsic
);
emit_concurrency_warning!(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc.clone());
let res_stmt = self.codegen_expr_to_place(p, var1);
Expand Down Expand Up @@ -688,10 +698,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
warn!(
"Kani does not support concurrency for now. {} treated as a sequential operation.",
intrinsic
);
emit_concurrency_warning!(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc.clone());
let tmp = self.gen_temp_variable(var1.typ().clone(), loc.clone()).to_expr();
Expand Down Expand Up @@ -727,10 +734,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
warn!(
"Kani does not support concurrency for now. {} treated as a sequential operation.",
intrinsic
);
emit_concurrency_warning!(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc.clone());
let tmp = self.gen_temp_variable(var1.typ().clone(), loc.clone()).to_expr();
Expand All @@ -743,10 +747,7 @@ impl<'tcx> GotocCtx<'tcx> {

/// Atomic no-ops (e.g., atomic_fence) are transformed into SKIP statements
fn codegen_atomic_noop(&mut self, intrinsic: &str, loc: Location) -> Stmt {
warn!(
"Kani does not support concurrency for now. {} treated as a sequential operation.",
intrinsic
);
emit_concurrency_warning!(intrinsic, loc);
let skip_stmt = Stmt::skip(loc.clone());
Stmt::atomic_block(vec![skip_stmt], loc)
}
Expand Down Expand Up @@ -1005,4 +1006,27 @@ impl<'tcx> GotocCtx<'tcx> {
.collect();
self.codegen_expr_to_place(p, Expr::vector_expr(cbmc_ret_ty, elems))
}

/// A volatile write of a memory location:
/// https://doc.rust-lang.org/std/ptr/fn.write_volatile.html
///
/// Undefined behavior if any of these conditions are violated:
/// * `dst` must be valid for writes (done by `--pointer-check`)
/// * `dst` must be properly aligned (done by `align_check` below)
fn codegen_volatile_store(
&mut self,
instance: Instance<'tcx>,
intrinsic: &str,
mut fargs: Vec<Expr>,
loc: Location,
) -> Stmt {
emit_concurrency_warning!(intrinsic, loc);
let dst = fargs.remove(0);
let src = fargs.remove(0);
let typ = instance.substs.type_at(0);
adpaco-aws marked this conversation as resolved.
Show resolved Hide resolved
let align = self.is_aligned(typ, dst.clone());
let align_check = Stmt::assert(align, "`dst` is properly aligned", loc.clone());
adpaco-aws marked this conversation as resolved.
Show resolved Hide resolved
adpaco-aws marked this conversation as resolved.
Show resolved Hide resolved
let expr = dst.dereference().assign(src, loc.clone());
Stmt::block(vec![align_check, expr], loc)
}
}
2 changes: 0 additions & 2 deletions src/kani-compiler/rustc_codegen_kani/src/overrides/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,8 @@ impl<'tcx> GotocHook<'tcx> for PtrWrite {
let name = with_no_trimmed_paths(|| tcx.def_path_str(instance.def_id()));
name == "core::ptr::write"
|| name == "core::ptr::write_unaligned"
|| name == "core::ptr::write_volatile"
|| name == "std::ptr::write"
|| name == "std::ptr::write_unaligned"
|| name == "std::ptr::write_volatile"
}

fn handle(
Expand Down
12 changes: 12 additions & 0 deletions src/kani-compiler/rustc_codegen_kani/src/utils/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use super::super::codegen::TypeExt;
use crate::GotocCtx;
use cbmc::btree_string_map;
use cbmc::goto_program::{Expr, ExprValue, Location, Stmt, SymbolTable, Type};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::TyS;
use tracing::debug;

// Should move into rvalue
Expand Down Expand Up @@ -67,6 +69,16 @@ impl<'tcx> GotocCtx<'tcx> {
Expr::statement_expression(body, t).with_location(loc)
}

/// Generates an expression `((dst as usize) % align_of(T) == 0`
/// to determine if `dst` is aligned.
pub fn is_aligned(&mut self, typ: &'tcx TyS<'_>, dst: Expr) -> Expr {
adpaco-aws marked this conversation as resolved.
Show resolved Hide resolved
let layout = self.layout_of(typ);
let align = Expr::int_constant(layout.align.abi.bytes(), Type::size_t());
let cast_dst = dst.cast_to(Type::size_t());
let zero = Type::size_t().zero();
cast_dst.rem(align).eq(zero)
danielsn marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn unsupported_msg(item: &str, url: Option<&str>) -> String {
let mut s = format!("{} is not currently supported by Kani", item);
if url.is_some() {
Expand Down
27 changes: 27 additions & 0 deletions tests/kani/Intrinsics/Volatile/store.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR MIT

// Check that `volatile_store` passes when it writes to an aligned value.
// This example is similar to the one that appears in the documentation for
// `write_unaligned`:
// https://doc.rust-lang.org/std/ptr/fn.write_unaligned.html
#![feature(core_intrinsics)]

// In contrast to the `Packed` struct in `store_fail.rs`, this struct includes
// padding so that each field is aligned.
struct NonPacked {
_padding: u8,
unaligned: u32,
}

fn main() {
let mut packed: NonPacked = unsafe { std::mem::zeroed() };
// Take the address of a 32-bit integer which is not aligned.
// In contrast to `&packed.unaligned as *mut _`, this has no undefined behavior.
let unaligned = std::ptr::addr_of_mut!(packed.unaligned);

// Store the value with `volatile_store`.
// This includes an alignment check for `unaligned` which should pass.
unsafe { std::intrinsics::volatile_store(unaligned, 42) };
assert!(packed.unaligned == 42);
}
29 changes: 29 additions & 0 deletions tests/kani/Intrinsics/Volatile/store_fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR MIT
// kani-verify-fail

// Check that `volatile_store` fails when it writes to an unaligned value.
// This example is similar to the one that appears in the documentation for
// `write_unaligned`:
// https://doc.rust-lang.org/std/ptr/fn.write_unaligned.html
#![feature(core_intrinsics)]

// `repr(packed)` forces the struct to be stripped of any padding and only align
// its fields to a byte.
#[repr(packed)]
struct Packed {
_padding: u8,
unaligned: u32,
}

fn main() {
let mut packed: Packed = unsafe { std::mem::zeroed() };
// Take the address of a 32-bit integer which is not aligned.
// In contrast to `&packed.unaligned as *mut _`, this has no undefined behavior.
let unaligned = std::ptr::addr_of_mut!(packed.unaligned);

// Store the value with `volatile_store`.
// This includes an alignment check for `unaligned` which should fail.
unsafe { std::intrinsics::volatile_store(unaligned, 42) };
assert!(packed.unaligned == 42);
}