Improve gnu lib generation and msvc lib reproducibility (#2016)

.github/workflows/build.yml

@@ -187,7 +187,6 @@ jobs:
           cargo clippy -p test_window_long &&
           cargo clippy -p test_winrt &&
           cargo clippy -p tool_gnu &&
-          cargo clippy -p tool_gnullvm &&
           cargo clippy -p tool_ilrs &&
           cargo clippy -p tool_lib &&
           cargo clippy -p tool_msvc &&

.github/workflows/test.yml

@@ -171,7 +171,6 @@ jobs:
           cargo test --target ${{ matrix.target }} -p test_window_long &&
           cargo test --target ${{ matrix.target }} -p test_winrt &&
           cargo test --target ${{ matrix.target }} -p tool_gnu &&
-          cargo test --target ${{ matrix.target }} -p tool_gnullvm &&
           cargo test --target ${{ matrix.target }} -p tool_ilrs &&
           cargo test --target ${{ matrix.target }} -p tool_lib &&
           cargo test --target ${{ matrix.target }} -p tool_msvc &&

crates/tools/gnu/readme.md

@@ -1,21 +1,9 @@
-The Windows umbrella lib (targeting GNU tooling) is generated using the following steps:
+The Windows umbrella lib (targeting GNU and LLVM tooling) is generated using the following steps:
 
 0. Ensure MSYS2 MinGW environment is installed (https://www.mingw-w64.org/downloads/)
-1. Ensure `~\.cargo\config` has the following blocks to override toolchain defaults (customize paths as needed):
-
-    ```toml
-    [target.i686-pc-windows-gnu]
-    linker = "C:\\msys64\\mingw32\\bin\\i686-w64-mingw32-gcc.exe"
-    ar = "C:\\msys64\\mingw32\\bin\\ar.exe"
-
-    [target.x86_64-pc-windows-gnu]
-    linker = "C:\\msys64\\mingw64\\bin\\x86_64-w64-mingw32-gcc.exe"
-    ar = "C:\\msys64\\mingw64\\bin\\ar.exe"
-    ```
-
-1. Open `MSYS2 MinGW 32-bit`
+1. Open `MSYS2 MinGW 64-bit`
 2. Execute: `pacman -Syuu --noconfirm` (repeat until no further updates available)
-3. Repeat step 1 if needed
-4. Navigate to crate root
-5. Execute: `PATH=$USERPROFILE/.cargo/bin:$PATH cargo run -p tool_gnu --target i686-pc-windows-gnu`
-6. Repeat steps 1-5, using the `MSYS MinGW 64-bit` environment and `x86_64-pc-windows-gnu` target
+3. Execute: `pacman --needed -S mingw-w64-x86_64-llvm`
+4. Repeat step 1 if needed
+5. Navigate to crate root
+6. Execute: `PATH=$USERPROFILE/.cargo/bin:$PATH cargo run -p tool_gnu -- all`

crates/tools/gnu/src/main.rs

@@ -1,28 +1,39 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::io::prelude::*;
+use std::process::{Command, Stdio};
 
 fn main() {
-    let mut cmd = std::process::Command::new("where");
-    cmd.arg("dlltool.exe");
-
-    let output = cmd.output().unwrap();
-
-    if !output.status.success() {
-        println!("dlltool.exe not found");
-        return;
+    const ALL_PLATFORMS: [&str; 4] = ["x86_64_gnu", "i686_gnu", "x86_64_gnullvm", "aarch64_gnullvm"];
+    let mut platforms = BTreeSet::new();
+    for platform in std::env::args().skip(1) {
+        if ALL_PLATFORMS.contains(&&*platform) {
+            platforms.insert(platform);
+        } else if platform == "all" {
+            platforms.extend(ALL_PLATFORMS.iter().map(|s| s.to_string()));
+        } else {
+            eprintln!("Unknown platform: {}", platform);
+            return;
+        }
     }
-
-    let output = unsafe { core::str::from_utf8_unchecked(&output.stdout) };
-
-    let platform = if output.contains("mingw64") {
-        "x86_64_gnu"
-    } else if output.contains("mingw32") {
-        "i686_gnu"
-    } else {
-        println!("mingw not found");
+    if platforms.is_empty() {
+        eprintln!("Please specify at least one platform or use 'all' argument");
         return;
     };
 
+    for platform in platforms {
+        let tools = if platform.ends_with("_gnu") { &["dlltool", "ar", "objcopy"][..] } else { &["llvm-dlltool", "llvm-ar"][..] };
+        for tool in tools {
+            if Command::new(tool).stdout(Stdio::null()).stderr(Stdio::null()).spawn().is_err() {
+                eprintln!("Could not find {}. Is it in your $PATH?", tool);
+                return;
+            }
+        }
+
+        build_platform(&platform, tools[0], tools[1]);
+    }
+}
+
+fn build_platform(platform: &str, dlltool: &str, ar: &str) {
     println!("Platform: {}", platform);
 
     let libraries = lib::libraries();
@@ -31,17 +42,17 @@ fn main() {
     std::fs::create_dir_all(&output).unwrap();
 
     for (library, functions) in &libraries {
-        build_library(&output, library, functions, platform);
+        build_library(&output, dlltool, library, functions, platform);
     }
 
-    build_mri(&output, &libraries);
+    build_mri(&output, ar, &libraries);
 
     for library in libraries.keys() {
         std::fs::remove_file(output.join(format!("lib{}.a", library))).unwrap();
     }
 }
 
-fn build_library(output: &std::path::Path, library: &str, functions: &BTreeMap<String, lib::CallingConvention>, platform: &str) {
+fn build_library(output: &std::path::Path, dlltool: &str, library: &str, functions: &BTreeMap<String, lib::CallingConvention>, platform: &str) {
     println!("{}", library);
 
     // Note that we don't use set_extension as it confuses PathBuf when the library name includes a period.
@@ -63,14 +74,14 @@ EXPORTS
 
     for (function, calling_convention) in functions {
         match calling_convention {
-            lib::CallingConvention::Stdcall(params) if platform.eq("i686_gnu") => def.write_all(format!("{}@{}\n", function, params).as_bytes()).unwrap(),
-            _ => def.write_all(format!("{}\n", function).as_bytes()).unwrap(),
+            lib::CallingConvention::Stdcall(params) if platform.eq("i686_gnu") => def.write_all(format!("{}@{} @ 0\n", function, params).as_bytes()).unwrap(),
+            _ => def.write_all(format!("{} @ 0\n", function).as_bytes()).unwrap(),
         }
     }
 
     drop(def);
 
-    let mut cmd = std::process::Command::new("dlltool");
+    let mut cmd = Command::new(dlltool);
     cmd.current_dir(output);
 
     if platform.eq("i686_gnu") {
@@ -81,40 +92,58 @@ EXPORTS
     cmd.arg(format!("{}.def", library));
     cmd.arg("-l");
     cmd.arg(format!("lib{}.a", library));
-    // Ensure consistency in the prefixes used by dlltool.
-    cmd.arg("-t");
-    if library.contains('.') {
-        cmd.arg(format!("{}_", library).replace('.', "_").replace('-', "_"));
-    } else {
-        cmd.arg(format!("{}_dll_", library).replace('-', "_"));
+    cmd.arg("-m");
+    cmd.arg(match platform {
+        "i686_gnu" => "i386",
+        "x86_64_gnu" | "x86_64_gnullvm" => "i386:x86-64",
+        "aarch64_gnullvm" => "arm64",
+        _ => unreachable!(),
+    });
+    if dlltool == "dlltool" {
+        // Work around https://sourceware.org/bugzilla/show_bug.cgi?id=29497
+        cmd.arg("-f");
+        cmd.arg(match platform {
+            "i686_gnu" => "--32",
+            "x86_64_gnu" => "--64",
+            _ => unreachable!(),
+        });
+        // Ensure consistency in the prefixes used by dlltool.
+        cmd.arg("-t");
+        if library.contains('.') {
+            cmd.arg(format!("{}_", library).replace('.', "_").replace('-', "_"));
+        } else {
+            cmd.arg(format!("{}_dll_", library).replace('-', "_"));
+        }
     }
     cmd.output().unwrap();
 
-    // Work around lack of determinism in dlltool output, and at the same time remove
-    // unnecessary sections and symbols.
-    std::fs::rename(output.join(format!("lib{}.a", library)), output.join("tmp.a")).unwrap();
-    let mut cmd = std::process::Command::new("objcopy");
-    cmd.current_dir(output);
-    cmd.arg("--remove-section=.bss");
-    cmd.arg("--remove-section=.data");
-    cmd.arg("--strip-unneeded-symbol=fthunk");
-    cmd.arg("--strip-unneeded-symbol=hname");
-    cmd.arg("--strip-unneeded-symbol=.file");
-    cmd.arg("--strip-unneeded-symbol=.text");
-    cmd.arg("--strip-unneeded-symbol=.data");
-    cmd.arg("--strip-unneeded-symbol=.bss");
-    cmd.arg("--strip-unneeded-symbol=.idata$7");
-    cmd.arg("--strip-unneeded-symbol=.idata$5");
-    cmd.arg("--strip-unneeded-symbol=.idata$4");
-    cmd.arg("tmp.a");
-    cmd.arg(format!("lib{}.a", library));
-    cmd.output().unwrap();
-
-    std::fs::remove_file(output.join("tmp.a")).unwrap();
+    if dlltool == "dlltool" {
+        // Work around lack of determinism in dlltool output, and at the same time remove
+        // unnecessary sections and symbols.
+        std::fs::rename(output.join(format!("lib{}.a", library)), output.join("tmp.a")).unwrap();
+        let mut cmd = Command::new("objcopy");
+        cmd.current_dir(output);
+        cmd.arg("--remove-section=.bss");
+        cmd.arg("--remove-section=.data");
+        cmd.arg("--strip-unneeded-symbol=fthunk");
+        cmd.arg("--strip-unneeded-symbol=hname");
+        cmd.arg("--strip-unneeded-symbol=.file");
+        cmd.arg("--strip-unneeded-symbol=.text");
+        cmd.arg("--strip-unneeded-symbol=.data");
+        cmd.arg("--strip-unneeded-symbol=.bss");
+        cmd.arg("--strip-unneeded-symbol=.idata$7");
+        cmd.arg("--strip-unneeded-symbol=.idata$5");
+        cmd.arg("--strip-unneeded-symbol=.idata$4");
+        cmd.arg("tmp.a");
+        cmd.arg(format!("lib{}.a", library));
+        cmd.output().unwrap();
+
+        std::fs::remove_file(output.join("tmp.a")).unwrap();
+    }
     std::fs::remove_file(output.join(format!("{}.def", library))).unwrap();
 }
 
-fn build_mri(output: &std::path::Path, libraries: &BTreeMap<String, BTreeMap<String, lib::CallingConvention>>) {
+fn build_mri(output: &std::path::Path, ar: &str, libraries: &BTreeMap<String, BTreeMap<String, lib::CallingConvention>>) {
     let mri_path = output.join("unified.mri");
     let mut mri = std::fs::File::create(&mri_path).unwrap();
     println!("Generating {}", mri_path.to_string_lossy());
@@ -127,7 +156,7 @@ fn build_mri(output: &std::path::Path, libraries: &BTreeMap<String, BTreeMap<Str
 
     mri.write_all(b"SAVE\nEND\n").unwrap();
 
-    let mut cmd = std::process::Command::new("ar");
+    let mut cmd = Command::new(ar);
     cmd.current_dir(output);
     cmd.arg("-M");
     cmd.stdin(std::fs::File::open(&mri_path).unwrap());