immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.79k stars 219 forks source link

analyze: omit unused hypothetical lifetimes during rewriting #1015

Closed spernsteiner closed 10 months ago

spernsteiner commented 11 months ago

This change removes hypothetical lifetimes for pointers that are rewritten into non-ref types, such as raw pointers or Box<T>. The AdtMetadataTable is computed normally at first so it can be used in borrowck, but once all pointer permissions have been determined, it's recomputed with an extra filter that skips creating hypothetical region params for pointers whose type_desc::Ownership is not Imm, Cell, or Mut (&T, &Cell<T>, or &mut T).

For example:

struct Foo {
   p: *mut u8,
   #[c2rust_analyze_test::fixed_signature]
   q: *mut u8,
}

Previously, Foo would be rewritten to have two new region parameters, one for p and one for q. But the region for q would unused: q is marked FIXED, so it's left as *mut u8 rather than rewritten to a safe reference type. With this change, Foo is rewritten to have only one region parameter, the one for p. This avoids rustc errors about unused generic parameters in the rewritten code.

spernsteiner commented 11 months ago

For 5a88caa, were you seeing known ptr permissions change? Which ones?

Opened #1017 with the specific cases I saw in lighttpd

aneksteind commented 11 months ago

@spernsteiner are we somehow constraining things, resulting in a different set of rewrites, by giving hypothetical lifetimes to polonius that won't end up being used?

aneksteind commented 11 months ago

This apparently also fixes #994

spernsteiner commented 11 months ago

are we somehow constraining things, resulting in a different set of rewrites, by giving hypothetical lifetimes to polonius that won't end up being used?

For variables that get rewritten to Box/Rc, the presence or absence of UNIQUE doesn't affect the rewrite. So I guess the concern here would be that including these lifetimes could add constraints on other variables where UNIQUE does matter. I don't know for sure whether or not this can happen, but I would guess not - borrowing the data inside a box is pretty similar from a borrowck perspective to borrowing the data behind an &mut.

If this does turn out to be an issue, we can think about borrowck improvements then (and having a real example to look at will help).