diff --git a/src/include/OpenImageIO/detail/fmt.h b/src/include/OpenImageIO/detail/fmt.h index 74e6fae78c..6978311933 100644 --- a/src/include/OpenImageIO/detail/fmt.h +++ b/src/include/OpenImageIO/detail/fmt.h @@ -19,13 +19,18 @@ # define FMT_EXCEPTIONS 0 #endif -// Redefining FMT_THROW to something benign seems to avoid some UB or possibly -// gcc 11+ compiler bug triggered by the definition of FMT_THROW in fmt 10.1+ -// when FMT_EXCEPTIONS=0, which results in mangling SIMD math. This nugget -// below works around the problems for hard to understand reasons. -#if !defined(FMT_THROW) && !FMT_EXCEPTIONS && OIIO_GNUC_VERSION >= 110000 -# define FMT_THROW(x) \ - OIIO_ASSERT_MSG(0, "fmt exception: %s", (x).what()), std::terminate() +OIIO_NAMESPACE_BEGIN +namespace pvt { +OIIO_UTIL_API void +log_fmt_error(const char* message); +}; +OIIO_NAMESPACE_END + +// Redefining FMT_THROW to print and log the error. This should only occur if +// we've made a mistake and mismatched a format string and its arguments. +// Hopefully this will help us track it down. +#if !defined(FMT_THROW) && !FMT_EXCEPTIONS +# define FMT_THROW(x) OIIO::pvt::log_fmt_error((x).what()) #endif // Use the grisu fast floating point formatting for old fmt versions diff --git a/src/include/OpenImageIO/strutil.h b/src/include/OpenImageIO/strutil.h index e0c5ec302f..cb92333dcf 100644 --- a/src/include/OpenImageIO/strutil.h +++ b/src/include/OpenImageIO/strutil.h @@ -281,7 +281,11 @@ using sync::print; namespace pvt { +// Internal use only OIIO_UTIL_API void debug(string_view str); +OIIO_UTIL_API void append_error(string_view str); +OIIO_UTIL_API bool has_error(); +OIIO_UTIL_API std::string geterror(bool clear); } /// `debug(format, ...)` prints debugging message when attribute "debug" is diff --git a/src/include/imageio_pvt.h b/src/include/imageio_pvt.h index 08fe2bb52f..4b854bcded 100644 --- a/src/include/imageio_pvt.h +++ b/src/include/imageio_pvt.h @@ -39,6 +39,7 @@ extern std::string output_format_list; extern std::string extension_list; extern std::string library_list; extern OIIO_UTIL_API int oiio_print_debug; +extern OIIO_UTIL_API int oiio_print_uncaught_errors; extern int oiio_log_times; extern int openexr_core; extern int limit_channels; diff --git a/src/libOpenImageIO/imageio.cpp b/src/libOpenImageIO/imageio.cpp index e7712b19cc..ba859c607c 100644 --- a/src/libOpenImageIO/imageio.cpp +++ b/src/libOpenImageIO/imageio.cpp @@ -62,7 +62,6 @@ std::string output_format_list; // comma-separated list of writable formats std::string extension_list; // list of all extensions for all formats std::string library_list; // list of all libraries for all formats int oiio_log_times = Strutil::stoi(Sysutil::getenv("OPENIMAGEIO_LOG_TIMES")); -int oiio_print_uncaught_errors(1); std::vector oiio_missingcolor; } // namespace pvt @@ -285,51 +284,10 @@ openimageio_version() -// ErrorHolder houses a string, with the addition that when it is destroyed, -// it will disgorge any un-retrieved error messages, in an effort to help -// beginning users diagnose their problems if they have forgotten to call -// geterror(). -struct ErrorHolder { - std::string error_msg; - - ~ErrorHolder() - { - if (!error_msg.empty() && pvt::oiio_print_uncaught_errors) { - OIIO::print( - "OpenImageIO exited with a pending error message that was never\n" - "retrieved via OIIO::geterror(). This was the error message:\n{}\n", - error_msg); - } - } -}; - - - -// To avoid thread oddities, we have the storage area buffering error -// messages for append_error()/geterror() be thread-specific. -static thread_local ErrorHolder error_msg_holder; - - void pvt::append_error(string_view message) { - // Remove a single trailing newline - if (message.size() && message.back() == '\n') - message.remove_suffix(1); - std::string& error_msg(error_msg_holder.error_msg); - OIIO_ASSERT( - error_msg.size() < 1024 * 1024 * 16 - && "Accumulated error messages > 16MB. Try checking return codes!"); - // If we are appending to existing error messages, separate them with - // a single newline. - if (error_msg.size() && error_msg.back() != '\n') - error_msg += '\n'; - error_msg += std::string(message); - - // Remove a single trailing newline - if (message.size() && message.back() == '\n') - message.remove_suffix(1); - error_msg = std::string(message); + Strutil::pvt::append_error(message); } @@ -337,8 +295,7 @@ pvt::append_error(string_view message) bool has_error() { - std::string& error_msg(error_msg_holder.error_msg); - return !error_msg.empty(); + return Strutil::pvt::has_error(); } @@ -346,11 +303,7 @@ has_error() std::string geterror(bool clear) { - std::string& error_msg(error_msg_holder.error_msg); - std::string e = error_msg; - if (clear) - error_msg.clear(); - return e; + return Strutil::pvt::geterror(clear); } diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp index 2435afd534..1a39b7798a 100644 --- a/src/libutil/strutil.cpp +++ b/src/libutil/strutil.cpp @@ -172,9 +172,86 @@ OIIO_UTIL_API int OIIO_UTIL_API int oiio_print_debug(oiio_debug_env ? Strutil::stoi(oiio_debug_env) : 1); #endif +OIIO_UTIL_API int oiio_print_uncaught_errors(1); } // namespace pvt + +// ErrorHolder houses a string, with the addition that when it is destroyed, +// it will disgorge any un-retrieved error messages, in an effort to help +// beginning users diagnose their problems if they have forgotten to call +// geterror(). +struct ErrorHolder { + std::string error_msg; + + ~ErrorHolder() + { + if (!error_msg.empty() && pvt::oiio_print_uncaught_errors) { + OIIO::print( + "OpenImageIO exited with a pending error message that was never\n" + "retrieved via OIIO::geterror(). This was the error message:\n{}\n", + error_msg); + } + } +}; + + +// To avoid thread oddities, we have the storage area buffering error +// messages for append_error()/geterror() be thread-specific. +static thread_local ErrorHolder error_msg_holder; + + +void +Strutil::pvt::append_error(string_view message) +{ + // Remove a single trailing newline + if (message.size() && message.back() == '\n') + message.remove_suffix(1); + std::string& error_msg(error_msg_holder.error_msg); + OIIO_ASSERT( + error_msg.size() < 1024 * 1024 * 16 + && "Accumulated error messages > 16MB. Try checking return codes!"); + // If we are appending to existing error messages, separate them with + // a single newline. + if (error_msg.size() && error_msg.back() != '\n') + error_msg += '\n'; + error_msg += std::string(message); + + // Remove a single trailing newline + if (message.size() && message.back() == '\n') + message.remove_suffix(1); + error_msg = std::string(message); +} + + +bool +Strutil::pvt::has_error() +{ + std::string& error_msg(error_msg_holder.error_msg); + return !error_msg.empty(); +} + + +std::string +Strutil::pvt::geterror(bool clear) +{ + std::string& error_msg(error_msg_holder.error_msg); + std::string e = error_msg; + if (clear) + error_msg.clear(); + return e; +} + + +void +pvt::log_fmt_error(const char* message) +{ + print("fmt exception: {}\n", message); + Strutil::pvt::append_error(std::string("fmt exception: ") + message); +} + + + void Strutil::pvt::debug(string_view message) { diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index bcfcc7139e..3e8a261054 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -6395,6 +6395,12 @@ Oiiotool::getargs(int argc, char* argv[]) ap.arg("--crash") .hidden() .action(crash_me); + ap.arg("--test-bad-format") + .hidden() + .action([&](cspan){ + print("{}\n", Strutil::fmt::format("hey hey {:d} {}", + "foo", "bar", "oops")); + }); ap.separator("Commands that read images:"); ap.arg("-i %s:FILENAME") diff --git a/testsuite/oiiotool/ref/out.txt b/testsuite/oiiotool/ref/out.txt index e5532a1f56..e5b24d1dfc 100644 --- a/testsuite/oiiotool/ref/out.txt +++ b/testsuite/oiiotool/ref/out.txt @@ -57,6 +57,12 @@ half data[2][2][3] = { /* (0, 1): */ { 0.000000000, 0.000000000, 0.000000000 }, /* (1, 1): */ { 1.000000000, 1.000000000, 0.000000000 } }, }; +testing bad format +fmt exception: invalid format specifier +hey hey foo bar +OpenImageIO exited with a pending error message that was never +retrieved via OIIO::geterror(). This was the error message: +fmt exception: invalid format specifier Comparing "filled.tif" and "ref/filled.tif" PASS Comparing "autotrim.tif" and "ref/autotrim.tif" diff --git a/testsuite/oiiotool/run.py b/testsuite/oiiotool/run.py index 315406f7f0..ec5c661544 100755 --- a/testsuite/oiiotool/run.py +++ b/testsuite/oiiotool/run.py @@ -234,6 +234,9 @@ command += oiiotool ("-echo dumpdata: --dumpdata dump.exr") command += oiiotool ("-echo dumpdata:C --dumpdata:C=data dump.exr") +# Test printing uncaught errors (and finding bad fmt strings) +command += oiiotool ("--echo \"testing bad format\" --test-bad-format") + # To add more tests, just append more lines like the above and also add # the new 'feature.tif' (or whatever you call it) to the outputs list, # below.