libtock2: Enable overriding the toolchain for the assembler

kupiakos · kupiakos · commit 6c635ca29a3d · 2022-01-20T10:42:33.000-08:00
After some testing and upstream support, a variant of this will be
upstreamed and this CL will be replaced by that. This uses the standard
Make flags AS, ASFLAGS, and AR. STRIP is non-standard but was confirmed
as fine to use by jrvanwhy.

This also fixes a particularly weird issue with using llvm-mc currently:
it cannot handle a comment on the same line as a `mv` instruction,
unless it ends in `;`. Sounds like a tokenizer bug.

BUG=b:210846576
TEST=make flash
SOURCE=CHROME-ONLY

Change-Id: I1a4c38cf918e43e4f3de45a0fb1851466f540c93
diff --git a/runtime/asm/asm_riscv32.S b/runtime/asm/asm_riscv32.S
@@ -33,7 +33,8 @@ start:
 	 * check is performed by comparing the program counter at the start to the
 	 * address of `start`, which is stored in rt_header. */
 	auipc s0, 0            /* s0 = pc */
-	mv a5, a0              /* Save rt_header so syscalls don't overwrite it */
+	/* Note: the below semicolon is required due to an llvm-mc bug */
+	mv a5, a0;             /* Save rt_header so syscalls don't overwrite it */
 	lw s1, 0(a5)           /* s1 = rt_header.start */
 	beq s0, s1, .Lset_brk  /* Skip error handling code if pc is correct */
 	/* If the beq on the previous line did not jump, then the binary is not at
diff --git a/runtime/extern_asm.rs b/runtime/extern_asm.rs
@@ -2,118 +2,157 @@
 //! point) and linking it into the process binary. Requires out_dir to be added
 //! to rustc's link search path.
 
-/// Compiles the external assembly and tells cargo/rustc to link the resulting
-/// library into the crate. Panics if it is unable to find a working assembly
-/// toolchain or if the assembly fails to compile.
-pub(crate) fn build_and_link(out_dir: &str) {
-    use std::env::var;
-    let arch = var("CARGO_CFG_TARGET_ARCH").expect("Unable to read CARGO_CFG_TARGET_ARCH");
-
-    // Identify the toolchain configurations to try for the target architecture.
-    // We support trying multiple toolchains because not all toolchains are
-    // available on every OS that we want to support development on.
-    let build_configs: &[AsmBuildConfig] = match arch.as_str() {
-        "arm" => &[AsmBuildConfig {
-            prefix: "arm-none-eabi",
-            as_extra_args: &[],
-            strip: false,
-        }],
-        "riscv32" => &[
-            // First try riscv64-unknown-elf, as it is the toolchain used by
-            // libtock-c and the toolchain used in the CI environment.
-            AsmBuildConfig {
-                prefix: "riscv64-unknown-elf",
-                as_extra_args: &["-march=rv32imc"],
-                strip: true,
-            },
-            // Second try riscv32-unknown-elf. This is the best match for Tock's
-            // risc-v targets, but is not as widely available (and has not been
-            // tested with libtock-rs yet).
-            AsmBuildConfig {
-                prefix: "riscv32-unknown-elf",
-                as_extra_args: &[],
-                strip: false, // Untested, may need to change.
-            },
-            // Last try riscv64-linux-gnu, as it is the only option on Debian 10
-            AsmBuildConfig {
-                prefix: "riscv64-linux-gnu",
-                as_extra_args: &["-march=rv32imc"],
-                strip: true,
-            },
-        ],
-        unknown_arch => {
-            panic!("Unsupported architecture {}", unknown_arch);
-        }
-    };
+use std::{
+    env, io,
+    process::{self, Command},
+};
+
+#[derive(Clone)]
+struct ToolchainInfo {
+    arch: String,
+
+    /// The library archive command.
+    ar_cmd: String,
+
+    /// The object file strip command, if used.
+    /// If `None`, no strip will take place.
+    strip_cmd: Option<String>,
+
+    /// The assembler command.
+    as_cmd: String,
 
-    // Loop through toolchain configs until one works.
-    for &build_config in build_configs {
-        if try_build(&arch, build_config, out_dir).is_ok() {
-            return;
+    /// Additional flags to pass to the assembler.
+    as_flags: Vec<String>,
+}
+
+impl ToolchainInfo {
+    fn from_gcc_prefix(arch: &str, prefix: &str, strip: bool) -> Self {
+        Self {
+            arch: arch.to_string(),
+            ar_cmd: format!("{}-ar", prefix),
+            strip_cmd: strip.then(|| format!("{}-strip", prefix)),
+            as_cmd: format!("{}-as", prefix),
+            as_flags: Vec::new(),
         }
     }
-
-    panic!("Unable to find a toolchain for architecture {}", arch);
 }
 
-#[derive(Clone, Copy)]
-struct AsmBuildConfig {
-    // Prefix, which is prepended to the command names.
-    prefix: &'static str,
+/// Reads toolchain info from local environment variables, merging with a
+/// default toolchain if provided.
+///
+/// If a complete toolchain could not be constructed, this returns `None`.
+fn env_toolchain(arch: &str, default: Option<ToolchainInfo>) -> Option<ToolchainInfo> {
+    let env_ar_cmd = env::var("AR").ok();
+    let env_as_cmd = env::var("AS").ok();
+    // TODO: this cannot handle whitespace in flags.
+    let env_as_flags = env::var("ASFLAGS")
+        .ok()
+        .map(|x| x.split_ascii_whitespace().map(str::to_string).collect());
+    let env_strip_cmd = env::var("STRIP").ok().filter(|x| !x.is_empty());
+
+    let ar_cmd;
+    let as_cmd;
+    let as_flags;
+    let strip_cmd;
+    if let Some(default) = default {
+        // TODO: Should this merging occur or be all-or-nothing with
+        //       environment-provided toolchain variables?
+        ar_cmd = env_ar_cmd.unwrap_or(default.ar_cmd);
+        as_cmd = env_as_cmd.unwrap_or(default.as_cmd);
+        as_flags = env_as_flags.unwrap_or(default.as_flags);
+        strip_cmd = env_strip_cmd.or(default.strip_cmd);
+    } else {
+        ar_cmd = env_ar_cmd?;
+        as_cmd = env_as_cmd?;
+        as_flags = env_as_flags.unwrap_or(Vec::new());
+        strip_cmd = env_strip_cmd;
+    }
+
+    Some(ToolchainInfo {
+        arch: arch.to_string(),
+        ar_cmd,
+        strip_cmd,
+        as_cmd,
+        as_flags,
+    })
+}
 
-    // Extra arguments to pass to the assembler.
-    as_extra_args: &'static [&'static str],
+/// Checks a toolchain, by running `--version` on the provided commands.
+fn test_toolchain(toolchain: &ToolchainInfo) -> io::Result<()> {
+    Command::new(&toolchain.ar_cmd).arg("--version").status()?;
+    Command::new(&toolchain.as_cmd).arg("--version").status()?;
+    if let Some(ref path) = toolchain.strip_cmd {
+        Command::new(path).arg("--version").status()?;
+    }
+    Ok(())
+}
 
-    // Do we need to strip the object file before packing it into the library
-    // archive? This should be set to true on platforms where the assembler adds
-    // local symbols to the object file.
-    strip: bool,
+fn find_default_toolchain(arch: &str) -> Option<ToolchainInfo> {
+    // The default toolchain is the first GCC-like toolchain that works as
+    // expected. We support trying multiple toolchains because not all
+    // toolchains are available on every OS that we want to support development.
+    match arch {
+        "arm" => {
+            let toolchain = ToolchainInfo::from_gcc_prefix(arch, "arm-none-eabi", false);
+            test_toolchain(&toolchain).ok().map(|_| toolchain)
+        }
+        "riscv32" => {
+            for toolchain in [
+                ToolchainInfo {
+                    as_flags: vec!["-march=rv32imc".to_string()],
+                    ..ToolchainInfo::from_gcc_prefix(arch, "riscv64-unknown-elf", true)
+                },
+                // strip: false is untested here, may need to change
+                ToolchainInfo::from_gcc_prefix(arch, "riscv32-unknown-elf", false),
+                ToolchainInfo {
+                    as_flags: vec!["-march=rv32imc".to_string()],
+                    ..ToolchainInfo::from_gcc_prefix(arch, "riscv64-linux-gnu", true)
+                },
+            ] {
+                if test_toolchain(&toolchain).is_ok() {
+                    return Some(toolchain);
+                }
+            }
+            None
+        }
+        unknown_arch => {
+            panic!("Unsupported architecture {}", unknown_arch)
+        }
+    }
 }
 
-// Indicates the toolchain in the build config is unavailable.
-struct ToolchainUnavailable;
+/// Compiles the external assembly and tells cargo/rustc to link the resulting
+/// library into the crate. Panics if it is unable to find a working assembly
+/// toolchain or if the assembly fails to compile.
+pub(crate) fn build_and_link(out_dir: &str) {
+    let arch = env::var("CARGO_CFG_TARGET_ARCH").expect("Unable to read CARGO_CFG_TARGET_ARCH");
+
+    match env_toolchain(&arch, find_default_toolchain(&arch)) {
+        Some(toolchain) => build(&toolchain, out_dir),
+        None => panic!("Unable to find a toolchain for architecture {}", arch),
+    }
+}
 
-fn try_build(
-    arch: &str,
-    build_config: AsmBuildConfig,
-    out_dir: &str,
-) -> Result<(), ToolchainUnavailable> {
+fn build(toolchain: &ToolchainInfo, out_dir: &str) {
     use std::path::PathBuf;
-    use std::process::Command;
 
     // Invoke the assembler to produce an object file.
-    let asm_source = &format!("asm/asm_{}.S", arch);
+    let as_cmd = &toolchain.as_cmd;
+    let asm_source = &format!("asm/asm_{}.S", toolchain.arch);
     let obj_file_path = [out_dir, "libtock_rt_asm.o"].iter().collect::<PathBuf>();
     let obj_file = obj_file_path.to_str().expect("Non-Unicode obj_file_path");
-    let as_result = Command::new(format!("{}-as", build_config.prefix))
-        .args(build_config.as_extra_args)
+    let status = Command::new(as_cmd)
+        .args(&toolchain.as_flags)
         .args(&[asm_source, "-o", obj_file])
-        .status();
-
-    match as_result {
-        Err(error) => {
-            if error.kind() == std::io::ErrorKind::NotFound {
-                // This `as` command does not exist. Return an error so
-                // build_an_link can try another config (if one is available).
-                return Err(ToolchainUnavailable);
-            } else {
-                panic!("Error invoking assembler: {}", error);
-            }
-        }
-        Ok(status) => {
-            assert!(status.success(), "Assembler returned an error");
-        }
-    }
-
-    // At this point, we know this toolchain is installed. We will fail if later
-    // commands are uninstalled rather than trying a different build config.
+        .status()
+        .unwrap_or_else(|_| panic!("Failed to invoke {}", as_cmd));
+    assert!(status.success(), "{} returned an error", as_cmd);
 
     println!("cargo:rerun-if-changed={}", asm_source);
 
     // Run `strip` if necessary.
-    if build_config.strip {
-        let strip_cmd = format!("{}-strip", build_config.prefix);
-        let status = Command::new(&strip_cmd)
+    if let Some(strip_cmd) = &toolchain.strip_cmd {
+        let status = Command::new(strip_cmd)
             .args(&["-K", "start", "-K", "rust_start", obj_file])
             .status()
             .unwrap_or_else(|_| panic!("Failed to invoke {}", strip_cmd));
@@ -127,15 +166,15 @@ fn try_build(
         .iter()
         .collect();
     if let Err(error) = std::fs::remove_file(&archive_path) {
-        if error.kind() != std::io::ErrorKind::NotFound {
+        if error.kind() != io::ErrorKind::NotFound {
             panic!("Unable to remove archive file {}", archive_path.display());
         }
     }
 
     // Create the library archive.
-    let ar_cmd = format!("{}-ar", build_config.prefix);
+    let ar_cmd = &toolchain.ar_cmd;
     let archive = archive_path.to_str().expect("Non-Unicode archive_path");
-    let status = std::process::Command::new(&ar_cmd)
+    let status = process::Command::new(ar_cmd)
         // c == Do not complain if archive needs to be created.
         // r == Insert or replace file in archive.
         .args(&["cr", archive, obj_file])
@@ -145,6 +184,4 @@ fn try_build(
 
     // Tell rustc to link the binary against the library archive.
     println!("cargo:rustc-link-lib=static={}", ARCHIVE_NAME);
-
-    Ok(())
 }
