(c2rust-analyze) Add more (still incomplete) dataflow constraints for ptr casts
#947
Conversation
…:Misc` (still incomplete).
…ointer` (previously incomplete). More tests have also been added to `string_casts.rs` exercising as many of the `PointerCast`s as possible, but many of these crash later on, so they're disabled.
|
|
||
| /// [`PointerCast::ReifyFnPointer`] | ||
| /// Can't figure out how to create a [`PointerCast::ReifyFnPointer`]. | ||
| #[cfg(any())] |
There was a problem hiding this comment.
where do these tests crash downstream? create an issue outlining them?
There was a problem hiding this comment.
i see the comments outline them, so not a huge priority to make issues of them
| // 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, | ||
| }; |
There was a problem hiding this comment.
I think it would be clearer to match on the "from" and "to" TyKinds, or else to have a single case per PointerCast 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 on TyKinds is necessary to distinguish (x: &[u8; 3]) as &[u8] from (x: &[u8; 3]) as &dyn Debug (both are PointerCast::Unsize, but they need different handling here).
There was a problem hiding this comment.
That's a good point about dyn fat ptrs; I hadn't thought of them. Should I just error! on as &dyn _s since there shouldn't be any dyns in lighttpd or any other c2rust transpiled codebase?
There was a problem hiding this comment.
Yes, error! on dyn seems like the best approach for now.
| 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)); | ||
| } |
There was a problem hiding this comment.
Similar to my previous comment, I think it would be good to be clear about how many args we expect to see for each PointerCast case. For example, Unsize should always have 1 arg in both from_lty and to_lty, while for UnsafeFnPointer we only expect that from_lty and to_lty will have the same amount.
I also think it's okay to panic on the tricky cases ReifyFnPointer and ClosureFnPointer for now.
There was a problem hiding this comment.
How tricky is ReifyFnPointer? If it's not too complex, I'd rather not error on it as it is used commonly in lighttpd and other c2rust transpiled code for fn ptrs.
There was a problem hiding this comment.
With ReifyFnPointer, I haven't thought about what constraints we ought to generate for pointer arguments in the function's signature. For example, given fn f(p: *mut /*p0*/ i32) and the expression f as fn(*mut /*p1*/ i32), do we require p1's permissions to be a subset of p0's, or do we require the reverse, or do we require the two to be equal?
…ate a `PointerCast::ReifyFnPointer`.
kkysen
force-pushed
the
kkysen/analyze-ptr-casts-dataflow
branch
from
e02903c
to
19ce2bd
Compare
3c03860 adds the simple (still incomplete) dataflow constraints for
CastKind::Misc(fixes the minimum outlined in #839 (comment)). The full constraints are trickier since the types can be recursive, so I wanted to split this simpler part out into its own smaller PR first.b20cea7 adds complete dataflow constraints for
CastKind::Pointer, which is modeled ondo_assign, but adjusted given its a ptr cast (fixes #839 (comment)).