Auto merge of rust-lang#102988 - dpaoliello:inlinerawdylib, r=dpaoliello

Support raw-dylib functions being used inside inlined functions

Fixes rust-lang#102714

Issue Details:
When generating the import library for `raw-dylib` symbols, we currently only use the functions and variables declared within the current crate. This works fine if all crates are static libraries or `rlib`s as the generated import library will be contained in the static library or `rlib` itself, but if a dependency is a dynamic library AND the use of a `raw-dylib` function or variable is inlined or part of a generic instantiation then the current crate won't see its dependency's import library and so linking will fail.

Fix Details:
Instead, when we generate the import library for a `dylib` or `bin` crate, we will now generate it for the symbols both for the current crate and all upstream crates. We do this in two steps so that the import library for the current crate is passed into the linker first, thus it is preferred if there are any ambiguous symbols.

Author: bors

compiler/rustc_codegen_cranelift/src/archive.rs

@@ -38,6 +38,7 @@ impl ArchiveBuilderBuilder for ArArchiveBuilderBuilder {
         _lib_name: &str,
         _dll_imports: &[rustc_session::cstore::DllImport],
         _tmpdir: &Path,
+        _is_direct_dependency: bool,
     ) -> PathBuf {
         bug!("creating dll imports is not supported");
     }

compiler/rustc_codegen_gcc/src/archive.rs

@@ -47,6 +47,7 @@ impl ArchiveBuilderBuilder for ArArchiveBuilderBuilder {
         _lib_name: &str,
         _dll_imports: &[DllImport],
         _tmpdir: &Path,
+        _is_direct_dependency: bool,
     ) -> PathBuf {
         unimplemented!();
     }

compiler/rustc_codegen_llvm/src/back/archive.rs

