c2rust icon indicating copy to clipboard operation
c2rust copied to clipboard

analyze: add NON_NULL rewrites

Open spernsteiner opened this issue 1 year ago • 2 comments

This branch implements rewriting of nullable pointers (those lacking PermissionSet::NON_NULL) to Option. This includes the following kinds of rewrites:

  • Type annotations: Option<&mut T> instead of &mut T
  • Casts between nullable and non-nullable via p.unwrap() and Some(p). For most permissions, we implement only one direction because the other is invalid/nonsensical, but here we implement both in order to support "unsound" rewrites involving overridden NON_NULL flags (as in #1088).
  • Borrows: when casting p: Option<&mut T>, we use p.as_deref_mut().unwrap() instead of p.unwrap() to avoid consuming the original p. (In the code, this is called a "downgrade", since it allows borrowing Option<Box<T>> as Option<&T> and similar.)
  • Pointer projections on nullable pointers. Where NON_NULL pointers would use &p[0], nullable ones use p.map(|ptr| &ptr[0]). Internally, this is represented similar to Some(&p.unwrap()[0]), but it's handled specially by rewrite::expr::convert to produce a map call instead, which passes through None without a panic.
  • unwrap() calls on derefs. *p is rewritten to *p.unwrap(), or to *p.as_deref().unwrap() if a downgrade/borrow is necessary to avoid moving p.
  • ptr::null() and 0 as *const _ to None, and p.is_null() to p.is_none().

The new non_null_rewrites.rs test case passes, and the rewritten code compiles.

This might be easier to review commit-by-commit.

spernsteiner avatar May 16 '24 00:05 spernsteiner

I realized while writing the PR that I never tested field projections (q = &(*p).field). I'll test it but will leave any fixes to a separate PR, since this one is already fairly complex.

spernsteiner avatar May 16 '24 00:05 spernsteiner

Left a couple of comments. This is a large PR, it will take me a while to get through.

ahomescu avatar May 21 '24 06:05 ahomescu

@ahomescu I've addressed all the previous feedback - any objections to merging this now?

spernsteiner avatar Jun 25 '24 20:06 spernsteiner