Evaluate global constructors (life before main)

[oli-obk]

@eddyb is lawful evil https://github.com/neon-bindings/neon/blame/master/src/lib.rs#L49

basically we should look in platform specific link sections for statics and run their value before anything else

#[cfg_attr(target_os = "linux", link_section = ".ctors")]
#[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
#[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
pub static __FOOMP: extern "C" fn() = {
    extern "C" fn __foomp() {
        // life before main goes here
    }
    __foomp
}

This would also help support the inventory crate.

[Ericson2314]

🤮 @eddyb!

[RalfJung]

Are you sure about the "lawful" part? :P

More seriously though -- shouldn't we instead lobby for some way to express this in Rust proper? I already feel bad about that TLS emulation we are doing...

[eddyb]

shouldn't we instead lobby for some way to express this in Rust proper?

No, Rust as a language is uninterested in life-before-main. The only reason any of this works is because it's an OS feature we can't really stop you from opting into.

[RalfJung]

See https://github.com/rust-lang/rust/issues/66862#issuecomment-559862805 for more information on these sections.

[joshtriplett]

You might find the following C program useful for experimenting with init section order:

#include <stdio.h>

typedef void (*FN)(void);

#define MKINIT(function, section) \
    void function(void) { puts(section); } \
    __attribute__((__section__(section))) FN function##_section[] = { function };

MKINIT(init_array, ".init_array")
MKINIT(init_array_00001, ".init_array.00001")
MKINIT(init_array_00002, ".init_array.00002")
MKINIT(init_array_65533, ".init_array.65533")
MKINIT(init_array_65534, ".init_array.65534")
MKINIT(fini_array, ".fini_array")
MKINIT(fini_array_00001, ".fini_array.00001")
MKINIT(fini_array_00002, ".fini_array.00002")
MKINIT(fini_array_65533, ".fini_array.65533")
MKINIT(fini_array_65534, ".fini_array.65534")
MKINIT(ctors, ".ctors")
MKINIT(ctors_00001, ".ctors.00001")
MKINIT(ctors_00002, ".ctors.00002")
MKINIT(ctors_65533, ".ctors.65533")
MKINIT(ctors_65534, ".ctors.65534")
MKINIT(dtors, ".dtors")
MKINIT(dtors_00001, ".dtors.00001")
MKINIT(dtors_00002, ".dtors.00002")
MKINIT(dtors_65533, ".dtors.65533")
MKINIT(dtors_65534, ".dtors.65534")

int main(void)
{
    puts("main");
    return 0;
}

This prints:

.ctors.65534
.init_array.00001
.ctors.65533
.init_array.00002
.ctors.00002
.init_array.65533
.ctors.00001
.init_array.65534
.init_array
.ctors
main
.dtors
.fini_array
.fini_array.65534
.dtors.00001
.fini_array.65533
.dtors.00002
.fini_array.00002
.dtors.65533
.fini_array.00001
.dtors.65534

Note that weird things happen if you try using 00000 or 65535 or numbers outside that range, or non-numeric suffixes; the entire list gets sorted strangely if you do that. I don't think you need to worry about reproducing that exact behavior, though; I think that's a linker script quirk that doesn't normally come up because GCC never emits sections with such "weird" suffixes. Also note that the aforementioned weirdness may differ between GNU ld and LLVM's lld.

[RalfJung]

Thanks! We'll need to turn this into a Rust program as a test for this feature, but that shouldn't be too hard I guess.

Note that weird things happen if you try using 00000 or 65535 or numbers outside that range, or non-numeric suffixes; the entire list gets sorted strangely if you do that. I don't think you need to worry about reproducing that exact behavior, though; I think that's a linker script quirk that doesn't normally come up because GCC never emits sections with such "weird" suffixes. Also note that the aforementioned weirdness may differ between GNU ld and LLVM's lld.

Sounds like it would be best to just make Miri error in those cases.

[oli-obk]

We also need to bail if two entries for the same link section exist

[joshtriplett]

@oli-obk No, you can have arbitrarily many entries in a section; it's an array of function pointers. Entries for the same section get concatenated in link order.

[RalfJung]

How does one detect the length of the array? Do link sections have a notion of "length"?

[oli-obk]

Entries for the same section get concatenated in link order.

I don't think miri can figure out link order. For now we can bail out on multiple entries for the same section, but in the future we can check that they are order independent instead

[RalfJung]

in the future we can check that they are order independent instead

That sounds like fantasy to me. ;) I don't see a feasible implementation strategy for this.

[eddyb]

Do link sections have a notion of "length"?

@RalfJung yes, in some sense. AFAIK, linker scripts can define a symbol before and one after, so that code could iterate between those two symbols.

When you're operating before linking (as miri would need to do), any static in a relevant section is effectively a part of the final array, so it must contain 0, 1 or more fn pointers.

That is, ignoring the Rust type of the static, take the resulting memory allocation and divide it into pointer-sized elements that you then read out as fn pointers.

I don't think link order is a serious concern, and other than the numeric priorities which @joshtriplett explained, you can just pick some visiting order (and maybe lint the fact that a linker might differ?).

[RalfJung]

@oli-obk @bjorn3 do you know a good way to get "all statics that are in a particular linker section"?

I suspect this issue may be responsible for https://github.com/rust-lang/rust/issues/123583. Maybe it's time to stop using this ad-hoc hack for our Windows thread-local destructors.

[bjorn3]

