From c59b82353d70122b59f5bc428be9464f49a99326 Mon Sep 17 00:00:00 2001
From: James Farrell <jamesfarrell@google.com>
Date: Tue, 13 Jun 2023 16:16:06 +0000
Subject: [PATCH] Better diagnostics for dlltool errors.

When dlltool fails, show the full command that was executed. In
particular, llvm-dlltool is not very helpful, printing a generic usage
message rather than what actually went wrong, so stdout and stderr
aren't of much use when troubleshooting.
---
 compiler/rustc_codegen_llvm/messages.ftl      |  3 +-
 .../rustc_codegen_llvm/src/back/archive.rs    | 41 +++++++++++--------
 compiler/rustc_codegen_llvm/src/errors.rs     |  2 +
 .../rfcs/rfc-2627-raw-dylib/dlltool-failed.rs |  4 ++
 .../rfc-2627-raw-dylib/dlltool-failed.stderr  |  3 +-
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl
index de1622951fe..aed4a8f3c85 100644
--- a/compiler/rustc_codegen_llvm/messages.ftl
+++ b/compiler/rustc_codegen_llvm/messages.ftl
@@ -1,7 +1,8 @@
 codegen_llvm_copy_bitcode = failed to copy bitcode to object file: {$err}
 
 codegen_llvm_dlltool_fail_import_library =
-    Dlltool could not create import library: {$stdout}
+    Dlltool could not create import library with {$dlltool_path} {$dlltool_args}:
+    {$stdout}
     {$stderr}
 
 codegen_llvm_dynamic_linking_with_lto =
diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs
index a6416e9540c..10bf954d242 100644
--- a/compiler/rustc_codegen_llvm/src/back/archive.rs
+++ b/compiler/rustc_codegen_llvm/src/back/archive.rs
@@ -198,25 +198,24 @@ fn create_dll_import_lib(
                 "arm" => ("arm", "--32"),
                 _ => panic!("unsupported arch {}", sess.target.arch),
             };
-            let result = std::process::Command::new(&dlltool)
-                .args([
-                    "-d",
-                    def_file_path.to_str().unwrap(),
-                    "-D",
-                    lib_name,
-                    "-l",
-                    output_path.to_str().unwrap(),
-                    "-m",
-                    dlltool_target_arch,
-                    "-f",
-                    dlltool_target_bitness,
-                    "--no-leading-underscore",
-                    "--temp-prefix",
-                    temp_prefix.to_str().unwrap(),
-                ])
-                .output();
+            let mut dlltool_cmd = std::process::Command::new(&dlltool);
+            dlltool_cmd.args([
+                "-d",
+                def_file_path.to_str().unwrap(),
+                "-D",
+                lib_name,
+                "-l",
+                output_path.to_str().unwrap(),
+                "-m",
+                dlltool_target_arch,
+                "-f",
+                dlltool_target_bitness,
+                "--no-leading-underscore",
+                "--temp-prefix",
+                temp_prefix.to_str().unwrap(),
+            ]);
 
-            match result {
+            match dlltool_cmd.output() {
                 Err(e) => {
                     sess.emit_fatal(ErrorCallingDllTool {
                         dlltool_path: dlltool.to_string_lossy(),
@@ -226,6 +225,12 @@ fn create_dll_import_lib(
                 // dlltool returns '0' on failure, so check for error output instead.
                 Ok(output) if !output.stderr.is_empty() => {
                     sess.emit_fatal(DlltoolFailImportLibrary {
+                        dlltool_path: dlltool.to_string_lossy(),
+                        dlltool_args: dlltool_cmd
+                            .get_args()
+                            .map(|arg| arg.to_string_lossy())
+                            .collect::<Vec<_>>()
+                            .join(" "),
                         stdout: String::from_utf8_lossy(&output.stdout),
                         stderr: String::from_utf8_lossy(&output.stderr),
                     })
diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs
index 44869ced1ae..fced6d504d2 100644
--- a/compiler/rustc_codegen_llvm/src/errors.rs
+++ b/compiler/rustc_codegen_llvm/src/errors.rs
@@ -81,6 +81,8 @@ pub(crate) struct ErrorCallingDllTool<'a> {
 #[derive(Diagnostic)]
 #[diag(codegen_llvm_dlltool_fail_import_library)]
 pub(crate) struct DlltoolFailImportLibrary<'a> {
+    pub dlltool_path: Cow<'a, str>,
+    pub dlltool_args: String,
     pub stdout: Cow<'a, str>,
     pub stderr: Cow<'a, str>,
 }
diff --git a/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.rs b/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.rs
index d7a418959bf..880907b24b4 100644
--- a/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.rs
+++ b/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.rs
@@ -6,6 +6,10 @@
 // compile-flags: --crate-type lib --emit link
 // normalize-stderr-test: "[^ ']*/dlltool.exe" -> "$$DLLTOOL"
 // normalize-stderr-test: "[^ ]*/foo.def" -> "$$DEF_FILE"
+// normalize-stderr-test: "[^ ]*/foo.lib" -> "$$LIB_FILE"
+// normalize-stderr-test: "-m [^ ]*" -> "$$TARGET_MACHINE"
+// normalize-stderr-test: "-f [^ ]*" -> "$$ASM_FLAGS"
+// normalize-stderr-test: "--temp-prefix [^ ]*/foo.dll" -> "$$TEMP_PREFIX"
 #[link(name = "foo", kind = "raw-dylib")]
 extern "C" {
     // `@1` is an invalid name to export, as it usually indicates that something
diff --git a/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.stderr b/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.stderr
index 020ac6a2b67..cc532ccc451 100644
--- a/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.stderr
+++ b/tests/ui/rfcs/rfc-2627-raw-dylib/dlltool-failed.stderr
@@ -1,4 +1,5 @@
-error: Dlltool could not create import library: 
+error: Dlltool could not create import library with $DLLTOOL -d $DEF_FILE -D foo.dll -l $LIB_FILE $TARGET_MACHINE $ASM_FLAGS --no-leading-underscore $TEMP_PREFIX:
+       
        $DLLTOOL: Syntax error in def file $DEF_FILE:1
 
 error: aborting due to previous error