Auto merge of #98989 - dpaoliello:rawdylibbin, r=michaelwoerister

bors · bors · commit daaae25022ce · 2022-07-26T01:47:34.000Z
Enable raw-dylib for bin crates Fixes #93842 When `raw-dylib` is used in a `bin` crate, we need to collect all of the `raw-dylib` functions, generate the import library and add that to the linker command line. I also changed the tests so that 1) the C++ dlls are created after the Rust dlls, thus there is no chance of accidentally using them in the Rust linking process and 2) disabled generating import libraries when building with MSVC.
diff --git a/compiler/rustc_codegen_cranelift/src/archive.rs b/compiler/rustc_codegen_cranelift/src/archive.rs
@@ -204,12 +204,16 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> {
         any_members
     }
 
-    fn inject_dll_import_lib(
-        &mut self,
+    fn sess(&self) -> &Session {
+        self.sess
+    }
+
+    fn create_dll_import_lib(
+        _sess: &Session,
         _lib_name: &str,
         _dll_imports: &[rustc_session::cstore::DllImport],
-        _tmpdir: &rustc_data_structures::temp_dir::MaybeTempDir,
-    ) {
-        bug!("injecting dll imports is not supported");
+        _tmpdir: &Path,
+    ) -> PathBuf {
+        bug!("creating dll imports is not supported");
     }
 }
diff --git a/compiler/rustc_codegen_gcc/src/archive.rs b/compiler/rustc_codegen_gcc/src/archive.rs
@@ -4,7 +4,6 @@ use std::path::{Path, PathBuf};
 use rustc_codegen_ssa::back::archive::ArchiveBuilder;
 use rustc_session::Session;
 
-use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_session::cstore::DllImport;
 
 struct ArchiveConfig<'a> {
@@ -177,7 +176,16 @@ impl<'a> ArchiveBuilder<'a> for ArArchiveBuilder<'a> {
         any_members
     }
 
-    fn inject_dll_import_lib(&mut self, _lib_name: &str, _dll_imports: &[DllImport], _tmpdir: &MaybeTempDir) {
+    fn sess(&self) -> &Session {
+        self.config.sess
+    }
+
+    fn create_dll_import_lib(
+        _sess: &Session,
+        _lib_name: &str,
+        _dll_imports: &[DllImport],
+        _tmpdir: &Path,
+    ) -> PathBuf {
         unimplemented!();
     }
 }
diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs
@@ -11,7 +11,6 @@ use std::str;
 use crate::llvm::archive_ro::{ArchiveRO, Child};
 use crate::llvm::{self, ArchiveKind, LLVMMachineType, LLVMRustCOFFShortExport};
 use rustc_codegen_ssa::back::archive::ArchiveBuilder;
-use rustc_data_structures::temp_dir::MaybeTempDir;
 use rustc_session::cstore::{DllCallingConvention, DllImport};
 use rustc_session::Session;
 
@@ -96,19 +95,23 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
         }
     }
 
