Skip to content

Commit 3ae60f3

Browse files
celinvaltedinski
authored andcommitted
Fix handling of dyn T (rust-lang#1073)
During codegen we were handling `dyn T` the same way as `&dyn T` which was causing a bunch of type mismatch warnings. To fix that, I first changed how we encode trait fat pointers. We used to encode them as: ``` struct RefToTrait { void* data; Tag* vtable; } ``` But now we encode them as: ``` struct RefToTrait { T* data; // This is a typedef to void*. Tag* vtable; } ``` Then I modified encoding `dyn T` to codegen a thin pointer `T*`. I had also to modify places where we were forcing `void*` to be the type of the `data` field. * Fix vtable restriction after `dyn T` refactoring. The vtable restriction code was relying on the fact that `dyn T` and `&dyn T` were generating the same gotoc expression. Instead, we now ensure that we pass the fat pointer type to the function that creates the vtable call site. * Fix Rc<dyn> and add a bunch of debug stuff. I'm still seeing tons of type mismatch on std crate and unable to find vtable warnings. * Add testcase for issue-990 * Clean up code for PR * Codegen unimplemented for dropping unsized struct We were incorrectly handling them. I'm mitigation this issue for now and I created a testcase. Note: Without this mitigation but with the fix to fat pointers not using void*, this was failing codegen of firecracker. * Replace bool by Unit * PR comments + better tests * Add assertion and improve comments
1 parent d8f7ad4 commit 3ae60f3

File tree

15 files changed

+395
-71
lines changed

15 files changed

+395
-71
lines changed

kani-compiler/src/codegen_cprover_gotoc/codegen/function.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_middle::ty::{self, Instance};
1515
use std::collections::BTreeMap;
1616
use std::convert::TryInto;
1717
use std::iter::FromIterator;
18-
use tracing::{debug, info_span, warn};
18+
use tracing::{debug, info_span};
1919

2020
/// Utility to skip functions that can't currently be successfully codgenned.
2121
impl<'tcx> GotocCtx<'tcx> {
@@ -73,7 +73,7 @@ impl<'tcx> GotocCtx<'tcx> {
7373
let _trace_span =
7474
info_span!("CodegenFunction", name = self.current_fn().readable_name()).entered();
7575
if old_sym.is_function_definition() {
76-
warn!("Double codegen of {:?}", old_sym);
76+
tracing::info!("Double codegen of {:?}", old_sym);
7777
} else if self.should_skip_current_fn() {
7878
debug!("Skipping function {}", self.current_fn().readable_name());
7979
let body = self.codegen_fatal_error(

kani-compiler/src/codegen_cprover_gotoc/codegen/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ mod rvalue;
1414
mod span;
1515
mod statement;
1616
mod static_var;
17-
mod typ;
17+
18+
// Visible for all codegen module.
19+
pub(super) mod typ;
1820

1921
pub use assert::PropertyClass;
2022
pub use typ::TypeExt;

kani-compiler/src/codegen_cprover_gotoc/codegen/place.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use rustc_middle::{
1616
ty::{self, Ty, TypeAndMut, VariantDef},
1717
};
1818
use rustc_target::abi::{TagEncoding, Variants};
19-
use tracing::{debug, warn};
19+
use tracing::{debug, trace, warn};
2020

2121
/// A projection in Kani can either be to a type (the normal case),
2222
/// or a variant in the case of a downcast.
@@ -114,6 +114,14 @@ impl<'tcx> ProjectedPlace<'tcx> {
114114
{
115115
None
116116
}
117+
// TODO: Do we really need this?
118+
// https://github.com/model-checking/kani/issues/1092
119+
ty::Dynamic(..)
120+
if expr_ty.is_pointer()
121+
&& *expr_ty.base_type().unwrap() == type_from_mir =>
122+
{
123+
None
124+
}
117125
_ => Some((expr_ty, type_from_mir)),
118126
}
119127
} else {
@@ -325,6 +333,7 @@ impl<'tcx> GotocCtx<'tcx> {
325333
let before = before?;
326334
match proj {
327335
ProjectionElem::Deref => {
336+
trace!(?before, ?proj, "codegen_projection");
328337
let base_type = before.mir_typ();
329338
let inner_goto_expr = if base_type.is_box() {
330339
self.deref_box(before.goto_expr)
@@ -341,7 +350,6 @@ impl<'tcx> GotocCtx<'tcx> {
341350
} else {
342351
before.fat_ptr_mir_typ
343352
};
344-
345353
let fat_ptr_goto_expr =
346354
if self.is_box_of_unsized(base_type) || self.is_ref_of_unsized(base_type) {
347355
Some(inner_goto_expr.clone())

kani-compiler/src/codegen_cprover_gotoc/codegen/rvalue.rs

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl<'tcx> GotocCtx<'tcx> {
140140

141141
// The thin pointer in the resulting fat pointer is a pointer to the value
142142
let thin_pointer = if place_goto_expr.typ().is_pointer() {
143-
// The value is itself a pointer (eg, a void pointer), just use this pointer
143+
// The value is itself a pointer, just use this pointer
144144
place_goto_expr
145145
} else if place_goto_expr.typ().is_array_like() {
146146
// The value is an array (eg, a flexible struct member), point to the first array element
@@ -162,12 +162,7 @@ impl<'tcx> GotocCtx<'tcx> {
162162
if self.use_slice_fat_pointer(place_mir_type) {
163163
slice_fat_ptr(result_goto_type, thin_pointer, metadata, &self.symbol_table)
164164
} else if self.use_vtable_fat_pointer(place_mir_type) {
165-
dynamic_fat_ptr(
166-
result_goto_type,
167-
thin_pointer.cast_to(Type::void_pointer()),
168-
metadata,
169-
&self.symbol_table,
170-
)
165+
dynamic_fat_ptr(result_goto_type, thin_pointer, metadata, &self.symbol_table)
171166
} else {
172167
unreachable!();
173168
}
@@ -720,10 +715,12 @@ impl<'tcx> GotocCtx<'tcx> {
720715
t: Ty<'tcx>,
721716
idx: usize,
722717
) -> Expr {
718+
debug!(?instance, typ=?t, %idx, "codegen_vtable_method_field");
723719
let vtable_field_name = self.vtable_field_name(instance.def_id(), idx);
724720
let vtable_type = Type::struct_tag(self.vtable_name(t));
725721
let field_type =
726722
vtable_type.lookup_field_type(vtable_field_name, &self.symbol_table).unwrap();
723+
debug!(?vtable_field_name, ?vtable_type, "codegen_vtable_method_field");
727724

728725
// Lookup in the symbol table using the full symbol table name/key
729726
let fn_name = self.symbol_name(instance);
@@ -741,6 +738,7 @@ impl<'tcx> GotocCtx<'tcx> {
741738
// Create a pointer to the method
742739
// Note that the method takes a self* as the first argument, but the vtable field type has a void* as the first arg.
743740
// So we need to cast it at the end.
741+
debug!(?fn_symbol, fn_typ=?fn_symbol.typ, ?field_type, "codegen_vtable_method_field");
744742
Expr::symbol_expression(fn_symbol.name, fn_symbol.typ.clone())
745743
.address_of()
746744
.cast_to(field_type)
@@ -773,6 +771,10 @@ impl<'tcx> GotocCtx<'tcx> {
773771
);
774772
}
775773

774+
debug!(?ty, ?trait_ty, "codegen_drop_in_place");
775+
debug!(?drop_instance, ?trait_fn_ty, "codegen_drop_in_place");
776+
debug!(drop_sym=?drop_sym.clone().typ, "codegen_drop_in_place");
777+
776778
Expr::symbol_expression(drop_sym_name, drop_sym.clone().typ)
777779
.address_of()
778780
.cast_to(trait_fn_ty)
@@ -959,20 +961,23 @@ impl<'tcx> GotocCtx<'tcx> {
959961
if src_mir_type.kind() == dst_mir_type.kind() {
960962
return None; // no cast required, nothing to do
961963
}
964+
debug!(?src_goto_expr, ?src_mir_type, ?dst_mir_type, "cast_unsized_dyn_trait");
962965

963966
// The source destination must be a fat pointers to a dyn trait object
964967
assert!(self.is_vtable_fat_pointer(src_mir_type));
965968
assert!(self.is_vtable_fat_pointer(dst_mir_type));
966969

970+
let dst_goto_type = self.codegen_ty(dst_mir_type);
971+
972+
// Cast the data type.
967973
let dst_mir_dyn_ty = pointee_type(dst_mir_type).unwrap();
974+
let dst_data_type = self.codegen_trait_data_pointer(dst_mir_dyn_ty);
975+
let data =
976+
src_goto_expr.to_owned().member("data", &self.symbol_table).cast_to(dst_data_type);
968977

969-
// Get the fat pointer data and vtable fields, and cast the type of
970-
// the vtable.
971-
let dst_goto_type = self.codegen_ty(dst_mir_type);
972-
let data = src_goto_expr.to_owned().member("data", &self.symbol_table);
978+
// Retrieve the vtable and cast the vtable type.
973979
let vtable_name = self.vtable_name(dst_mir_dyn_ty);
974980
let vtable_ty = Type::struct_tag(vtable_name).to_pointer();
975-
976981
let vtable = src_goto_expr.member("vtable", &self.symbol_table).cast_to(vtable_ty);
977982

978983
// Construct a fat pointer with the same (casted) fields and new type
@@ -1108,15 +1113,19 @@ impl<'tcx> GotocCtx<'tcx> {
11081113
fn cast_sized_pointer_to_trait_fat_pointer(
11091114
&mut self,
11101115
src_goto_expr: Expr,
1111-
_src_mir_type: Ty<'tcx>,
1116+
src_mir_type: Ty<'tcx>,
11121117
dst_mir_type: Ty<'tcx>,
11131118
src_pointee_type: Ty<'tcx>,
11141119
dst_pointee_type: Ty<'tcx>,
11151120
) -> Option<Expr> {
1121+
tracing::trace!(?src_pointee_type, ?dst_pointee_type, "cast_thin_2_fat_ptr");
1122+
tracing::trace!(?src_mir_type, ?dst_mir_type, "cast_thin_2_fat_ptr");
11161123
if let Some((concrete_type, trait_type)) =
11171124
self.nested_pair_of_concrete_and_trait_types(src_pointee_type, dst_pointee_type)
11181125
{
1119-
let dst_goto_expr = src_goto_expr.cast_to(Type::void_pointer());
1126+
tracing::trace!(?concrete_type, ?trait_type, "cast_thin_2_fat_ptr");
1127+
let dst_goto_expr =
1128+
src_goto_expr.cast_to(self.codegen_ty(dst_pointee_type).to_pointer());
11201129
let dst_goto_type = self.codegen_ty(dst_mir_type);
11211130
let vtable = self.codegen_vtable(concrete_type, trait_type);
11221131
let vtable_expr = vtable.address_of();

kani-compiler/src/codegen_cprover_gotoc/codegen/statement.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use super::typ::TypeExt;
44
use super::typ::FN_RETURN_VOID_VAR_NAME;
55
use super::PropertyClass;
6+
use crate::codegen_cprover_gotoc::codegen::typ::pointee_type;
67
use crate::codegen_cprover_gotoc::utils;
78
use crate::codegen_cprover_gotoc::{GotocCtx, VtableCtx};
89
use crate::unwrap_or_return_codegen_unimplemented_stmt;
@@ -20,7 +21,7 @@ use rustc_middle::ty::layout::LayoutOf;
2021
use rustc_middle::ty::{Instance, InstanceDef, Ty};
2122
use rustc_span::Span;
2223
use rustc_target::abi::{FieldsShape, Primitive, TagEncoding, Variants};
23-
use tracing::{debug, info_span};
24+
use tracing::{debug, info_span, warn};
2425

2526
impl<'tcx> GotocCtx<'tcx> {
2627
fn codegen_ret_unit(&mut self) -> Stmt {
@@ -152,6 +153,7 @@ impl<'tcx> GotocCtx<'tcx> {
152153
// https://github.com/model-checking/kani/issues/221
153154
fn codegen_drop(&mut self, location: &Place<'tcx>, target: &BasicBlock) -> Stmt {
154155
let loc_ty = self.place_ty(location);
156+
debug!(?loc_ty, "codegen_drop");
155157
let drop_instance = Instance::resolve_drop_in_place(self.tcx, loc_ty);
156158
if let Some(hk) = self.hooks.hook_applies(self.tcx, drop_instance) {
157159
let le =
@@ -174,18 +176,34 @@ impl<'tcx> GotocCtx<'tcx> {
174176
)
175177
.fat_ptr_goto_expr
176178
.unwrap();
179+
debug!(?trait_fat_ptr, "codegen_drop: ");
177180

178181
// Pull the function off of the fat pointer's vtable pointer
179182
let vtable_ref =
180183
trait_fat_ptr.to_owned().member("vtable", &self.symbol_table);
184+
181185
let vtable = vtable_ref.dereference();
182186
let fn_ptr = vtable.member("drop", &self.symbol_table);
183187

184188
// Pull the self argument off of the fat pointer's data pointer
185-
let self_ref =
189+
if let Some(typ) = pointee_type(self.local_ty(location.local)) {
190+
if !(typ.is_trait() || typ.is_box()) {
191+
warn!(self_type=?typ, "Unsupported drop of unsized");
192+
return self
193+
.codegen_unimplemented(
194+
format!("Unsupported drop unsized struct: {:?}", typ)
195+
.as_str(),
196+
Type::Empty,
197+
Location::None,
198+
"https://github.com/model-checking/kani/issues/1072",
199+
)
200+
.as_stmt(Location::None);
201+
}
202+
}
203+
let self_data =
186204
trait_fat_ptr.to_owned().member("data", &self.symbol_table);
187205
let self_ref =
188-
self_ref.cast_to(trait_fat_ptr.typ().clone().to_pointer());
206+
self_data.clone().cast_to(trait_fat_ptr.typ().clone().to_pointer());
189207

190208
let call =
191209
fn_ptr.dereference().call(vec![self_ref]).as_stmt(Location::none());

0 commit comments

Comments
 (0)