Handle exceptions in main.

Fixes #89

components/conformance-tests/tests/tests.rs

@@ -1,4 +1,4 @@
-use anyhow::Result;
+use anyhow::{Result, Context};
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -41,6 +41,10 @@ struct TestArgs {
 struct TestOut {
     outfile: PathBuf,
     outfile_sourcemap: Option<PathBuf>,
+    /// Indicates the run succeeded.
+    success: bool,
+    /// For the binaries, they include the typical command output.
+    proc_output: Option<std::process::Output>,
 }
 
 fn run_test(args: TestArgs) -> Result<()> {
@@ -57,14 +61,25 @@ fn run_test(args: TestArgs) -> Result<()> {
     let rust_out = run_test_rust(&args, &rust_tempdir)?;
     let api_out = run_test_api(&args, &api_tempdir)?;
 
-    let binaryen_out_file = fs::read(binaryen_out.outfile)?;
-    let rust_out_file = fs::read(rust_out.outfile)?;
+    assert_eq!(binaryen_out.success, rust_out.success);
+    assert_eq!(binaryen_out.success, api_out.success);
 
-    assert_eq!(binaryen_out_file, rust_out_file);
+    assert_eq!(binaryen_out.proc_output, rust_out.proc_output);
 
-    let api_out_file = fs::read(api_out.outfile)?;
+    let has_output = binaryen_out.outfile.exists();
+    if has_output {
+        let binaryen_out_file = fs::read(binaryen_out.outfile)?;
+        let rust_out_file = fs::read(rust_out.outfile)?;
+
+        assert_eq!(binaryen_out_file, rust_out_file);
+
+        let api_out_file = fs::read(api_out.outfile)?;
 
-    assert_eq!(api_out_file, rust_out_file);
+        assert_eq!(api_out_file, rust_out_file);
+    } else {
+        assert!(!rust_out.outfile.exists());
+        assert!(!api_out.outfile.exists());
+    }
 
     match (
         binaryen_out.outfile_sourcemap,
@@ -73,12 +88,18 @@ fn run_test(args: TestArgs) -> Result<()> {
     ) {
         (None, None, None) => {}
         (Some(binaryen_out), Some(rust_out), Some(api_out)) => {
-            let binaryen_out_sourcemap = fs::read(binaryen_out)?;
-            let rust_out_sourcemap = fs::read(rust_out)?;
-            assert_eq!(binaryen_out_sourcemap, rust_out_sourcemap);
-
-            let api_out_sourcemap = fs::read(api_out)?;
-            assert_eq!(api_out_sourcemap, rust_out_sourcemap);
+            let has_sourcemap_output = binaryen_out.exists();
+            if has_sourcemap_output {
+                let binaryen_out_sourcemap = fs::read(binaryen_out)?;
+                let rust_out_sourcemap = fs::read(rust_out)?;
+                assert_eq!(binaryen_out_sourcemap, rust_out_sourcemap);
+
+                let api_out_sourcemap = fs::read(api_out)?;
+                assert_eq!(api_out_sourcemap, rust_out_sourcemap);
+            } else {
+                assert!(!rust_out.exists());
+                assert!(!api_out.exists());
+            }
         }
         _ => anyhow::bail!("output_sourcemap test failed"),
     }
@@ -123,13 +144,15 @@ fn run_test_binaryen(args: &TestArgs, tempdir: &Path) -> Result<TestOut> {
         });
     }
 
-    if !cmd.status()?.success() {
-        anyhow::bail!("run binaryen failed");
-    }
+    let proc_output = cmd.output().context("run binaryen failed")?;
+    let success = proc_output.status.success();
+    let proc_output = Some(proc_output);
 
     Ok(TestOut {
         outfile,
         outfile_sourcemap,
+        success,
+        proc_output,
     })
 }
 
@@ -164,13 +187,15 @@ fn run_test_rust(args: &TestArgs, tempdir: &Path) -> Result<TestOut> {
         });
     }
 
-    if !cmd.status()?.success() {
-        anyhow::bail!("run rust wasm-opt failed");
-    }
+    let proc_output = cmd.output().context("run rust wasm-opt failed")?;
+    let success = proc_output.status.success();
+    let proc_output = Some(proc_output);
 
     Ok(TestOut {
         outfile,
         outfile_sourcemap,
+        success,
+        proc_output,
     })
 }
 
@@ -207,11 +232,13 @@ fn run_test_api(args: &TestArgs, tempdir: &Path) -> Result<TestOut> {
         });
     }
 
