immunant / c2rust

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

Fix int-to-ptr casts in `analysis/test` #682

Open kkysen opened 2 years ago

kkysen commented 2 years ago

analysis/test contains many int-to-ptr casts for null pointers, like 0 as *const _ and 0 as *mut _. These use ptr::from_exposed_addr as part of strict provenance. Since we don't actually need to use ptr::from_exposed_addr here, we should switch to using str::ptr::null{,_mut}() instead. miri warns on these, so it's easier to find and fix UB bugs if these aren't in the way, and it's best practice to do this as well.

See the miri warning:

warning: integer-to-pointer cast
   --> src/pointers.rs:82:17
    |
82  |         field3: 0 as *const S,
    |                 ^^^^^^^^^^^^^ integer-to-pointer cast
    |
    = help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
    = help: which means that Miri might miss pointer bugs in this program.
    = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
    = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead.
    = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
    = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
    = note: backtrace:
    = note: inside `pointers::simple` at src/pointers.rs:82:17
aneksteind commented 2 years ago

Similar to https://github.com/immunant/c2rust/pull/683#issuecomment-1259581307, it would be good to know if this is a result of the transpiler. I believe that should be the focus over changes to analysis/test specifically, if so.

kkysen commented 2 years ago

Similar to #683 (comment), it would be good to know if this is a result of the transpiler. I believe that should be the focus over changes to analysis/test specifically, if so.

This is a result of the transpiler, at least the 0 as *{const,mut} _s, but is intended for the time being. See #202 for a more thorough discussion. We had previously decided the current way was better, but that was before strict provenance was added, so I made this comment: https://github.com/immunant/c2rust/issues/202#issuecomment-1228089251, suggesting {core,std}::ptr::{null,null_mut}() be preferred.