From f25c90a83f661d39d9d55e61eaaf4b1beaabaf20 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 21 Feb 2024 11:17:07 +0000 Subject: [PATCH] Unify dylib loading between proc macros and codegen backends As bonus this makes the errors when failing to load a proc macro more informative to match the backend loading errors. In addition it makes it slightly easier to patch rustc to work on platforms that don't support dynamic linking like wasm. --- Cargo.lock | 1 - compiler/rustc_interface/Cargo.toml | 1 - compiler/rustc_interface/src/lib.rs | 1 - compiler/rustc_interface/src/util.rs | 33 +++++---------- compiler/rustc_metadata/messages.ftl | 2 +- compiler/rustc_metadata/src/creader.rs | 56 +++++++++++++++++++------- compiler/rustc_metadata/src/errors.rs | 1 + compiler/rustc_metadata/src/lib.rs | 2 + compiler/rustc_metadata/src/locator.rs | 8 ++-- 9 files changed, 60 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61f9c130e38d7..4dbe4b1b82285 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4047,7 +4047,6 @@ dependencies = [ name = "rustc_interface" version = "0.0.0" dependencies = [ - "libloading", "rustc-rayon", "rustc-rayon-core", "rustc_ast", diff --git a/compiler/rustc_interface/Cargo.toml b/compiler/rustc_interface/Cargo.toml index a238eacda44ba..0e90836145efa 100644 --- a/compiler/rustc_interface/Cargo.toml +++ b/compiler/rustc_interface/Cargo.toml @@ -5,7 +5,6 @@ edition = "2021" [dependencies] # tidy-alphabetical-start -libloading = "0.8.0" rustc-rayon = { version = "0.5.0", optional = true } rustc-rayon-core = { version = "0.5.0", optional = true } rustc_ast = { path = "../rustc_ast" } diff --git a/compiler/rustc_interface/src/lib.rs b/compiler/rustc_interface/src/lib.rs index 24c2e29053488..d0ce23dacb5ef 100644 --- a/compiler/rustc_interface/src/lib.rs +++ b/compiler/rustc_interface/src/lib.rs @@ -1,5 +1,4 @@ #![feature(decl_macro)] -#![feature(error_iter)] #![feature(generic_nonzero)] #![feature(lazy_cell)] #![feature(let_chains)] diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 087c43075f175..823614e1f0619 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -1,10 +1,10 @@ use crate::errors; use info; -use libloading::Library; use rustc_ast as ast; use rustc_codegen_ssa::traits::CodegenBackend; #[cfg(parallel_compiler)] use rustc_data_structures::sync; +use rustc_metadata::{load_symbol_from_dylib, DylibError}; use rustc_parse::validate_attr; use rustc_session as session; use rustc_session::config::{self, Cfg, CrateType, OutFileName, OutputFilenames, OutputTypes}; @@ -17,7 +17,6 @@ use rustc_span::symbol::{sym, Symbol}; use session::EarlyDiagCtxt; use std::env; use std::env::consts::{DLL_PREFIX, DLL_SUFFIX}; -use std::mem; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::OnceLock; @@ -162,29 +161,19 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( } fn load_backend_from_dylib(early_dcx: &EarlyDiagCtxt, path: &Path) -> MakeBackendFn { - fn format_err(e: &(dyn std::error::Error + 'static)) -> String { - e.sources().map(|e| format!(": {e}")).collect() - } - let lib = unsafe { Library::new(path) }.unwrap_or_else(|err| { - let err = format!("couldn't load codegen backend {path:?}{}", format_err(&err)); - early_dcx.early_fatal(err); - }); - - let backend_sym = unsafe { lib.get::(b"__rustc_codegen_backend") } - .unwrap_or_else(|e| { + match unsafe { load_symbol_from_dylib::(path, "__rustc_codegen_backend") } { + Ok(backend_sym) => backend_sym, + Err(DylibError::DlOpen(path, err)) => { + let err = format!("couldn't load codegen backend {path}{err}"); + early_dcx.early_fatal(err); + } + Err(DylibError::DlSym(_path, err)) => { let e = format!( - "`__rustc_codegen_backend` symbol lookup in the codegen backend failed{}", - format_err(&e) + "`__rustc_codegen_backend` symbol lookup in the codegen backend failed{err}", ); early_dcx.early_fatal(e); - }); - - // Intentionally leak the dynamic library. We can't ever unload it - // since the library can make things that will live arbitrarily long. - let backend_sym = unsafe { backend_sym.into_raw() }; - mem::forget(lib); - - *backend_sym + } + } } /// Get the codegen backend based on the name and specified sysroot. diff --git a/compiler/rustc_metadata/messages.ftl b/compiler/rustc_metadata/messages.ftl index 8da6f0007f03a..a1c6fba4d435e 100644 --- a/compiler/rustc_metadata/messages.ftl +++ b/compiler/rustc_metadata/messages.ftl @@ -45,7 +45,7 @@ metadata_crate_not_panic_runtime = the crate `{$crate_name}` is not a panic runtime metadata_dl_error = - {$err} + {$path}{$err} metadata_empty_link_name = link name must not be empty diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index f6d3dba247090..8fea1a722239b 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -692,20 +692,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { path: &Path, stable_crate_id: StableCrateId, ) -> Result<&'static [ProcMacro], CrateError> { - // Make sure the path contains a / or the linker will search for it. - let path = try_canonicalize(path).unwrap(); - let lib = load_dylib(&path, 5).map_err(|err| CrateError::DlOpen(err))?; - let sym_name = self.sess.generate_proc_macro_decls_symbol(stable_crate_id); - let sym = unsafe { lib.get::<*const &[ProcMacro]>(sym_name.as_bytes()) } - .map_err(|err| CrateError::DlSym(err.to_string()))?; - - // Intentionally leak the dynamic library. We can't ever unload it - // since the library can make things that will live arbitrarily long. - let sym = unsafe { sym.into_raw() }; - std::mem::forget(lib); - - Ok(unsafe { **sym }) + Ok(unsafe { *load_symbol_from_dylib::<*const &[ProcMacro]>(path, &sym_name)? }) } fn inject_panic_runtime(&mut self, krate: &ast::Crate) { @@ -1116,6 +1104,10 @@ fn alloc_error_handler_spans(krate: &ast::Crate) -> Vec { f.spans } +fn format_dlopen_err(e: &(dyn std::error::Error + 'static)) -> String { + e.sources().map(|e| format!(": {e}")).collect() +} + // On Windows the compiler would sometimes intermittently fail to open the // proc-macro DLL with `Error::LoadLibraryExW`. It is suspected that something in the // system still holds a lock on the file, so we retry a few times before calling it @@ -1154,9 +1146,43 @@ fn load_dylib(path: &Path, max_attempts: usize) -> Result for CrateError { + fn from(err: DylibError) -> CrateError { + match err { + DylibError::DlOpen(path, err) => CrateError::DlOpen(path, err), + DylibError::DlSym(path, err) => CrateError::DlSym(path, err), + } + } +} + +pub unsafe fn load_symbol_from_dylib( + path: &Path, + sym_name: &str, +) -> Result { + // Make sure the path contains a / or the linker will search for it. + let path = try_canonicalize(path).unwrap(); + let lib = + load_dylib(&path, 5).map_err(|err| DylibError::DlOpen(path.display().to_string(), err))?; + + let sym = unsafe { lib.get::(sym_name.as_bytes()) } + .map_err(|err| DylibError::DlSym(path.display().to_string(), format_dlopen_err(&err)))?; + + // Intentionally leak the dynamic library. We can't ever unload it + // since the library can make things that will live arbitrarily long. + let sym = unsafe { sym.into_raw() }; + std::mem::forget(lib); + + Ok(*sym) +} diff --git a/compiler/rustc_metadata/src/errors.rs b/compiler/rustc_metadata/src/errors.rs index 7e0a4fb72d45c..9a05d9ac0def1 100644 --- a/compiler/rustc_metadata/src/errors.rs +++ b/compiler/rustc_metadata/src/errors.rs @@ -535,6 +535,7 @@ pub struct StableCrateIdCollision { pub struct DlError { #[primary_span] pub span: Span, + pub path: String, pub err: String, } diff --git a/compiler/rustc_metadata/src/lib.rs b/compiler/rustc_metadata/src/lib.rs index 70ad859895724..f133a2f5f73b2 100644 --- a/compiler/rustc_metadata/src/lib.rs +++ b/compiler/rustc_metadata/src/lib.rs @@ -3,6 +3,7 @@ #![feature(rustdoc_internals)] #![allow(internal_features)] #![feature(decl_macro)] +#![feature(error_iter)] #![feature(extract_if)] #![feature(coroutines)] #![feature(generic_nonzero)] @@ -39,6 +40,7 @@ pub mod errors; pub mod fs; pub mod locator; +pub use creader::{load_symbol_from_dylib, DylibError}; pub use fs::{emit_wrapper_file, METADATA_FILENAME}; pub use native_libs::find_native_static_library; pub use rmeta::{encode_metadata, rendered_const, EncodedMetadata, METADATA_HEADER}; diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index 1ab965e28769f..15f56db6278b1 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -921,8 +921,8 @@ pub(crate) enum CrateError { MultipleCandidates(Symbol, CrateFlavor, Vec), SymbolConflictsCurrent(Symbol), StableCrateIdCollision(Symbol, Symbol), - DlOpen(String), - DlSym(String), + DlOpen(String, String), + DlSym(String, String), LocatorCombined(Box), NotFound(Symbol), } @@ -967,8 +967,8 @@ impl CrateError { CrateError::StableCrateIdCollision(crate_name0, crate_name1) => { dcx.emit_err(errors::StableCrateIdCollision { span, crate_name0, crate_name1 }); } - CrateError::DlOpen(s) | CrateError::DlSym(s) => { - dcx.emit_err(errors::DlError { span, err: s }); + CrateError::DlOpen(path, err) | CrateError::DlSym(path, err) => { + dcx.emit_err(errors::DlError { span, path, err }); } CrateError::LocatorCombined(locator) => { let crate_name = locator.crate_name;