I implemented this in https://github.com/rust-lang/rust/commit/850e1e34149a0dfa7513ae2e597434ab4b937263, but it requires a hack in the exported_symbols query override to also preserve the Instances of all #[used] symbols.

[RalfJung]

Oh, neat!

it requires a hack in the exported_symbols query override to also preserve the Instances of all #[used] symbols.

Does the regular query do that and Miri just lost it? Or do we somehow have to do something the regular query does not do? I assume regular rustc also has to preserve these statics for codegen as well...

[bjorn3]

Does the regular query do that and Miri just lost it?

The regular query doesn't do that.

I assume regular rustc also has to preserve these statics for codegen as well...

Statics are codegened in the crate that defines them and as such don't need to end up in exported_symbols if they aren't exported from the current crate (and not used by an exported inlinable function). For calling global constructors however miri would need all statics even if they aren't exported for as long as they are marked as #[used].

[ChrisDenton]

I think it'd be better for the compiler to have some basic support for platform specific linker magic. The library code is already a bit too hacky for my liking so it'd be an improvement there too.

[RalfJung]

Yeah it would be better but also a lot more work to implement. I won't stop anyone who tries to do that but I don't have the time to do it.

Statics are codegened in the crate that defines them and as such don't need to end up in exported_symbols if they aren't exported from the current crate (and not used by an exported inlinable function). For calling global constructors however miri would need all statics even if they aren't exported for as long as they are marked as #[used].

The static in question is not marked used

#[link_section = ".CRT$XLB"]
pub static p_thread_callback: unsafe extern "system" fn(c::LPVOID, c::DWORD, c::LPVOID) =
    on_tls_callback;

So there's something here I don't understand.

Statics are codegened in the crate that defines them

The query we override runs in that crate, so why is that relevant?

For finding extern functions with a link_name we also just do exactly what codegen does, I think. So it seems strange to me that here we'd have to do more than what codegen does.

[joboet]

The static in question is not marked used

See rust-lang/rust#121596 for why the #[used] was removed.

[bjorn3]

Statics are codegened in the crate that defines them

The query we override runs in that crate, so why is that relevant?

We need to override the query to include #[used] items because otherwise they wouldn't be encoded in the crate metadata as exported symbols and thus have no way to find all used statics for dependencies.

For finding extern functions with a link_name we also just do exactly what codegen does, I think. So it seems strange to me that here we'd have to do more than what codegen does.

When doing codegen the static initializer ends up an object file and codegen doesn't have to care about it anymore. Only the linker needs to find it. Miri however will need to know all static initializers of all dependencies without inspecting any (non existent) object files. As such the list of static initializers needs to be encoded in the crate metadata, which I did by ensuring they end up in the exported_symbols query.

[RalfJung]

When doing codegen the static initializer ends up an object file and codegen doesn't have to care about it anymore. Only the linker needs to find it. Miri however will need to know all static initializers of all dependencies without inspecting any (non existent) object files. As such the list of static initializers needs to be encoded in the crate metadata, which I did by ensuring they end up in the exported_symbols query.

I see... well I think I see. ;)

But it seems to me Miri should preserve all statics with a link_name, not all statics with #[used]. That is what we do for functions as well, isn't it? (I don't see a check for link_name in our exported_symbols query, but we only really care about the ones with a link_name.)

[bjorn3]

We do preserve those with link_name already, but not those with link_section.

I don't see a check for link_name in our exported_symbols query, but we only really care about the ones with a link_name.

I believe that is covered by the contains_extern_indicator check.

[RalfJung]

Ah right I mixed up link_name and link_section.

Sounds like a non-used static with a magic link_section has a really strange semantics - the function may or may not be called based on optimizations. That's very fragile and tricky to emulate in Miri. I would prefer if we didn't do hacks like that...

[ChrisDenton]

Which is one reason why I'd prefer proper compiler support for TLS destructors 😉

But it is just an optimization. Considering it to be always called is fine.

[RalfJung]

But it is just an optimization. Considering it to be always called is fine.

What Miri should be doing is randomly sometimes call it and sometimes not. That reflects the fact that it is unspecified whether the static ends up in the final binary or not.

Which is one reason why I'd prefer proper compiler support for TLS destructors 😉

I don't disagree with that. But I strongly disagree with using hacks relying on unspecified behavior to work around language limitations. The fix for a missing language feature is to add the feature. If something can't be done properly then doing it improperly is not a viable alternative IMO. In this case, using #[used] is a perfectly viable option IMO; the further optimization is not worth the significant headache it causes.

[RalfJung]

In this case I think the worst thing that can happen is memory leaks. Or can unsafe code somehow rely on TLS destructors being called? If yes then I would say https://github.com/rust-lang/rust/pull/121596 introduced a soundness issue, as we no longer can guarantee that TLS destructors are called.

[ChrisDenton]

I'm not sure I understand your point. If there is ever a TLS dtor then it will be called. Always.

If there is not then it's a no-op in any case so is eligible for removal.

[RalfJung]

I'm not sure I understand your point. If there is ever a TLS dtor then it will be called. Always.

You are relying on the completely unspecified assumption that these volatile reads "pull in" the static.

[ChrisDenton]

Sure but it's not the only place in the standard library that does that. In fact within that very function there's a volatile read being used to pull in another symbol _tls_used).

[RalfJung]

Relying on unspecified behavior more than once doesn't make it any or correct to do so.