From aad563327f248190a17ad48de8e0a3f667b76597 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 13 Oct 2022 12:31:39 -0600 Subject: [PATCH] Handle exceptions in main. Fixes #89 --- components/conformance-tests/tests/tests.rs | 84 ++++++++++++++----- components/wasm-opt-sys/build.rs | 15 +++- .../wasm-opt-sys/src/wasm-opt-main-shim.cpp | 27 ++++++ 3 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 components/wasm-opt-sys/src/wasm-opt-main-shim.cpp diff --git a/components/conformance-tests/tests/tests.rs b/components/conformance-tests/tests/tests.rs index 6fd32fd..dd06b61 100644 --- a/components/conformance-tests/tests/tests.rs +++ b/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, + /// Indicates the run succeeded. + success: bool, + /// For the binaries, they include the typical command output. + proc_output: Option, } 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 { }); } - 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 { }); } - 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 { }); } - 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::; + let outfile_sourcemap = None::; + + let args = vec![]; + + run_test(TestArgs { + infile, + infile_sourcemap, + outfile, + outfile_sourcemap, + args, + }) +} diff --git a/components/wasm-opt-sys/build.rs b/components/wasm-opt-sys/build.rs index cc69446..c2d177f 100644 --- a/components/wasm-opt-sys/build.rs +++ b/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 { } } +/// 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 { 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 { 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(()) diff --git a/components/wasm-opt-sys/src/wasm-opt-main-shim.cpp b/components/wasm-opt-sys/src/wasm-opt-main-shim.cpp new file mode 100644 index 0000000..11099da --- /dev/null +++ b/components/wasm-opt-sys/src/wasm-opt-main-shim.cpp @@ -0,0 +1,27 @@ +#include // _Exit +#include // runtime_error +#include + +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); + } +}