From 07ce394c646127c64ea401dd21924401b0f6c7ae Mon Sep 17 00:00:00 2001 From: "Stephen M. Coakley" Date: Tue, 16 Nov 2021 22:34:48 -0600 Subject: [PATCH] Refactor init to avoid link bugs on macOS Monterey Compilation currently fails for curl on macOS Monterey due to upstream rustc issue https://github.com/rust-lang/rust/issues/90342. To make this problem hurt users less, we can work around this by avoiding the specific issue that this bug causes. To avoid the rustc issue, we cannot directly reference any symbol that is configured to be in a constructor linker section, which we were previously doing intentionally to work around a different rustc issue https://github.com/rust-lang/rust/issues/47384. We should be able to avoid both bugs by defining our constructor symbol as a public item in the root module, though not directly referencing it in other code. Since the root module is always used (`init` is called on-demand in key places in the code) it should not be removed by optimization. Also add a quick unit test to make sure the constructor is still working for the platforms we have CI for. Fixes #417. --- src/lib.rs | 102 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 66c492b1f..2965e2bed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,6 +70,9 @@ pub mod easy; pub mod multi; mod panic; +#[cfg(test)] +static INITIALIZED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + /// Initializes the underlying libcurl library. /// /// The underlying libcurl library must be initialized before use, and must be @@ -90,48 +93,62 @@ pub fn init() { /// Used to prevent concurrent or duplicate initialization. static INIT: Once = Once::new(); - /// An exported constructor function. On supported platforms, this will be - /// invoked automatically before the program's `main` is called. - #[cfg_attr( - any(target_os = "linux", target_os = "freebsd", target_os = "android"), - link_section = ".init_array" - )] - #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")] - #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")] - static INIT_CTOR: extern "C" fn() = init_inner; + INIT.call_once(|| { + #[cfg(need_openssl_init)] + openssl_probe::init_ssl_cert_env_vars(); + #[cfg(need_openssl_init)] + openssl_sys::init(); + + unsafe { + assert_eq!(curl_sys::curl_global_init(curl_sys::CURL_GLOBAL_ALL), 0); + } + + #[cfg(test)] + { + INITIALIZED.store(true, std::sync::atomic::Ordering::SeqCst); + } + + // Note that we explicitly don't schedule a call to + // `curl_global_cleanup`. The documentation for that function says + // + // > You must not call it when any other thread in the program (i.e. a + // > thread sharing the same memory) is running. This doesn't just mean + // > no other thread that is using libcurl. + // + // We can't ever be sure of that, so unfortunately we can't call the + // function. + }); +} +/// An exported constructor function. On supported platforms, this will be +/// invoked automatically before the program's `main` is called. This is done +/// for the convenience of library users since otherwise the thread-safety rules +/// around initialization can be difficult to fulfill. +/// +/// This is a hidden public item to ensure the symbol isn't optimized away by a +/// rustc/LLVM bug: https://github.com/rust-lang/rust/issues/47384. As long as +/// any item in this module is used by the final binary (which `init` will be) +/// then this symbol should be preserved. +#[used] +#[doc(hidden)] +#[cfg_attr( + any(target_os = "linux", target_os = "freebsd", target_os = "android"), + link_section = ".init_array" +)] +#[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")] +#[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")] +pub static INIT_CTOR: extern "C" fn() = { /// This is the body of our constructor function. #[cfg_attr( any(target_os = "linux", target_os = "android"), link_section = ".text.startup" )] - extern "C" fn init_inner() { - INIT.call_once(|| { - #[cfg(need_openssl_init)] - openssl_probe::init_ssl_cert_env_vars(); - #[cfg(need_openssl_init)] - openssl_sys::init(); - - unsafe { - assert_eq!(curl_sys::curl_global_init(curl_sys::CURL_GLOBAL_ALL), 0); - } - - // Note that we explicitly don't schedule a call to - // `curl_global_cleanup`. The documentation for that function says - // - // > You must not call it when any other thread in the program (i.e. - // > a thread sharing the same memory) is running. This doesn't just - // > mean no other thread that is using libcurl. - // - // We can't ever be sure of that, so unfortunately we can't call the - // function. - }); + extern "C" fn init_ctor() { + init(); } - // We invoke our init function through our static to ensure the symbol isn't - // optimized away by a bug: https://github.com/rust-lang/rust/issues/47384 - INIT_CTOR(); -} + init_ctor +}; unsafe fn opt_str<'a>(ptr: *const libc::c_char) -> Option<&'a str> { if ptr.is_null() { @@ -148,3 +165,20 @@ fn cvt(r: curl_sys::CURLcode) -> Result<(), Error> { Err(Error::new(r)) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[cfg(any( + target_os = "linux", + target_os = "macos", + target_os = "windows", + target_os = "freebsd", + target_os = "android" + ))] + fn is_initialized_before_main() { + assert!(INITIALIZED.load(std::sync::atomic::Ordering::SeqCst)); + } +}