Auto merge of rust-lang#113923 - DianQK:restore-no-builtins-lto, r=<try>

Restore `#![no_builtins]` crates participation in LTO. After rust-lang#113716, we can make `#![no_builtins]` crates participate in LTO again. `#![no_builtins]` with LTO does not result in undefined references to the error. I believe this type of issue won't happen again. ＼（＾▽＾）／ I will test the following issues later to verify. The task format is `Fixes {issue} {nightly-2023-07-20 result} {PR rust-lang#113923 result}`. - [x] Fixes rust-lang#72140. ❌ ✅ - [x] Fixes rust-lang#112245. ❌ ✅ - [x] Fixes rust-lang#110606. ❌ ✅ - [ ] Fixes rust-lang#105734. - [ ] Fixes rust-lang#96486. - [ ] Fixes rust-lang#108853. - [x] Fixes rust-lang/compiler-builtins#347. ❌ ✅ - [ ] Fixes rust-lang#108893. - [ ] Fixes rust-lang#78744. Fixes rust-lang#91158. Fixes rust-lang/cargo#10118. The `nightly-2023-07-20` version does not always reproduce problems due to changes in compiler-builtins, core, and user code. That's why this issue recurs and disappears. Some issues were not tested due to the difficulty of reproducing them. r? pnkfelix cc `@bjorn3` `@japaric` `@alexcrichton` `@Amanieu`
rust-lang-ci · Sep 14, 2023 · 807017a · 807017a
2 parents df63c5f + f7751fa
commit 807017a
Show file tree

Hide file tree

Showing 15 changed files with 152 additions and 78 deletions.
diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs
@@ -41,6 +41,24 @@ pub fn crate_type_allows_lto(crate_type: CrateType) -> bool {
     }
 }
 
