-
Notifications
You must be signed in to change notification settings - Fork 248
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
(c2rust-analyze
) Add more (still incomplete) dataflow constraints for ptr casts
#947
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ use rustc_middle::mir::{ | |
}; | ||
use rustc_middle::ty::adjustment::PointerCast; | ||
use rustc_middle::ty::{SubstsRef, Ty, TyKind}; | ||
use std::iter; | ||
|
||
/// Visitor that walks over the MIR, computing types of rvalues/operands/places and generating | ||
/// constraints as a side effect. | ||
|
@@ -135,12 +136,35 @@ impl<'tcx> TypeChecker<'tcx, '_> { | |
PointerCast::ArrayToPointer => {} | ||
PointerCast::Unsize => {} | ||
} | ||
self.do_assign_pointer_ids(to_lty.label, from_lty.label) | ||
// TODO add other dataflow constraints | ||
|
||
// We would normally do `self.do_assign`, | ||
// but we can't do a normal `self.do_equivalence_nested` (inside `self.do_assign`), | ||
// so we do the `self.do_assign_pointer_ids` from `self.do_assign`, | ||
// and then do the body of `self.do_equivalence_nested`, | ||
// but after the extra outer layers are stripped. | ||
|
||
self.do_assign_pointer_ids(to_lty.label, from_lty.label); | ||
|
||
// Each of these ptr casts needs at least the outer layer stripped (e.x. `&`, `*mut`, `fn`), | ||
// but then after that, these ptr casts below | ||
// needs more layers stripped to reach the types that are equivalent. | ||
let strip_from = |lty: LTy<'tcx>| match ptr_cast { | ||
PointerCast::ArrayToPointer => lty.args[0], | ||
PointerCast::Unsize => lty.args[0], | ||
_ => lty, | ||
}; | ||
let strip_to = |lty: LTy<'tcx>| match ptr_cast { | ||
PointerCast::Unsize => lty.args[0], | ||
_ => lty, | ||
}; | ||
for (&from_lty, &to_lty) in iter::zip(from_lty.args, to_lty.args) { | ||
self.do_unify(strip_from(from_lty), strip_to(to_lty)); | ||
} | ||
Comment on lines
+160
to
+162
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to my previous comment, I think it would be good to be clear about how many args we expect to see for each I also think it's okay to panic on the tricky cases There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How tricky is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With |
||
} | ||
CastKind::Misc => { | ||
match is_transmutable_ptr_cast(from_ty, to_ty) { | ||
Some(true) => { | ||
self.do_assign_pointer_ids(to_lty.label, from_lty.label); | ||
// TODO add other dataflow constraints | ||
}, | ||
Some(false) => ::log::error!("TODO: unsupported ptr-to-ptr cast between pointee types not yet supported as safely transmutable: `{from_ty:?} as {to_ty:?}`"), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clearer to match on the "from" and "to"
TyKind
s, or else to have a single case perPointerCast
variant and assert that the types have a particular shape (for both sanity-checking and documentation purposes). As is, it's hard for me to tell whether or not these rules are correct. Also, I think matching onTyKind
s is necessary to distinguish(x: &[u8; 3]) as &[u8]
from(x: &[u8; 3]) as &dyn Debug
(both arePointerCast::Unsize
, but they need different handling here).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point about
dyn
fat ptrs; I hadn't thought of them. Should I justerror!
onas &dyn _
s since there shouldn't be anydyn
s inlighttpd
or any otherc2rust transpile
d codebase?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes,
error!
ondyn
seems like the best approach for now.