rust-lang / unsafe-code-guidelines

Forum for discussion about what unsafe code can and can't do
https://rust-lang.github.io/unsafe-code-guidelines
Apache License 2.0
658 stars 57 forks source link

What are the special magic rules around `malloc`? #535

Open RalfJung opened 1 week ago

RalfJung commented 1 week ago

Taken from https://github.com/rust-lang/unsafe-code-guidelines/issues/534:

// use a mutable reference to prevent the MIR opt from happening
#[no_mangle]
pub fn src(x: &mut &u8) -> impl Sized {
    let y = **x;
    let mut z = Box::new(0);
    // a bunch of code that operates on the `Box`, however, 
    // nothing else can potentially access the underlying `u8`
    // that's behind the double reference besides the `__rust_alloc` call.

    // optimizable to `true`?
    **x == y
}

Currently, LLVM doesn't do the second optimization. However, it does perform it if you manually set System to be the global allocator: https://rust.godbolt.org/z/a77PWjeKE [^1]. This is due to this line, which is used by their GVN pass.

There are clearly special magic rules applying specifically for malloc that mean that its memory must be truly fresh for the Abstract Machine, and cannot be part of any previously existing stack/heap/other allocation. This is "fine" as long as malloc is called via FFI and all the state it works in is completely hidden from the current compilation unit. It becomes rather incoherent if there is ever a chance of malloc itself being inlined into surrounding code, or exchanging data with surrounding code via global state -- so we better have rules in place against things like that. I think we should say that malloc is reserved to be provided by the underlying runtime system, and it must be called via FFI in a way that no inlining is possible.

Note that this is separate from Rust's #[global_allocator] attribute, which does not get all the same magic that malloc gets. See https://github.com/rust-lang/unsafe-code-guidelines/issues/442 for discussion of the semantics of that attribute.

[^1]: You also get the malloc -> calloc transformation for types other than these hardcoded ones if you set System to be the global allocator manually.

VorpalBlade commented 1 week ago

The issue with this magic that I see is if you implement malloc itself in Rust.

Another issue is LTO or even cross-language LTO.

RalfJung commented 1 week ago

I agree that this magic is potentially problematic. I don't know if LLVM has a way to disable it though.

VorpalBlade commented 1 week ago

I agree that this magic is potentially problematic. I don't know if LLVM has a way to disable it though.

Fair enough. But I do believe rust / llvm need an answer for how to properly handle the above scenarios. How do I do these things soundly in Rust? Can I or can I not use LTO when making a libc for example?

Also, as I understand it, any soundness issues that cannot be traced to an unsafe block (or unsafe attribute, unsafe command line flags (though I don't think those exist yet?), etc) are compiler bugs? Though in this case I guess the unsafe bit is the no-mangle export of a function called malloc, but that feels like a cop-out and would make it really difficult to write a libc in Rust.

Diggsey commented 1 week ago

There are clearly special magic rules applying specifically for malloc that mean that its memory must be truly fresh for the Abstract Machine, and cannot be part of any previously existing stack/heap/other allocation.

Could I dig a bit more into why this is important? Could we avoid such issues by having the malloc implementation explicitly "carve out" an existing allocation and give it back to the Abstract Machine, minting a new allocation? I imagine this "carving out" would come with significant limitations, such as no access being allowed to that region of memory until it is returned.

In this model, the malloc implementation accessing the memory after carving it out would be UB.

RalfJung commented 6 days ago

why this is important?

It's important because LLVM does optimizations and we have to ensure they don't break our model.

This is a descriptivist issue, not a prescriptivist one. There are special magic rules for malloc on LLVM. I don't know the full extent of this magic, it is AFAIK not documented. I can't tell you how important they are, you'll have to ask that on the LLVM forums. I also don't know whether a libc written in C needs to do anything special wrt its malloc symbol to avoid trouble here.

In this model, the malloc implementation accessing the memory after carving it out would be UB.

That's already part of the model for regular Rust global allocators. This issue is about malloc magic that goes beyond this. My understanding is that LLVM does more things for malloc than it does for our __rust_alloc, and I linked to an example of that in the issue description. In particular, what you describe is nowhere near enough to justify the optimization in the issue description.

Diggsey commented 5 days ago

It's important because LLVM does optimizations and we have to ensure they don't break our model. This is a descriptivist issue, not a prescriptivist one.

Right, I'm trying to understand what part of the model these optimizations would break if malloc were implemented in the same compilation unit.

In particular, what you describe is nowhere near enough to justify the optimization in the issue description.

I think I was misunderstanding the nature of this issue, due to:

There are clearly special magic rules applying specifically for malloc that mean that its memory must be truly fresh for the Abstract Machine, and cannot be part of any previously existing stack/heap/other allocation.

I thought that meant this was something to do with LLVM's intrinsic understanding of memory allocation and deallocation, but actually it could happen with any function that LLVM "knows" doesn't access any IR visible value. LLVM has chosen to only special-case malloc and related functions because they are already "known" and are presumably the most common example of such a function?

Do we know when LLVM considers something to be malloc? Maybe it already handles the case we are worried about (where malloc is implemented in the same compilation unit) by not considering it to be a malloc-like function in that case?