ill-typed unused FFI declarations can cause UB

[arielb1]

This compiles and prints "p is not null and 0x0":

pub mod bad {
    #[allow(improper_ctypes)]
    extern {
        pub fn malloc(x: usize) -> &'static mut ();
    }
    
    #[no_mangle]
    pub fn bar() {
        let _m = malloc as unsafe extern "C" fn(usize) -> &'static mut ();
    }
}

pub mod good {
    extern {
        fn malloc(x: usize) -> *const u8;
    }
    
    pub fn foo() {
        unsafe {
            let p = malloc(0x13371337deadbeef); // your computer doesn't have enough memory
            if p.is_null() {
                panic!("p is null");
            } else {
                panic!("p is not null and {:?}", p);
            }
        }
    }
}

fn main() {
    bad::bar();
    good::foo();
}

The problem is that we have two declarations of the "malloc" symbol, but LLVM uses a global namespace for these. So during codegen, the 2nd declaration we generate overwrites the first. In this case, the "ill-typed" malloc declaration (bad::malloc) comes last, up putting a nonnull attribute on malloc, which causes mod good to be miscompiled.

Here's another example that does not involve malloc. It does not get miscompiled currently, but it demonstrates the issue.

See here for a document that summarizes why this happens, and what our options are.

[arielb1]

I haven't actually seen this in practice, but in brendanzab/gl-rs#438 I think someone has (erroneously) declared GetProcAddress as returning a (non-null) function pointer, which might cause other random code in the same executable to break because the erroneous nonnull reaches them.

[nikomatsakis]

Some questions:

• Is it possible for us to have two distinct definitions in LLVM for malloc, such that the call which is not dead does not inherit the non-null attribute?
• Alternatively, or perhaps just additionally, we could detect and lint against the mismatch.

[nikomatsakis]

triage: P-medium

We should fix this, but not a burning issue.

[gnzlbg]

Note that this is not restricted to extern "C", but it also impacts Rust function declarations: https://llvm.godbolt.org/z/03s-aL

I wondered whether, in some cases, LLVM could merge the attributes of the declarations with the definition, introducing UB in safe Rust code like this:

fn foo(x: *mut u8) { ... }
extern "Rust" {
    fn foos_mangled_name(x: &mut u8);
}

The above link shows that, currently, when merging noalias, the noalias of the declaration can end up being removed when a definition without noalias is present in the same module.

It is unclear to me whether making the definition noalias would be a sound optimization of LLVM-IR. noalias is modeled after C's restrict, and in C, the behavior of programs with a declaration like the above, which doesn't match the "restrict"/"noalias" qualifier of the definition, is undefined. So LLVM can assume that it never happens, and if it does, merging noalias in any way would be "ok" for C (not for Rust, and unclear whether it's ok for LLVM-IR, hopefully not).

[gnzlbg]

Alternatively, or perhaps just additionally, we could detect and lint against the mismatch.

@nikomatsakis if the incorrect declaration is in a different crate, could it become visible here due to LTO ? If so, then warning against this would need some kind of whole-program analysis.