Skip to content

Commit

Permalink
Auto merge of rust-lang#113923 - DianQK:restore-no-builtins-lto, r=pn…
Browse files Browse the repository at this point in the history
…kfelix

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`
  • Loading branch information
bors committed Oct 12, 2023
2 parents 19149d1 + 4df4d43 commit 4a10160
Show file tree
Hide file tree
Showing 15 changed files with 154 additions and 79 deletions.
25 changes: 24 additions & 1 deletion compiler/rustc_codegen_llvm/src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,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,
Expand All @@ -59,8 +77,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
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,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()),
Expand Down Expand Up @@ -678,15 +677,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)
}

Expand Down Expand Up @@ -789,7 +787,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,
Expand Down Expand Up @@ -824,7 +822,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,
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2139,13 +2139,8 @@ extern "C" {
ArgsCstrBuff: *const c_char,
ArgsCstrBuffLen: usize,
) -> *mut TargetMachine;

pub fn LLVMRustDisposeTargetMachine(T: *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>,
Expand All @@ -2167,7 +2162,6 @@ extern "C" {
UnrollLoops: bool,
SLPVectorize: bool,
LoopVectorize: bool,
DisableSimplifyLibCalls: bool,
EmitLifetimeMarkers: bool,
SanitizerOptions: Option<&SanitizerOptions>,
PGOGenPath: *const c_char,
Expand All @@ -2193,6 +2187,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>;
Expand Down
36 changes: 4 additions & 32 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,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));
Expand Down Expand Up @@ -1250,24 +1249,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(
Expand Down Expand Up @@ -2733,10 +2714,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.
Expand All @@ -2762,9 +2739,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;
}
Expand All @@ -2778,8 +2753,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(
Expand All @@ -2796,9 +2769,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;
}

Expand Down
16 changes: 1 addition & 15 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1040,9 +1029,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()));
}));

Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,9 +885,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
Expand Down
23 changes: 16 additions & 7 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,12 +535,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));
}

Expand Down Expand Up @@ -707,7 +704,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,
Expand Down Expand Up @@ -813,8 +810,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);
Expand Down Expand Up @@ -1125,6 +1120,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) {
Expand Down
18 changes: 12 additions & 6 deletions tests/run-make/no-builtins-lto/Makefile
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions tests/run-make/no-builtins-lto/filecheck.lto.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
CHECK: define{{.*}} void @bar
CHECK-NEXT: call void @no_builtins
CHECK-NEXT: call void @llvm.memcpy

CHECK: define{{.*}} i32 @main
CHECK: call void @bar

CHECK: define{{.*}} void @foo
CHECK-NEXT: call void @llvm.memcpy

CHECK: define{{.*}} void @no_builtins
CHECK-SAME: #[[ATTR:[0-9]+]] {
CHECK: call void @foo
CHECK-NEXT: call{{.*}} @memcpy

CHECK: attributes #[[ATTR]]
CHECK-SAME: no-builtins
33 changes: 33 additions & 0 deletions tests/run-make/no-builtins-lto/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#![feature(lang_items, no_core)]
#![no_std]
#![no_core]
#![crate_type = "lib"]

#[inline(never)]
#[no_mangle]
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);
}
27 changes: 26 additions & 1 deletion tests/run-make/no-builtins-lto/main.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Loading

0 comments on commit 4a10160

Please sign in to comment.