`#[pre_init]` safety is questionable

[jonas-schievink]

#[pre_init] requires that the function it is applied to is unsafe, since the function is run before .bss/.data is initialized. However, unsafe fn means that the function is unsafe to call, while a #[pre_init] function is actually unsafe to implement (it might additionally be unsafe to call from any other place than the pre-init hook). One way to express this would be to require the function body to contain only an unsafe {} block (similar to how #[naked] functions work).

Another issue is that it is neither clear nor documented what happens to promoted statics, which are easy to accidentally create. Presumably, accessing them and creating them is immediate UB.

[japaric]

A #[pre_init] function affects safety in a non-standard way: it makes some safe functions unsound to call. I think no version of unsafe we have (unsafe fn (unsafe to call), unsafe impl / unsafe trait (asserts a property about the type)) properly conveys this fact.

One way to express this would be to require the function body to contain only an unsafe {} block (similar to how #[naked] functions work).

I don't think this helps convey what #[pre_init] actually does. It can also be misleading; for example, the following code would trigger a warning telling the user to remove the unsafe block.

#[pre_init]
fn foo() {
    unsafe { safe_but_unsound_to_call_here(); }
    //~^ WARN unnecessary `unsafe` block
}

fn safe_but_unsound_to_call_here() -> u32 {
    static X: AtomicU32 = AtomicU32::new(0);
    X.fetch_add(1, Ordering::Relaxed)
}

Unless someone is willing to write a lint that walks the call graph that spans from #[pre_init] to detect uses of static variables I think we should rely on documentation to convey what's sound and unsound to do in this function. If we want to point more heavily the user to the docs we could change the attribute to always be called like this:

#[pre_init(yes_I_have_read_the_safety_section_of_the_docs_and_know_what_I_am_doing)]
unsafe fn foo() {
    // ..
}

[jonas-schievink]

unsafe impl / unsafe trait (asserts a property about the type))

These can also assert properties of functions in the impl (like Alloc), which we could theoretically make use of too, but having to define some type and implement a trait seems excessive.

It seems that #[naked] doesn't currently work like what I proposed either, so I guess we should leave this be for now (and maybe revisit once Rust has to express something similar in some place).

It can also be misleading; for example, the following code would trigger a warning telling the user to remove the unsafe block.

Oh, I thought that didn't trigger on macro output? Ah, I guess rustc knows that these tokens come from the user... We could still make it not warn by slightly modifying the macro output.

[jonas-schievink]

Another issue is that it is neither clear nor documented what happens to promoted statics, which are easy to accidentally create. Presumably, accessing them and creating them is immediate UB.

This part was addressed by #248. Closing.