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

Enable concrete playback for contract and stubs #3389

Merged
merged 13 commits into from
Jul 31, 2024
45 changes: 25 additions & 20 deletions kani-compiler/src/kani_middle/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use std::collections::BTreeMap;

use kani_metadata::{CbmcSolver, HarnessAttributes, Stub};
use kani_metadata::{CbmcSolver, HarnessAttributes, HarnessKind, Stub};
use rustc_ast::{
attr,
token::Token,
Expand Down Expand Up @@ -310,7 +310,7 @@ impl<'tcx> KaniAttributes<'tcx> {
/// the session and emit all errors found.
pub(super) fn check_attributes(&self) {
// Check that all attributes are correctly used and well formed.
let is_harness = self.is_harness();
let is_harness = self.is_proof_harness();
for (&kind, attrs) in self.map.iter() {
let local_error = |msg| self.tcx.dcx().span_err(attrs[0].span, msg);

Expand Down Expand Up @@ -454,7 +454,7 @@ impl<'tcx> KaniAttributes<'tcx> {

/// Is this item a harness? (either `proof` or `proof_for_contract`
/// attribute are present)
fn is_harness(&self) -> bool {
fn is_proof_harness(&self) -> bool {
self.map.contains_key(&KaniAttributeKind::Proof)
|| self.map.contains_key(&KaniAttributeKind::ProofForContract)
}
Expand All @@ -469,13 +469,18 @@ impl<'tcx> KaniAttributes<'tcx> {
panic!("Expected a local item, but got: {:?}", self.item);
};
trace!(?self, "extract_harness_attributes");
assert!(self.is_harness());
self.map.iter().fold(HarnessAttributes::default(), |mut harness, (kind, attributes)| {
assert!(self.is_proof_harness());
let harness_attrs = if let Some(Ok(harness)) = self.proof_for_contract() {
HarnessAttributes::new(HarnessKind::ProofForContract { target_fn: harness.to_string() })
} else {
HarnessAttributes::new(HarnessKind::Proof)
};
self.map.iter().fold(harness_attrs, |mut harness, (kind, attributes)| {
match kind {
KaniAttributeKind::ShouldPanic => harness.should_panic = true,
KaniAttributeKind::Recursion => {
self.tcx.dcx().span_err(self.tcx.def_span(self.item), "The attribute `kani::recursion` should only be used in combination with function contracts.");
},
}
KaniAttributeKind::Solver => {
harness.solver = parse_solver(self.tcx, attributes[0]);
}
Expand All @@ -485,7 +490,7 @@ impl<'tcx> KaniAttributes<'tcx> {
KaniAttributeKind::Unwind => {
harness.unwind_value = parse_unwind(self.tcx, attributes[0])
}
KaniAttributeKind::Proof => harness.proof = true,
KaniAttributeKind::Proof => { /* no-op */ }
KaniAttributeKind::ProofForContract => self.handle_proof_for_contract(&mut harness),
KaniAttributeKind::StubVerified => self.handle_stub_verified(&mut harness),
KaniAttributeKind::Unstable => {
Expand All @@ -498,7 +503,7 @@ impl<'tcx> KaniAttributes<'tcx> {
| KaniAttributeKind::InnerCheck
| KaniAttributeKind::ReplacedWith => {
self.tcx.dcx().span_err(self.tcx.def_span(self.item), format!("Contracts are not supported on harnesses. (Found the kani-internal contract attribute `{}`)", kind.as_ref()));
},
}
KaniAttributeKind::DisableChecks => {
// Internal attribute which shouldn't exist here.
unreachable!()
Expand Down Expand Up @@ -552,14 +557,14 @@ impl<'tcx> KaniAttributes<'tcx> {
self.item_name(),
),
)
.with_span_note(
self.tcx.def_span(def_id),
format!(
"Try adding a contract to this function or use the unsound `{}` attribute instead.",
KaniAttributeKind::Stub.as_ref(),
.with_span_note(
self.tcx.def_span(def_id),
format!(
"Try adding a contract to this function or use the unsound `{}` attribute instead.",
KaniAttributeKind::Stub.as_ref(),
),
)
)
.emit();
.emit();
continue;
}
Some(Ok(replacement_name)) => replacement_name,
Expand Down Expand Up @@ -689,7 +694,7 @@ fn has_kani_attribute<F: Fn(KaniAttributeKind) -> bool>(
tcx.get_attrs_unchecked(def_id).iter().filter_map(|a| attr_kind(tcx, a)).any(predicate)
}

/// Same as [`KaniAttributes::is_harness`] but more efficient because less
/// Same as [`KaniAttributes::is_proof_harness`] but more efficient because less
/// attribute parsing is performed.
pub fn is_proof_harness(tcx: TyCtxt, instance: InstanceStable) -> bool {
let def_id = rustc_internal::internal(tcx, instance.def.def_id());
Expand Down Expand Up @@ -896,7 +901,7 @@ fn parse_stubs(tcx: TyCtxt, harness: DefId, attributes: &[&Attribute]) -> Vec<St
Err(error_span) => {
tcx.dcx().span_err(
error_span,
"attribute `kani::stub` takes two path arguments; found argument that is not a path",
"attribute `kani::stub` takes two path arguments; found argument that is not a path",
);
None
}
Expand All @@ -910,9 +915,9 @@ fn parse_solver(tcx: TyCtxt, attr: &Attribute) -> Option<CbmcSolver> {
const ATTRIBUTE: &str = "#[kani::solver]";
let invalid_arg_err = |attr: &Attribute| {
tcx.dcx().span_err(
attr.span,
format!("invalid argument for `{ATTRIBUTE}` attribute, expected one of the supported solvers (e.g. `kissat`) or a SAT solver binary (e.g. `bin=\"<SAT_SOLVER_BINARY>\"`)")
)
attr.span,
format!("invalid argument for `{ATTRIBUTE}` attribute, expected one of the supported solvers (e.g. `kissat`) or a SAT solver binary (e.g. `bin=\"<SAT_SOLVER_BINARY>\"`)"),
)
};

let attr_args = attr.meta_item_list().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion kani-compiler/src/kani_middle/codegen_units.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl CodegenUnits {
/// Generate [KaniMetadata] for the target crate.
fn generate_metadata(&self) -> KaniMetadata {
let (proof_harnesses, test_harnesses) =
self.harness_info.values().cloned().partition(|md| md.attributes.proof);
self.harness_info.values().cloned().partition(|md| md.attributes.is_proof_harness());
KaniMetadata {
crate_name: self.crate_info.name.clone(),
proof_harnesses,
Expand Down
4 changes: 2 additions & 2 deletions kani-compiler/src/kani_middle/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use std::path::Path;

use crate::kani_middle::attributes::test_harness_name;
use kani_metadata::{ArtifactType, HarnessAttributes, HarnessMetadata};
use kani_metadata::{ArtifactType, HarnessAttributes, HarnessKind, HarnessMetadata};
use rustc_middle::ty::TyCtxt;
use stable_mir::mir::mono::Instance;
use stable_mir::CrateDef;
Expand Down Expand Up @@ -61,7 +61,7 @@ pub fn gen_test_metadata(
original_file: loc.filename,
original_start_line: loc.start_line,
original_end_line: loc.end_line,
attributes: HarnessAttributes::default(),
attributes: HarnessAttributes::new(HarnessKind::Test),
// TODO: This no longer needs to be an Option.
goto_file: Some(model_file),
contract: Default::default(),
Expand Down
13 changes: 3 additions & 10 deletions kani-driver/src/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,13 +559,6 @@ impl ValidateArgs for VerificationArgs {
--output-format=old.",
));
}
if self.concrete_playback.is_some() && self.is_stubbing_enabled() {
// Concrete playback currently does not work with contracts or stubbing.
return Err(Error::raw(
ErrorKind::ArgumentConflict,
"Conflicting options: --concrete-playback isn't compatible with stubbing.",
));
}
if self.concrete_playback.is_some() && self.jobs() != Some(1) {
// Concrete playback currently embeds a lot of assumptions about the order in which harnesses get called.
return Err(Error::raw(
Expand Down Expand Up @@ -869,13 +862,13 @@ mod tests {
let res = parse_unstable_disabled("--harness foo -Z stubbing").unwrap();
assert!(res.verify_opts.is_stubbing_enabled());

// `-Z stubbing` cannot be called with `--concrete-playback`
// `-Z stubbing` can now be called with concrete playback.
let res = parse_unstable_disabled(
"--harness foo --concrete-playback=print -Z concrete-playback -Z stubbing",
)
.unwrap();
let err = res.validate().unwrap_err();
assert_eq!(err.kind(), ErrorKind::ArgumentConflict);
// Note that `res.validate()` fails because input file does not exist.
assert!(matches!(res.verify_opts.validate(), Ok(())));
}

#[test]
Expand Down
85 changes: 70 additions & 15 deletions kani-driver/src/concrete_playback/test_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@

use crate::args::ConcretePlaybackMode;
use crate::call_cbmc::VerificationResult;
use crate::cbmc_output_parser::Property;
use crate::session::KaniSession;
use anyhow::{Context, Result};
use concrete_vals_extractor::{extract_harness_values, ConcreteVal};
use kani_metadata::HarnessMetadata;
use kani_metadata::{HarnessKind, HarnessMetadata};
use std::collections::hash_map::DefaultHasher;
use std::ffi::OsString;
use std::fs::{read_to_string, File};
Expand All @@ -32,7 +33,7 @@ impl KaniSession {
};

if let Ok(result_items) = &verification_result.results {
let harness_values: Vec<Vec<ConcreteVal>> = extract_harness_values(result_items);
let harness_values = extract_harness_values(result_items);

if harness_values.is_empty() {
println!(
Expand All @@ -43,9 +44,9 @@ impl KaniSession {
} else {
let mut unit_tests: Vec<UnitTest> = harness_values
.iter()
.map(|concrete_vals| {
.map(|(prop, concrete_vals)| {
let pretty_name = harness.get_harness_name_unqualified();
format_unit_test(&pretty_name, &concrete_vals)
format_unit_test(&pretty_name, &concrete_vals, gen_test_doc(harness, prop))
})
.collect();
unit_tests.dedup_by(|a, b| a.name == b.name);
Expand Down Expand Up @@ -168,6 +169,9 @@ impl KaniSession {
writeln!(temp_file, "{line}")?;
if curr_line_num == proof_harness_end_line {
for unit_test in unit_tests.iter() {
// Write an empty line before the unit test.
writeln!(temp_file)?;

for unit_test_line in unit_test.code.iter() {
curr_line_num += 1;
writeln!(temp_file, "{unit_test_line}")?;
Expand All @@ -176,7 +180,7 @@ impl KaniSession {
}
}

// Renames are usually automic, so we won't corrupt the user's source file during a
// Renames are usually atomic, so we won't corrupt the user's source file during a
// crash; but first flush all updates to disk, which persist wouldn't take care of.
temp_file.as_file().sync_all()?;
temp_file.persist(source_path).expect("Could not rename file");
Expand Down Expand Up @@ -231,8 +235,52 @@ impl KaniSession {
}
}

fn gen_test_doc(harness: &HarnessMetadata, property: &Property) -> String {
let mut doc_str = match &harness.attributes.kind {
HarnessKind::Proof => {
format!("/// Test generated for harness `{}` \n", harness.pretty_name)
}
HarnessKind::ProofForContract { target_fn } => {
format!(
"/// Test generated for harness `{}` that checks contract for `{target_fn}`\n",
harness.pretty_name
)
}
celinval marked this conversation as resolved.
Show resolved Hide resolved
HarnessKind::Test => {
unreachable!("Concrete playback for tests is not supported")
}
};
doc_str.push_str("///\n");
celinval marked this conversation as resolved.
Show resolved Hide resolved
doc_str.push_str(&format!(
"/// Check for `{}`: \"{}\"\n",
property.property_class(),
property.description
));
if !harness.attributes.stubs.is_empty() {
doc_str.push_str(
r#"///
/// # Warning
///
/// Concrete playback tests combined with stubs or contracts is highly
/// experimental, and subject to change.
///
/// The original harness has stubs which are not applied to this test.
/// This may cause a mismatch of non-deterministic values if the stub
/// creates any non-deterministic value.
/// The execution path may also differ, which can be used to refine the stub
/// logic.
"#,
);
}
doc_str
}

/// Generate a formatted unit test from a list of concrete values.
fn format_unit_test(harness_name: &str, concrete_vals: &[ConcreteVal]) -> UnitTest {
fn format_unit_test(
harness_name: &str,
concrete_vals: &[ConcreteVal],
doc_str: String,
) -> UnitTest {
// Hash the concrete values along with the proof harness name.
let mut hasher = DefaultHasher::new();
harness_name.hash(&mut hasher);
Expand All @@ -241,6 +289,7 @@ fn format_unit_test(harness_name: &str, concrete_vals: &[ConcreteVal]) -> UnitTe
let func_name = format!("kani_concrete_playback_{harness_name}_{hash}");

let func_before_concrete_vals = [
doc_str,
"#[test]".to_string(),
format!("fn {func_name}() {{"),
format!("{:<4}let concrete_vals: Vec<Vec<u8>> = vec![", " "),
Expand Down Expand Up @@ -324,7 +373,7 @@ mod concrete_vals_extractor {
/// Extract a set of concrete values that trigger one assertion
/// failure. Each element of the outer vector corresponds to
/// inputs triggering one assertion failure or cover statement.
pub fn extract_harness_values(result_items: &[Property]) -> Vec<Vec<ConcreteVal>> {
pub fn extract_harness_values(result_items: &[Property]) -> Vec<(&Property, Vec<ConcreteVal>)> {
result_items
.iter()
.filter(|prop| {
Expand All @@ -340,7 +389,7 @@ mod concrete_vals_extractor {
let concrete_vals: Vec<ConcreteVal> =
trace.iter().filter_map(&extract_from_trace_item).collect();

concrete_vals
(property, concrete_vals)
})
.collect()
}
Expand All @@ -359,7 +408,7 @@ mod concrete_vals_extractor {
{
if trace_item.step_type == "assignment"
&& lhs.starts_with("goto_symex$$return_value")
&& func.starts_with("kani::any_raw_internal")
&& func.starts_with("kani::any_raw_")
{
let declared_width = width_u64 as usize;
let actual_width = bit_concrete_val.len();
Expand Down Expand Up @@ -484,9 +533,10 @@ mod tests {
/// Since hashes can not be relied on in tests, this compares all parts of a unit test except the hash.
#[test]
fn format_unit_test_full_func() {
let doc_str = "/// Test documentation";
let harness_name = "test_proof_harness";
let concrete_vals = [ConcreteVal { byte_arr: vec![0, 0], interp_val: "0".to_string() }];
let unit_test = format_unit_test(harness_name, &concrete_vals);
let unit_test = format_unit_test(harness_name, &concrete_vals, doc_str.to_string());
let full_func = unit_test.code;
let split_unit_test_name = split_unit_test_name(&unit_test.name);
let expected_after_func_name = vec![
Expand All @@ -498,18 +548,23 @@ mod tests {
"}".to_string(),
];

assert_eq!(full_func[0], "#[test]");
assert_eq!(full_func[0], doc_str);
assert_eq!(full_func[1], "#[test]");
assert_eq!(
split_unit_test_name.before_hash,
format!("kani_concrete_playback_{harness_name}")
);
assert_eq!(full_func[1], format!("fn {}() {{", unit_test.name));
assert_eq!(full_func[2..], expected_after_func_name);
assert_eq!(full_func[2], format!("fn {}() {{", unit_test.name));
assert_eq!(full_func[3..], expected_after_func_name);
}

/// Generates a unit test and returns its hash.
fn extract_hash_from_unit_test(harness_name: &str, concrete_vals: &[ConcreteVal]) -> String {
let unit_test = format_unit_test(harness_name, concrete_vals);
let unit_test = format_unit_test(
harness_name,
concrete_vals,
"/// Harness created for unit test".to_string(),
);
split_unit_test_name(&unit_test.name).hash
}

Expand Down Expand Up @@ -603,7 +658,7 @@ mod tests {
}),
}]),
}];
let concrete_vals = extract_harness_values(&processed_items).pop().unwrap();
let (_, concrete_vals) = extract_harness_values(&processed_items).pop().unwrap();
let concrete_val = &concrete_vals[0];

assert_eq!(concrete_val.byte_arr, vec![1, 3]);
Expand Down
Loading
Loading