-    integration::run_from_command_args(cmd)?;
+    let success = integration::run_from_command_args(cmd).is_ok();
 
     Ok(TestOut {
         outfile,
         outfile_sourcemap,
+        success,
+        proc_output: None,
     })
 }
 
@@ -809,3 +836,22 @@ fn wasm_to_wasm_converge() -> Result<()> {
         args,
     })
 }
+
+#[test]
+fn input_file_does_not_exist() -> Result<()> {
+    let infile = PathBuf::from("bogus.wasm");
+    let outfile = PathBuf::from("bogus-out.wasm");
+
+    let infile_sourcemap = None::<PathBuf>;
+    let outfile_sourcemap = None::<PathBuf>;
+
+    let args = vec![];
+
+    run_test(TestArgs {
+        infile,
+        infile_sourcemap,
+        outfile,
+        outfile_sourcemap,
+        args,
+    })
+}

components/wasm-opt-sys/build.rs

@@ -22,6 +22,10 @@ fn main() -> anyhow::Result<()> {
 
     let wasm_intrinsics_src = get_converted_wasm_intrinsics_cpp(&src_dir)?;
 
+    let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")?;
+    let manifest_dir = Path::new(&manifest_dir);
+    let wasm_opt_main_shim = manifest_dir.join("src/wasm-opt-main-shim.cpp");
+
     create_config_header()?;
 
     let mut builder = cc::Build::new();
@@ -45,6 +49,7 @@ fn main() -> anyhow::Result<()> {
         .include(src_dir)
         .include(tools_dir)
         .include(output_dir)
+        .file(wasm_opt_main_shim)
         .files(src_files)
         .file(wasm_opt_src)
         .file(wasm_intrinsics_src);
@@ -85,6 +90,9 @@ fn get_binaryen_dir() -> anyhow::Result<PathBuf> {
     }
 }
 
+/// Replaces the `main` declaration with a C ABI and a different name.
+///
+/// It can be called from Rust and doesn't clash with Rust's `main`.
 fn get_converted_wasm_opt_cpp(src_dir: &Path) -> anyhow::Result<PathBuf> {
     let wasm_opt_file = File::open(src_dir)?;
     let reader = BufReader::new(wasm_opt_file);
@@ -100,7 +108,7 @@ fn get_converted_wasm_opt_cpp(src_dir: &Path) -> anyhow::Result<PathBuf> {
         let mut line = line?;
 
         if line.contains("int main") {
-            line = line.replace("int main", "extern \"C\" int wasm_opt_main");
+            line = line.replace("int main", "extern \"C\" int wasm_opt_main_actual");
         }
 
         writer.write_all(line.as_bytes())?;
@@ -374,7 +382,10 @@ fn check_cxx17_support() -> anyhow::Result<()> {
     };
 
     if !builder.is_flag_supported(cxx17_flag)? {
-        return Err(anyhow::anyhow!("C++ compiler does not support `{}` flag", cxx17_flag));
+        return Err(anyhow::anyhow!(
+            "C++ compiler does not support `{}` flag",
+            cxx17_flag
+        ));
     }
 
     Ok(())

components/wasm-opt-sys/src/wasm-opt-main-shim.cpp

@@ -0,0 +1,27 @@
+#include <cstdlib> // _Exit
+#include <stdexcept> // runtime_error
+#include <iostream>
+
+extern "C" int wasm_opt_main_actual(int argc, const char* argv[]);
+
+// A wrapper for the C++ `main` function that catches exceptions.
+//
+// This is needed because we have asked the `Fatal` type to throw instead of
+// exit, for use as a library. But for use as a `bin` we still need to handle
+// those exceptions in a similar way to the "real" `wasm-opt` bin.
+//
+// Since the bin does not use `cxx` it doesn't get baked-in exception handling,
+// so we do that here, and the bin calls this main function.
+//
+// This design is also influenced by the need to maintain binary compatibility
+// between `wasm-opt` crate 0.110.0 and 0.110.1. We might otherwise use `cxx`
+// for the exception handling.
+extern "C" int wasm_opt_main(int argc, const char* argv[]) {
+  try {
+    return wasm_opt_main_actual(argc, argv);
+  } catch (const std::exception &e) {
+    std::cerr << e.what() << std::endl;
+    // See comments in `Fatal` about `_Exit` and static destructors.
+    _Exit(EXIT_FAILURE);
+  }
+}