-    fn inject_dll_import_lib(
-        &mut self,
+    fn sess(&self) -> &Session {
+        self.sess
+    }
+
+    fn create_dll_import_lib(
+        sess: &Session,
         lib_name: &str,
         dll_imports: &[DllImport],
-        tmpdir: &MaybeTempDir,
-    ) {
+        tmpdir: &Path,
+    ) -> PathBuf {
         let output_path = {
-            let mut output_path: PathBuf = tmpdir.as_ref().to_path_buf();
+            let mut output_path: PathBuf = tmpdir.to_path_buf();
             output_path.push(format!("{}_imports", lib_name));
             output_path.with_extension("lib")
         };
 
-        let target = &self.sess.target;
+        let target = &sess.target;
         let mingw_gnu_toolchain = target.vendor == "pc"
             && target.os == "windows"
             && target.env == "gnu"
@@ -117,7 +120,7 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
         let import_name_and_ordinal_vector: Vec<(String, Option<u16>)> = dll_imports
             .iter()
             .map(|import: &DllImport| {
-                if self.sess.target.arch == "x86" {
+                if sess.target.arch == "x86" {
                     (
                         LlvmArchiveBuilder::i686_decorated_name(import, mingw_gnu_toolchain),
                         import.ordinal,
@@ -134,8 +137,7 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
             // 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.
-            let def_file_path =
-                tmpdir.as_ref().join(format!("{}_imports", lib_name)).with_extension("def");
+            let def_file_path = tmpdir.join(format!("{}_imports", lib_name)).with_extension("def");
 
             let def_file_content = format!(
                 "EXPORTS\n{}",
@@ -154,11 +156,11 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
             match std::fs::write(&def_file_path, def_file_content) {
                 Ok(_) => {}
                 Err(e) => {
-                    self.sess.fatal(&format!("Error writing .DEF file: {}", e));
+                    sess.fatal(&format!("Error writing .DEF file: {}", e));
                 }
             };
 
-            let dlltool = find_binutils_dlltool(self.sess);
+            let dlltool = find_binutils_dlltool(sess);
             let result = std::process::Command::new(dlltool)
                 .args([
                     "-d",
@@ -172,9 +174,9 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
 
             match result {
                 Err(e) => {
-                    self.sess.fatal(&format!("Error calling dlltool: {}", e));
+                    sess.fatal(&format!("Error calling dlltool: {}", e));
                 }
-                Ok(output) if !output.status.success() => self.sess.fatal(&format!(
+                Ok(output) if !output.status.success() => sess.fatal(&format!(
                     "Dlltool could not create import library: {}\n{}",
                     String::from_utf8_lossy(&output.stdout),
                     String::from_utf8_lossy(&output.stderr)
@@ -220,27 +222,21 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
                     output_path_z.as_ptr(),
                     ffi_exports.as_ptr(),
                     ffi_exports.len(),
-                    llvm_machine_type(&self.sess.target.arch) as u16,
-                    !self.sess.target.is_like_msvc,
+                    llvm_machine_type(&sess.target.arch) as u16,
+                    !sess.target.is_like_msvc,
                 )
             };
 
             if result == crate::llvm::LLVMRustResult::Failure {
-                self.sess.fatal(&format!(
+                sess.fatal(&format!(
                     "Error creating import library for {}: {}",
                     lib_name,
                     llvm::last_error().unwrap_or("unknown LLVM error".to_string())
                 ));
             }
         };
 
-        self.add_archive(&output_path, |_| false).unwrap_or_else(|e| {
-            self.sess.fatal(&format!(
-                "failed to add native library {}: {}",
-                output_path.display(),
-                e
-            ));
-        });
+        output_path
     }
 }
 
diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs
@@ -51,10 +51,37 @@ pub trait ArchiveBuilder<'a> {
 
     fn build(self) -> bool;
 
+    fn sess(&self) -> &Session;
+
+    /// Creates a DLL Import Library <https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-creation#creating-an-import-library>.
+    /// and returns the path on disk to that import library.
+    /// This functions doesn't take `self` so that it can be called from
+    /// `linker_with_args`, which is specialized on `ArchiveBuilder` but
+    /// doesn't take or create an instance of that type.
+    fn create_dll_import_lib(
+        sess: &Session,
+        lib_name: &str,
+        dll_imports: &[DllImport],
+        tmpdir: &Path,
+    ) -> PathBuf;
+
+    /// Creates a DLL Import Library <https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-creation#creating-an-import-library>
+    /// and adds it to the current compilation's set of archives.
     fn inject_dll_import_lib(
         &mut self,
         lib_name: &str,
         dll_imports: &[DllImport],
         tmpdir: &MaybeTempDir,
-    );
+    ) {
+        let output_path =
+            Self::create_dll_import_lib(self.sess(), lib_name, dll_imports, tmpdir.as_ref());
+
+        self.add_archive(&output_path, |_| false).unwrap_or_else(|e| {
+            self.sess().fatal(&format!(
+                "failed to add native library {}: {}",
+                output_path.display(),
+                e
+            ));
+        });
+    }
 }
diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -120,7 +120,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(
                         &out_filename,
                         codegen_results,
                         path.as_ref(),
-                    );
+                    )?;
                 }
             }
             if sess.opts.json_artifact_notifications {
@@ -650,7 +650,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>(
     out_filename: &Path,
     codegen_results: &CodegenResults,
     tmpdir: &Path,
-) {
+) -> Result<(), ErrorGuaranteed> {
     info!("preparing {:?} to {:?}", crate_type, out_filename);
     let (linker_path, flavor) = linker_and_flavor(sess);
     let mut cmd = linker_with_args::<B>(
@@ -661,7 +661,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>(
         tmpdir,
         out_filename,
         codegen_results,
-    );
+    )?;
 
     linker::disable_localization(&mut cmd);
 
@@ -1000,6 +1000,8 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>(
             (Strip::None, _) => {}
         }
     }
+
+    Ok(())
 }
 
 // Temporarily support both -Z strip and -C strip
@@ -1848,7 +1850,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
     tmpdir: &Path,
     out_filename: &Path,
     codegen_results: &CodegenResults,
-) -> Command {
+) -> Result<Command, ErrorGuaranteed> {
     let crt_objects_fallback = crt_objects_fallback(sess, crate_type);
     let cmd = &mut *super::linker::get_linker(
         sess,
@@ -1955,6 +1957,18 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
         add_upstream_native_libraries(cmd, sess, codegen_results);
     }
 
+    // Link with the import library generated for any raw-dylib functions.
+    for (raw_dylib_name, raw_dylib_imports) in
+        collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)?
+    {
+        cmd.add_object(&B::create_dll_import_lib(
+            sess,
+            &raw_dylib_name,
+            &raw_dylib_imports,
+            tmpdir,
+        ));
+    }
+
     // Library linking above uses some global state for things like `-Bstatic`/`-Bdynamic` to make
     // command line shorter, reset it to default here before adding more libraries.
     cmd.reset_per_library_state();
@@ -1998,7 +2012,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
     // to it and remove the option.
     add_post_link_args(cmd, sess, flavor);
 
-    cmd.take_cmd()
+    Ok(cmd.take_cmd())
 }
 
 fn add_order_independent_options(
@@ -2227,8 +2241,7 @@ fn add_local_native_libraries(
                 }
             }
             NativeLibKind::RawDylib => {
-                // FIXME(#58713): Proper handling for raw dylibs.
-                bug!("raw_dylib feature not yet implemented");
+                // Ignore RawDylib here, they are handled separately in linker_with_args().
             }
         }
     }
diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk
@@ -90,7 +90,7 @@ NATIVE_STATICLIB = $(TMPDIR)/$(call NATIVE_STATICLIB_FILE,$(1))
 OUT_EXE=-Fe:`cygpath -w $(TMPDIR)/$(call BIN,$(1))` \
 	-Fo:`cygpath -w $(TMPDIR)/$(1).obj`
 else
-COMPILE_OBJ = $(CC) -c -o $(1) $(2)
+COMPILE_OBJ = $(CC) -v -c -o $(1) $(2)
 COMPILE_OBJ_CXX = $(CXX) -c -o $(1) $(2)
 NATIVE_STATICLIB_FILE = lib$(1).a
 NATIVE_STATICLIB = $(call STATICLIB,$(1))
diff --git a/src/test/run-make/raw-dylib-alt-calling-convention/Makefile b/src/test/run-make/raw-dylib-alt-calling-convention/Makefile
@@ -6,14 +6,14 @@
 -include ../../run-make-fulldeps/tools.mk
 
 all:
+	$(RUSTC) --crate-type lib --crate-name raw_dylib_alt_calling_convention_test lib.rs
+	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	$(call COMPILE_OBJ,"$(TMPDIR)"/extern.obj,extern.c)
 ifdef IS_MSVC
-	$(CC) "$(TMPDIR)"/extern.obj -link -dll -out:"$(TMPDIR)"/extern.dll
+	$(CC) "$(TMPDIR)"/extern.obj -link -dll -out:"$(TMPDIR)"/extern.dll -noimplib
 else
 	$(CC) "$(TMPDIR)"/extern.obj -shared -o "$(TMPDIR)"/extern.dll
 endif
-	$(RUSTC) --crate-type lib --crate-name raw_dylib_alt_calling_convention_test lib.rs
-	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	"$(TMPDIR)"/driver > "$(TMPDIR)"/output.txt
 
 ifdef RUSTC_BLESS_TEST
diff --git a/src/test/run-make/raw-dylib-c/Makefile b/src/test/run-make/raw-dylib-c/Makefile
@@ -5,21 +5,24 @@
 -include ../../run-make-fulldeps/tools.mk
 
 all:
+	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
+	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
+	$(RUSTC) --crate-type bin --crate-name raw_dylib_test_bin lib.rs
 	$(call COMPILE_OBJ,"$(TMPDIR)"/extern_1.obj,extern_1.c)
 	$(call COMPILE_OBJ,"$(TMPDIR)"/extern_2.obj,extern_2.c)
 ifdef IS_MSVC
-	$(CC) "$(TMPDIR)"/extern_1.obj -link -dll -out:"$(TMPDIR)"/extern_1.dll
-	$(CC) "$(TMPDIR)"/extern_2.obj -link -dll -out:"$(TMPDIR)"/extern_2.dll
+	$(CC) "$(TMPDIR)"/extern_1.obj -link -dll -out:"$(TMPDIR)"/extern_1.dll -noimplib
+	$(CC) "$(TMPDIR)"/extern_2.obj -link -dll -out:"$(TMPDIR)"/extern_2.dll -noimplib
 else
 	$(CC) "$(TMPDIR)"/extern_1.obj -shared -o "$(TMPDIR)"/extern_1.dll
 	$(CC) "$(TMPDIR)"/extern_2.obj -shared -o "$(TMPDIR)"/extern_2.dll
 endif
-	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
-	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	"$(TMPDIR)"/driver > "$(TMPDIR)"/output.txt
+	"$(TMPDIR)"/raw_dylib_test_bin > "$(TMPDIR)"/output_bin.txt
 
 ifdef RUSTC_BLESS_TEST
 	cp "$(TMPDIR)"/output.txt output.txt
 else
 	$(DIFF) output.txt "$(TMPDIR)"/output.txt
+	$(DIFF) output.txt "$(TMPDIR)"/output_bin.txt
 endif
diff --git a/src/test/run-make/raw-dylib-c/lib.rs b/src/test/run-make/raw-dylib-c/lib.rs
@@ -20,3 +20,7 @@ pub fn library_function() {
         extern_fn_3();
     }
 }
+
+fn main() {
+    library_function();
+}
diff --git a/src/test/run-make/raw-dylib-link-ordinal/Makefile b/src/test/run-make/raw-dylib-link-ordinal/Makefile
@@ -5,14 +5,14 @@
 -include ../../run-make-fulldeps/tools.mk
 
 all:
+	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
+	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	$(call COMPILE_OBJ,"$(TMPDIR)"/exporter.obj,exporter.c)
 ifdef IS_MSVC
-	$(CC) "$(TMPDIR)"/exporter.obj exporter.def -link -dll -out:"$(TMPDIR)"/exporter.dll
+	$(CC) "$(TMPDIR)"/exporter.obj exporter.def -link -dll -out:"$(TMPDIR)"/exporter.dll -noimplib
 else
 	$(CC) "$(TMPDIR)"/exporter.obj exporter.def -shared -o "$(TMPDIR)"/exporter.dll
 endif
-	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
-	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	"$(TMPDIR)"/driver > "$(TMPDIR)"/output.txt
 
 ifdef RUSTC_BLESS_TEST
diff --git a/src/test/run-make/raw-dylib-stdcall-ordinal/Makefile b/src/test/run-make/raw-dylib-stdcall-ordinal/Makefile
@@ -6,14 +6,14 @@
 -include ../../run-make-fulldeps/tools.mk
 
 all:
+	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
+	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	$(call COMPILE_OBJ,"$(TMPDIR)"/exporter.obj,exporter.c)
 ifdef IS_MSVC
-	$(CC) "$(TMPDIR)"/exporter.obj exporter-msvc.def -link -dll -out:"$(TMPDIR)"/exporter.dll
+	$(CC) "$(TMPDIR)"/exporter.obj exporter-msvc.def -link -dll -out:"$(TMPDIR)"/exporter.dll -noimplib
 else
 	$(CC) "$(TMPDIR)"/exporter.obj exporter-gnu.def -shared -o "$(TMPDIR)"/exporter.dll
 endif
-	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs
-	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)"
 	"$(TMPDIR)"/driver > "$(TMPDIR)"/actual_output.txt
 
 ifdef RUSTC_BLESS_TEST

Original file line number	Diff line number	Diff line change
`@@ -20,3 +20,7 @@ pub fn library_function() {`
`20`	`20`	`extern_fn_3();`
`21`	`21`	`}`
`22`	`22`	`}`
	`23`	`+`
	`24`	`+fn main() {`
	`25`	`+ library_function();`
	`26`	`+}`