From 70ac80aedc734ae0bf4da119231508e830cab050 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 25 Sep 2022 22:18:15 -0400 Subject: [PATCH 1/6] Have file.cpp call 'Fatal' instead of 'exit' --- src/support/file.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/support/file.cpp b/src/support/file.cpp index dc9707a1b9d..cfd656391be 100644 --- a/src/support/file.cpp +++ b/src/support/file.cpp @@ -16,6 +16,7 @@ #include "support/file.h" #include "support/debug.h" +#include "support/utilities.h" #include #include @@ -58,18 +59,16 @@ T wasm::read_file(const std::string& filename, Flags::BinaryOption binary) { } infile.open(filename, flags); if (!infile.is_open()) { - std::cerr << "Failed opening '" << filename << "'" << std::endl; - exit(EXIT_FAILURE); + Fatal() << "Failed opening '" << filename << "'"; } infile.seekg(0, std::ios::end); std::streampos insize = infile.tellg(); if (uint64_t(insize) >= std::numeric_limits::max()) { // Building a 32-bit executable where size_t == 32 bits, we are not able to // create strings larger than 2^32 bytes in length, so must abort here. - std::cerr << "Failed opening '" << filename - << "': Input file too large: " << insize - << " bytes. Try rebuilding in 64-bit mode." << std::endl; - exit(EXIT_FAILURE); + Fatal() << "Failed opening '" << filename + << "': Input file too large: " << insize + << " bytes. Try rebuilding in 64-bit mode."; } T input(size_t(insize) + (binary == Flags::Binary ? 0 : 1), '\0'); if (size_t(insize) == 0) { @@ -115,8 +114,7 @@ wasm::Output::Output(const std::string& filename, Flags::BinaryOption binary) } outfile.open(filename, flags); if (!outfile.is_open()) { - std::cerr << "Failed opening '" << filename << "'" << std::endl; - exit(EXIT_FAILURE); + Fatal() << "Failed opening '" << filename << "'"; } buffer = outfile.rdbuf(); } From faa1669fd11bf5d7a96d8eae63f73807cc7c9e1d Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 25 Sep 2022 22:23:30 -0400 Subject: [PATCH 2/6] Change Fatal to exit with EXIT_FAILURE instead of 1 --- src/support/utilities.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/support/utilities.h b/src/support/utilities.h index 57d17026983..b2470c645fb 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -73,7 +73,7 @@ class Fatal { // Use _Exit here to avoid calling static destructors. This avoids deadlocks // in (for example) the thread worker pool, where workers hold a lock while // performing their work. - _Exit(1); + _Exit(EXIT_FAILURE); } }; From ab2a5fd393c6085344037ca24fc6cd74a4f5c49f Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 25 Sep 2022 23:15:01 -0400 Subject: [PATCH 3/6] Buffer fatal error messages for display in the destructor --- src/support/utilities.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/support/utilities.h b/src/support/utilities.h index b2470c645fb..ef4037b36ab 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "support/bits.h" @@ -62,14 +63,16 @@ std::unique_ptr make_unique(Args&&... args) { // For fatal errors which could arise from input (i.e. not assertion failures) class Fatal { +private: + std::stringstream buffer; public: - Fatal() { std::cerr << "Fatal: "; } + Fatal() { buffer << "Fatal: "; } template Fatal& operator<<(T arg) { - std::cerr << arg; + buffer << arg; return *this; } [[noreturn]] ~Fatal() { - std::cerr << "\n"; + std::cerr << buffer.str() << std::endl; // Use _Exit here to avoid calling static destructors. This avoids deadlocks // in (for example) the thread worker pool, where workers hold a lock while // performing their work. From 96f01b9cae4d73166e33db5308db87f7be968824 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 26 Sep 2022 02:13:28 -0400 Subject: [PATCH 4/6] Add THROW_ON_FATAL option --- src/support/utilities.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/support/utilities.h b/src/support/utilities.h index ef4037b36ab..447e7d4b2f7 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -71,6 +71,7 @@ class Fatal { buffer << arg; return *this; } +#ifndef THROW_ON_FATAL [[noreturn]] ~Fatal() { std::cerr << buffer.str() << std::endl; // Use _Exit here to avoid calling static destructors. This avoids deadlocks @@ -78,6 +79,18 @@ class Fatal { // performing their work. _Exit(EXIT_FAILURE); } +#else + // This variation is a best-effort attempt to make fatal errors recoverable + // for embedders of Binaryen as a library, namely wasm-opt-rs. + // + // Throwing in destructors is strongly discouraged, since it is easy to + // accidentally throw during unwinding, which will trigger an abort. Since + // `Fatal` is a special type that only occurs on error paths, we are hoping it + // is never constructed during unwinding or while destructing another type. + [[noreturn]] ~Fatal() noexcept(false) { + throw std::runtime_error(buffer.str()); + } +#endif }; [[noreturn]] void handle_unreachable(const char* msg = nullptr, From a70d1494ad1f336acf2700402d1e74c42d6c437b Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 30 Sep 2022 10:07:03 -0400 Subject: [PATCH 5/6] Add THROW_ON_FATAL to changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eafb6447181..2298928bbb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ Current Trunk - `TypeBuilderSetSubType` now takes a supertype as the second argument. - `call_ref` can now take a signature type immediate in the text format. The type immediate will become mandatory in the future. +- If `THROW_ON_FATAL` is defined at compile-time, then fatal errors will throw a + `std::runtime_error` instead of terminating the process. This may be used by + embedders of Binaryen to recover from errors. v110 ---- From d348f4e8c61af67a77f5f5ac98092a71915503dc Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 30 Sep 2022 16:26:09 -0400 Subject: [PATCH 6/6] Tidy --- src/support/utilities.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/support/utilities.h b/src/support/utilities.h index 447e7d4b2f7..ad28e0ff455 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -65,6 +65,7 @@ std::unique_ptr make_unique(Args&&... args) { class Fatal { private: std::stringstream buffer; + public: Fatal() { buffer << "Fatal: "; } template Fatal& operator<<(T arg) {