about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-25 04:57:51 +0000
committerbors <bors@rust-lang.org>2022-10-25 04:57:51 +0000
commit31d754a1dfba4d091a05589459708f286d5a231e (patch)
tree25ea743dd19ec2f0eb44d53ee688235c2cb614c8
parentf2702e922ba31e49d6167f5651d4545646dcf22d (diff)
parent3a1ef50b34d3c88ddc423411be4521499cf9def5 (diff)
downloadrust-31d754a1dfba4d091a05589459708f286d5a231e.tar.gz
rust-31d754a1dfba4d091a05589459708f286d5a231e.zip
Auto merge of #102988 - dpaoliello:inlinerawdylib, r=dpaoliello
Support raw-dylib functions being used inside inlined functions

Fixes #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.
-rw-r--r--compiler/rustc_codegen_cranelift/src/archive.rs1
-rw-r--r--compiler/rustc_codegen_gcc/src/archive.rs1
-rw-r--r--compiler/rustc_codegen_llvm/src/back/archive.rs7
-rw-r--r--compiler/rustc_codegen_ssa/src/back/archive.rs1
-rw-r--r--compiler/rustc_codegen_ssa/src/back/link.rs41
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/Makefile31
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/driver.rs21
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/extern_1.c11
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/extern_2.c6
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/lib.rs21
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/lib_wrapper.rs6
-rw-r--r--src/test/run-make/raw-dylib-inline-cross-dylib/output.txt6
12 files changed, 146 insertions, 7 deletions
diff --git a/compiler/rustc_codegen_cranelift/src/archive.rs b/compiler/rustc_codegen_cranelift/src/archive.rs
index 31d3d0e0615..f2e3bf16e61 100644
--- a/compiler/rustc_codegen_cranelift/src/archive.rs
+++ b/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");
     }
diff --git a/compiler/rustc_codegen_gcc/src/archive.rs b/compiler/rustc_codegen_gcc/src/archive.rs
index ac0342f6b80..f18ae7ea5e9 100644
--- a/compiler/rustc_codegen_gcc/src/archive.rs
+++ b/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!();
     }
diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs
index 20a063f80fd..082665bba38 100644
--- a/compiler/rustc_codegen_llvm/src/back/archive.rs
+++ b/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{}",
diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs
index bad58d0a8a0..bb76ca5d2b9 100644
--- a/compiler/rustc_codegen_ssa/src/back/archive.rs
+++ b/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(
diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index a0b5e3b6daf..0dc0dee862c 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/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,
         ));
     }
 
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/Makefile b/src/test/run-make/raw-dylib-inline-cross-dylib/Makefile
new file mode 100644
index 00000000000..9e603f95835
--- /dev/null
+++ b/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
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/driver.rs b/src/test/run-make/raw-dylib-inline-cross-dylib/driver.rs
new file mode 100644
index 00000000000..f72ded7d9f6
--- /dev/null
+++ b/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();
+    }
+}
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/extern_1.c b/src/test/run-make/raw-dylib-inline-cross-dylib/extern_1.c
new file mode 100644
index 00000000000..e5baaf5f040
--- /dev/null
+++ b/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);
+}
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/extern_2.c b/src/test/run-make/raw-dylib-inline-cross-dylib/extern_2.c
new file mode 100644
index 00000000000..30aa4692238
--- /dev/null
+++ b/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);
+}
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/lib.rs b/src/test/run-make/raw-dylib-inline-cross-dylib/lib.rs
new file mode 100644
index 00000000000..00c2c1c42d1
--- /dev/null
+++ b/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();
+    }
+}
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/lib_wrapper.rs b/src/test/run-make/raw-dylib-inline-cross-dylib/lib_wrapper.rs
new file mode 100644
index 00000000000..47191b8de21
--- /dev/null
+++ b/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();
+}
diff --git a/src/test/run-make/raw-dylib-inline-cross-dylib/output.txt b/src/test/run-make/raw-dylib-inline-cross-dylib/output.txt
new file mode 100644
index 00000000000..e7009baa0d4
--- /dev/null
+++ b/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