rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.33k stars 12.58k forks source link

Casts, including "no-op" casts, are not in-place #120211

Open gfaster opened 8 months ago

gfaster commented 8 months ago

According to the Rust Reference, integer casts to other integers of the same width (e.g. i32 -> u32) are a no-op. With that in mind, I would expect the following code to pass the assertion.

(Playground)

fn main() {
    let mut val: usize = 42;
    unsafe {
        modify(&mut (val as usize));
    };
    assert_eq!(val, 0);
}

unsafe fn modify(val: *mut usize) {
    unsafe { *val = 0 };
}

This issue originally came about doing pointee casting to call a C function that had this signature:

void my_free(void **ptr);

It didn't occur to me that the pointer cast in order to call it was what was causing unexpected behavior until quite a bit of debugging. As far as I can tell, these semantics are mentioned nowhere in the Rust Reference, nor the Rustonomicon.

When optimized, it appears that LLVM decides that this just always fails the assertion, and it doesn't mark the whole function as unreachable.

It's also worth noting that rustc does warn about val having an unused mut.

I'm not sure if this entails a language semantics change, or just a documentation and/or lint change, but either way this behavior is both unexpected and undocumented.

Meta

Occurs on both stable and latest nightly rustc --version --verbose:

rustc 1.75.0 (82e1608df 2023-12-21)
binary: rustc
commit-hash: 82e1608dfa6e0b5569232559e3d385fea5a93112
commit-date: 2023-12-21
host: x86_64-unknown-linux-gnu
release: 1.75.0
LLVM version: 17.0.6
rustc 1.77.0-nightly (88189a71e 2024-01-19)
binary: rustc
commit-hash: 88189a71e4e4376eea82ac61db6a539612eb200a
commit-date: 2024-01-19
host: x86_64-unknown-linux-gnu
release: 1.77.0-nightly
LLVM version: 17.0.6
asquared31415 commented 8 months ago

Note the "variable does not need to be mutable" warning that the compiler emits. This should indicate that the variable isn't actually being mutated. The expression (val as usize) is creating a copy of val. The docs should probably be improved to say "does not change the bit representation" instead, since the expression does have effects.

@rustbot label +A-docs -needs-triage

quaternic commented 8 months ago

@gfaster The Reference defines as like this:

Executing an as expression casts the value on the left-hand side to the type on the right-hand side.

The key is that <expr> as <type> is a value expression, whereas your expectation seems consistent with a hypothetical where it could instead be a place expression. For the definitions of these, see https://doc.rust-lang.org/reference/expressions.html#place-expressions-and-value-expressions Since the type is the same, val as usize is equivalent to {val}, which also does a copy of the value.

While I could imagine it casting the place in these no-op cases, it wouldn't be possible in general since the the types may be different sizes (u8 as u32), or representation (f32 as u32).

To get your expected behaviour, you could cast the reference/pointer: (&mut val) as *mut usize, or to avoid the intermediate reference, addr_of_mut!(val)

workingjubilee commented 8 months ago

According to the Rust Reference, integer casts to other integers of the same width (e.g. i32 -> u32) are a no-op. With that in mind, I would expect the following code to pass the assertion.

This issue originally came about doing pointee casting to call a C function that had this signature:

It didn't occur to me that the pointer cast in order to call it was what was causing unexpected behavior until quite a bit of debugging. As far as I can tell, these semantics are mentioned nowhere in the Rust Reference, nor the Rustonomicon.

@gfaster You have still not stated what the actual problematic cast was.

Pointers and integers are T: Copy, and what is causing this behavior is documented in Copy.

workingjubilee commented 8 months ago

In my experience, it is very common for people to have a more subtle and nefarious problem when doing a pointer cast, one which falls outside the bounds of the described issue here. The problem is often something like

let mut val: usize = 42;
let val_mut = &mut val;
let ptr_to_val = val_mut as *mut _ as *mut *mut usize;
// or perhaps...
let ptr_to_val = val_mut as *mut _ as usize;

...or something like that.

And I am mildly surprised this issue has been opened, because both the Rust program

fn main() {
    let mut val: usize = 42;
    unsafe {
        modify(&mut val as usize);
    };
    assert_eq!(val, 0);
}

unsafe fn modify(val: *mut usize) {
    unsafe { *val = 0 };
}

and the C program

#include <stddef.h>
#include <assert.h>

void modify(size_t* val) {
    *val = 0;
}

int main() {
    size_t val = 42;
    modify(&((size_t)val));
    assert(val == 0);
    return 0;
}

...simply do not compile. On a CPU, a "nop" still has an "effect" (that is, it indicates the instruction pointer should move on to the next instruction). Thus "no-op" in computing is used in a similar sense: the operation may still increase the amount of spacetime used by the program, it just has no other semantic import. It's not clear to me what you were expecting, given all that, so it will not be easy to improve the documentation in a satisfactory way with the actual problem obscured so.

gfaster commented 7 months ago

In retrospect, this was something I Should Have Known. The reason I ended up in the scenario was because I was trying to wrangle &mut decay.

I was trying to call a C function that looked like this:

void my_free(void **ptr);

I had a pointer to pass to it, but casting it to the right type was causing trouble. Something like this:

extern "C" {
    /// Nulls `*ptr` to prevent double frees
    fn my_free(ptr: *mut *mut c_void) -> c_void;
}
let mut my_ptr: *mut i32 = (/* ... */);
my_free(&mut my_ptr);

The problem was that this wouldn't compile, so I tried to cast the pointer first, thinking (incorrectly) that it would be in-place:

my_free(&mut (my_ptr as *mut c_void));

I do get that there is probably no good way of solving this, but it was a pretty major unexpected footgun for me.

workingjubilee commented 6 months ago

I do get that there is probably no good way of solving this, but it was a pretty major unexpected footgun for me.

Hmm. Probably we could dial up the lints and emit more warnings, but if a lint already was emitted and it was ignored, I'm not entirely sure if we can expect another warning to not also be ignored, here?