Skip to content

Commit

Permalink
Clean up some code before migrating rvalue (#2928)
Browse files Browse the repository at this point in the history
Clean up old unreachable check due to missing component which has been a
long time by the MirLinker. Also remove an old TODO code to add the def
name to virtual calls. This has regressed before, so the risk of
regressing doesn't seem worth for debug purpose.
  • Loading branch information
celinval committed Dec 9, 2023
1 parent abd3690 commit b7ece82
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 55 deletions.
36 changes: 6 additions & 30 deletions kani-compiler/src/codegen_cprover_gotoc/codegen/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::kani_middle::coercion::{
};
use crate::unwrap_or_return_codegen_unimplemented;
use cbmc::goto_program::{
arithmetic_overflow_result_type, BinaryOperator, Expr, Location, Stmt, Symbol, Type,
arithmetic_overflow_result_type, BinaryOperator, Expr, Location, Stmt, Type,
ARITH_OVERFLOW_OVERFLOWED_FIELD, ARITH_OVERFLOW_RESULT_FIELD,
};
use cbmc::MachineModel;
Expand Down Expand Up @@ -1169,7 +1169,7 @@ impl<'tcx> GotocCtx<'tcx> {
idx: usize,
) -> Expr {
debug!(?instance, typ=?t, %idx, "codegen_vtable_method_field");
let vtable_field_name = self.vtable_field_name(instance.def_id(), idx);
let vtable_field_name = self.vtable_field_name(idx);
let vtable_type = Type::struct_tag(self.vtable_name(t));
let field_type =
vtable_type.lookup_field_type(vtable_field_name, &self.symbol_table).unwrap();
Expand Down Expand Up @@ -1232,34 +1232,10 @@ impl<'tcx> GotocCtx<'tcx> {
.address_of()
.cast_to(trait_fn_ty)
} else {
// We skip an entire submodule of the standard library, so drop is missing
// for it. Build and insert a function that just calls an unimplemented block
// to maintain soundness.
let drop_sym_name = format!("drop_unimplemented_{}", self.symbol_name(drop_instance));
let pretty_name =
format!("drop_unimplemented<{}>", self.readable_instance_name(drop_instance));
let drop_sym = self.ensure(&drop_sym_name, |ctx, name| {
// Function body
let unimplemented = ctx.codegen_unimplemented_stmt(
format!("drop_in_place for {drop_instance}").as_str(),
Location::none(),
"https://github.com/model-checking/kani/issues/281",
);

// Declare symbol for the single, self parameter
let param_typ = ctx.codegen_ty(trait_ty).to_pointer();
let param_sym = ctx.gen_function_parameter(0, &drop_sym_name, param_typ);

// Build and insert the function itself
Symbol::function(
name,
Type::code(vec![param_sym.to_function_parameter()], Type::empty()),
Some(Stmt::block(vec![unimplemented], Location::none())),
pretty_name,
Location::none(),
)
});
drop_sym.to_expr().address_of().cast_to(trait_fn_ty)
unreachable!(
"Missing drop implementation for {}",
self.readable_instance_name(drop_instance)
);
}
}

Expand Down
15 changes: 3 additions & 12 deletions kani-compiler/src/codegen_cprover_gotoc/codegen/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use super::PropertyClass;
use crate::codegen_cprover_gotoc::{GotocCtx, VtableCtx};
use crate::unwrap_or_return_codegen_unimplemented_stmt;
use cbmc::goto_program::{Expr, Location, Stmt, Type};
use rustc_hir::def_id::DefId;
use rustc_middle::mir;
use rustc_middle::mir::{
AssertKind, BasicBlock, NonDivergingIntrinsic, Operand, Place, Statement, StatementKind,
Expand Down Expand Up @@ -541,16 +540,9 @@ impl<'tcx> GotocCtx<'tcx> {
return Stmt::goto(self.current_fn().find_label(&target.unwrap()), loc);
}
// Handle a virtual function call via a vtable lookup
InstanceDef::Virtual(def_id, idx) => {
InstanceDef::Virtual(_, idx) => {
let self_ty = self.operand_ty(&args[0]);
self.codegen_virtual_funcall(
self_ty,
def_id,
idx,
destination,
&mut fargs,
loc,
)
self.codegen_virtual_funcall(self_ty, idx, destination, &mut fargs, loc)
}
// Normal, non-virtual function calls
InstanceDef::Item(..)
Expand Down Expand Up @@ -623,13 +615,12 @@ impl<'tcx> GotocCtx<'tcx> {
fn codegen_virtual_funcall(
&mut self,
self_ty: Ty<'tcx>,
def_id: DefId,
idx: usize,
place: &Place<'tcx>,
fargs: &mut [Expr],
loc: Location,
) -> Vec<Stmt> {
let vtable_field_name = self.vtable_field_name(def_id, idx);
let vtable_field_name = self.vtable_field_name(idx);
trace!(?self_ty, ?place, ?vtable_field_name, "codegen_virtual_funcall");
debug!(?fargs, "codegen_virtual_funcall");

Expand Down
2 changes: 1 addition & 1 deletion kani-compiler/src/codegen_cprover_gotoc/codegen/typ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ impl<'tcx> GotocCtx<'tcx> {
let fn_ptr = fn_ty.to_pointer();

// vtable field name, i.e., 3_vol (idx_method)
let vtable_field_name = self.vtable_field_name(instance.def_id(), idx);
let vtable_field_name = self.vtable_field_name(idx);

DatatypeComponent::field(vtable_field_name, fn_ptr)
}
Expand Down
5 changes: 0 additions & 5 deletions kani-compiler/src/codegen_cprover_gotoc/context/goto_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,6 @@ impl<'tcx> GotocCtx<'tcx> {
self.gen_stack_variable(c, fname, "var", t, Location::none(), false)
}

// Generate a Symbol Expression representing a function parameter from the MIR
pub fn gen_function_parameter(&mut self, c: u64, fname: &str, t: Type) -> Symbol {
self.gen_stack_variable(c, fname, "var", t, Location::none(), true)
}

/// Given a counter `c` a function name `fname, and a prefix `prefix`, generates a new function local variable
/// It is an error to reuse an existing `c`, `fname` `prefix` tuple.
fn gen_stack_variable(
Expand Down
9 changes: 2 additions & 7 deletions kani-compiler/src/codegen_cprover_gotoc/utils/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

use crate::codegen_cprover_gotoc::GotocCtx;
use cbmc::InternedString;
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_middle::mir::mono::CodegenUnitNameBuilder;
use rustc_middle::mir::Local;
Expand Down Expand Up @@ -83,12 +82,8 @@ impl<'tcx> GotocCtx<'tcx> {
/// The name for the struct field on a vtable for a given function. Because generic
/// functions can share the same name, we need to use the index of the entry in the
/// vtable. This is the same index that will be passed in virtual function calls as
/// InstanceDef::Virtual(def_id, idx). We could use solely the index as a key into
/// the vtable struct, but we add the method name for debugging readability.
/// Example: 3_vol
pub fn vtable_field_name(&self, _def_id: DefId, idx: usize) -> InternedString {
// format!("{}_{}", idx, with_no_trimmed_paths!(|| self.tcx.item_name(def_id)))
// TODO: use def_id https://github.com/model-checking/kani/issues/364
/// InstanceDef::Virtual(def_id, idx).
pub fn vtable_field_name(&self, idx: usize) -> InternedString {
idx.to_string().into()
}

Expand Down

0 comments on commit b7ece82

Please sign in to comment.