Lint against #[no_mangle] for non-repr(C) pub statics

[ojeda]

What it does

The lint should lint against a #[no_mangle] pub static which has a type which is not repr(C) (or that has no explicit repr(Rust) set).

This would be similar to no_mangle_with_rust_abi from #10347, but for pub statics.

Advantage

Avoids potential subtle mistakes and UB in programs that mix Rust and other languages like C: one cannot predict the layout of repr(Rust) types and thus an exported static marked with #[no_mangle] has a high chance of being a mistake.

Drawbacks

No response

Example

pub struct S(u8, u16);

#[no_mangle]
pub static X: S = S(0xFF, 0xFFFF);

Should be written as:

#[repr(C)]
pub struct S(u8, u16);

#[no_mangle]
pub static X: S = S(0xFF, 0xFFFF);

Otherwise, a C program with manually written bindings (e.g. projects not using cbindgen, which in this case would generate an incomplete type) trying to access X will break sooner or later, potentially silently (using -Zrandomize-layout would increase the chances of the issue being spotted, though), e.g.

#include <assert.h>
#include <stdint.h>

struct S {
    uint8_t a;
    uint16_t b;
};

extern const struct S X;

int main(void) {
    assert(X.a == 0xFF);
    assert(X.b == 0xFFFF);
    return 0;
}

[ojeda]

I got the idea from the __rust_no_alloc_shim_is_unstable that we have to export in the kernel once we upgrade to Rust 1.71 (though that is just a u8) -- Cc @bjorn3.

[blyxyas]

Couldn't this be an extension to no_mangle_with_rust_abi?

[ojeda]

Yeah, that sounds sensible; although I don't know how lint "granularity" is decided in Clippy, or whether somebody would want one to get diagnostics for one (but not the other) in practice.

[Centri3]

I think merging them into one lint is fine. I can't think of a single reason somebody would use #[no_mangle] with the rust ABI (implicitly, of course), so having one enabled but not the other seems really rare. Maybe the crate defining it and the crate that uses it are in the same compiler run so it wouldn't be causing UB but even then, why would you have #[no_mangle] in the first place? I doubt the compiler would guarantee they have the same layout if it's a cdylib, so then it's (probably) UB anyway.

It's a bit unfortunate though that you can't use #[repr(Rust)] explicitly, so it's not possible to make this explicit. But I think this behavior is fine (Though maybe I should open a PR for Rust to allow this.)

[blyxyas]

I think #[repr(transparent)] is equivalent to what #[repr(Rust)] would be

Maybe I'm wrong though, it's 8 in the morning 🐉

[Centri3]

Not entirely, it uses the same layout as the one non-marker (more accurately, non-PhantomData) field. It also prevents field randomization as technically there are no fields, which is most of what causes issues in the first place, so it's a stable ABI if the underlying type is also. I like to think of it as the newtype representation, I suppose.

#[repr(Rust)] would function the same as no #[repr(...)] at all, but is explicit.