From d053ccb45fafc12a52629f209122c1ce9bbfabed Mon Sep 17 00:00:00 2001 From: klutzy Date: Sat, 10 Jan 2015 22:53:22 +0900 Subject: [PATCH] std::dynamic_lib: Fix Windows error handling This is a [breaking-change] since `std::dynamic_lib::dl` is now private. When `LoadLibraryW()` fails, original code called `errno()` to get error code. However, there was local allocation of `Vec` before `LoadLibraryW()`, and it drops before `errno()`, and the drop (deallocation) changed `errno`! Therefore `dynamic_lib::open()` thought it always succeeded. This commit fixes the issue. This commit also sets Windows error mode during `LoadLibrary()` to prevent "dll load failed" dialog. --- src/libstd/dynamic_lib.rs | 141 +++++++++++++++++++++++++----------- src/libstd/sys/windows/c.rs | 7 ++ 2 files changed, 105 insertions(+), 43 deletions(-) diff --git a/src/libstd/dynamic_lib.rs b/src/libstd/dynamic_lib.rs index 3eeb09b79da..db1239ae5b5 100644 --- a/src/libstd/dynamic_lib.rs +++ b/src/libstd/dynamic_lib.rs @@ -52,21 +52,14 @@ 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 { - unsafe { - let maybe_library = dl::check_for_errors_in(|| { - match filename { - Some(name) => dl::open_external(name.as_vec()), - None => dl::open_internal() - } - }); + let maybe_library = dl::open(filename.map(|path| path.as_vec())); - // The dynamic library must not be constructed if there is - // an error opening the library so the destructor does not - // run. - match maybe_library { - Err(err) => Err(err), - Ok(handle) => Ok(DynamicLibrary { handle: handle }) - } + // The dynamic library must not be constructed if there is + // an error opening the library so the destructor does not + // run. + match maybe_library { + Err(err) => Err(err), + Ok(handle) => Ok(DynamicLibrary { handle: handle }) } } @@ -198,8 +191,7 @@ mod test { target_os = "ios", target_os = "freebsd", target_os = "dragonfly"))] -pub mod dl { - pub use self::Rtld::*; +mod dl { use prelude::v1::*; use ffi::{self, CString}; @@ -207,13 +199,26 @@ pub mod dl { use libc; use ptr; - pub unsafe fn open_external(filename: &[u8]) -> *mut u8 { - let s = CString::from_slice(filename); - dlopen(s.as_ptr(), Lazy as libc::c_int) as *mut u8 + pub fn open(filename: Option<&[u8]>) -> Result<*mut u8, String> { + check_for_errors_in(|| { + unsafe { + match filename { + Some(filename) => open_external(filename), + None => open_internal(), + } + } + }) } - pub unsafe fn open_internal() -> *mut u8 { - dlopen(ptr::null(), Lazy as libc::c_int) as *mut u8 + const LAZY: libc::c_int = 1; + + unsafe fn open_external(filename: &[u8]) -> *mut u8 { + let s = CString::from_slice(filename); + dlopen(s.as_ptr(), LAZY) as *mut u8 + } + + unsafe fn open_internal() -> *mut u8 { + dlopen(ptr::null(), LAZY) as *mut u8 } pub fn check_for_errors_in(f: F) -> Result where @@ -249,14 +254,6 @@ pub mod dl { dlclose(handle as *mut libc::c_void); () } - #[derive(Copy)] - pub enum Rtld { - Lazy = 1, - Now = 2, - Global = 256, - Local = 0, - } - #[link_name = "dl"] extern { fn dlopen(filename: *const libc::c_char, @@ -269,11 +266,13 @@ pub mod dl { } #[cfg(target_os = "windows")] -pub mod dl { +mod dl { use iter::IteratorExt; use libc; + use libc::consts::os::extra::ERROR_CALL_NOT_IMPLEMENTED; use ops::FnOnce; use os; + use option::Option::{self, Some, None}; use ptr; use result::Result; use result::Result::{Ok, Err}; @@ -282,19 +281,75 @@ pub mod dl { use str; use string::String; use vec::Vec; + use sys::c::compat::kernel32::SetThreadErrorMode; - pub unsafe fn open_external(filename: &[u8]) -> *mut u8 { - // Windows expects Unicode data - let filename_str = str::from_utf8(filename).unwrap(); - let mut filename_str: Vec = filename_str.utf16_units().collect(); - filename_str.push(0); - LoadLibraryW(filename_str.as_ptr() as *const libc::c_void) as *mut u8 - } + pub fn open(filename: Option<&[u8]>) -> Result<*mut u8, String> { + // disable "dll load failed" error dialog. + let mut use_thread_mode = true; + let prev_error_mode = unsafe { + // SEM_FAILCRITICALERRORS 0x01 + let new_error_mode = 1; + let mut prev_error_mode = 0; + // Windows >= 7 supports thread error mode. + let result = SetThreadErrorMode(new_error_mode, &mut prev_error_mode); + if result == 0 { + let err = os::errno(); + if err as libc::c_int == ERROR_CALL_NOT_IMPLEMENTED { + use_thread_mode = false; + // SetThreadErrorMode not found. use fallback solution: SetErrorMode() + // Note that SetErrorMode is process-wide so this can cause race condition! + // However, since even Windows APIs do not care of such problem (#20650), + // we just assume SetErrorMode race is not a great deal. + prev_error_mode = SetErrorMode(new_error_mode); + } + } + prev_error_mode + }; - pub unsafe fn open_internal() -> *mut u8 { - let mut handle = ptr::null_mut(); - GetModuleHandleExW(0 as libc::DWORD, ptr::null(), &mut handle); - handle as *mut u8 + unsafe { + SetLastError(0); + } + + let result = match filename { + Some(filename) => { + let filename_str = str::from_utf8(filename).unwrap(); + let mut filename_str: Vec = filename_str.utf16_units().collect(); + filename_str.push(0); + let result = unsafe { + LoadLibraryW(filename_str.as_ptr() as *const libc::c_void) + }; + // beware: Vec/String may change errno during drop! + // so we get error here. + if result == ptr::null_mut() { + let errno = os::errno(); + Err(os::error_string(errno)) + } else { + Ok(result as *mut u8) + } + } + None => { + let mut handle = ptr::null_mut(); + let succeeded = unsafe { + GetModuleHandleExW(0 as libc::DWORD, ptr::null(), &mut handle) + }; + if succeeded == libc::FALSE { + let errno = os::errno(); + Err(os::error_string(errno)) + } else { + Ok(handle as *mut u8) + } + } + }; + + unsafe { + if use_thread_mode { + SetThreadErrorMode(prev_error_mode, ptr::null_mut()); + } else { + SetErrorMode(prev_error_mode); + } + } + + result } pub fn check_for_errors_in(f: F) -> Result where @@ -326,10 +381,10 @@ pub mod dl { fn SetLastError(error: libc::size_t); fn LoadLibraryW(name: *const libc::c_void) -> *mut libc::c_void; fn GetModuleHandleExW(dwFlags: libc::DWORD, name: *const u16, - handle: *mut *mut libc::c_void) - -> *mut libc::c_void; + handle: *mut *mut libc::c_void) -> libc::BOOL; fn GetProcAddress(handle: *mut libc::c_void, name: *const libc::c_char) -> *mut libc::c_void; fn FreeLibrary(handle: *mut libc::c_void); + fn SetErrorMode(uMode: libc::c_uint) -> libc::c_uint; } } diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index 37ed32fa367..da3b7ee2f2f 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -226,6 +226,7 @@ pub mod compat { /// * `CreateSymbolicLinkW`: Windows XP, Windows Server 2003 /// * `GetFinalPathNameByHandleW`: Windows XP, Windows Server 2003 pub mod kernel32 { + use libc::c_uint; use libc::types::os::arch::extra::{DWORD, LPCWSTR, BOOLEAN, HANDLE}; use libc::consts::os::extra::ERROR_CALL_NOT_IMPLEMENTED; @@ -249,6 +250,12 @@ pub mod compat { unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); 0 } } } + + compat_fn! { + kernel32::SetThreadErrorMode(_dwNewMode: DWORD, _lpOldMode: *mut DWORD) -> c_uint { + unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); 0 } + } + } } }