replace dynamic library module with libloading

Cargo.lock

@@ -1921,6 +1921,16 @@ dependencies = [
  "pkg-config",
 ]
 
+[[package]]
+name = "libloading"
+version = "0.7.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c0cf036d15402bea3c5d4de17b3fce76b3e4a56ebc1f577be0e7a72f7c607cf0"
+dependencies = [
+ "cfg-if 1.0.0",
+ "winapi",
+]
+
 [[package]]
 name = "libm"
 version = "0.1.4"
@@ -3708,6 +3718,7 @@ dependencies = [
  "bitflags",
  "cstr",
  "libc",
+ "libloading",
  "measureme 10.0.0",
  "rustc-demangle",
  "rustc_arena",
@@ -3992,6 +4003,7 @@ name = "rustc_interface"
 version = "0.0.0"
 dependencies = [
  "libc",
+ "libloading",
  "rustc-rayon",
  "rustc-rayon-core",
  "rustc_ast",
@@ -4104,7 +4116,7 @@ dependencies = [
 name = "rustc_metadata"
 version = "0.0.0"
 dependencies = [
- "libc",
+ "libloading",
  "odht",
  "rustc_ast",
  "rustc_attr",
@@ -4124,7 +4136,6 @@ dependencies = [
  "smallvec",
  "snap",
  "tracing",
- "winapi",
 ]
 
 [[package]]
@@ -4297,6 +4308,7 @@ dependencies = [
 name = "rustc_plugin_impl"
 version = "0.0.0"
 dependencies = [
+ "libloading",
  "rustc_ast",
  "rustc_errors",
  "rustc_hir",

compiler/rustc_codegen_llvm/Cargo.toml

@@ -11,6 +11,7 @@ doctest = false
 bitflags = "1.0"
 cstr = "0.2"
 libc = "0.2"
+libloading = "0.7.1"
 measureme = "10.0.0"
 snap = "1"
 tracing = "0.1"

compiler/rustc_codegen_llvm/src/llvm_util.rs

@@ -1,9 +1,9 @@
 use crate::back::write::create_informational_target_machine;
 use crate::{llvm, llvm_util};
 use libc::c_int;
+use libloading::Library;
 use rustc_codegen_ssa::target_features::supported_target_features;
 use rustc_data_structures::fx::FxHashSet;
-use rustc_metadata::dynamic_lib::DynamicLibrary;
 use rustc_middle::bug;
 use rustc_session::config::PrintRequest;
 use rustc_session::Session;
@@ -13,7 +13,6 @@ use std::ffi::{CStr, CString};
 use tracing::debug;
 
 use std::mem;
-use std::path::Path;
 use std::ptr;
 use std::slice;
 use std::str;
@@ -120,14 +119,14 @@ unsafe fn configure_llvm(sess: &Session) {
 
     llvm::LLVMInitializePasses();
 
+    // Register LLVM plugins by loading them into the compiler process.
     for plugin in &sess.opts.debugging_opts.llvm_plugins {
-        let path = Path::new(plugin);
+        let lib = Library::new(plugin).unwrap_or_else(|e| bug!("couldn't load plugin: {}", e));
-        let res = DynamicLibrary::open(path);
+        debug!("LLVM plugin loaded successfully {:?} ({})", lib, plugin);
-        match res {
+
-            Ok(_) => debug!("LLVM plugin loaded succesfully {} ({})", path.display(), plugin),
+        // Intentionally leak the dynamic library. We can't ever unload it
-            Err(e) => bug!("couldn't load plugin: {}", e),
+        // since the library can make things that will live arbitrarily long.
-        }
+        mem::forget(lib);
-        mem::forget(res);
     }
 
     rustc_llvm::initialize_available_targets();

compiler/rustc_interface/Cargo.toml

@@ -8,6 +8,7 @@ doctest = false
 
 [dependencies]
 libc = "0.2"
+libloading = "0.7.1"
 tracing = "0.1"
 rustc-rayon-core = "0.3.1"
 rayon = { version = "0.3.1", package = "rustc-rayon" }

compiler/rustc_interface/src/util.rs

@@ -1,3 +1,4 @@
+use libloading::Library;
 use rustc_ast::mut_visit::{visit_clobber, MutVisitor, *};
 use rustc_ast::ptr::P;
 use rustc_ast::{self as ast, AttrVec, BlockCheckMode};
@@ -7,7 +8,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::jobserver;
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::registry::Registry;
-use rustc_metadata::dynamic_lib::DynamicLibrary;
 #[cfg(parallel_compiler)]
 use rustc_middle::ty::tls;
 use rustc_parse::validate_attr;
@@ -39,6 +39,9 @@ use std::sync::{Arc, Mutex};
 use std::thread;
 use tracing::info;
 
+/// Function pointer type that constructs a new CodegenBackend.
+pub type MakeBackendFn = fn() -> Box<dyn CodegenBackend>;
+
 /// Adds `target_feature = "..."` cfgs for a variety of platform
 /// specific features (SSE, NEON etc.).
 ///
@@ -211,28 +214,24 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals<F: FnOnce() -> R + Se
     })
 }
 
-fn load_backend_from_dylib(path: &Path) -> fn() -> Box<dyn CodegenBackend> {
+fn load_backend_from_dylib(path: &Path) -> MakeBackendFn {
-    let lib = DynamicLibrary::open(path).unwrap_or_else(|err| {
+    let lib = unsafe { Library::new(path) }.unwrap_or_else(|err| {
-        let err = format!("couldn't load codegen backend {:?}: {:?}", path, err);
+        let err = format!("couldn't load codegen backend {:?}: {}", path, err);
         early_error(ErrorOutputType::default(), &err);
     });
-    unsafe {
+
-        match lib.symbol("__rustc_codegen_backend") {
+    let backend_sym = unsafe { lib.get::<MakeBackendFn>(b"__rustc_codegen_backend") }
-            Ok(f) => {
+        .unwrap_or_else(|e| {
-                mem::forget(lib);
+            let err = format!("couldn't load codegen backend: {}", e);
-                mem::transmute::<*mut u8, _>(f)
+            early_error(ErrorOutputType::default(), &err);
-            }
+        });
-            Err(e) => {
+
-                let err = format!(
+    // Intentionally leak the dynamic library. We can't ever unload it
-                    "couldn't load codegen backend as it \
+    // since the library can make things that will live arbitrarily long.
-                                   doesn't export the `__rustc_codegen_backend` \
+    let backend_sym = unsafe { backend_sym.into_raw() };
-                                   symbol: {:?}",
+    mem::forget(lib);
-                    e
+
-                );
+    *backend_sym
-                early_error(ErrorOutputType::default(), &err);
-            }
-        }
-    }
 }
 
 /// Get the codegen backend based on the name and specified sysroot.
@@ -380,10 +379,7 @@ fn sysroot_candidates() -> Vec<PathBuf> {
     }
 }
 
-pub fn get_codegen_sysroot(
+pub fn get_codegen_sysroot(maybe_sysroot: &Option<PathBuf>, backend_name: &str) -> MakeBackendFn {
-    maybe_sysroot: &Option<PathBuf>,
-    backend_name: &str,
-) -> fn() -> Box<dyn CodegenBackend> {
     // For now we only allow this function to be called once as it'll dlopen a
     // few things, which seems to work best if we only do that once. In
     // general this assertion never trips due to the once guard in `get_codegen_backend`,

compiler/rustc_metadata/Cargo.toml

@@ -7,7 +7,7 @@ edition = "2021"
 doctest = false
 
 [dependencies]
-libc = "0.2"
+libloading = "0.7.1"
 odht = { version = "0.3.1", features = ["nightly"] }
 snap = "1"
 tracing = "0.1"
@@ -27,6 +27,3 @@ rustc_ast = { path = "../rustc_ast" }
 rustc_expand = { path = "../rustc_expand" }
 rustc_span = { path = "../rustc_span" }
 rustc_session = { path = "../rustc_session" }
-
-[target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3", features = ["errhandlingapi", "libloaderapi"] }

compiler/rustc_metadata/src/creader.rs

@@ -1,6 +1,5 @@
 //! Validates all used crates and extern libraries and loads their metadata
 
-use crate::dynamic_lib::DynamicLibrary;
 use crate::locator::{CrateError, CrateLocator, CratePaths};
 use crate::rmeta::{CrateDep, CrateMetadata, CrateNumMap, CrateRoot, MetadataBlob};
 
@@ -676,25 +675,19 @@ impl<'a> CrateLoader<'a> {
     ) -> Result<&'static [ProcMacro], CrateError> {
         // 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(&path) {
+        let lib = unsafe { libloading::Library::new(path) }
-            Ok(lib) => lib,
+            .map_err(|err| CrateError::DlOpen(err.to_string()))?;
-            Err(s) => return Err(CrateError::DlOpen(s)),
-        };
 
-        let sym = self.sess.generate_proc_macro_decls_symbol(stable_crate_id);
+        let sym_name = self.sess.generate_proc_macro_decls_symbol(stable_crate_id);
-        let decls = unsafe {
+        let sym = unsafe { lib.get::<*const &[ProcMacro]>(sym_name.as_bytes()) }
-            let sym = match lib.symbol(&sym) {
+            .map_err(|err| CrateError::DlSym(err.to_string()))?;
-                Ok(f) => f,
-                Err(s) => return Err(CrateError::DlSym(s)),
-            };
-            *(sym as *const &[ProcMacro])
-        };
 
         // 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(decls)
+        Ok(unsafe { **sym })
     }
 
     fn inject_panic_runtime(&mut self, krate: &ast::Crate) {

compiler/rustc_metadata/src/dynamic_lib.rs

@@ -1,194 +0,0 @@
-//! Dynamic library facilities.
-//!
-//! A simple wrapper over the platform's dynamic library facilities
-
-use std::ffi::CString;
-use std::path::Path;
-
-pub struct DynamicLibrary {
-    handle: *mut u8,
-}
-
-impl Drop for DynamicLibrary {
-    fn drop(&mut self) {
-        unsafe { dl::close(self.handle) }
-    }
-}
-
-impl DynamicLibrary {
-    /// 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
-        // run.
-        match maybe_library {
-            Err(err) => Err(err),
-            Ok(handle) => Ok(DynamicLibrary { handle }),
-        }
-    }
-
-    /// Accesses the value at the symbol of the dynamic library.
-    pub unsafe fn symbol<T>(&self, symbol: &str) -> Result<*mut T, String> {
-        // This function should have a lifetime constraint of 'a on
-        // T but that feature is still unimplemented
-
-        let raw_string = CString::new(symbol).unwrap();
-        let maybe_symbol_value = dl::symbol(self.handle, raw_string.as_ptr());
-
-        // The value must not be constructed if there is an error so
-        // the destructor does not run.
-        match maybe_symbol_value {
-            Err(err) => Err(err),
-            Ok(symbol_value) => Ok(symbol_value as *mut T),
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests;
-
-#[cfg(unix)]
-mod dl {
-    use std::ffi::{CString, OsStr};
-    use std::os::unix::prelude::*;
-
-    // As of the 2017 revision of the POSIX standard (IEEE 1003.1-2017), it is
-    // implementation-defined whether `dlerror` is thread-safe (in which case it returns the most
-    // recent error in the calling thread) or not thread-safe (in which case it returns the most
-    // recent error in *any* thread).
-    //
-    // There's no easy way to tell what strategy is used by a given POSIX implementation, so we
-    // lock around all calls that can modify `dlerror` in this module lest we accidentally read an
-    // error from a different thread. This is bulletproof when we are the *only* code using the
-    // dynamic library APIs at a given point in time. However, it's still possible for us to race
-    // with other code (see #74469) on platforms where `dlerror` is not thread-safe.
-    mod error {
-        use std::ffi::CStr;
-        use std::lazy::SyncLazy;
-        use std::sync::{Mutex, MutexGuard};
-
-        pub fn lock() -> MutexGuard<'static, Guard> {
-            static LOCK: SyncLazy<Mutex<Guard>> = SyncLazy::new(|| Mutex::new(Guard));
-            LOCK.lock().unwrap()
-        }
-
-        #[non_exhaustive]
-        pub struct Guard;
-
-        impl Guard {
-            pub fn get(&mut self) -> Result<(), String> {
-                let msg = unsafe { libc::dlerror() };
-                if msg.is_null() {
-                    Ok(())
-                } else {
-                    let msg = unsafe { CStr::from_ptr(msg as *const _) };
-                    Err(msg.to_string_lossy().into_owned())
-                }
-            }
-
-            pub fn clear(&mut self) {
-                let _ = unsafe { libc::dlerror() };
-            }
-        }
-    }
-
-    pub(super) fn open(filename: &OsStr) -> Result<*mut u8, String> {
-        let s = CString::new(filename.as_bytes()).unwrap();
-
-        let mut dlerror = error::lock();
-        let ret = unsafe { libc::dlopen(s.as_ptr(), libc::RTLD_LAZY | libc::RTLD_LOCAL) };
-
-        if !ret.is_null() {
-            return Ok(ret.cast());
-        }
-
-        // A null return from `dlopen` indicates that an error has definitely occurred, so if
-        // nothing is in `dlerror`, we are racing with another thread that has stolen our error
-        // message. See the explanation on the `dl::error` module for more information.
-        dlerror.get().and_then(|()| Err("Unknown error".to_string()))
-    }
-
-    pub(super) unsafe fn symbol(
-        handle: *mut u8,
-        symbol: *const libc::c_char,
-    ) -> Result<*mut u8, String> {
-        let mut dlerror = error::lock();
-
-        // Unlike `dlopen`, it's possible for `dlsym` to return null without overwriting `dlerror`.
-        // Because of this, we clear `dlerror` before calling `dlsym` to avoid picking up a stale
-        // error message by accident.
-        dlerror.clear();
-
-        let ret = libc::dlsym(handle as *mut libc::c_void, symbol);
-
-        if !ret.is_null() {
-            return Ok(ret.cast());
-        }
-
-        // If `dlsym` returns null but there is nothing in `dlerror` it means one of two things:
-        // - We tried to load a symbol mapped to address 0. This is not technically an error but is
-        //   unlikely to occur in practice and equally unlikely to be handled correctly by calling
-        //   code. Therefore we treat it as an error anyway.
-        // - An error has occurred, but we are racing with another thread that has stolen our error
-        //   message. See the explanation on the `dl::error` module for more information.
-        dlerror.get().and_then(|()| Err("Tried to load symbol mapped to address 0".to_string()))
-    }
-
-    pub(super) unsafe fn close(handle: *mut u8) {
-        libc::dlclose(handle as *mut libc::c_void);
-    }
-}
-
-#[cfg(windows)]
-mod dl {
-    use std::ffi::OsStr;
-    use std::io;
-    use std::os::windows::prelude::*;
-    use std::ptr;
-
-    use winapi::shared::minwindef::HMODULE;
-    use winapi::um::errhandlingapi::SetThreadErrorMode;
-    use winapi::um::libloaderapi::{FreeLibrary, GetProcAddress, LoadLibraryW};
-    use winapi::um::winbase::SEM_FAILCRITICALERRORS;
-
-    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;
-            let mut prev_error_mode = 0;
-            let result = SetThreadErrorMode(new_error_mode, &mut prev_error_mode);
-            if result == 0 {
-                return Err(io::Error::last_os_error().to_string());
-            }
-            prev_error_mode
-        };
-
-        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());
-        }
-
-        result
-    }
-
-    pub(super) unsafe fn symbol(
-        handle: *mut u8,
-        symbol: *const libc::c_char,
-    ) -> Result<*mut u8, String> {
-        let ptr = GetProcAddress(handle as HMODULE, symbol) as *mut u8;
-        ptr_result(ptr)
-    }
-
-    pub(super) unsafe fn close(handle: *mut u8) {
-        FreeLibrary(handle as HMODULE);
-    }
-
-    fn ptr_result<T>(ptr: *mut T) -> Result<*mut T, String> {
-        if ptr.is_null() { Err(io::Error::last_os_error().to_string()) } else { Ok(ptr) }
-    }
-}

compiler/rustc_metadata/src/dynamic_lib/tests.rs

@@ -1,18 +0,0 @@
-use super::*;
-
-#[test]
-fn test_errors_do_not_crash() {
-    use std::path::Path;
-
-    if !cfg!(unix) {
-        return;
-    }
-
-    // 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(&path) {
-        Err(_) => {}
-        Ok(_) => panic!("Successfully opened the empty library."),
-    }
-}

compiler/rustc_metadata/src/lib.rs

@@ -28,7 +28,6 @@ mod native_libs;
 mod rmeta;
 
 pub mod creader;
-pub mod dynamic_lib;
 pub mod locator;
 
 pub use rmeta::{encode_metadata, EncodedMetadata, METADATA_HEADER};

compiler/rustc_plugin_impl/Cargo.toml

@@ -8,6 +8,7 @@ edition = "2021"
 doctest = false
 
 [dependencies]
+libloading = "0.7.1"
 rustc_middle = { path = "../rustc_middle" }
 rustc_errors = { path = "../rustc_errors" }
 rustc_hir = { path = "../rustc_hir" }

compiler/rustc_plugin_impl/src/load.rs

@@ -1,6 +1,7 @@
 //! Used by `rustc` when loading a plugin.
 
 use crate::Registry;
+use libloading::Library;
 use rustc_ast::Crate;
 use rustc_errors::struct_span_err;
 use rustc_metadata::locator;
@@ -56,37 +57,28 @@ fn load_plugin(
     ident: Ident,
 ) {
     let lib = locator::find_plugin_registrar(sess, metadata_loader, ident.span, ident.name);
-    let fun = dylink_registrar(sess, ident.span, lib);
+    let fun = dylink_registrar(lib).unwrap_or_else(|err| {
+        // This is fatal: there are almost certainly macros we need inside this crate, so
+        // continuing would spew "macro undefined" errors.
+        sess.span_fatal(ident.span, &err.to_string());
+    });
     plugins.push(fun);
 }
 
-// Dynamically link a registrar function into the compiler process.
+/// Dynamically link a registrar function into the compiler process.
-fn dylink_registrar(sess: &Session, span: Span, path: PathBuf) -> PluginRegistrarFn {
+fn dylink_registrar(lib_path: PathBuf) -> Result<PluginRegistrarFn, libloading::Error> {
-    use rustc_metadata::dynamic_lib::DynamicLibrary;
-
     // Make sure the path contains a / or the linker will search for it.
-    let path = env::current_dir().unwrap().join(&path);
+    let lib_path = env::current_dir().unwrap().join(&lib_path);
 
-    let lib = match DynamicLibrary::open(&path) {
+    let lib = unsafe { Library::new(&lib_path) }?;
-        Ok(lib) => lib,
-        // this is fatal: there are almost certainly macros we need
-        // inside this crate, so continue would spew "macro undefined"
-        // errors
-        Err(err) => sess.span_fatal(span, &err),
-    };
 
-    unsafe {
+    let registrar_sym = unsafe { lib.get::<PluginRegistrarFn>(b"__rustc_plugin_registrar") }?;
-        let registrar = match lib.symbol("__rustc_plugin_registrar") {
-            Ok(registrar) => mem::transmute::<*mut u8, PluginRegistrarFn>(registrar),
-            // again fatal if we can't register macros
-            Err(err) => sess.span_fatal(span, &err),
-        };
 
-        // Intentionally leak the dynamic library. We can't ever unload it
+    // Intentionally leak the dynamic library. We can't ever unload it
-        // since the library can make things that will live arbitrarily long
+    // since the library can make things that will live arbitrarily long
-        // (e.g., an Rc cycle or a thread).
+    // (e.g., an Rc cycle or a thread).
-        mem::forget(lib);
+    let registrar_sym = unsafe { registrar_sym.into_raw() };
+    mem::forget(lib);
 
-        registrar
+    Ok(*registrar_sym)
-    }
 }

src/test/run-make-fulldeps/extern-fn-reachable/Makefile

@@ -1,9 +1,25 @@
 -include ../tools.mk
 
+# ignore-windows-msvc
+
+NM=nm -D
+
+ifeq ($(UNAME),Darwin)
+NM=nm -gU
+endif
+
+ifdef IS_WINDOWS
+NM=nm -g
+endif
+
 # This overrides the LD_LIBRARY_PATH for RUN
 TARGET_RPATH_DIR:=$(TARGET_RPATH_DIR):$(TMPDIR)
 
 all:
 	$(RUSTC) dylib.rs -o $(TMPDIR)/libdylib.so -C prefer-dynamic
-	$(RUSTC) main.rs -C prefer-dynamic
+
-	$(call RUN,main)
+	[ "$$($(NM) $(TMPDIR)/libdylib.so | grep -v __imp_ | grep -c fun1)" -eq "1" ]
+	[ "$$($(NM) $(TMPDIR)/libdylib.so | grep -v __imp_ | grep -c fun2)" -eq "1" ]
+	[ "$$($(NM) $(TMPDIR)/libdylib.so | grep -v __imp_ | grep -c fun3)" -eq "1" ]
+	[ "$$($(NM) $(TMPDIR)/libdylib.so | grep -v __imp_ | grep -c fun4)" -eq "1" ]
+	[ "$$($(NM) $(TMPDIR)/libdylib.so | grep -v __imp_ | grep -c fun5)" -eq "1" ]

src/test/run-make-fulldeps/extern-fn-reachable/main.rs

@@ -1,18 +0,0 @@
-#![feature(rustc_private)]
-
-extern crate rustc_metadata;
-
-use rustc_metadata::dynamic_lib::DynamicLibrary;
-use std::path::Path;
-
-pub fn main() {
-    unsafe {
-        let path = Path::new("libdylib.so");
-        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());
-        assert!(a.symbol::<isize>("fun4").is_ok());
-        assert!(a.symbol::<isize>("fun5").is_ok());
-    }
-}

src/tools/tidy/src/deps.rs

@@ -15,6 +15,7 @@ const LICENSES: &[&str] = &[
     "Apache-2.0 OR MIT",
     "Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT", // wasi license
     "MIT",
+    "ISC",
     "Unlicense/MIT",
     "Unlicense OR MIT",
     "0BSD OR MIT OR Apache-2.0", // adler license
@@ -53,7 +54,6 @@ const EXCEPTIONS_CRANELIFT: &[(&str, &str)] = &[
     ("cranelift-module", "Apache-2.0 WITH LLVM-exception"),
     ("cranelift-native", "Apache-2.0 WITH LLVM-exception"),
     ("cranelift-object", "Apache-2.0 WITH LLVM-exception"),
-    ("libloading", "ISC"),
     ("mach", "BSD-2-Clause"),
     ("regalloc", "Apache-2.0 WITH LLVM-exception"),
     ("target-lexicon", "Apache-2.0 WITH LLVM-exception"),
@@ -129,6 +129,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[
     "jobserver",
     "lazy_static",
     "libc",
+    "libloading",
     "libz-sys",
     "lock_api",
     "log",