Since printf is user-facing now, make formatter errors less fatal

renderdoc/common/formatting.h

@@ -32,6 +32,8 @@ struct Args
 {
   virtual void reset() = 0;
 
+  virtual void error(const char *err) = 0;
+
   virtual int get_int() = 0;
   virtual unsigned int get_uint() = 0;
   virtual double get_double() = 0;

renderdoc/driver/vulkan/vk_shader_feedback.cpp

@@ -53,7 +53,8 @@ struct feedbackData
 
 struct PrintfData
 {
-  rdcstr format;
+  rdcstr user_format;
+  rdcstr effective_format;
   // vectors are expanded so there's one for each component (as printf will expect)
   rdcarray<rdcspv::Scalar> argTypes;
   size_t payloadWords;
@@ -72,6 +73,7 @@ public:
     m_Cur = m_Start;
     m_Idx = 0;
   }
+  void error(const char *err) override { m_Error = err; }
   int get_int() override
   {
     int32_t ret = *(int32_t *)m_Cur;
@@ -125,11 +127,13 @@ public:
   }
 
   size_t get_size() override { return sizeof(size_t) == 8 ? (size_t)get_uint64() : get_uint(); }
+  rdcstr get_error() { return m_Error; }
 private:
   const uint32_t *m_Cur;
   const uint32_t *m_Start;
   size_t m_Idx;
   const PrintfData &m_Formats;
+  rdcstr m_Error;
 };
 
 rdcstr PatchFormatString(rdcstr format)
@@ -886,7 +890,8 @@ void AnnotateShader(const ShaderReflection &refl, const SPIRVPatchData &patchDat
 
           {
             rdcspv::OpString str(editor.GetID(rdcspv::Id::fromWord(extinst.params[0])));
-            format.format = PatchFormatString(str.string);
+            format.user_format = str.string;
+            format.effective_format = PatchFormatString(str.string);
           }
 
           rdcarray<rdcspv::Id> packetWords;
@@ -1848,7 +1853,10 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId)
           RDCLOG("pixel %u, %u", msg.location.pixel.x, msg.location.pixel.y);
         }
 
-        msg.message = StringFormat::FmtArgs(fmt.format.c_str(), args);
+        msg.message = StringFormat::FmtArgs(fmt.effective_format.c_str(), args);
+
+        if(!args.get_error().empty())
+          msg.message = args.get_error() + " in \"" + fmt.user_format + "\"";
 
         result.messages.push_back(msg);
       }

renderdoc/strings/utf8printf.cpp

@@ -1401,11 +1401,6 @@ void formatargument(char type, void *rawarg, FormatterParams formatter, char *&o
 
     PrintFloat(argd, formatter, e, f, g, a, uppercaseDigits, output, actualsize, end);
   }
-  else
-  {
-    // Unrecognised format specifier
-    RDCDUMPMSG("Unrecognised % formatter");
-  }
 }
 
 struct va_arg_getter
@@ -1417,6 +1412,8 @@ struct va_arg_getter
   {
     return va_arg(list, T);
   }
+
+  void error(const char *err) {}
 };
 
 struct custom_arg_getter
@@ -1425,6 +1422,8 @@ struct custom_arg_getter
   custom_arg_getter(StringFormat::Args &f) : formatter(f) {}
   template <typename T>
   inline T get_next();
+
+  void error(const char *err) { formatter.error(err); }
 };
 
 template <>
@@ -1460,6 +1459,18 @@ inline size_t custom_arg_getter::get_next<size_t>()
 }
 #endif
 
+int utf8print_error(char *buf, size_t bufsize, const char *str)
+{
+  // copy the string with no formatting in the error case
+  size_t ret = strlen(str);
+  if(bufsize > 0)
+  {
+    memcpy(buf, str, RDCMIN(bufsize - 1, ret));
+    buf[RDCMIN(bufsize - 1, ret)] = 0;
+  }
+  return int(ret);
+}
+
 template <typename arg_getter>
 int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter args)
 {
@@ -1481,7 +1492,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar
       iter++;
 
       if(*iter == 0)
-        RDCDUMPMSG("unterminated formatter (should be %% if you want a literal %)");
+      {
+        args.error("unterminated formatter (should be %% if you want a literal %)");
+        return utf8print_error(buf, bufsize, fmt);
+      }
 
       if(*iter == '%')    // %% found, insert single % and continue copying
       {
@@ -1556,7 +1570,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar
 
         // unterminated formatter
         if(*iter == 0)
-          RDCDUMPMSG("Unterminated % formatter found after width");
+        {
+          args.error("Unterminated % formatter found after width");
+          return utf8print_error(buf, bufsize, fmt);
+        }
       }
       else
       {
@@ -1597,7 +1614,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar
 
           // unterminated formatter
           if(*iter == 0)
-            RDCDUMPMSG("Unterminated % formatter found after precision");
+          {
+            args.error("Unterminated % formatter found after precision");
+            return utf8print_error(buf, bufsize, fmt);
+          }
         }
       }
       else
@@ -1688,7 +1708,8 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar
     }
     else
     {
-      RDCDUMPMSG("Unrecognised % formatter");
+      args.error("Unrecognised % formatter");
+      return utf8print_error(buf, bufsize, fmt);
     }
 
     formatargument(type, arg, formatter, output, actualsize, end);
@@ -1915,6 +1936,24 @@ TEST_CASE("utf8printf buffer sizing", "[utf8printf]")
   };
 };
 
+TEST_CASE("utf8printf error cases", "[utf8printf]")
+{
+  // unrecognised format type (covers most errors where an invalid character happens after optional
+  // precision stuff
+  CHECK(StringFormat::Fmt("%f foo %s: %q", 1.234f, "blah", 777) == "%f foo %s: %q");
+  CHECK(StringFormat::Fmt("%f foo %s: %8q", 1.234f, "blah", 777) == "%f foo %s: %8q");
+  CHECK(StringFormat::Fmt("%f foo %s: %1.8q", 1.234f, "blah", 777) == "%f foo %s: %1.8q");
+
+  // unterminated % at the end of the string
+  CHECK(StringFormat::Fmt("%f foo %s: %", 1.234f, "blah", 777) == "%f foo %s: %");
+  CHECK(StringFormat::Fmt("%f foo %s: %8", 1.234f, "blah", 777) == "%f foo %s: %8");
+  CHECK(StringFormat::Fmt("%f foo %s: %1.8", 1.234f, "blah", 777) == "%f foo %s: %1.8");
+  CHECK(StringFormat::Fmt("%f foo %s: %1.", 1.234f, "blah", 777) == "%f foo %s: %1.");
+
+  // precision as vararg
+  CHECK(StringFormat::Fmt("%f foo %*s", 1.234f, "blah", 777) == "%f foo %*s");
+};
+
 TEST_CASE("utf8printf standard string formatters", "[utf8printf]")
 {
   const wchar_t wc = L'\xe1';