Can no longer set custom section flags in LLVM IR

[nikic]

Bugzilla Link	51207
Version	trunk
OS	Linux
Blocks	#50580
CC	@MaskRay,@cuviper,@tstellar
Fixed by commit(s)	1c198b3

Extended Description

After 82d0749 this IR

module asm ".section .llvmbc,\22e\22"
@​rustc.embedded.module = private constant [3 x i8] c"ABC", section ".llvmbc"

when run through llc results in an "Unknown section kind" assertion failure.

rustc emits this IR here, which has a comment explaining why it does so: https://github.com/rust-lang/rust/blob/bddb59cf07efcf6e606f16b87f85e3ecd2c1ca69/compiler/rustc_codegen_llvm/src/back/write.rs#L970-L1017

I'm not sure where the bug is here -- what rustc is doing certainly looks fishy, but I'm not sure what it is supposed to be doing instead. Any advice?

[MaskRay]

82d0749 is innocent. The Rust code is incorrect.

With the inline asm .section directive, SeenSectionNameBefore is true and we don't take the return MCContext::GenericSectionID; shortcut.
getELFSectionNameForGlobal doesn't intend to work for non-SHF_ALLOC sections => assertion failure.

The Rust abused the previous LLVM behavior when an inline asm .section directive can override the section attributes of section ".llvmbc".
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp doesn't have such guarantee.
D100944 may be when this changed.

To support such usage properly, we need an LLVM IR construct to encode the section attributes.
It can be quite challenging to make a design suitable for multiple binary formats.
In the absence of that, the best way forward for Rust is to change the section after MC object emission.

[nikic]

I don't think that modifying object files after the fact is an acceptable outcome for us. Prior to your change, LLVM provided a way to set section flags on the llvmbc section, even if it was roundabout -- and after your change there is no longer any IR-level way to achieve that, if I understood correctly.

As such, I believe this change should be reverted for the LLVM 13 release, and be reapplied once there is an IR-level mechanism to specify the flags.

[nikic]

After further testing, it looks like reverting https://reviews.llvm.org/D86374 just fixes the assertion failure, but still results in undesirable behavior: We'll get two .llvmbc sections, one with the EXCLUDE flag, and one (containing the actual bitcode) with the ALLOC flag. This breaks bitcode extraction entirely.

The actual problem here is https://reviews.llvm.org/D100944. Reverting that change (and just that change) restores the previous behavior.

Actually, it looks like this problem was brought up in post-commit review for D100944, but didn't reach a satisfactory conclusion.

[nikic]

Revert posted at https://reviews.llvm.org/D107216.

[nikic]

We have now finished the LLVM 13 upgrade with D100944 reverted in our fork.

[cuviper]

In further discussion on D107216, we agreed to apply the revert on 13.x only, leaving the change on main, so Rust will have time to find a new way for 14. Fangrui suggested different wording for the commit message though.

[tstellar]

Merged: 1c198b3

[tstellar]

mentioned in issue #50580

[asl]

@tstellar Move to 13.0.1?

[nikic]

This issue has been addressed on the rust side (see rust-lang/rust#90326), so closing this. Adding proper support for section flags in LLVM would still be nice, but it's no longer relevant as far as the release is concerned.