[gold] Ignore bitcode from sections inside object files

[tstellar]

Summary

-fembed-bitcode will put bitcode into special sections within object
files, but this is not meant to be used by LTO, so the gold plugin
should ignore it.

https://github.com/llvm/llvm-project/issues/47216

[tejohnson]

A few comments and questions.

First, it looks like lld works because it has a similar check guarding its call of InputFile::create (which is embedded in the BitcodeFile constructor).

Note though I am seeing inconsistent info online as to what embedded bitcode is meant to be used for. It would be good to document this somewhere, like where fembed-bitcode is documented (which I am not finding). The initial submitter text when bcsection.ll was added suggests it was intended to be useful for LTO (https://reviews.llvm.org/D4371). I did find this recent discussion which indicates it was added for Apple recompilation: https://groups.google.com/g/llvm-dev/c/d1WSRvLefHQ/m/Iwm39FboAAAJ. I also found official ARM documentation online that points to the embedded .llvmbc section as being there to support LTO: https://developer.arm.com/documentation/dui0773/i/sbu1510240545830.

But in any case, as noted above, lld does not support its use in LTO links, so I think this is just in need of clear documentation somewhere.

Can you add a test?

If bcsection.ll is deleted, its input file bcsection.s should as well.

[tstellar]

In D116995#3234756, @tejohnson wrote:

A few comments and questions.

First, it looks like lld works because it has a similar check guarding its call of InputFile::create (which is embedded in the BitcodeFile constructor).

Note though I am seeing inconsistent info online as to what embedded bitcode is meant to be used for. It would be good to document this somewhere, like where fembed-bitcode is documented (which I am not finding). The initial submitter text when bcsection.ll was added suggests it was intended to be useful for LTO (https://reviews.llvm.org/D4371). I did find this recent discussion which indicates it was added for Apple recompilation: https://groups.google.com/g/llvm-dev/c/d1WSRvLefHQ/m/Iwm39FboAAAJ. I also found official ARM documentation online that points to the embedded .llvmbc section as being there to support LTO: https://developer.arm.com/documentation/dui0773/i/sbu1510240545830.

This discussion from rust is also relevant: https://github.com/rust-lang/rust/issues/90326

But in any case, as noted above, lld does not support its use in LTO links, so I think this is just in need of clear documentation somewhere.

Can you add a test?

If bcsection.ll is deleted, its input file bcsection.s should as well.

[tstellar]

Add test case.

[tstellar]

The documentation here: https://llvm.org/docs/BitCodeFormat.html#native-object-file-wrapper-format also says that the section is useful for LTO.

As you can see from the linked issue, there are some cases where adding the .llvmbc section causes ld.bfd to fail to link, when it was able to link successfully without the section. I originally tried to fix the failures in the plugin, but after reading the comments on https://github.com/rust-lang/rust/issues/90326 it seemed like it might be better to just ignore these sections if they aren't intended to be use this way.

[tstellar]

@jyknight You commented here that -fembed-bitcode was not intended for LTO, but the llvm documentation says the .llvmbc section is useful for LTO. Do you have any more background on the history of the .llvmbc section and do you think the gold plugin should try to use it for LTO?

[jyknight]

-fembed-bitcode was originally added by Apple to enable them to recompile
3rd-party-developers' software with a newer compiler, or for new CPU
targets, without the source code. It uses the normal non-LTO optimization
pipeline, and emits bitcode corresponding to the binary object also
emitted.

For LTO bitcode generation, we use a different pass manager configuration.

That said, various other people seem to have had various other ideas about
how things should work over time. So, I couldn't say conclusively that
nobody will complain if we have gold not try to parse these sections...but
it seems potentially reasonable to me.

[tstellar]

Add documentation about the change.

[MaskRay]

In D116995#3481661, @jyknight wrote:

-fembed-bitcode was originally added by Apple to enable them to recompile
3rd-party-developers' software with a newer compiler, or for new CPU
targets, without the source code. It uses the normal non-LTO optimization
pipeline, and emits bitcode corresponding to the binary object also
emitted.

For LTO bitcode generation, we use a different pass manager configuration.

That said, various other people seem to have had various other ideas about
how things should work over time. So, I couldn't say conclusively that
nobody will complain if we have gold not try to parse these sections...but
it seems potentially reasonable to me.

I agree. I think for Mach-O the section is for LTO use. For ELF there are two usage:

• lld/LLVMgold.so: the section should be ignored. This patch makes LLVMgold.so behavior match lld, therefore LGTM. The use case is similar to something like https://github.com/travitch/whole-program-llvm ; .llvmbc is to collect bitcode files, not for doing LTO.
• llgo (https://reviews.llvm.org/D4371): it places the bitcode for LTO into a special section .llvmbc.

llvm/docs/ReleaseNotes.rst

170

llvm/tools/gold/gold-plugin.cpp

540

special sections => .llvmbc

Be specific.

541

"not meant for LTO use " when using LLVMgold.so.

For llgo(?) it was for LTO use: https://reviews.llvm.org/D4371

[tstellar]

Update comments and documentation.

[tstellar]

@tejohnson Are you OK with this change?

[tejohnson]

LGTM but should https://llvm.org/docs/BitCodeFormat.html#native-object-file-wrapper-format also be updated?

[tstellar]

Updated documentation in BitCodeFormat.rst.

[tstellar]

@tejohnson Do the updates to the BitCodeFormat document look OK now?

[tejohnson]

In D116995#3600590, @tstellar wrote:

@tejohnson Do the updates to the BitCodeFormat document look OK now?

Yep, thanks!

[MaskRay]

llvm/docs/ReleaseNotes.rst

170

The LLVM gold plugin now ignores bitcode ...

[tstellar]

Updated relase notes.

[MaskRay]

llvm/docs/GoldPlugin.rst

20

[tstellar]

Fix documentation formatting.

[tstellar]

Rebased.

llvm/docs/GoldPlugin.rst

 
 The LLVM gold plugin implements the gold plugin interface on top of
 :ref:`libLTO`.  The same plugin can also be used by other tools such as
 ``ar`` and ``nm``.  Note that ld.bfd from binutils version 2.21.51.0.2
 and above also supports LTO via plugins.  However, usage of the LLVM
 gold plugin with ld.bfd is not tested and therefore not officially
 supported or recommended.
 
+As of LLVM 15, the gold plugin will ignore bitcode from the .llvmbc
+section inside of ELF object files.  However, LTO with bitcode files
+is still supported.
+
 .. _`gold linker`: http://sourceware.org/binutils
 .. _`GCC LTO`: http://gcc.gnu.org/wiki/LinkTimeOptimization
 .. _`gold plugin interface`: http://gcc.gnu.org/wiki/whopr/driver
 
 .. _lto-how-to-build:
 
 How to build it
 ===============

llvm/docs/ReleaseNotes.rst

 Changes to the Debug Info
 ---------------------------------
 
 During this release ...
 
 Changes to the LLVM tools
 ---------------------------------
 
+* The LLVM gold plugin will now ignore bitcode from the ``.llvmbc`` section of ELF
+  files when doing LTO.  https://github.com/llvm/llvm-project/issues/47216
+
 Changes to LLDB
 ---------------------------------
 
 Changes to Sanitizers
 ---------------------
 
 
 Other Changes

llvm/test/tools/gold/X86/Inputs/bcsection-lib.ll

+declare void @elf_func()
+
+define i32 @lib_func() {
+  call void @elf_func()
+  ret i32 0
+}

llvm/test/tools/gold/X86/Inputs/bcsection.s

+.global elf_func
+
+elf_func:
+   ret
+
 .section .llvmbc
 .incbin "bcsection.bc"

llvm/test/tools/gold/X86/bcsection.ll

 ; RUN: rm -rf %t && mkdir -p %t
 ; RUN: llvm-as -o %t/bcsection.bc %s
 
 ; RUN: llvm-mc -I=%t -filetype=obj -triple=x86_64-unknown-unknown -o %t/bcsection.bco %p/Inputs/bcsection.s
-; RUN: llvm-nm --no-llvm-bc %t/bcsection.bco 2>&1 | FileCheck %s -check-prefix=NO-SYMBOLS
-; NO-SYMBOLS: no symbols
+; RUN: llc -filetype=obj -mtriple=x86_64-unknown-unknown -o %t/bcsection-lib.o %p/Inputs/bcsection-lib.ll
 
-; RUN: %gold -r -o %t/bcsection.o -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext %t/bcsection.bco
-; RUN: llvm-nm --no-llvm-bc %t/bcsection.o | FileCheck %s
+; RUN: %gold -shared --no-undefined -o %t/bcsection.so -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold%shlibext %t/bcsection.bco %t/bcsection-lib.o
+
+; This test checks that the gold plugin does not attempt to use the bitcode
+; in the .llvmbc section for LTO.  bcsection-lib.o calls a function that is
+; present the symbol table of bcsection.bco, but not included in the embedded
+; bitcode.  If the linker were to use the bitcode, then the symbols in the
+; symbol table of bcsection.bco will be ignored and the link will fail.
+;
+; bcsection.bco:
+;  .text:
+;    elf_func
+;  .llvmbc:
+;    bitcode_func
+;
+; bcsection-lib.o:
+;   calls elf_func()
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-unknown"
 
 ; CHECK: main
-define i32 @main() {
+define i32 @bitcode_func() {
   ret i32 0
 }

llvm/tools/gold/gold-plugin.cpp

if (get_view) {
     if (std::error_code EC = BufferOrErr.getError()) {
       message(LDPL_ERROR, EC.message().c_str());
       return LDPS_ERR;
     }
     Buffer = std::move(BufferOrErr.get());
     BufferRef = Buffer->getMemBufferRef();
   }
 
+  // Only use bitcode files for LTO.  InputFile::create() will load bitcode
+  // from the .llvmbc section within a binary object, this bitcode is typically
+  // generated by -fembed-bitcode and is not to be used by LLVMgold.so for LTO.
+  if (identify_magic(BufferRef.getBuffer()) != file_magic::bitcode) {
+    *claimed = 0;
+    return LDPS_OK;
+  }
+
   *claimed = 1;
 
   Expected<std::unique_ptr<InputFile>> ObjOrErr = InputFile::create(BufferRef);
   if (!ObjOrErr) {
     handleAllErrors(ObjOrErr.takeError(), [&](const ErrorInfoBase &EI) {
       std::error_code EC = EI.convertToErrorCode();
       if (EC == object::object_error::invalid_file_type ||
           EC == object::object_error::bitcode_section_not_found)