Bug 1767917 - Remove rust compiler-builtins-hack. r=firefox-build-sys…

…tem-reviewers,andi The underlying issue in the LLVM gold plugin now has a proposed fix that we can use to remove the hack. Differential Revision: https://phabricator.services.mozilla.com/D145539
iaxat · May 6, 2022 · c893adc · c893adc
1 parent 75abcd9
commit c893adc
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 79 deletions.
diff --git a/build/build-clang/D116995.diff b/build/build-clang/D116995.diff
@@ -0,0 +1,87 @@
+[gold] Ignore bitcode from sections inside object files
+
+-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
+
+diff --git a/llvm/test/tools/gold/X86/Inputs/bcsection-lib.ll b/llvm/test/tools/gold/X86/Inputs/bcsection-lib.ll
+new file mode 100644
+--- /dev/null
++++ b/llvm/test/tools/gold/X86/Inputs/bcsection-lib.ll
+@@ -0,0 +1,6 @@
++declare void @elf_func()
++
++define i32 @lib_func() {
++  call void @elf_func()
++  ret i32 0
++}
+diff --git a/llvm/test/tools/gold/X86/Inputs/bcsection.s b/llvm/test/tools/gold/X86/Inputs/bcsection.s
+--- a/llvm/test/tools/gold/X86/Inputs/bcsection.s
++++ b/llvm/test/tools/gold/X86/Inputs/bcsection.s
+@@ -1,2 +1,7 @@
++.global elf_func
++
++elf_func:
++   ret
++
+ .section .llvmbc
+ .incbin "bcsection.bc"
+diff --git a/llvm/test/tools/gold/X86/bcsection.ll b/llvm/test/tools/gold/X86/bcsection.ll
+--- a/llvm/test/tools/gold/X86/bcsection.ll
++++ b/llvm/test/tools/gold/X86/bcsection.ll
+@@ -2,16 +2,29 @@
+ ; 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
+ }
+diff --git a/llvm/tools/gold/gold-plugin.cpp b/llvm/tools/gold/gold-plugin.cpp
+--- a/llvm/tools/gold/gold-plugin.cpp
++++ b/llvm/tools/gold/gold-plugin.cpp
+@@ -540,6 +540,14 @@
+     BufferRef = Buffer->getMemBufferRef();
+   }
+
++  // Only use bitcode files for LTO.  InputFile::create() will load bitcode
++  // from special sections within a binary object, this bitcode is typically
++  // generated by -fembed-bitcode and is not meant for LTO use.
++  if (identify_magic(BufferRef.getBuffer()) != file_magic::bitcode) {
++    *claimed = 0;
++    return LDPS_OK;
++  }
++
+   *claimed = 1;
+
+   Expected<std::unique_ptr<InputFile>> ObjOrErr = InputFile::create(BufferRef);
+
diff --git a/build/build-clang/clang-14.json b/build/build-clang/clang-14.json
@@ -9,6 +9,7 @@
       "Remove-FlushViewOfFile-when-unmaping-gcda-files.patch",
       "fuzzing_ccov_build_clang_12.patch",
       "win64-no-symlink.patch",
+      "D116995.diff",
       "revert-llvmorg-14-init-14141-gd6d3000a2f6d.patch",
       "revert-llvmorg-14-init-11890-gf86deb18cab6.patch",
       "llvmorg-15-init-283-g4db89e23190d.patch"

diff --git a/taskcluster/ci/toolchain/rust.yml b/taskcluster/ci/toolchain/rust.yml
@@ -22,12 +22,8 @@ linux64-rust-1.57:
             '--target', 'i686-unknown-linux-gnu',
             '--target', 'aarch64-unknown-linux-gnu',
             '--target', 'wasm32-wasi',
-            '--compiler-builtins-hack',
         ]
         toolchain-alias: linux64-rust-toolchain
-    fetches:
-        toolchain:
-            - linux64-clang-toolchain
 
 linux64-rust-1.59:
     treeherder:
@@ -40,11 +36,7 @@ linux64-rust-1.59:
             '--target', 'i686-unknown-linux-gnu',
             '--target', 'aarch64-unknown-linux-gnu',
             '--target', 'wasm32-wasi',
-            '--compiler-builtins-hack',
         ]
-    fetches:
-        toolchain:
-            - linux64-clang-toolchain
 
 linux64-rust-1.60:
     treeherder:
@@ -57,12 +49,8 @@ linux64-rust-1.60:
             '--target', 'i686-unknown-linux-gnu',
             '--target', 'aarch64-unknown-linux-gnu',
             '--target', 'wasm32-wasi',
-            '--compiler-builtins-hack',
         ]
         toolchain-alias: linux64-rust
-    fetches:
-        toolchain:
-            - linux64-clang-toolchain
 
 # A patched rust toolchain that allows us to use sanitizers in our vendored
 # build environment. See the rust fetch's comments for more details.
@@ -78,7 +66,6 @@ linux64-rust-dev:
             '--channel', 'dev',
             '--host', 'x86_64-unknown-linux-gnu',
             '--target', 'x86_64-unknown-linux-gnu',
-            '--compiler-builtins-hack',
         ]
     fetches:
         fetch:
@@ -168,12 +155,8 @@ linux64-rust-android-1.60:
             '--target', 'aarch64-linux-android',
             '--target', 'i686-linux-android',
             '--target', 'x86_64-linux-android',
-            '--compiler-builtins-hack',
         ]
         toolchain-alias: linux64-rust-android
-    fetches:
-        toolchain:
-            - linux64-clang-toolchain
 
 linux64-rust-windows-1.57:
     description: "rust repack with windows-cross support"

diff --git a/taskcluster/scripts/misc/repack_rust.py b/taskcluster/scripts/misc/repack_rust.py
@@ -13,14 +13,12 @@
 
 import argparse
 import errno
-import glob
 import hashlib
 import os
 import shutil
 import subprocess
 from contextlib import contextmanager
 import tarfile
-import tempfile
 import textwrap
 
 import requests
@@ -459,7 +457,6 @@ def repack(
     targets,
     channel="stable",
     cargo_channel=None,
-    compiler_builtins_hack=False,
     patches=[],
 ):
     install_dir = "rustc"
@@ -509,60 +506,6 @@ def repack(
         for std in stds:
             install(os.path.basename(std["url"]), install_dir)
             pass
-    # Workaround for https://github.com/rust-lang/rust/issues/74657:
-    # Remove the .llvmbc and .llvmcmd sections (sections for the LLVM bitcode)
-    # from the compiler_builtins rlib.
-    hack_targets = ()
-    if compiler_builtins_hack:
-        hack_targets = (
-            "x86_64-unknown-linux-gnu",
-            "i686-unknown-linux-gnu",
-            "x86_64-linux-android",
-            "i686-linux-android",
-            "thumbv7neon-linux-androideabi",
-            "aarch64-linux-android",
-        )
-        llvm_bin = os.path.join(os.environ["MOZ_FETCHES_DIR"], "clang", "bin")
-    for t in hack_targets:
-        if t not in targets:
-            continue
-        for lib in glob.glob(
-            os.path.join(
-                install_dir, "lib", "rustlib", t, "lib", "libcompiler_builtins*"
-            )
-        ):
-            log("Postprocessing %s" % lib)
-            with tempfile.TemporaryDirectory() as d:
-                # Extract all the files from the .rlib
-                subprocess.check_call(
-                    [os.path.join(llvm_bin, "llvm-ar"), "x", os.path.abspath(lib)],
-                    cwd=d,
-                )
-                files = os.listdir(d)
-                for f in files:
-                    if not f.endswith(".o"):
-                        continue
-                    # For each .o file, remove the aforementioned sections.
-                    subprocess.check_call(
-                        [
-                            os.path.join(llvm_bin, "llvm-objcopy"),
-                            "-R",
-                            ".llvmbc",
-                            "-R",
-                            ".llvmcmd",
-                            f,
-                        ],
-                        cwd=d,
-                    )
-                # Create a new .rlib with the updated object files.
-                subprocess.check_call(
-                    [os.path.join(llvm_bin, "llvm-ar"), "crv", os.path.abspath(lib)]
-                    + files,
-                    cwd=d,
-                )
-                subprocess.check_call(
-                    [os.path.join(llvm_bin, "llvm-ranlib"), os.path.abspath(lib)], cwd=d
-                )
 
     log("Creating archive...")
     tar_file = install_dir + ".tar.zst"
@@ -690,11 +633,6 @@ def args():
         help="Release channel version to use for cargo."
         " Defaults to the same as --channel.",
     )
-    parser.add_argument(
-        "--compiler-builtins-hack",
-        action="store_true",
-        help="Enable workaround for " "https://github.com/rust-lang/rust/issues/74657.",
-    )
     parser.add_argument(
         "--host",
         help="Host platform for the toolchain executable:"