@@ -165,10 +165,12 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder {
         lib_name: &str,
         dll_imports: &[DllImport],
         tmpdir: &Path,
+        is_direct_dependency: bool,
     ) -> PathBuf {
+        let name_suffix = if is_direct_dependency { "_imports" } else { "_imports_indirect" };
         let output_path = {
             let mut output_path: PathBuf = tmpdir.to_path_buf();
-            output_path.push(format!("{}_imports", lib_name));
+            output_path.push(format!("{}{}", lib_name, name_suffix));
             output_path.with_extension("lib")
         };
 
@@ -195,7 +197,8 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder {
             // 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.join(format!("{}_imports", lib_name)).with_extension("def");
+            let def_file_path =
+                tmpdir.join(format!("{}{}", lib_name, name_suffix)).with_extension("def");
 
             let def_file_content = format!(
                 "EXPORTS\n{}",

compiler/rustc_codegen_ssa/src/back/archive.rs

@@ -25,6 +25,7 @@ pub trait ArchiveBuilderBuilder {
         lib_name: &str,
         dll_imports: &[DllImport],
         tmpdir: &Path,
+        is_direct_dependency: bool,
     ) -> PathBuf;
 
     fn extract_bundled_libs(

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -391,13 +391,14 @@ fn link_rlib<'a>(
     }
 
     for (raw_dylib_name, raw_dylib_imports) in
-        collate_raw_dylibs(sess, &codegen_results.crate_info.used_libraries)?
+        collate_raw_dylibs(sess, codegen_results.crate_info.used_libraries.iter())?
     {
         let output_path = archive_builder_builder.create_dll_import_lib(
             sess,
             &raw_dylib_name,
             &raw_dylib_imports,
             tmpdir.as_ref(),
+            true,
         );
 
         ab.add_archive(&output_path, Box::new(|_| false)).unwrap_or_else(|error| {
@@ -449,9 +450,9 @@ fn link_rlib<'a>(
 /// then the CodegenResults value contains one NativeLib instance for each block.  However, the
 /// linker appears to expect only a single import library for each library used, so we need to
 /// collate the symbols together by library name before generating the import libraries.
-fn collate_raw_dylibs(
-    sess: &Session,
-    used_libraries: &[NativeLib],
+fn collate_raw_dylibs<'a, 'b>(
+    sess: &'a Session,
+    used_libraries: impl IntoIterator<Item = &'b NativeLib>,
 ) -> Result<Vec<(String, Vec<DllImport>)>, ErrorGuaranteed> {
     // Use index maps to preserve original order of imports and libraries.
     let mut dylib_table = FxIndexMap::<String, FxIndexMap<Symbol, &DllImport>>::default();
@@ -2068,13 +2069,43 @@ fn linker_with_args<'a>(
 
     // 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)?
+        collate_raw_dylibs(sess, codegen_results.crate_info.used_libraries.iter())?
+    {
+        cmd.add_object(&archive_builder_builder.create_dll_import_lib(
+            sess,
+            &raw_dylib_name,
+            &raw_dylib_imports,
+            tmpdir,
+            true,
+        ));
+    }
+    // As with add_upstream_native_libraries, we need to add the upstream raw-dylib symbols in case
+    // they are used within inlined functions or instantiated generic functions. We do this *after*
+    // handling the raw-dylib symbols in the current crate to make sure that those are chosen first
+    // by the linker.
+    let (_, dependency_linkage) = codegen_results
+        .crate_info
+        .dependency_formats
+        .iter()
+        .find(|(ty, _)| *ty == crate_type)
+        .expect("failed to find crate type in dependency format list");
+    let native_libraries_from_nonstatics = codegen_results
+        .crate_info
+        .native_libraries
+        .iter()
+        .filter_map(|(cnum, libraries)| {
+            (dependency_linkage[cnum.as_usize() - 1] != Linkage::Static).then(|| libraries)
+        })
+        .flatten();
+    for (raw_dylib_name, raw_dylib_imports) in
+        collate_raw_dylibs(sess, native_libraries_from_nonstatics)?
     {
         cmd.add_object(&archive_builder_builder.create_dll_import_lib(
             sess,
             &raw_dylib_name,
             &raw_dylib_imports,
             tmpdir,
+            false,
         ));
     }
 

src/test/run-make/raw-dylib-inline-cross-dylib/Makefile

@@ -0,0 +1,31 @@
+# Regression test for calling an inline function that uses a raw-dylib function.
+
+# only-windows
+
+include ../../run-make-fulldeps/tools.mk
+
+all:
+	$(RUSTC) --crate-type dylib --crate-name raw_dylib_test lib.rs -C prefer-dynamic
+	$(RUSTC) --crate-type dylib --crate-name raw_dylib_test_wrapper lib_wrapper.rs -C prefer-dynamic
+	$(RUSTC) --crate-type bin driver.rs -L "$(TMPDIR)" -C prefer-dynamic
+	# Make sure we don't find an import to the functions we expect to be inlined.
+	"$(LLVM_BIN_DIR)"/llvm-objdump -p $(TMPDIR)/driver.exe | $(CGREP) -v -e "inline_library_function"
+	"$(LLVM_BIN_DIR)"/llvm-objdump -p $(TMPDIR)/driver.exe | $(CGREP) -v -e "inline_library_function_calls_inline"
+	# Make sure we do find an import to the functions we expect to be imported.
+	"$(LLVM_BIN_DIR)"/llvm-objdump -p $(TMPDIR)/driver.exe | $(CGREP) -e "library_function"
+	$(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 -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
+	$(call RUN,driver) > "$(TMPDIR)"/output.txt
+
+ifdef RUSTC_BLESS_TEST
+	cp "$(TMPDIR)"/output.txt output.txt
+else
+	$(DIFF) output.txt "$(TMPDIR)"/output.txt
+endif

src/test/run-make/raw-dylib-inline-cross-dylib/driver.rs

@@ -0,0 +1,21 @@
+#![feature(raw_dylib)]
+
+extern crate raw_dylib_test;
+extern crate raw_dylib_test_wrapper;
+
+#[link(name = "extern_2", kind = "raw-dylib")]
+extern {
+    fn extern_fn_2();
+}
+
+fn main() {
+    // NOTE: The inlined call to `extern_fn_2` links against the function in extern_2.dll instead
+    // of extern_1.dll since raw-dylib symbols from the current crate are passed to the linker
+    // first, so any ambiguous names will prefer the current crate's definition.
+    raw_dylib_test::inline_library_function();
+    raw_dylib_test::library_function();
+    raw_dylib_test_wrapper::inline_library_function_calls_inline();
+    unsafe {
+        extern_fn_2();
+    }
+}

src/test/run-make/raw-dylib-inline-cross-dylib/extern_1.c

@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+__declspec(dllexport) void extern_fn_1() {
+    printf("extern_fn_1\n");
+    fflush(stdout);
+}
+
+__declspec(dllexport) void extern_fn_2() {
+    printf("extern_fn_2 in extern_1\n");
+    fflush(stdout);
+}

src/test/run-make/raw-dylib-inline-cross-dylib/extern_2.c

@@ -0,0 +1,6 @@
+#include <stdio.h>
+
+__declspec(dllexport) void extern_fn_2() {
+    printf("extern_fn_2 in extern_2\n");
+    fflush(stdout);
+}

src/test/run-make/raw-dylib-inline-cross-dylib/lib.rs

@@ -0,0 +1,21 @@
+#![feature(raw_dylib)]
+
+#[link(name = "extern_1", kind = "raw-dylib")]
+extern {
+    fn extern_fn_1();
+    fn extern_fn_2();
+}
+
+#[inline]
+pub fn inline_library_function() {
+    unsafe {
+        extern_fn_1();
+        extern_fn_2();
+    }
+}
+
+pub fn library_function() {
+    unsafe {
+        extern_fn_2();
+    }
+}

src/test/run-make/raw-dylib-inline-cross-dylib/lib_wrapper.rs

@@ -0,0 +1,6 @@
+extern crate raw_dylib_test;
+
+#[inline]
+pub fn inline_library_function_calls_inline() {
+    raw_dylib_test::inline_library_function();
+}

src/test/run-make/raw-dylib-inline-cross-dylib/output.txt

@@ -0,0 +1,6 @@
+extern_fn_1
+extern_fn_2 in extern_2
+extern_fn_2 in extern_1
+extern_fn_1
+extern_fn_2 in extern_2
+extern_fn_2 in extern_2