+fn get_llvm_preserved_symbols() -> Vec<String> {
+    let mut len = 0;
+    unsafe {
+        let symbols = llvm::LLVMRustPreservedSymbols(&mut len);
+        let symbols: &[*const _] = slice::from_raw_parts(symbols, len);
+        symbols
+            .iter()
+            .filter_map(|&symbol| {
+                if symbol.is_null() {
+                    None
+                } else {
+                    Some(String::from_utf8(CStr::from_ptr(symbol).to_bytes().to_vec()).unwrap())
+                }
+            })
+            .collect()
+    }
+}
+
 fn prepare_lto(
     cgcx: &CodegenContext<LlvmCodegenBackend>,
     diag_handler: &Handler,
@@ -55,8 +73,13 @@ fn prepare_lto(
         Lto::No => panic!("didn't request LTO but we're doing LTO"),
     };
 
+    let llvm_reserved_symbols = get_llvm_preserved_symbols();
+
     let symbol_filter = &|&(ref name, info): &(String, SymbolExportInfo)| {
-        if info.level.is_below_threshold(export_threshold) || info.used {
+        if info.level.is_below_threshold(export_threshold)
+            || info.used
+            || llvm_reserved_symbols.contains(name)
+        {
             Some(CString::new(name.as_str()).unwrap())
         } else {
             None

diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs
@@ -558,7 +558,6 @@ pub(crate) unsafe fn llvm_optimize(
         unroll_loops,
         config.vectorize_slp,
         config.vectorize_loop,
-        config.no_builtins,
         config.emit_lifetime_markers,
         sanitizer_options.as_ref(),
         pgo_gen_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
@@ -673,15 +672,14 @@ pub(crate) unsafe fn codegen(
         unsafe fn with_codegen<'ll, F, R>(
             tm: &'ll llvm::TargetMachine,
             llmod: &'ll llvm::Module,
-            no_builtins: bool,
             f: F,
         ) -> R
         where
             F: FnOnce(&'ll mut PassManager<'ll>) -> R,
         {
             let cpm = llvm::LLVMCreatePassManager();
             llvm::LLVMAddAnalysisPasses(tm, cpm);
-            llvm::LLVMRustAddLibraryInfo(cpm, llmod, no_builtins);
+            llvm::LLVMRustAddLibraryInfo(cpm, llmod);
             f(cpm)
         }
 
@@ -784,7 +782,7 @@ pub(crate) unsafe fn codegen(
             } else {
                 llmod
             };
-            with_codegen(tm, llmod, config.no_builtins, |cpm| {
+            with_codegen(tm, llmod, |cpm| {
                 write_output_file(
                     diag_handler,
                     tm,
@@ -819,7 +817,7 @@ pub(crate) unsafe fn codegen(
                     (_, SplitDwarfKind::Split) => Some(dwo_out.as_path()),
                 };
 
-                with_codegen(tm, llmod, config.no_builtins, |cpm| {
+                with_codegen(tm, llmod, |cpm| {
                     write_output_file(
                         diag_handler,
                         tm,

diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
@@ -2138,11 +2138,7 @@ extern "C" {
     ) -> Option<&'static mut TargetMachine>;
 
     pub fn LLVMRustDisposeTargetMachine(T: &'static mut TargetMachine);
-    pub fn LLVMRustAddLibraryInfo<'a>(
-        PM: &PassManager<'a>,
-        M: &'a Module,
-        DisableSimplifyLibCalls: bool,
-    );
+    pub fn LLVMRustAddLibraryInfo<'a>(PM: &PassManager<'a>, M: &'a Module);
     pub fn LLVMRustWriteOutputFile<'a>(
         T: &'a TargetMachine,
         PM: &PassManager<'a>,
@@ -2164,7 +2160,6 @@ extern "C" {
         UnrollLoops: bool,
         SLPVectorize: bool,
         LoopVectorize: bool,
-        DisableSimplifyLibCalls: bool,
         EmitLifetimeMarkers: bool,
         SanitizerOptions: Option<&SanitizerOptions>,
         PGOGenPath: *const c_char,
@@ -2190,6 +2185,7 @@ extern "C" {
     pub fn LLVMRustSetLLVMOptions(Argc: c_int, Argv: *const *const c_char);
     pub fn LLVMRustPrintPasses();
     pub fn LLVMRustSetNormalizedTarget(M: &Module, triple: *const c_char);
+    pub fn LLVMRustPreservedSymbols(len: *mut usize) -> *const *const c_char;
     pub fn LLVMRustRunRestrictionPass(M: &Module, syms: *const *const c_char, len: size_t);
 
     pub fn LLVMRustOpenArchive(path: *const c_char) -> Option<&'static mut Archive>;

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -516,8 +516,7 @@ fn link_staticlib<'a>(
         &codegen_results.crate_info,
         Some(CrateType::Staticlib),
         &mut |cnum, path| {
-            let lto = are_upstream_rust_objects_already_included(sess)
-                && !ignored_for_lto(sess, &codegen_results.crate_info, cnum);
+            let lto = are_upstream_rust_objects_already_included(sess);
 
             let native_libs = codegen_results.crate_info.native_libraries[&cnum].iter();
             let relevant = native_libs.clone().filter(|lib| relevant_lib(sess, &lib));
@@ -1256,24 +1255,6 @@ fn link_sanitizer_runtime(sess: &Session, linker: &mut dyn Linker, name: &str) {
     }
 }
 
-/// Returns a boolean indicating whether the specified crate should be ignored
-/// during LTO.
-///
-/// Crates ignored during LTO are not lumped together in the "massive object
-/// file" that we create and are linked in their normal rlib states. See
-/// comments below for what crates do not participate in LTO.
-///
-/// It's unusual for a crate to not participate in LTO. Typically only
-/// compiler-specific and unstable crates have a reason to not participate in
-/// LTO.
-pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool {
-    // If our target enables builtin function lowering in LLVM then the
-    // crates providing these functions don't participate in LTO (e.g.
-    // no_builtins or compiler builtins crates).
-    !sess.target.no_builtins
-        && (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum))
-}
-
 /// This functions tries to determine the appropriate linker (and corresponding LinkerFlavor) to use
 pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) {
     fn infer_from(
@@ -2739,10 +2720,6 @@ fn rehome_sysroot_lib_dir<'a>(sess: &'a Session, lib_dir: &Path) -> PathBuf {
 // symbols). We must continue to include the rest of the rlib, however, as
 // it may contain static native libraries which must be linked in.
 //
-// (*) Crates marked with `#![no_builtins]` don't participate in LTO and
-// their bytecode wasn't included. The object files in those libraries must
-// still be passed to the linker.
-//
 // Note, however, that if we're not doing LTO we can just pass the rlib
 // blindly to the linker (fast) because it's fine if it's not actually
 // included as we're at the end of the dependency chain.
@@ -2768,9 +2745,7 @@ fn add_static_crate<'a>(
         cmd.link_rlib(&rlib_path);
     };
 
-    if !are_upstream_rust_objects_already_included(sess)
-        || ignored_for_lto(sess, &codegen_results.crate_info, cnum)
-    {
+    if !are_upstream_rust_objects_already_included(sess) {
         link_upstream(cratepath);
         return;
     }
@@ -2784,8 +2759,6 @@ fn add_static_crate<'a>(
         let canonical_name = name.replace('-', "_");
         let upstream_rust_objects_already_included =
             are_upstream_rust_objects_already_included(sess);
-        let is_builtins =
-            sess.target.no_builtins || !codegen_results.crate_info.is_no_builtins.contains(&cnum);
 
         let mut archive = archive_builder_builder.new_archive_builder(sess);
         if let Err(error) = archive.add_archive(
@@ -2802,9 +2775,8 @@ fn add_static_crate<'a>(
 
                 // If we're performing LTO and this is a rust-generated object
                 // file, then we don't need the object file as it's part of the
-                // LTO module. Note that `#![no_builtins]` is excluded from LTO,
-                // though, so we let that object file slide.
-                if upstream_rust_objects_already_included && is_rust_object && is_builtins {
+                // LTO module.
+                if upstream_rust_objects_already_included && is_rust_object {
                     return true;
                 }
 

diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs
@@ -149,23 +149,12 @@ impl ModuleConfig {
 
         let emit_obj = if !should_emit_obj {
             EmitObj::None
-        } else if sess.target.obj_is_bitcode
-            || (sess.opts.cg.linker_plugin_lto.enabled() && !no_builtins)
-        {
+        } else if sess.target.obj_is_bitcode || sess.opts.cg.linker_plugin_lto.enabled() {
             // This case is selected if the target uses objects as bitcode, or
             // if linker plugin LTO is enabled. In the linker plugin LTO case
             // the assumption is that the final link-step will read the bitcode
             // and convert it to object code. This may be done by either the
             // native linker or rustc itself.
-            //
-            // Note, however, that the linker-plugin-lto requested here is
-            // explicitly ignored for `#![no_builtins]` crates. These crates are
-            // specifically ignored by rustc's LTO passes and wouldn't work if
-            // loaded into the linker. These crates define symbols that LLVM
-            // lowers intrinsics to, and these symbol dependencies aren't known
-            // until after codegen. As a result any crate marked
-            // `#![no_builtins]` is assumed to not participate in LTO and
-            // instead goes on to generate object code.
             EmitObj::Bitcode
         } else if need_bitcode_in_object(tcx) {
             EmitObj::ObjectCode(BitcodeSection::Full)
@@ -1033,9 +1022,6 @@ fn start_executing_work<B: ExtraBackendMethods>(
 
     let mut each_linked_rlib_for_lto = Vec::new();
     drop(link::each_linked_rlib(crate_info, None, &mut |cnum, path| {
-        if link::ignored_for_lto(sess, crate_info, cnum) {
-            return;
-        }
         each_linked_rlib_for_lto.push((cnum, path.to_path_buf()));
     }));
 

diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs
@@ -870,9 +870,7 @@ impl CrateInfo {
         // If global LTO is enabled then almost everything (*) is glued into a single object file,
         // so this logic is not necessary and can cause issues on some targets (due to weak lang
         // item symbols being "privatized" to that object file), so we disable it.
-        // (*) Native libs, and `#[compiler_builtins]` and `#[no_builtins]` crates are not glued,
-        // and we assume that they cannot define weak lang items. This is not currently enforced
-        // by the compiler, but that's ok because all this stuff is unstable anyway.
+        // (*) Native libs are not glued, and we assume that they cannot define weak lang items.
         let target = &tcx.sess.target;
         if !are_upstream_rust_objects_already_included(tcx.sess) {
             let missing_weak_lang_items: FxHashSet<Symbol> = info

diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
@@ -525,12 +525,9 @@ extern "C" void LLVMRustDisposeTargetMachine(LLVMTargetMachineRef TM) {
 
 // Unfortunately, the LLVM C API doesn't provide a way to create the
 // TargetLibraryInfo pass, so we use this method to do so.
-extern "C" void LLVMRustAddLibraryInfo(LLVMPassManagerRef PMR, LLVMModuleRef M,
-                                       bool DisableSimplifyLibCalls) {
+extern "C" void LLVMRustAddLibraryInfo(LLVMPassManagerRef PMR, LLVMModuleRef M) {
   Triple TargetTriple(unwrap(M)->getTargetTriple());
   TargetLibraryInfoImpl TLII(TargetTriple);
-  if (DisableSimplifyLibCalls)
-    TLII.disableAllFunctions();
   unwrap(PMR)->add(new TargetLibraryInfoWrapperPass(TLII));
 }
 
@@ -689,7 +686,7 @@ LLVMRustOptimize(
     bool IsLinkerPluginLTO,
     bool NoPrepopulatePasses, bool VerifyIR, bool UseThinLTOBuffers,
     bool MergeFunctions, bool UnrollLoops, bool SLPVectorize, bool LoopVectorize,
-    bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
+    bool EmitLifetimeMarkers,
     LLVMRustSanitizerOptions *SanitizerOptions,
     const char *PGOGenPath, const char *PGOUsePath,
     bool InstrumentCoverage, const char *InstrProfileOutput,
@@ -781,8 +778,6 @@ LLVMRustOptimize(
 
   Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr<TargetLibraryInfoImpl> TLII(new TargetLibraryInfoImpl(TargetTriple));
-  if (DisableSimplifyLibCalls)
-    TLII->disableAllFunctions();
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
 
   PB.registerModuleAnalyses(MAM);
@@ -1107,6 +1102,20 @@ extern "C" void LLVMRustPrintPasses() {
   PB.printPassNames(outs());
 }
 
+// from https://github.com/llvm/llvm-project/blob/7021182d6b43de9488ab70de626192ce70b3a4a6/llvm/lib/Object/IRSymtab.cpp#L48-L57
+static const char *PreservedSymbols[] = {
+#define HANDLE_LIBCALL(code, name) name,
+#include "llvm/IR/RuntimeLibcalls.def"
+#undef HANDLE_LIBCALL
+    "__ssp_canary_word",
+    "__stack_chk_guard",
+};
+
+extern "C" const char **LLVMRustPreservedSymbols(size_t *len) {
+  *len = sizeof(PreservedSymbols) / sizeof(PreservedSymbols[0]);
+  return PreservedSymbols;
+}
+
 extern "C" void LLVMRustRunRestrictionPass(LLVMModuleRef M, char **Symbols,
                                            size_t Len) {
   auto PreserveFunctions = [=](const GlobalValue &GV) {

diff --git a/tests/run-make/no-builtins-lto/Makefile b/tests/run-make/no-builtins-lto/Makefile
@@ -1,9 +1,15 @@
 include ../tools.mk
 
+# only-x86_64
+
+# We want to check that `no_builtins` is correctly participating in LTO.
+# First, verify that the `foo::foo` symbol can be found when linking.
+# Next, verify that `memcpy` can be customized using `no_builtins` under LTO.
+# Others will use the built-in memcpy.
+
 all:
-	# Compile a `#![no_builtins]` rlib crate
-	$(RUSTC) no_builtins.rs
-	# Build an executable that depends on that crate using LTO. The no_builtins crate doesn't
-	# participate in LTO, so its rlib must be explicitly linked into the final binary. Verify this by
-	# grepping the linker arguments.
-	$(RUSTC) main.rs -C lto --print link-args | $(CGREP) 'libno_builtins.rlib'
+	$(RUSTC) -C linker-plugin-lto -C opt-level=2 -C debuginfo=0 foo.rs
+	$(RUSTC) -C linker-plugin-lto -C opt-level=2 -C debuginfo=0 no_builtins.rs
+	$(RUSTC) main.rs -C lto -C opt-level=2 -C debuginfo=0 -C save-temps -C metadata=1 -C codegen-units=1
+	$(LLVM_BIN_DIR)/llvm-dis $(TMPDIR)/main.main.*-cgu.0.rcgu.lto.input.bc -o $(TMPDIR)/lto.ll
+	cat "$(TMPDIR)"/lto.ll | "$(LLVM_FILECHECK)" filecheck.lto.txt
diff --git a/tests/run-make/no-builtins-lto/filecheck.lto.txt b/tests/run-make/no-builtins-lto/filecheck.lto.txt
@@ -0,0 +1,17 @@
+CHECK: define{{.*}} void @bar
+CHECK-NEXT: call void @_ZN11no_builtins11no_builtins17hcda8eeb3c587012fE
+CHECK-NEXT: call void @llvm.memcpy
+
+CHECK: define{{.*}} i32 @main
+CHECK: call void @bar
+
+CHECK: define{{.*}} void @_ZN3foo3foo17hfad400915e51b713E
+CHECK-NEXT: call void @llvm.memcpy
+
+CHECK: define{{.*}} void @_ZN11no_builtins11no_builtins17hcda8eeb3c587012fE
+CHECK-SAME: #[[ATTR:[0-9]+]] {
+CHECK: call void @_ZN3foo3foo17hfad400915e51b713E
+CHECK-NEXT: call{{.*}} @memcpy
+
+CHECK: attributes #[[ATTR]] 
+CHECK-SAME: no-builtins
diff --git a/tests/run-make/no-builtins-lto/foo.rs b/tests/run-make/no-builtins-lto/foo.rs
@@ -0,0 +1,32 @@
+#![feature(lang_items, no_core)]
+#![no_std]
+#![no_core]
+#![crate_type = "lib"]
+
+#[inline(never)]
+pub unsafe fn foo(dest: *mut u8, src: *const u8) {
+    // should call `@llvm.memcpy`.
+    memcpy(dest, src, 1024);
+}
+
+#[no_mangle]
+#[inline(never)]
+pub unsafe extern "C" fn memcpy(dest: *mut u8, src: *const u8, _n: usize) -> *mut u8 {
+    *dest = 0;
+    return src as *mut u8;
+}
+
+#[lang = "sized"]
+trait Sized {}
+#[lang = "copy"]
+trait Copy {}
+impl Copy for *mut u8 {}
+impl Copy for *const u8 {}
+
+#[lang = "drop_in_place"]
+#[allow(unconditional_recursion)]
+pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
+    // Code here does not matter - this is replaced by the
+    // real drop glue by the compiler.
+    drop_in_place(to_drop);
+}
diff --git a/tests/run-make/no-builtins-lto/main.rs b/tests/run-make/no-builtins-lto/main.rs
@@ -1,3 +1,28 @@
+#![feature(no_core, start, lang_items)]
+#![no_std]
+// We use `no_core` to reduce the LTO products is small enough.
+#![no_core]
+
 extern crate no_builtins;
+extern crate foo;
+
+#[link(name = "c")]
+extern "C" {}
+
+#[start]
+fn main(_: isize, p: *const *const u8) -> isize {
+    // Make sure the symbols are retained.
+    unsafe { bar(*p as *mut u8, *p); }
+    0
+}
+
+#[no_mangle]
+#[inline(never)]
+pub unsafe extern "C" fn bar(dest: *mut u8, src: *const u8) {
+    no_builtins::no_builtins(dest, src);
+    // should call `@llvm.memcpy`
+    foo::memcpy(dest, src, 1024);
+}
 
-fn main() {}
+#[lang = "eh_personality"]
+fn eh_personality() {}