Skip to content
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
27 changes: 6 additions & 21 deletions kani-compiler/src/codegen_cprover_gotoc/codegen/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,7 @@ use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{Instance, InstanceDef};
use rustc_span::Span;
use tracing::{debug, warn};

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

struct SizeAlign {
size: Expr,
Expand Down Expand Up @@ -345,7 +330,7 @@ impl<'tcx> GotocCtx<'tcx> {
macro_rules! codegen_atomic_binop {
($op: ident) => {{
let loc = self.codegen_span_option(span);
emit_concurrency_warning!(intrinsic, loc);
self.store_concurrent_construct(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference();
let (tmp, decl_stmt) =
Expand Down Expand Up @@ -833,7 +818,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
emit_concurrency_warning!(intrinsic, loc);
self.store_concurrent_construct(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc);
let res_stmt = self.codegen_expr_to_place(p, var1);
Expand Down Expand Up @@ -861,7 +846,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
emit_concurrency_warning!(intrinsic, loc);
self.store_concurrent_construct(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc);
let (tmp, decl_stmt) =
Expand Down Expand Up @@ -897,7 +882,7 @@ impl<'tcx> GotocCtx<'tcx> {
p: &Place<'tcx>,
loc: Location,
) -> Stmt {
emit_concurrency_warning!(intrinsic, loc);
self.store_concurrent_construct(intrinsic, loc);
let var1_ref = fargs.remove(0);
let var1 = var1_ref.dereference().with_location(loc);
let (tmp, decl_stmt) =
Expand All @@ -910,7 +895,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 {
emit_concurrency_warning!(intrinsic, loc);
self.store_concurrent_construct(intrinsic, loc);
let skip_stmt = Stmt::skip(loc);
Stmt::atomic_block(vec![skip_stmt], loc)
}
Expand Down
4 changes: 2 additions & 2 deletions kani-compiler/src/codegen_cprover_gotoc/codegen/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::codegen_cprover_gotoc::{GotocCtx, VtableCtx};
use crate::kani_middle::coercion::{
extract_unsize_casting, CoerceUnsizedInfo, CoerceUnsizedIterator, CoercionBase,
};
use crate::{emit_concurrency_warning, unwrap_or_return_codegen_unimplemented};
use crate::unwrap_or_return_codegen_unimplemented;
use cbmc::goto_program::{Expr, Location, Stmt, Symbol, Type};
use cbmc::MachineModel;
use cbmc::{btree_string_map, InternString, InternedString};
Expand Down Expand Up @@ -408,7 +408,7 @@ impl<'tcx> GotocCtx<'tcx> {
}
Rvalue::ThreadLocalRef(def_id) => {
// Since Kani is single-threaded, we treat a thread local like a static variable:
emit_concurrency_warning!("thread local", loc, "a static variable");
self.store_concurrent_construct("thread local (replaced by static variable)", loc);
self.codegen_static_pointer(*def_id, true)
}
// A CopyForDeref is equivalent to a read from a place at the codegen level.
Expand Down
11 changes: 11 additions & 0 deletions kani-compiler/src/codegen_cprover_gotoc/compiler_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,17 @@ fn print_report(ctx: &GotocCtx, tcx: TyCtxt) {
details.";
tcx.sess.warn(&msg);
}

if !ctx.concurrent_constructs.is_empty() {
let mut msg = String::from(
"Kani currently does not support concurrency. The following constructs will be treated \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This messaging could also be improved. In particular, I think we should avoid using "Kani" in our messages, and say something like "Concurrency is not supported at the moment". But I'm afraid this will break some stuff, so let's not do it here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should avoid using "Kani" in our messages

Why is that?

Copy link
Contributor

@adpaco-aws adpaco-aws Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're running Kani, so it's odd printing messages that mention Kani in any way when you're already in that context. If I'm not mistaken, the Rust compiler won't use its own name in messages unless it's talking about specific editions (e.g., "this construct isn't supported in Rust 2018"). Otherwise, I feel like it becomes repetitive for users to see messages like "Kani does this" or "Kani does that" (there may be exceptions, of course).

as sequential operations:\n",
);
for (construct, locations) in ctx.concurrent_constructs.iter() {
writeln!(&mut msg, " - {construct} ({})", locations.len()).unwrap();
}
tcx.sess.warn(&msg);
}
}

/// Return a struct that contains information about the codegen results as expected by `rustc`.
Expand Down
5 changes: 5 additions & 0 deletions kani-compiler/src/codegen_cprover_gotoc/context/goto_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ pub struct GotocCtx<'tcx> {
pub global_checks_count: u64,
/// A map of unsupported constructs that were found while codegen
pub unsupported_constructs: FxHashMap<InternedString, Vec<Location>>,
/// A map of concurrency constructs that are treated sequentially.
/// We collect them and print one warning at the end if not empty instead of printing one
/// warning at each occurrence.
pub concurrent_constructs: FxHashMap<InternedString, Vec<Location>>,
}

/// Constructor
Expand All @@ -96,6 +100,7 @@ impl<'tcx> GotocCtx<'tcx> {
test_harnesses: vec![],
global_checks_count: 0,
unsupported_constructs: FxHashMap::default(),
concurrent_constructs: FxHashMap::default(),
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions kani-compiler/src/codegen_cprover_gotoc/utils/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
use super::super::codegen::TypeExt;
use crate::codegen_cprover_gotoc::codegen::typ::{is_pointer, pointee_type};
use crate::codegen_cprover_gotoc::GotocCtx;
use cbmc::btree_string_map;
use cbmc::goto_program::{Expr, ExprValue, SymbolTable, Type};
use cbmc::goto_program::{Expr, ExprValue, Location, SymbolTable, Type};
use cbmc::{btree_string_map, InternedString};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{Instance, Ty};
use tracing::debug;

// Should move into rvalue
//make this a member function
Expand Down Expand Up @@ -66,6 +67,20 @@ impl<'tcx> GotocCtx<'tcx> {
_ => None,
}
}

/// Store an occurrence of a concurrent construct that was treated as a sequential operation.
///
/// Kani does not currently support concurrency and the compiler assumes that when generating
/// code for some specialized concurrent constructs that this is the case. We store all types of
/// operations that had this special handling and print a warning at the end of the compilation.
pub fn store_concurrent_construct(&mut self, operation_name: &str, loc: Location) {
Copy link
Contributor

@adpaco-aws adpaco-aws Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned, printing the location isn't useful at the moment. A better thing to do would be storing locations from function calls up to here, then print whichever is closest to the user's code. But we don't have anything like that at the moment, right?

debug!(op=?operation_name, location=?loc.short_string(), "store_seq_construct");

// Save this occurrence so we can emit a warning in the compilation report.
let key: InternedString = operation_name.into();
let entry = self.concurrent_constructs.entry(key).or_default();
entry.push(loc);
}
}

/// Members traverse path to get to the raw pointer of a box (b.0.pointer.pointer).
Expand Down