interpret #[used] as #[used(compiler)] on illumos

[iximeow]

helps #146169 not be as painful: fixes the illumos regression in #140872, but #[used(linker)] is still erroneous on illumos generally.

illumos' ld does not support a flag like either SHF_GNU_RETAIN or SHF_SUNW_NODISCARD, so there is no way to communicate #[used(linker)] for that target. Setting USED_LINKER to try results in LLVM setting SHF_SUNW_NODISCARD for Solaris-like targets, which is an unknown section header flag for illumos ld and prevents sections from being merged that otherwise would.

As a motivating example, the inventory crate produces #[used] items to merge into .init_array. Because those items have an unknown section header flag they are not merged with the default .init_array with frame_dummy, and end up never executed.

Downgrading #[used] to #[used(compiler)] on illumos keeps so-attributed items as preserved as they had been before #140872. As was the case before that change, because rustc passes -z ignore to illumos ld, it's possible that used sections are GC'd at link time. #146169 describes this unfortunate circumstance.

---

as it turns out, tests/ui/attributes/used_with_archive.rs had broken on x86_64-unknown-illumos, and this patch fixes it. the trials and tribulations of tier 2 :(

r? @Noratrieb probably?

[rustbot]

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann

[rustbot]

Noratrieb is not on the review rotation at the moment.
They may take a while to respond.

[workingjubilee]

Hm. Isn't used(linker) simply broken on illumos? Is there a purpose to adding a variant to UsedBy when used(linker) remains incorrectly behaving?

[iximeow]

i didn't think we can get at the target OS while parsing used, mainly. and it seems a bit funky to parse attributes in a target-sensitive way like that, but i have no sense for how normal that would be to do?

[workingjubilee]

Wouldn't it be simpler to only change codegen_attrs.rs?

[iximeow]

in a "lines changed" sense, yes, i suppose? as a very infrequent contributor and a basically-never-touched-parsing contributor, i primarily want to make a change that is not surprising to regular contributors.

if you're saying that this is surprising, and you would expect codegen_attrs.rs to care about the target OS (and presumably it'd be available under cx somewhere here), i'm happy to make that change. the diff here is what i thought would be least surprising, happy to defer to whatever makes more sense to the regular readers here.

[workingjubilee]

sorry, I meant in cg_ssa specifically. adding a variant during option parsing to bypass broken behavior on a target where the behavior for one of the variants is already broken and will remain broken is what puzzles me.

[iximeow]

by cg_ssa we can't distinguish between the source having been #[used] or #[used(linker)]. so to reinterpret UsedBy::Linker as USED_COMPILER there would make #[used(linker)] in source pretty awkward. i recognize that #[used(linker)] is unstable but it feels more fitting to warn about it than silently downgrade it.

so that's how i ended up agreeing with bjorn3 on a third variant here. and separately i figure i'll add a warning if UsedBy::Linker makes it to codegen on illumos?

(relatedly, i'm not totally sure that -z ignore does what we want with illumos' ld, but i haven't experimented much. if it doesn't and it makes sense to condition that on is-Solaris-like-and-not-illumos, then #[used(linker)] is trivially satisfied as USED_COMPILER. but i think the status quo is that -z ignore means a #[used(linker)] section could end up GC'd. that's also something i want to follow up on but.. i figure one thing at a time)

[iximeow]

hm. i'd missed that moving from passing to failing among the other failures on illumos, i suppose i should have run ./x test on a tier 1 target too :)

[jdonszelmann]

This seems like the right solution to me (having written most attribute parsers, but not this one specifically. Did review it at the time). Parsing it to a form that's target agnostic but let's you still decode which form it was (Any) and then choosing what to do with it in SSA. r=me for the attribute parsing stuff if you need my blessing nora

[workingjubilee]

by cg_ssa we can't distinguish between the source having been #[used] or #[used(linker)]. so to reinterpret UsedBy::Linker as USED_COMPILER there would make #[used(linker)] in source pretty awkward. i recognize that #[used(linker)] is unstable but it feels more fitting to warn about it than silently downgrade it.

Yes, it's mostly a matter of taste then. If people prefer this route then that's fine, as it does reflect the source most closely during parsing before it reaches codegen.

[Noratrieb]

Thanks, can you clean up the commits to squash the third one into the first one? Then we can merge it

View changes since this review

[iximeow]

i went ahead and squashed all three since it feels like there's no use for the intermediate handles-warnings-bad state. thanks for the reviews 🙏

[Noratrieb]

Makes sense, thanks!
@bors r+

[bors]

📌 Commit c721fa2 has been approved by Noratrieb

It is now in the queue for this repository.