-
Notifications
You must be signed in to change notification settings - Fork 242
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 2 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:?}`"), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,3 +33,57 @@ pub fn cast_from_literal() { | |
pub fn cast_from_literal_explicit() { | ||
std::ptr::addr_of!(*b"\0") as *const u8 as *const core::ffi::c_char; | ||
} | ||
|
||
/// [`PointerCast::ReifyFnPointer`] | ||
/// Can't figure out how to create a [`PointerCast::ReifyFnPointer`]. | ||
kkysen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[cfg(any())] | ||
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. where do these tests crash downstream? create an issue outlining them? 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. i see the comments outline them, so not a huge priority to make issues of them |
||
pub fn cast_fn_item_to_fn_ptr(f: impl Fn(u8) -> i8) { | ||
f as fn(u8) -> i8; | ||
} | ||
|
||
/// [`PointerCast::UnsafeFnPointer`] | ||
/// | ||
/// ```shell | ||
/// thread 'rustc' panicked at 'not yet implemented', c2rust-analyze/src/labeled_ty.rs:372:17 | ||
/// ``` | ||
#[cfg(any())] | ||
pub fn cast_fn_ptr_to_unsafe_fn_ptr(f: fn(u8) -> i8) { | ||
f as unsafe fn(u8) -> i8; | ||
} | ||
|
||
/// [`PointerCast::ClosureFnPointer`] | ||
/// Unhandled very early on. | ||
#[cfg(any())] | ||
pub fn cast_closure_to_fn_ptr() { | ||
(|b: u8| b as i8) as fn(u8) -> i8; | ||
} | ||
|
||
/// [`PointerCast::MutToConstPointer`] | ||
/// | ||
/// ```shell | ||
/// thread 'rustc' panicked at 'not yet implemented', c2rust-analyze/src/labeled_ty.rs:372:17 | ||
/// ``` | ||
#[cfg(any())] | ||
pub fn cast_mut_to_const_ptr(p: *mut i32) { | ||
p as *const i32; | ||
} | ||
|
||
/// Meant to be [`PointerCast::ArrayToPointer`], but is [`CastKind::Misc`]. | ||
pub fn cast_array_ptr_to_ptr(p: *const [i32; 1]) { | ||
p as *const i32; | ||
} | ||
|
||
/// [`PointerCast::Unsize`] | ||
/// | ||
/// ```shell | ||
/// thread 'rustc' panicked at 'expected to find only one Assign statement, but got multiple', c2rust-analyze/src/rewrite/expr/hir_op.rs:126:21 | ||
/// ``` | ||
#[cfg(any())] | ||
pub fn cast_unsize_direct(a: &[i32; 1]) { | ||
a as &[i32]; | ||
} | ||
|
||
/// [`PointerCast::Unsize`] | ||
pub fn cast_unsize_indirect(a: &[i32; 1]) { | ||
let _ = a.as_ptr(); | ||
} |
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.