diff --git a/src/cmake/testing.cmake b/src/cmake/testing.cmake index db23a9da2..eaf39e27e 100644 --- a/src/cmake/testing.cmake +++ b/src/cmake/testing.cmake @@ -342,7 +342,7 @@ macro (osl_add_all_tests) debugnan debug-uninit derivs derivs-muldiv-clobber draw_string - error-dupes error-serialized + error-dupes error-malformed error-serialized example-deformer example-batched-deformer exit exponential diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp index 919266647..490237bd5 100644 --- a/src/liboslcomp/typecheck.cpp +++ b/src/liboslcomp/typecheck.cpp @@ -1221,7 +1221,7 @@ ASTfunction_call::typecheck_printf_args(const char* format, ASTNode* arg) && formatchar != 's') { errorfmt( "{} has mismatched format string and arguments (arg {} needs %s)", - m_name); + m_name, argnum); return false; } if (simpletype.basetype == TypeDesc::INT && formatchar != 'd' diff --git a/src/liboslexec/journal.cpp b/src/liboslexec/journal.cpp index a53556d3d..3273951ac 100644 --- a/src/liboslexec/journal.cpp +++ b/src/liboslexec/journal.cpp @@ -12,6 +12,36 @@ OSL_NAMESPACE_BEGIN +// Version of fmtformat_to_n that is "safe" in the sense of catching +// exceptions, not crashing (!), and turning an error into an output that +// hopefully will make it easier to track down where things are going awry. +template +OSL_NODISCARD inline auto +fmtformat_to_n_safe(OutIt& out, size_t n, string_view fmt, Args&&... args) +{ + // DOES NOT EXIST AS PUBLIC API + // return OIIO::Strutil::fmt::format_to_n(out, n, fmt, std::forward(args)...); + // So call directly into underlying fmt library OIIO is using + // TODO: Add format_to_n as a public API in OIIO + try { +#if OSL_CPLUSPLUS_VERSION >= 20 || FMT_VERSION >= 100000 + std::string str = fmtformat(fmt, std::forward(args)...); + return ::fmt::format_to_n(out, n, "{}", str); +#else + return ::fmt::format_to_n(out, n, + ::fmt::string_view { fmt.begin(), + fmt.length() }, + std::forward(args)...); +#endif + } catch (const std::exception& e) { + return ::fmt::format_to_n(out, n, + "MIS-FORMAT: format \"{}\" threw \"{}\"", fmt, + e.what()); + } +} + + + int decode_message(uint64_t format_hash, int32_t arg_count, const EncodedType* arg_types, const uint8_t* arg_values, @@ -81,7 +111,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); const char* arg_string = arg_value.c_str(); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length + 1, replacement_region, arg_string); @@ -91,7 +121,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, int32_t arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -101,7 +131,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, float arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -111,7 +141,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, double arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -121,7 +151,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, int64_t arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -131,7 +161,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, uint32_t arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -141,7 +171,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, uint64_t arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -152,7 +182,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, const void* arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); @@ -163,7 +193,7 @@ decode_message(uint64_t format_hash, int32_t arg_count, OSL::TypeDesc arg_value; memcpy(&arg_value, &arg_values[arg_offset], sizeof(arg_value)); - auto result = OSL::fmtformat_to_n(replacement_str, + auto result = fmtformat_to_n_safe(replacement_str, rs_max_length, replacement_region, arg_value); diff --git a/src/liboslexec/rendservices.cpp b/src/liboslexec/rendservices.cpp index 62eb7e83c..0e7cfebc5 100644 --- a/src/liboslexec/rendservices.cpp +++ b/src/liboslexec/rendservices.cpp @@ -206,7 +206,7 @@ RendererServices::errorfmt(OSL::ShaderGlobals* sg, OSL::decode_message(fmt_specification.hash(), arg_count, arg_types, arg_values, message); ShadingContext* ctx = (ShadingContext*)((ShaderGlobals*)sg)->context; - ctx->errorfmt(message.c_str()); + ctx->errorfmt("{}", message); } @@ -221,7 +221,7 @@ RendererServices::warningfmt(OSL::ShaderGlobals* sg, std::string message; OSL::decode_message(fmt_specification.hash(), arg_count, arg_types, arg_values, message); - ctx->warningfmt(message.c_str()); + ctx->warningfmt("{}", message); } } @@ -236,7 +236,7 @@ RendererServices::printfmt(OSL::ShaderGlobals* sg, OSL::decode_message(fmt_specification.hash(), arg_count, arg_types, arg_values, message); ShadingContext* ctx = (ShadingContext*)((ShaderGlobals*)sg)->context; - ctx->messagefmt(message.c_str()); + ctx->messagefmt("{}", message); } @@ -254,7 +254,7 @@ RendererServices::filefmt(OSL::ShaderGlobals* sg, // the message with the filename and hand it to the current error handler. auto file_message = OSL::fmtformat("{}:{}", filename_hash, message); ShadingContext* ctx = (ShadingContext*)((ShaderGlobals*)sg)->context; - ctx->messagefmt(file_message.c_str()); + ctx->messagefmt("{}", file_message); } diff --git a/testsuite/error-malformed/BATCHED b/testsuite/error-malformed/BATCHED new file mode 100644 index 000000000..e69de29bb diff --git a/testsuite/error-malformed/ref/out.txt b/testsuite/error-malformed/ref/out.txt new file mode 100644 index 000000000..c67d2f611 --- /dev/null +++ b/testsuite/error-malformed/ref/out.txt @@ -0,0 +1,7 @@ +Compiled test.osl -> test.oso + +Output Cout to out.tif +ERROR: [RendererServices::texture] Could not open file: {:d}.tif: No such file or directory +Printing {:f} +ERROR: Shader error [test]: Weird error: {:f} + diff --git a/testsuite/error-malformed/run.py b/testsuite/error-malformed/run.py new file mode 100755 index 000000000..804358d46 --- /dev/null +++ b/testsuite/error-malformed/run.py @@ -0,0 +1,8 @@ +#!/usr/bin/env python + +# Copyright Contributors to the Open Shading Language project. +# SPDX-License-Identifier: BSD-3-Clause +# https://github.com/AcademySoftwareFoundation/OpenShadingLanguage + +command += testshade("-g 1 1 --center -od uint8 -o Cout out.tif test") +outputs = [ "out.txt" ] diff --git a/testsuite/error-malformed/test.osl b/testsuite/error-malformed/test.osl new file mode 100644 index 000000000..cacff1063 --- /dev/null +++ b/testsuite/error-malformed/test.osl @@ -0,0 +1,16 @@ +// Copyright Contributors to the Open Shading Language project. +// SPDX-License-Identifier: BSD-3-Clause +// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage + +shader +test (output color Cout = 0) +{ + // Regression test: nonsensical filename that seems to contain formatting + // specifications. + string filename = (u < 2.0) ? "{:d}.tif" : "../common/textures/mandrill.tif"; + Cout = texture (filename, u, v); + + // For good measure, also test print and error with a bad string. + printf("Printing {:f}\n"); + error("Weird error: {:f}\n"); +}