Skip to content

Commit

Permalink
Rollup merge of #71537 - Mark-Simulacrum:no-self-open, r=davidtwco
Browse files Browse the repository at this point in the history
Remove support for self-opening

This was only used for linkage test cases, which is already covered by
the [run-make-fulldeps/symbol-visibility test](https://github.com/rust-lang/rust/blob/master/src/test/run-make-fulldeps/symbol-visibility/Makefile) -- which fairly extensively makes
sure we're correctly exporting the right symbols at the right visibility (for
various Rust crate types).

This fixes #10379 and resolves #10356 by removing the test case (and underlying support in the compiler). AFAICT, the better way to test visibility is via nm, like the symbol visibility test. It seems like that's sufficient; I suspect that given that we don't use this we should just drop it (android is tier 2 anyway). But happy to hear otherwise.
  • Loading branch information
Dylan-DPC authored Apr 26, 2020
2 parents 398d3ee + 97983af commit 4199ef1
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 117 deletions.
2 changes: 1 addition & 1 deletion src/librustc_interface/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub fn spawn_thread_pool<F: FnOnce() -> R + Send, R: Send>(
}

fn load_backend_from_dylib(path: &Path) -> fn() -> Box<dyn CodegenBackend> {
let lib = DynamicLibrary::open(Some(path)).unwrap_or_else(|err| {
let lib = DynamicLibrary::open(path).unwrap_or_else(|err| {
let err = format!("couldn't load codegen backend {:?}: {:?}", path, err);
early_error(ErrorOutputType::default(), &err);
});
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ impl<'a> CrateLoader<'a> {

// Make sure the path contains a / or the linker will search for it.
let path = env::current_dir().unwrap().join(path);
let lib = match DynamicLibrary::open(Some(&path)) {
let lib = match DynamicLibrary::open(&path) {
Ok(lib) => lib,
Err(err) => self.sess.span_fatal(span, &err),
};
Expand Down
47 changes: 11 additions & 36 deletions src/librustc_metadata/dynamic_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ impl Drop for DynamicLibrary {
}

impl DynamicLibrary {
/// Lazily open a dynamic library. When passed None it gives a
/// handle to the calling process
pub fn open(filename: Option<&Path>) -> Result<DynamicLibrary, String> {
let maybe_library = dl::open(filename.map(|path| path.as_os_str()));
/// Lazily open a dynamic library.
pub fn open(filename: &Path) -> Result<DynamicLibrary, String> {
let maybe_library = dl::open(filename.as_os_str());

// The dynamic library must not be constructed if there is
// an error opening the library so the destructor does not
Expand Down Expand Up @@ -57,24 +56,13 @@ mod dl {
use std::ptr;
use std::str;

pub(super) fn open(filename: Option<&OsStr>) -> Result<*mut u8, String> {
pub(super) fn open(filename: &OsStr) -> Result<*mut u8, String> {
check_for_errors_in(|| unsafe {
match filename {
Some(filename) => open_external(filename),
None => open_internal(),
}
let s = CString::new(filename.as_bytes()).unwrap();
libc::dlopen(s.as_ptr(), libc::RTLD_LAZY) as *mut u8
})
}

unsafe fn open_external(filename: &OsStr) -> *mut u8 {
let s = CString::new(filename.as_bytes()).unwrap();
libc::dlopen(s.as_ptr(), libc::RTLD_LAZY) as *mut u8
}

unsafe fn open_internal() -> *mut u8 {
libc::dlopen(ptr::null(), libc::RTLD_LAZY) as *mut u8
}

fn check_for_errors_in<T, F>(f: F) -> Result<T, String>
where
F: FnOnce() -> T,
Expand Down Expand Up @@ -124,10 +112,10 @@ mod dl {

use winapi::shared::minwindef::HMODULE;
use winapi::um::errhandlingapi::SetThreadErrorMode;
use winapi::um::libloaderapi::{FreeLibrary, GetModuleHandleExW, GetProcAddress, LoadLibraryW};
use winapi::um::libloaderapi::{FreeLibrary, GetProcAddress, LoadLibraryW};
use winapi::um::winbase::SEM_FAILCRITICALERRORS;

pub(super) fn open(filename: Option<&OsStr>) -> Result<*mut u8, String> {
pub(super) fn open(filename: &OsStr) -> Result<*mut u8, String> {
// disable "dll load failed" error dialog.
let prev_error_mode = unsafe {
let new_error_mode = SEM_FAILCRITICALERRORS;
Expand All @@ -139,22 +127,9 @@ mod dl {
prev_error_mode
};

let result = match filename {
Some(filename) => {
let filename_str: Vec<_> = filename.encode_wide().chain(Some(0)).collect();
let result = unsafe { LoadLibraryW(filename_str.as_ptr()) } as *mut u8;
ptr_result(result)
}
None => {
let mut handle = ptr::null_mut();
let succeeded = unsafe { GetModuleHandleExW(0, ptr::null(), &mut handle) };
if succeeded == 0 {
Err(io::Error::last_os_error().to_string())
} else {
Ok(handle as *mut u8)
}
}
};
let filename_str: Vec<_> = filename.encode_wide().chain(Some(0)).collect();
let result = unsafe { LoadLibraryW(filename_str.as_ptr()) } as *mut u8;
let result = ptr_result(result);

unsafe {
SetThreadErrorMode(prev_error_mode, ptr::null_mut());
Expand Down
30 changes: 1 addition & 29 deletions src/librustc_metadata/dynamic_lib/tests.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,4 @@
use super::*;
use std::mem;

#[test]
fn test_loading_atoi() {
if cfg!(windows) {
return;
}

// The C library does not need to be loaded since it is already linked in
let lib = match DynamicLibrary::open(None) {
Err(error) => panic!("Could not load self as module: {}", error),
Ok(lib) => lib,
};

let atoi: extern "C" fn(*const libc::c_char) -> libc::c_int = unsafe {
match lib.symbol("atoi") {
Err(error) => panic!("Could not load function atoi: {}", error),
Ok(atoi) => mem::transmute::<*mut u8, _>(atoi),
}
};

let argument = CString::new("1383428980").unwrap();
let expected_result = 0x52757374;
let result = atoi(argument.as_ptr());
if result != expected_result {
panic!("atoi({:?}) != {} but equaled {} instead", argument, expected_result, result)
}
}

#[test]
fn test_errors_do_not_crash() {
Expand All @@ -39,7 +11,7 @@ fn test_errors_do_not_crash() {
// Open /dev/null as a library to get an error, and make sure
// that only causes an error, and not a crash.
let path = Path::new("/dev/null");
match DynamicLibrary::open(Some(&path)) {
match DynamicLibrary::open(&path) {
Err(_) => {}
Ok(_) => panic!("Successfully opened the empty library."),
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_plugin_impl/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn dylink_registrar(
// Make sure the path contains a / or the linker will search for it.
let path = env::current_dir().unwrap().join(&path);

let lib = match DynamicLibrary::open(Some(&path)) {
let lib = match DynamicLibrary::open(&path) {
Ok(lib) => lib,
// this is fatal: there are almost certainly macros we need
// inside this crate, so continue would spew "macro undefined"
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-make-fulldeps/extern-fn-reachable/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::path::Path;
pub fn main() {
unsafe {
let path = Path::new("libdylib.so");
let a = DynamicLibrary::open(Some(&path)).unwrap();
let a = DynamicLibrary::open(&path).unwrap();
assert!(a.symbol::<isize>("fun1").is_ok());
assert!(a.symbol::<isize>("fun2").is_ok());
assert!(a.symbol::<isize>("fun3").is_ok());
Expand Down
35 changes: 0 additions & 35 deletions src/test/ui-fulldeps/auxiliary/linkage-visibility.rs

This file was deleted.

13 changes: 0 additions & 13 deletions src/test/ui-fulldeps/linkage-visibility.rs

This file was deleted.

0 comments on commit 4199ef1

Please sign in to comment.