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
}

[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 rust-lang/rust#66862 (comment) 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 rust-lang/rust#123583. Maybe it's time to stop using this ad-hoc hack for our Windows thread-local destructors.

[bjorn3]

I implemented this in rust-lang/rust@850e1e3, 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.

[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 rust-lang/rust#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]

#[used] does not provide the semantics we want either. It only guarantees that a symbol will end up in an object file. It does not guarantee that the linker ever evaluates that object. This can be a real concern, depending on how things end up being split. I've encountered this issue when trying to add a .drectve section in a library.

[RalfJung]

That sounds like a bug in #[used], maybe caused by limitations of linkers? If linkers have a way to force an object to persist, #[used] should set that.

But fixing #[used] is orthogonal to the issue in this thread.

[ChrisDenton]

It's documented behaviour. https://doc.rust-lang.org/reference/abi.html#the-used-attribute

This attribute forces the compiler to keep the variable in the output object file

But my point is that without compiler support there are no right options for including the global destructor.

[joboet]

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

How is that unspecified? read_volatile is guaranteed not to be optimized away, so there will always be a reference to the relevant symbol. And all referenced symbols must be placed in the final binary, right?

[RalfJung]

Unfortunately I don't think one can move individual comments. I opened a new issue: rust-lang/unsafe-code-guidelines#504. Would be great if you could summarize the relevant part of the discussion there. :)

I'll make some of our previous comments here as off-topic for the Miri side. When/if rust-lang/unsafe-code-guidelines#504 reaches a conclusion, we can try to implement that in Miri (but the proposal you made is unimplementable for Miri); meanwhile we can either keep using our current hack in Miri or go with a different hack, or with the best possible approximation of the unimplementable spec you proposed.

(I'll explain in rust-lang/unsafe-code-guidelines#504 why I claim this is unimplementable. I already explained it above, too.)

[RalfJung]

#[used] does not provide the semantics we want either.

But my point is that without compiler support there are no right options for including the global destructor.

There are options that are different degrees of wrong, and IMO rust-lang/rust#121596 made it more wrong than before. Now you're not only relying on whatever happens between the object file and the final binary keeping the symbol around, you are also relying on this volatile trick that has no proper justification.

IOW, there are two stages where the symbol can get lost, and your argument seems to be "because the second stage is lossy, it's okay to not use the attribute that makes the first stage definitely not-lossy". That's not a valid argument; we have been discussing all along what happens in the first stage (whether the symbol makes it to the object file) so the second stage (whether the symbol makes it from the object file to the binary) is orthogonal and entirely irrelevant.

[petrochenkov]

IMO rust-lang/rust#121596 made it more wrong than before

FWIW, I agree with this statement.
I've seen that PR, but didn't have time to investigate and interfere.
If #[used]/#[used(compiler)]/#[used(linker)] don't work, then they need to be fixed to work.
We have ways to both keep object files alive (symbols.o) and keep sections non-garbage-collectable (RETAIN sections), if they don't work on some platforms then other ways to make it work are likely possible.

[RalfJung]

With rust-lang/rust#123937 now has the basic infrastructure to deal with link_sections that are actually arrays, which should make it a lot easier to implement support for constructors as well.