Skip to content

Commit bf9b2c3

Browse files
committed
Auto merge of #20874 - klutzy:windows-dynamic-lib, r=alexcrichton
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.
2 parents bd8a43c + d053ccb commit bf9b2c3

File tree

2 files changed

+105
-43
lines changed

2 files changed

+105
-43
lines changed

src/libstd/dynamic_lib.rs

Lines changed: 98 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -52,21 +52,14 @@ impl DynamicLibrary {
5252
/// Lazily open a dynamic library. When passed None it gives a
5353
/// handle to the calling process
5454
pub fn open(filename: Option<&Path>) -> Result<DynamicLibrary, String> {
55-
unsafe {
56-
let maybe_library = dl::check_for_errors_in(|| {
57-
match filename {
58-
Some(name) => dl::open_external(name.as_vec()),
59-
None => dl::open_internal()
60-
}
61-
});
62-
63-
// The dynamic library must not be constructed if there is
64-
// an error opening the library so the destructor does not
65-
// run.
66-
match maybe_library {
67-
Err(err) => Err(err),
68-
Ok(handle) => Ok(DynamicLibrary { handle: handle })
69-
}
55+
let maybe_library = dl::open(filename.map(|path| path.as_vec()));
56+
57+
// The dynamic library must not be constructed if there is
58+
// an error opening the library so the destructor does not
59+
// run.
60+
match maybe_library {
61+
Err(err) => Err(err),
62+
Ok(handle) => Ok(DynamicLibrary { handle: handle })
7063
}
7164
}
7265

@@ -198,22 +191,34 @@ mod test {
198191
target_os = "ios",
199192
target_os = "freebsd",
200193
target_os = "dragonfly"))]
201-
pub mod dl {
202-
pub use self::Rtld::*;
194+
mod dl {
203195
use prelude::v1::*;
204196

205197
use ffi::{self, CString};
206198
use str;
207199
use libc;
208200
use ptr;
209201

210-
pub unsafe fn open_external(filename: &[u8]) -> *mut u8 {
202+
pub fn open(filename: Option<&[u8]>) -> Result<*mut u8, String> {
203+
check_for_errors_in(|| {
204+
unsafe {
205+
match filename {
206+
Some(filename) => open_external(filename),
207+
None => open_internal(),
208+
}
209+
}
210+
})
211+
}
212+
213+
const LAZY: libc::c_int = 1;
214+
215+
unsafe fn open_external(filename: &[u8]) -> *mut u8 {
211216
let s = CString::from_slice(filename);
212-
dlopen(s.as_ptr(), Lazy as libc::c_int) as *mut u8
217+
dlopen(s.as_ptr(), LAZY) as *mut u8
213218
}
214219

215-
pub unsafe fn open_internal() -> *mut u8 {
216-
dlopen(ptr::null(), Lazy as libc::c_int) as *mut u8
220+
unsafe fn open_internal() -> *mut u8 {
221+
dlopen(ptr::null(), LAZY) as *mut u8
217222
}
218223

219224
pub fn check_for_errors_in<T, F>(f: F) -> Result<T, String> where
@@ -249,14 +254,6 @@ pub mod dl {
249254
dlclose(handle as *mut libc::c_void); ()
250255
}
251256

252-
#[derive(Copy)]
253-
pub enum Rtld {
254-
Lazy = 1,
255-
Now = 2,
256-
Global = 256,
257-
Local = 0,
258-
}
259-
260257
#[link_name = "dl"]
261258
extern {
262259
fn dlopen(filename: *const libc::c_char,
@@ -269,11 +266,13 @@ pub mod dl {
269266
}
270267

271268
#[cfg(target_os = "windows")]
272-
pub mod dl {
269+
mod dl {
273270
use iter::IteratorExt;
274271
use libc;
272+
use libc::consts::os::extra::ERROR_CALL_NOT_IMPLEMENTED;
275273
use ops::FnOnce;
276274
use os;
275+
use option::Option::{self, Some, None};
277276
use ptr;
278277
use result::Result;
279278
use result::Result::{Ok, Err};
@@ -282,19 +281,75 @@ pub mod dl {
282281
use str;
283282
use string::String;
284283
use vec::Vec;
284+
use sys::c::compat::kernel32::SetThreadErrorMode;
285+
286+
pub fn open(filename: Option<&[u8]>) -> Result<*mut u8, String> {
287+
// disable "dll load failed" error dialog.
288+
let mut use_thread_mode = true;
289+
let prev_error_mode = unsafe {
290+
// SEM_FAILCRITICALERRORS 0x01
291+
let new_error_mode = 1;
292+
let mut prev_error_mode = 0;
293+
// Windows >= 7 supports thread error mode.
294+
let result = SetThreadErrorMode(new_error_mode, &mut prev_error_mode);
295+
if result == 0 {
296+
let err = os::errno();
297+
if err as libc::c_int == ERROR_CALL_NOT_IMPLEMENTED {
298+
use_thread_mode = false;
299+
// SetThreadErrorMode not found. use fallback solution: SetErrorMode()
300+
// Note that SetErrorMode is process-wide so this can cause race condition!
301+
// However, since even Windows APIs do not care of such problem (#20650),
302+
// we just assume SetErrorMode race is not a great deal.
303+
prev_error_mode = SetErrorMode(new_error_mode);
304+
}
305+
}
306+
prev_error_mode
307+
};
285308

286-
pub unsafe fn open_external(filename: &[u8]) -> *mut u8 {
287-
// Windows expects Unicode data
288-
let filename_str = str::from_utf8(filename).unwrap();
289-
let mut filename_str: Vec<u16> = filename_str.utf16_units().collect();
290-
filename_str.push(0);
291-
LoadLibraryW(filename_str.as_ptr() as *const libc::c_void) as *mut u8
292-
}
309+
unsafe {
310+
SetLastError(0);
311+
}
312+
313+
let result = match filename {
314+
Some(filename) => {
315+
let filename_str = str::from_utf8(filename).unwrap();
316+
let mut filename_str: Vec<u16> = filename_str.utf16_units().collect();
317+
filename_str.push(0);
318+
let result = unsafe {
319+
LoadLibraryW(filename_str.as_ptr() as *const libc::c_void)
320+
};
321+
// beware: Vec/String may change errno during drop!
322+
// so we get error here.
323+
if result == ptr::null_mut() {
324+
let errno = os::errno();
325+
Err(os::error_string(errno))
326+
} else {
327+
Ok(result as *mut u8)
328+
}
329+
}
330+
None => {
331+
let mut handle = ptr::null_mut();
332+
let succeeded = unsafe {
333+
GetModuleHandleExW(0 as libc::DWORD, ptr::null(), &mut handle)
334+
};
335+
if succeeded == libc::FALSE {
336+
let errno = os::errno();
337+
Err(os::error_string(errno))
338+
} else {
339+
Ok(handle as *mut u8)
340+
}
341+
}
342+
};
343+
344+
unsafe {
345+
if use_thread_mode {
346+
SetThreadErrorMode(prev_error_mode, ptr::null_mut());
347+
} else {
348+
SetErrorMode(prev_error_mode);
349+
}
350+
}
293351

294-
pub unsafe fn open_internal() -> *mut u8 {
295-
let mut handle = ptr::null_mut();
296-
GetModuleHandleExW(0 as libc::DWORD, ptr::null(), &mut handle);
297-
handle as *mut u8
352+
result
298353
}
299354

300355
pub fn check_for_errors_in<T, F>(f: F) -> Result<T, String> where
@@ -326,10 +381,10 @@ pub mod dl {
326381
fn SetLastError(error: libc::size_t);
327382
fn LoadLibraryW(name: *const libc::c_void) -> *mut libc::c_void;
328383
fn GetModuleHandleExW(dwFlags: libc::DWORD, name: *const u16,
329-
handle: *mut *mut libc::c_void)
330-
-> *mut libc::c_void;
384+
handle: *mut *mut libc::c_void) -> libc::BOOL;
331385
fn GetProcAddress(handle: *mut libc::c_void,
332386
name: *const libc::c_char) -> *mut libc::c_void;
333387
fn FreeLibrary(handle: *mut libc::c_void);
388+
fn SetErrorMode(uMode: libc::c_uint) -> libc::c_uint;
334389
}
335390
}

src/libstd/sys/windows/c.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ pub mod compat {
226226
/// * `CreateSymbolicLinkW`: Windows XP, Windows Server 2003
227227
/// * `GetFinalPathNameByHandleW`: Windows XP, Windows Server 2003
228228
pub mod kernel32 {
229+
use libc::c_uint;
229230
use libc::types::os::arch::extra::{DWORD, LPCWSTR, BOOLEAN, HANDLE};
230231
use libc::consts::os::extra::ERROR_CALL_NOT_IMPLEMENTED;
231232

@@ -249,6 +250,12 @@ pub mod compat {
249250
unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); 0 }
250251
}
251252
}
253+
254+
compat_fn! {
255+
kernel32::SetThreadErrorMode(_dwNewMode: DWORD, _lpOldMode: *mut DWORD) -> c_uint {
256+
unsafe { SetLastError(ERROR_CALL_NOT_IMPLEMENTED as DWORD); 0 }
257+
}
258+
}
252259
}
253260
}
254261

0 commit comments

Comments
 (0)