Rust: #[no_mangle] is unsafe

[geofft]

On some platforms (at least GNU/Linux, but I hear Windows and several others too), if you link together two static libraries that both export a symbol of the same name, it's undefined which symbol actually gets linked. In practice on my machine, the first library seems to win. This lets you defeat type/memory safety without the unsafe keyword, by having two crates export a #[no_mangle] pub fn with different signatures but compatible calling conventions:

// one.rs
#![crate_type = "lib"]

#[no_mangle]
pub fn convert(x: &'static i32) -> Result<i32, f32> {
    Ok(*x)
}

// two.rs
#![crate_type = "lib"]

#[no_mangle]
pub fn convert(x: &'static f32) -> Result<i32, f32> {
    Err(*x)
}

// transmute.rs
extern crate one;
extern crate two;             

fn main() {
    static X: f32 = 3.14;
    let y: i32 = two::convert(&X).unwrap();
    println!("{}", y);
}

geofft@titan:/tmp/transmute$ rustc one.rs
geofft@titan:/tmp/transmute$ rustc two.rs
geofft@titan:/tmp/transmute$ rustc transmute.rs -L .
geofft@titan:/tmp/transmute$ ./transmute 
1078523331

Despite the stated call to two::convert, it's actually one::convert that gets called, which interprets the argument as a &'static i32. (It may be clearer to understand with this simpler example, which doesn't break type safety.)

On at least GNU/Linux but _not_ other platforms like Windows or Darwin, dynamically-linked symbols have the same ambiguity.

I don't know what the right response is here. The following options all seem pretty reasonable:

1. Acknowledge it and ignore it. Maybe document it as a possible source of unsafety, despite not using the unsafe keyword.
2. Have #[no_mangle] export both a mangled and un-mangled name, and have Rust crates call each other via mangled names only, on the grounds that #[no_mangle] is for external interfaces, not for crates linking to crates. ("External interfaces" includes other Rust code using FFI, but FFI is unsafe.) This is analogous to how extern "C" fns export both a Rust-ABI symbol as well as a C-ABI shim, and a direct, safe Rust call to those function happens through the Rust ABI, not through the C ABI. I'm pretty sure that all production uses of #[no_mangle] are extern "C", anyway (see e.g. #10025).
3. Deprecate #[no_mangle] on safe functions and data, and introduce a new #[unsafe_no_mangle], so it substring-matches unsafe. (#[no_mangle] on unsafe functions or mutable statics is fine, since you need the unsafe keyword to get at them.)

All of these are, I think, backwards-compatible.

[alexcrichton]

I think this even leads to a linker error on some platforms (depending on the situation), so in that sense I'd also be fine just having a set of all #[no_mangle] symbols for each crate and if they overlap we generate a hard compile time error. While strictly not backwards compatible I believe it's practically fine and it's basically what most crates what anyway

[cuviper]

You can #[no_mangle] override an existing mangled function too:

fn main() {
  println!("ok")
}

#[no_mangle]
#[allow(non_snake_case)]
pub fn _ZN2io5stdio6_print20h94cd0587c9a534faX3gE() {
    unreachable!()
}

This makes the playpen timeout on stable 1.2: http://is.gd/sxdUl3

[geofft]

While strictly not backwards compatible I believe it's practically fine

Yeah, I'd argue that having multiply-defined #[no_mangle] symbols is, in essence, undefined behavior (in the sense that nobody defined it), so making it a hard error seems fine.

[nagisa]

I doubt there’s anything sensible (other than writing our own linker) that can be done here to truly fix the issue.

I’m not against the idea of making less of these cases possible, though. e.g.

#[no_mangle]
fn main(){}

doesn’t work already. But most of the proposed solutions (exception being keeping more symbols in memory) sound like a non-option to me.

[arielb1]

@nagisa

#[no_mangle] will always be always unsafe as you can use it to hijack C library functions, .e.g make malloc do something evil.

[nagisa]

@arielb1 i didn’t say anything about safety; only about our capability to do anything about this issue.

Either way, if you have to be wary of code you control unknowingly hijacking other code or doing evil things, something went very wrong where Rust (or any language, really) can’t help.

[arielb1]

@nagisa

JS is designed for that use-case. Rust is _theoretically_ supposed to fill that role too, but both rustc and the type-system are _way_ not sufficiently trustworthy for that.

[bstrie]

I agree that unmangled symbol collisions should be a compilation error. Breakage here is extraordinarily unlikely, and the only code that it could break is code that won't be working as intended anyway, which is exactly the sort of the code that we ought to break.

[nikomatsakis]

Reassigning to lang team. What a pain.

[nikomatsakis]

triage: P-low

Seems like a real problem, if not that concerning. @rust-lang/lang feels that the fix proposed by @alexcrichton in this comment is the correct one. Check at link time for duplicate #[no_mangle] symbols in rust code; don't try to solve similar problems that might occur with C code and other stuff out of our purview.

[vi]

Why is calling #[no_mangle] functions safe in the first place?

Also #[no_mangle] feels like a first step towards #[naked], which should also require unsafe to be called.

[geofft]

Why is calling #[no_mangle] functions safe in the first place?

They can't do anything that normal functions can't do. And there isn't in theory any way to use them to violate safety (this issue is a bug; it would be totally fine in theory for rustc to fail to compile code with any symbol collisions in any objects, whether they come from Rust or from another language).

#[naked], which should also require unsafe to be called.

As far as I can tell, this is basically because naked functions have no guaranteed ret opcode, so #[naked] fn foo() -> {println!("hi!")} will just let the instruction pointer run past the end of a function, which is rather memory-unsafe. If they did (and if there were a more coherent story about accessing arguments and returning values from pure Rust code), I'm having trouble seeing what makes them unsafe.

That is, the reason #[naked] functions should require unsafe is that the compiler expects a particular thing to be done by the implementation in order for them to be safely callable, and it has no way to check that. (I sort of think that, like unsafe traits, it should be possible for the implementor to say "Trust me on this" and others to be able to call the function from safe code, but maybe this is a discussion for the naked function RFC, not here.) There's nothing special that #[no_mangle] functions need to take care to do; they're just the same as normal functions except for their linkage name.

[vi]

They can't do anything that normal functions can't do.

It's not the function itself that is unsafe but the call. The call to something unmangled is FFI-ish, therefore looks (and is) unsafe.

#[naked] functions are unsafe per se, so calls to them should be unsafe not directly, but as a consequence.

[nikomatsakis]

There are some other examples of abusing #[no_mangle] that may be worth mentioning.

For example, this hack overrides main with custom assembly:

#![no_main]

#[link_section=".text"]
#[no_mangle]
pub static main: [u32; 9] = [
    3237986353,
    3355442993,
    120950088,
    822083584,
    252621522,
    1699267333,
    745499756,
    1919899424,
    169960556,
];

Sigh.

[vi]

#![forbid(unsafe)] code should be assumed free of such hacks...

[RalfJung]

![forbid(unsafe)] code should be assumed free of such hacks...

Yeah, my thinking exactly. Maybe that should forbid no_mangle as well, or so?

[ebkalderon]

The playground example in this comment no longer times out on Rust 1.35 stable and prints "ok" without any apparent issues.

[RalfJung]

That just means that the mangled name of the print function changed.

[ebkalderon]

@RalfJung Thanks, that makes sense. Just wanted to point it out for future readers, as I had stumbled across it and was surprised to see it work despite the issue not being resolved.

[RalfJung]

I think the current version (for Rust 1.35) would be

fn main() {
  println!("ok")
}

#[no_mangle]
#[allow(non_snake_case)]
pub fn _ZN3std2io5stdio6_print17ha4c0b9f4da5c9e13E() {
    unreachable!()
}

but that doesn't run either, it just makes the linker fail:

/rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e//src/libstd/io/stdio.rs:752: multiple definition of `std::io::stdio::_print'

[RalfJung]

In this comment, @dtolnay brings up the idea of "unsafety at the item level". I feel like that is also relevant for this issue here, which basically concerns an unsafe function-level attribute.