Unrolled build for rust-lang#130586

Rollup merge of rust-lang#130586 - dpaoliello:fixrawdylib, r=wesleywiser

Set "symbol name" in raw-dylib import libraries to the decorated name

`windows-rs` received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: <microsoft/windows-rs#3285>

The root cause turned out to be rust-lang#124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes rust-lang#124958

Passing i686 MSVC and MinGW build: <https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586>

r? `@ChrisDenton`

Author: rust-timer

compiler/rustc_codegen_gcc/src/archive.rs

@@ -2,6 +2,7 @@ use std::path::Path;
 
 use rustc_codegen_ssa::back::archive::{
     ArArchiveBuilder, ArchiveBuilder, ArchiveBuilderBuilder, DEFAULT_OBJECT_READER,
+    ImportLibraryItem,
 };
 use rustc_session::Session;
 
@@ -16,7 +17,7 @@ impl ArchiveBuilderBuilder for ArArchiveBuilderBuilder {
         &self,
         _sess: &Session,
         _lib_name: &str,
-        _import_name_and_ordinal_vector: Vec<(String, Option<u16>)>,
+        _items: Vec<ImportLibraryItem>,
         _output_path: &Path,
     ) {
         unimplemented!("creating dll imports is not yet supported");

compiler/rustc_codegen_llvm/src/callee.rs

@@ -44,6 +44,22 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
         let llfn = if tcx.sess.target.arch == "x86"
             && let Some(dllimport) = crate::common::get_dllimport(tcx, instance_def_id, sym)
         {
+            // When calling functions in generated import libraries, MSVC needs
+            // the fully decorated name (as would have been in the declaring
+            // object file), but MinGW wants the name as exported (as would be
+            // in the def file) which may be missing decorations.
+            let mingw_gnu_toolchain = common::is_mingw_gnu_toolchain(&tcx.sess.target);
+            let llfn = cx.declare_fn(
+                &common::i686_decorated_name(
+                    dllimport,
+                    mingw_gnu_toolchain,
+                    true,
+                    !mingw_gnu_toolchain,
+                ),
+                fn_abi,
+                Some(instance),
+            );
+
             // Fix for https://github.com/rust-lang/rust/issues/104453
             // On x86 Windows, LLVM uses 'L' as the prefix for any private
             // global symbols, so when we create an undecorated function symbol
@@ -55,15 +71,6 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
             // LLVM will prefix the name with `__imp_`. Ideally, we'd like the
             // existing logic below to set the Storage Class, but it has an
             // exemption for MinGW for backwards compatibility.
-            let llfn = cx.declare_fn(
-                &common::i686_decorated_name(
-                    dllimport,
-                    common::is_mingw_gnu_toolchain(&tcx.sess.target),
-                    true,
-                ),
-                fn_abi,
-                Some(instance),
-            );
             unsafe {
                 llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport);
             }

compiler/rustc_codegen_llvm/src/consts.rs

@@ -194,16 +194,10 @@ fn check_and_apply_linkage<'ll, 'tcx>(
         unsafe { llvm::LLVMSetInitializer(g2, g1) };
         g2
     } else if cx.tcx.sess.target.arch == "x86"
+        && common::is_mingw_gnu_toolchain(&cx.tcx.sess.target)
         && let Some(dllimport) = crate::common::get_dllimport(cx.tcx, def_id, sym)
     {
-        cx.declare_global(
-            &common::i686_decorated_name(
-                dllimport,
-                common::is_mingw_gnu_toolchain(&cx.tcx.sess.target),
-                true,
-            ),
-            llty,
-        )
+        cx.declare_global(&common::i686_decorated_name(dllimport, true, true, false), llty)
     } else {
         // Generate an external declaration.
         // FIXME(nagisa): investigate whether it can be changed into define_global

compiler/rustc_codegen_ssa/src/back/archive.rs

@@ -26,6 +26,35 @@ use crate::errors::{
     DlltoolFailImportLibrary, ErrorCallingDllTool, ErrorCreatingImportLibrary, ErrorWritingDEFFile,
 };
 
+/// An item to be included in an import library.
+/// This is a slimmed down version of `COFFShortExport` from `ar-archive-writer`.
+pub struct ImportLibraryItem {
+    /// The name to be exported.
+    pub name: String,
+    /// The ordinal to be exported, if any.
+    pub ordinal: Option<u16>,
+    /// The original, decorated name if `name` is not decorated.
+    pub symbol_name: Option<String>,
+    /// True if this is a data export, false if it is a function export.
+    pub is_data: bool,
+}
+
+impl From<ImportLibraryItem> for COFFShortExport {
+    fn from(item: ImportLibraryItem) -> Self {
+        COFFShortExport {
+            name: item.name,
+            ext_name: None,
+            symbol_name: item.symbol_name,
+            alias_target: None,
+            ordinal: item.ordinal.unwrap_or(0),
+            noname: item.ordinal.is_some(),
+            data: item.is_data,
+            private: false,
+            constant: false,
+        }
+    }
+}
+
 pub trait ArchiveBuilderBuilder {
     fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box<dyn ArchiveBuilder + 'a>;
 
@@ -38,7 +67,7 @@ pub trait ArchiveBuilderBuilder {
         &self,
         sess: &Session,
         lib_name: &str,
-        import_name_and_ordinal_vector: Vec<(String, Option<u16>)>,
+        items: Vec<ImportLibraryItem>,
         output_path: &Path,
     ) {
         if common::is_mingw_gnu_toolchain(&sess.target) {
@@ -47,21 +76,16 @@ pub trait ArchiveBuilderBuilder {
             // that loaded but crashed with an AV upon calling one of the imported
             // functions. Therefore, use binutils to create the import library instead,
             // by writing a .DEF file to the temp dir and calling binutils's dlltool.
-            create_mingw_dll_import_lib(
-                sess,
-                lib_name,
-                import_name_and_ordinal_vector,
-                output_path,
-            );
+            create_mingw_dll_import_lib(sess, lib_name, items, output_path);
         } else {
             trace!("creating import library");
             trace!("  dll_name {:#?}", lib_name);
             trace!("  output_path {}", output_path.display());
             trace!(
                 "  import names: {}",
-                import_name_and_ordinal_vector
+                items
                     .iter()
-                    .map(|(name, _ordinal)| name.clone())
+                    .map(|ImportLibraryItem { name, .. }| name.clone())
                     .collect::<Vec<_>>()
                     .join(", "),
             );
@@ -79,20 +103,7 @@ pub trait ArchiveBuilderBuilder {
                     .emit_fatal(ErrorCreatingImportLibrary { lib_name, error: error.to_string() }),
             };
 
-            let exports = import_name_and_ordinal_vector
-                .iter()
-                .map(|(name, ordinal)| COFFShortExport {
-                    name: name.to_string(),
-                    ext_name: None,
-                    symbol_name: None,
-                    alias_target: None,
-                    ordinal: ordinal.unwrap_or(0),
-                    noname: ordinal.is_some(),
-                    data: false,
-                    private: false,
-                    constant: false,
-                })
-                .collect::<Vec<_>>();
+            let exports = items.into_iter().map(Into::into).collect::<Vec<_>>();
             let machine = match &*sess.target.arch {
                 "x86_64" => MachineTypes::AMD64,
                 "x86" => MachineTypes::I386,
@@ -160,16 +171,16 @@ pub trait ArchiveBuilderBuilder {
 fn create_mingw_dll_import_lib(
     sess: &Session,
     lib_name: &str,
-    import_name_and_ordinal_vector: Vec<(String, Option<u16>)>,
+    items: Vec<ImportLibraryItem>,
     output_path: &Path,
 ) {
     let def_file_path = output_path.with_extension("def");
 
     let def_file_content = format!(
         "EXPORTS\n{}",
-        import_name_and_ordinal_vector
+        items
             .into_iter()
-            .map(|(name, ordinal)| {
+            .map(|ImportLibraryItem { name, ordinal, .. }| {
                 match ordinal {
                     Some(n) => format!("{name} @{n} NONAME"),
                     None => name,

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -45,7 +45,7 @@ use rustc_target::spec::{
 use tempfile::Builder as TempFileBuilder;
 use tracing::{debug, info, warn};
 
-use super::archive::{ArchiveBuilder, ArchiveBuilderBuilder};
+use super::archive::{ArchiveBuilder, ArchiveBuilderBuilder, ImportLibraryItem};
 use super::command::Command;
 use super::linker::{self, Linker};
 use super::metadata::{MetadataPosition, create_wrapper_file};
@@ -495,24 +495,43 @@ fn create_dll_import_libs<'a>(
 
             let mingw_gnu_toolchain = common::is_mingw_gnu_toolchain(&sess.target);
 
-            let import_name_and_ordinal_vector: Vec<(String, Option<u16>)> = raw_dylib_imports
+            let items: Vec<ImportLibraryItem> = raw_dylib_imports
                 .iter()
                 .map(|import: &DllImport| {
                     if sess.target.arch == "x86" {
-                        (
-                            common::i686_decorated_name(import, mingw_gnu_toolchain, false),
-                            import.ordinal(),
-                        )
+                        ImportLibraryItem {
+                            name: common::i686_decorated_name(
+                                import,
+                                mingw_gnu_toolchain,
+                                false,
+                                false,
+                            ),
+                            ordinal: import.ordinal(),
+                            symbol_name: import.is_missing_decorations().then(|| {
+                                common::i686_decorated_name(
+                                    import,
+                                    mingw_gnu_toolchain,
+                                    false,
+                                    true,
+                                )
+                            }),
+                            is_data: !import.is_fn,
+                        }
                     } else {
-                        (import.name.to_string(), import.ordinal())
+                        ImportLibraryItem {
+                            name: import.name.to_string(),
+                            ordinal: import.ordinal(),
+                            symbol_name: None,
+                            is_data: !import.is_fn,
+                        }
                     }
                 })
                 .collect();
 
             archive_builder_builder.create_dll_import_lib(
                 sess,
                 &raw_dylib_name,
-                import_name_and_ordinal_vector,
+                items,
                 &output_path,
             );
 

compiler/rustc_codegen_ssa/src/common.rs

@@ -187,12 +187,15 @@ pub fn i686_decorated_name(
     dll_import: &DllImport,
     mingw: bool,
     disable_name_mangling: bool,
+    force_fully_decorated: bool,
 ) -> String {
     let name = dll_import.name.as_str();
 
-    let (add_prefix, add_suffix) = match dll_import.import_name_type {
-        Some(PeImportNameType::NoPrefix) => (false, true),
-        Some(PeImportNameType::Undecorated) => (false, false),
+    let (add_prefix, add_suffix) = match (force_fully_decorated, dll_import.import_name_type) {
+        // No prefix is a bit weird, in that LLVM/ar_archive_writer won't emit it, so we will
+        // ignore `force_fully_decorated` and always partially decorate it.
+        (_, Some(PeImportNameType::NoPrefix)) => (false, true),
+        (false, Some(PeImportNameType::Undecorated)) => (false, false),
         _ => (true, true),
     };
 

compiler/rustc_session/src/cstore.rs

@@ -130,6 +130,11 @@ impl DllImport {
             None
         }
     }
+
+    pub fn is_missing_decorations(&self) -> bool {
+        self.import_name_type == Some(PeImportNameType::Undecorated)
+            || self.import_name_type == Some(PeImportNameType::NoPrefix)
+    }
 }
 
 /// Calling convention for a function defined in an external library.