After https://github.com/llvm/llvm-project/commit/82d0749749fafb0303414131dc570781376fdcfb 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?
82d0749749fafb0303414131dc570781376fdcfb 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.
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.
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.
Revert posted at https://reviews.llvm.org/D107216.
We have now finished the LLVM 13 upgrade with D100944 reverted in our fork.
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.
Merged: 1c198b3032e899003b5c1ffaa15c7648f24f1e69