Rollup merge of #109677 - dpaoliello:rawdylib, r=michaelwoerister,wes…

…leywiser

Stabilize raw-dylib, link_ordinal, import_name_type and -Cdlltool

This stabilizes the `raw-dylib` feature (#58713) for all architectures (i.e., `x86` as it is already stable for all other architectures).

Changes:
* Permit the use of the `raw-dylib` link kind for x86, the `link_ordinal` attribute and the `import_name_type` key for the `link` attribute.
* Mark the `raw_dylib` feature as stable.
* Stabilized the `-Zdlltool` argument as `-Cdlltool`.
* Note the path to `dlltool` if invoking it failed (we don't need to do this if `dlltool` returns an error since it prints its path in the error message).
* Adds tests for `-Cdlltool`.
* Adds tests for being unable to find the dlltool executable, and dlltool failing.
* Fixes a bug where we were checking the exit code of dlltool to see if it failed, but dlltool always returns 0 (indicating success), so instead we need to check if anything was written to `stderr`.

NOTE: As previously noted (#104218 (comment)) using dlltool within rustc is temporary, but this is not the first time that Rust has added a temporary tool use and argument: #104218 (comment)

Big thanks to ``````@tbu-`````` for the first version of this PR (#104218)

compiler/rustc_codegen_llvm/messages.ftl

@@ -24,7 +24,7 @@ codegen_llvm_error_writing_def_file =
     Error writing .DEF file: {$error}
 
 codegen_llvm_error_calling_dlltool =
-    Error calling dlltool: {$error}
+    Error calling dlltool '{$dlltool_path}': {$error}
 
 codegen_llvm_dlltool_fail_import_library =
     Dlltool could not create import library: {$stdout}

compiler/rustc_codegen_llvm/src/back/archive.rs

@@ -198,7 +198,7 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder {
                 "arm" => ("arm", "--32"),
                 _ => panic!("unsupported arch {}", sess.target.arch),
             };
-            let result = std::process::Command::new(dlltool)
+            let result = std::process::Command::new(&dlltool)
                 .args([
                     "-d",
                     def_file_path.to_str().unwrap(),
@@ -218,9 +218,13 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder {
 
             match result {
                 Err(e) => {
-                    sess.emit_fatal(ErrorCallingDllTool { error: e });
+                    sess.emit_fatal(ErrorCallingDllTool {
+                        dlltool_path: dlltool.to_string_lossy(),
+                        error: e,
+                    });
                 }
-                Ok(output) if !output.status.success() => {
+                // dlltool returns '0' on failure, so check for error output instead.
+                Ok(output) if !output.stderr.is_empty() => {
                     sess.emit_fatal(DlltoolFailImportLibrary {
                         stdout: String::from_utf8_lossy(&output.stdout),
                         stderr: String::from_utf8_lossy(&output.stderr),
@@ -431,7 +435,7 @@ fn string_to_io_error(s: String) -> io::Error {
 
 fn find_binutils_dlltool(sess: &Session) -> OsString {
     assert!(sess.target.options.is_like_windows && !sess.target.options.is_like_msvc);
-    if let Some(dlltool_path) = &sess.opts.unstable_opts.dlltool {
+    if let Some(dlltool_path) = &sess.opts.cg.dlltool {
         return dlltool_path.clone().into_os_string();
     }
 

compiler/rustc_codegen_llvm/src/errors.rs

@@ -67,7 +67,8 @@ pub(crate) struct ErrorWritingDEFFile {
 
 #[derive(Diagnostic)]
 #[diag(codegen_llvm_error_calling_dlltool)]
-pub(crate) struct ErrorCallingDllTool {
+pub(crate) struct ErrorCallingDllTool<'a> {
+    pub dlltool_path: Cow<'a, str>,
     pub error: std::io::Error,
 }
 

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -592,15 +592,6 @@ fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
 
 fn check_link_ordinal(tcx: TyCtxt<'_>, attr: &ast::Attribute) -> Option<u16> {
     use rustc_ast::{LitIntType, LitKind, MetaItemLit};
-    if !tcx.features().raw_dylib && tcx.sess.target.arch == "x86" {
-        feature_err(
-            &tcx.sess.parse_sess,
-            sym::raw_dylib,
-            attr.span,
-            "`#[link_ordinal]` is unstable on x86",
-        )
-        .emit();
-    }
     let meta_item_list = attr.meta_item_list();
     let meta_item_list = meta_item_list.as_deref();
     let sole_meta_list = match meta_item_list {

compiler/rustc_feature/src/accepted.rs

@@ -280,6 +280,8 @@ declare_features! (
     (accepted, pub_restricted, "1.18.0", Some(32409), None),
     /// Allows use of the postfix `?` operator in expressions.
     (accepted, question_mark, "1.13.0", Some(31436), None),
+    /// Allows the use of raw-dylibs (RFC 2627).
+    (accepted, raw_dylib, "CURRENT_RUSTC_VERSION", Some(58713), None),
     /// Allows keywords to be escaped for use as identifiers.
     (accepted, raw_identifiers, "1.30.0", Some(48589), None),
     /// Allows relaxing the coherence rules such that

compiler/rustc_feature/src/active.rs

@@ -489,8 +489,6 @@ declare_features! (
     (active, precise_pointer_size_matching, "1.32.0", Some(56354), None),
     /// Allows macro attributes on expressions, statements and non-inline modules.
     (active, proc_macro_hygiene, "1.30.0", Some(54727), None),
-    /// Allows the use of raw-dylibs (RFC 2627).
-    (active, raw_dylib, "1.65.0", Some(58713), None),
     /// Allows `&raw const $place_expr` and `&raw mut $place_expr` expressions.
     (active, raw_ref_op, "1.41.0", Some(64490), None),
     /// Allows using the `#[register_tool]` attribute.

compiler/rustc_interface/src/tests.rs

@@ -547,6 +547,7 @@ fn test_codegen_options_tracking_hash() {
     untracked!(ar, String::from("abc"));
     untracked!(codegen_units, Some(42));
     untracked!(default_linker_libraries, true);
+    untracked!(dlltool, Some(PathBuf::from("custom_dlltool.exe")));
     untracked!(extra_filename, String::from("extra-filename"));
     untracked!(incremental, Some(String::from("abc")));
     // `link_arg` is omitted because it just forwards to `link_args`.
@@ -651,7 +652,6 @@ fn test_unstable_options_tracking_hash() {
     untracked!(assert_incr_state, Some(String::from("loaded")));
     untracked!(deduplicate_diagnostics, false);
     untracked!(dep_tasks, true);
-    untracked!(dlltool, Some(PathBuf::from("custom_dlltool.exe")));
     untracked!(dont_buffer_diagnostics, true);
     untracked!(dump_dep_graph, true);
     untracked!(dump_drop_tracking_cfg, Some("cfg.dot".to_string()));

compiler/rustc_metadata/src/native_libs.rs

@@ -161,14 +161,6 @@ impl<'tcx> Collector<'tcx> {
                             "raw-dylib" => {
                                 if !sess.target.is_like_windows {
                                     sess.emit_err(errors::FrameworkOnlyWindows { span });
-                                } else if !features.raw_dylib && sess.target.arch == "x86" {
-                                    feature_err(
-                                        &sess.parse_sess,
-                                        sym::raw_dylib,
-                                        span,
-                                        "link kind `raw-dylib` is unstable on x86",
-                                    )
-                                    .emit();
                                 }
                                 NativeLibKind::RawDylib
                             }
@@ -251,16 +243,6 @@ impl<'tcx> Collector<'tcx> {
                                 continue;
                             }
                         };
-                        if !features.raw_dylib {
-                            let span = item.name_value_literal_span().unwrap();
-                            feature_err(
-                                &sess.parse_sess,
-                                sym::raw_dylib,
-                                span,
-                                "import name type is unstable",
-                            )
-                            .emit();
-                        }
                         import_name_type = Some((link_import_name_type, item.span()));
                     }
                     _ => {

compiler/rustc_session/src/options.rs

@@ -1235,6 +1235,8 @@ options! {
         line-tables-only, limited, or full; default: 0)"),
     default_linker_libraries: bool = (false, parse_bool, [UNTRACKED],
         "allow the linker to link its default libraries (default: no)"),
+    dlltool: Option<PathBuf> = (None, parse_opt_pathbuf, [UNTRACKED],
+        "import library generation tool (ignored except when targeting windows-gnu)"),
     embed_bitcode: bool = (true, parse_bool, [TRACKED],
         "emit bitcode in rlibs (default: yes)"),
     extra_filename: String = (String::new(), parse_string, [UNTRACKED],
@@ -1391,8 +1393,6 @@ options! {
         (default: no)"),
     diagnostic_width: Option<usize> = (None, parse_opt_number, [UNTRACKED],
         "set the current output width for diagnostic truncation"),
-    dlltool: Option<PathBuf> = (None, parse_opt_pathbuf, [UNTRACKED],
-        "import library generation tool (windows-gnu only)"),
     dont_buffer_diagnostics: bool = (false, parse_bool, [UNTRACKED],
         "emit diagnostics rather than buffering (breaks NLL error downgrading, sorting) \
         (default: no)"),

src/doc/rustc/src/codegen-options/index.md

@@ -90,6 +90,14 @@ It takes one of the following values:
 For example, for gcc flavor linkers, this issues the `-nodefaultlibs` flag to
 the linker.
 
+## dlltool
+
+On `windows-gnu` targets, this flag controls which dlltool `rustc` invokes to
+generate import libraries when using the [`raw-dylib` link kind](../../reference/items/external-blocks.md#the-link-attribute).
+It takes a path to [the dlltool executable](https://sourceware.org/binutils/docs/binutils/dlltool.html).
+If this flag is not specified, a dlltool executable will be inferred based on
+the host environment and target.
+
 ## embed-bitcode
 
 This flag controls whether or not the compiler embeds LLVM bitcode into object

src/tools/compiletest/src/header/needs.rs

@@ -115,6 +115,11 @@ pub(super) fn handle_needs(
             condition: cache.x86_64_dlltool,
             ignore_reason: "ignored when dlltool for x86_64 is not present",
         },
+        Need {
+            name: "needs-dlltool",
+            condition: cache.dlltool,
+            ignore_reason: "ignored when dlltool for the current architecture is not present",
+        },
         Need {
             name: "needs-git-hash",
             condition: config.git_hash,
@@ -183,13 +188,25 @@ pub(super) struct CachedNeedsConditions {
     rust_lld: bool,
     i686_dlltool: bool,
     x86_64_dlltool: bool,
+    dlltool: bool,
 }
 
 impl CachedNeedsConditions {
     pub(super) fn load(config: &Config) -> Self {
         let path = std::env::var_os("PATH").expect("missing PATH environment variable");
         let path = std::env::split_paths(&path).collect::<Vec<_>>();
 
+        // On Windows, dlltool.exe is used for all architectures.
+        #[cfg(windows)]
+        let dlltool = path.iter().any(|dir| dir.join("dlltool.exe").is_file());
+
+        // For non-Windows, there are architecture specific dlltool binaries.
+        #[cfg(not(windows))]
+        let i686_dlltool = path.iter().any(|dir| dir.join("i686-w64-mingw32-dlltool").is_file());
+        #[cfg(not(windows))]
+        let x86_64_dlltool =
+            path.iter().any(|dir| dir.join("x86_64-w64-mingw32-dlltool").is_file());
+
         let target = &&*config.target;
         Self {
             sanitizer_support: std::env::var_os("RUSTC_SANITIZER_SUPPORT").is_some(),
@@ -225,17 +242,26 @@ impl CachedNeedsConditions {
                 .join(if config.host.contains("windows") { "rust-lld.exe" } else { "rust-lld" })
                 .exists(),
 
-            // On Windows, dlltool.exe is used for all architectures.
             #[cfg(windows)]
-            i686_dlltool: path.iter().any(|dir| dir.join("dlltool.exe").is_file()),
+            i686_dlltool: dlltool,
             #[cfg(windows)]
-            x86_64_dlltool: path.iter().any(|dir| dir.join("dlltool.exe").is_file()),
+            x86_64_dlltool: dlltool,
+            #[cfg(windows)]
+            dlltool,
 
             // For non-Windows, there are architecture specific dlltool binaries.
             #[cfg(not(windows))]
-            i686_dlltool: path.iter().any(|dir| dir.join("i686-w64-mingw32-dlltool").is_file()),
+            i686_dlltool,
+            #[cfg(not(windows))]
+            x86_64_dlltool,
             #[cfg(not(windows))]
-            x86_64_dlltool: path.iter().any(|dir| dir.join("x86_64-w64-mingw32-dlltool").is_file()),
+            dlltool: if config.matches_arch("x86") {
+                i686_dlltool
+            } else if config.matches_arch("x86_64") {
+                x86_64_dlltool
+            } else {
+                false
+            },
         }
     }
 }

tests/run-make/raw-dylib-alt-calling-convention/lib.rs

@@ -1,5 +1,4 @@
 #![feature(abi_vectorcall)]
-#![cfg_attr(target_arch = "x86", feature(raw_dylib))]
 
 #[repr(C)]
 #[derive(Clone)]

tests/run-make/raw-dylib-c/lib.rs

@@ -1,5 +1,3 @@
-#![feature(raw_dylib)]
-
 #[link(name = "extern_1.dll", kind = "raw-dylib", modifiers = "+verbatim")]
 extern {
     fn extern_fn_1();

tests/run-make/raw-dylib-cross-compilation/lib.rs

@@ -1,4 +1,3 @@
-#![feature(raw_dylib)]
 #![feature(no_core, lang_items)]
 #![no_std]
 #![no_core]

tests/run-make/raw-dylib-custom-dlltool/Makefile

@@ -0,0 +1,11 @@
+# Test using -Cdlltool to change where raw-dylib looks for the dlltool binary.
+
+# only-windows
+# only-gnu
+# needs-dlltool
+
+include ../tools.mk
+
+all:
+	$(RUSTC) --crate-type lib --crate-name raw_dylib_test lib.rs -Cdlltool=$(CURDIR)/script.cmd
+	$(DIFF) output.txt "$(TMPDIR)"/output.txt

tests/run-make/raw-dylib-custom-dlltool/lib.rs

@@ -0,0 +1,10 @@
+#[link(name = "extern_1", kind = "raw-dylib")]
+extern {
+    fn extern_fn_1();
+}
+
+pub fn library_function() {
+    unsafe {
+        extern_fn_1();
+    }
+}

tests/run-make/raw-dylib-custom-dlltool/output.txt

@@ -0,0 +1 @@
+Called dlltool via script.cmd

tests/run-make/raw-dylib-custom-dlltool/script.cmd

@@ -0,0 +1,2 @@
+echo Called dlltool via script.cmd> %TMPDIR%\output.txt
+dlltool.exe %*

tests/run-make/raw-dylib-import-name-type/driver.rs

@@ -1,4 +1,3 @@
-#![feature(raw_dylib)]
 #![feature(abi_vectorcall)]
 
 #[link(name = "extern", kind = "raw-dylib", import_name_type = "undecorated")]

tests/run-make/raw-dylib-inline-cross-dylib/driver.rs

@@ -1,5 +1,3 @@
-#![feature(raw_dylib)]
-
 extern crate raw_dylib_test;
 extern crate raw_dylib_test_wrapper;
 

tests/run-make/raw-dylib-inline-cross-dylib/lib.rs

@@ -1,5 +1,3 @@
-#![feature(raw_dylib)]
-
 #[link(name = "extern_1", kind = "raw-dylib")]
 extern {
     fn extern_fn_1();

tests/run-make/raw-dylib-link-ordinal/lib.rs

@@ -1,5 +1,3 @@
-#![cfg_attr(target_arch = "x86", feature(raw_dylib))]
-
 #[link(name = "exporter", kind = "raw-dylib")]
 extern {
     #[link_ordinal(13)]

tests/run-make/raw-dylib-stdcall-ordinal/lib.rs

@@ -1,5 +1,3 @@
-#![cfg_attr(target_arch = "x86", feature(raw_dylib))]
-
 #[link(name = "exporter", kind = "raw-dylib")]
 extern "stdcall" {
     #[link_ordinal(15)]