[NFC] Make format() more amenable to format attributes

[fcloutier]

Summary

llvm/Support/Format.h provides format(), a variadic template function that accepts a printf-style format string and the corresponding fixed format arguments. While this function does forward its arguments to snprintf, which is type-checked thanks to the gnu::format attribute on platforms where that attribute is supported, it cannot itself have that attribute because GCC does not allow the format attribute on non-variadic functions (variadic template functions not excepted).

Regardless, we can change the implementation of format() so that it would be more amenable to this attribute. Recently, clang did gain support for gnu::format on non-variadic functions. We still can't use it because GCC would reject the code. However, this does allow vendors who never build LLVM with GCC themselves to maintain a patch which has that attribute enabled, catching a few errors in the process. The changes to the format_object class allow us to attribute its constructor with gnu::format (which, in turns, allows us to attribute format() with gnu::format as well).

In addition, we create a new function, format_chk(Expected, Fmt, T...), for the case where Fmt isn't known at compile-time. At runtime, Fmt is verified to have specifiers which are compatible with Expected's.

There will be another PR fixing any eventual format specifier mistakes found. In warning flags parlance, this would allow LLVM to build with -Wformat -Wformat-nonliteral.

[ahatanak]

If you write format_chk("<fallback: %u>", Format.c_str(), (unsigned char)(From)); in your code and at runtime the format string passed to the function is "%hu" (which means an argument of type short is expected), EnsureCompatible will just accept it. Is that correct?

Since clang emits a warning when compiling printf("%hu", (char)c); with -Wformat-type-confusion, it seems that you may want EnsureCompatible to reject it at runtime if you want to be strict and follow what clang is doing.

What are the rules for accepting or rejecting a format string at runtime?

[fcloutier]

Clang is being more zealous than the standard by warning you about this because integer-type variadic arguments undergo integer promotion (6.5.2.2:6 in n2310 numbering). It is effectively impossible to pass an integer smaller than int to a variadic function. This means %hu and %hhu are constraints that the format function needs to read an unsigned short or unsigned char from the va_list, but anything smaller than unsigned will be converted to unsigned as it is being passed to the variadic function, and va_arg(ap, unsigned char) is necessarily equivalent to (unsigned char)va_arg(ap, unsigned). This is reiterated and explicitly spelled out in 7.21.6.1:7, which describes format strings:

hh: Specifies that a following d, I, o, u, x, or X conversion specifier applies to a signed char or unsigned char argument (the argument will have been promoted according to the integer promotions, but its value shall be converted to signed char or unsigned char before printing); or that a following n conversion specifier applies to a pointer to a signed char argument.

This is the rationale for which EnsureCompatible lets you "confuse" any integer size up to int. The bar chosen is whether it would be undefined behavior to proceed, and in this case it isn't.

[ahatanak]

llvm/include/llvm/Support/Format.h

178

I don't think it's clear to the users of this function what they should pass for Expected. Can you leave a comment that explains what this function expects or in which cases Expected and Fmt would be incompatible?

llvm/lib/Support/Format.cpp

17

Can you leave comments that explain what these enums mean?

100

I think this rejects the combination of length modifier l and floating point conversions (e.g., %lf), but the standard doesn't say that's undefined behavior. It just says it has no effect.

158

Can you use StringRef::find or some other functions that is available instead of writing a loop here?

189

Flags are completely ignored here, but isn't the behavior undefined for some combinations of flags and conversions according to the standard (e.g., "%0p")? If it is, can you leave a comment that notes that this code ignores undefined behavior? Or can you leave a FIXME note if you plan to fix this in the future?

329

Don't you have to check that the lengths of both lists are equal? What happens if Expected is %d%d and Fmt is %d?

llvm/lib/TableGen/SetTheory.cpp

227–228

I think it's better to immediately fail here if the format string passed at runtime is invalid and inform the user of the location of the invalid string by calling a function in Error.h (e.g., PrintFatalError).

[fcloutier]

Address @ahatanak's comments.

llvm/include/llvm/Support/Format.h

178

Since there was just one use of this function and it was preferable to check a different way, I removed it entirely.

llvm/lib/Support/Format.cpp

158

I started off using StringRef and found it really cumbersome compared to using a raw const char *. In retrospective, it's not surprising given that printf was meant for C first. I removed next and replaced its only use with strchr to avoid the loop.

189

I implemented the missing checks.

329

It's not UB to call va_end before you've read each argument of a va_list, so the one thing that matters is that Fmt doesn't have more specifiers than Expected. This is ensured by breaking out of the loop when Fmt runs out of specifiers, regardless of whether Expected still has more. If we run out of Expected specifiers while Fmt still has specifiers, we error out.

[ahatanak]

llvm/include/llvm/Support/Format.h

42

Can you use char as the underlying type for enums?

llvm/lib/Support/Format.cpp

329

It's not UB, but why would a user want to pass a different number of specifiers? Isn't that an indication of some kind of error?

[ahatanak]

LGTM with minor comments.

llvm/lib/Support/Format.cpp

59

Function names should start with a lower case letter.

https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

Please fix this and other functions in this patch that start with an upper case letter.

184

The coding standard recommends using preincrement. A few other places are using postincrement too.

https://llvm.org/docs/CodingStandards.html#prefer-preincrement

[fcloutier]

Addressing final comments, running diff again for automated testing.

[fcloutier]

Failing tests are from clangd and WebAssembly backend, which are not touched by this change.

llvm/lib/Support/Format.cpp

329

There's just one use of it, so I'll change it to require the two sequences to have the same length.

[beanz]

In D132413#3886280, @fcloutier wrote:

Failing tests are from clangd and WebAssembly backend, which are not touched by this change.

You sure? I'm seeing failures on main now in the Support format tests on ubuntu.

[fcloutier]

That's what it complained about: https://buildkite.com/llvm-project/premerge-checks/builds/118482#01840c08-5dd5-40e8-8ebd-bcab1405d1ff

But I received the mail for Ubuntu and am on it. Sorry about that.

[thakis]

This is breaking check-llvm: http://45.33.8.238/linux/89951/step_12.txt

Please take a look and revert for now if it takes a while to fix.

[fcloutier]

I will revert now and continue the investigation since it has been about 15 minutes.

llvm/include/llvm/Support/Format.h

 #include "llvm/Support/DataTypes.h"
 #include <cassert>
 #include <cstdio>
 #include <tuple>
 #include <utility>
 
 namespace llvm {
 
+/// Utility class that parses printf-style format strings to yield the expected
+/// C type(s) of each specifier. This class is used to verify that a format
+/// string unknown at compile-time is equivalent to another format string (which
+/// itself is hopefully known at compile-time).
+class PrintfStyleFormatReader {
+public:
+  enum SpecifierType : char {
+    ST_EndOfFormatString,
+    ST_Unknown,
+    ST_WideChar,
+    ST_Int,
+    ST_Long,
+    ST_LongLong,
+    ST_IntMax,
+    ST_Size,
+    ST_Ptrdiff,
+    ST_Double,
+    ST_LongDouble,
+    ST_CString,
+    ST_WideCString,
+    ST_VoidPointer,
+    ST_Count_Char,
+    ST_Count_Short,
+    ST_Count_Int,
+    ST_Count_Long,
+    ST_Count_LongLong,
+    ST_Count_IntMax,
+    ST_Count_Size,
+    ST_Count_Ptrdiff
+  };
+
+private:
+  const char *Fmt;
+  llvm::SmallVector<SpecifierType, 3> SpecifierQueue;
+
+  void RefillSpecifierQueue();
+
+public:
+  /// Verify that the format specifiers in \p Fmt consume no more arguments than
+  /// those in \p Expected, and that all consumed arguments have a compatible
+  /// type. If \p Fmt is compatible with \p Expected in this way, \p Fmt is
+  /// returned. Otherwise, \p Expected is returned.
+  static const char *EnsureCompatible(const char *Expected, const char *Fmt);
+
+  PrintfStyleFormatReader(const char *Fmt) : Fmt(Fmt) {}
+
+  SpecifierType NextSpecifier() {
+    if (SpecifierQueue.empty())
+      RefillSpecifierQueue();
+    return SpecifierQueue.pop_back_val();
+  }
+};
+
 /// This is a helper class used for handling formatted output.  It is the
 /// abstract base class of a templated derived class.
 class format_object_base {
 protected:
-  const char *Fmt;
   ~format_object_base() = default; // Disallow polymorphic deletion.
   format_object_base(const format_object_base &) = default;
   virtual void home(); // Out of line virtual method.
 
   /// Call snprintf() for this object, on the given buffer and size.
   virtual int snprint(char *Buffer, unsigned BufferSize) const = 0;
 
 public:
-  format_object_base(const char *fmt) : Fmt(fmt) {}
+  format_object_base() = default;
 
   /// Format the object into the specified buffer.  On success, this returns
   /// the length of the formatted string.  If the buffer is too small, this
   /// returns a length to retry with, which will be larger than BufferSize.
   unsigned print(char *Buffer, unsigned BufferSize) const {
     assert(BufferSize && "Invalid buffer size!");
 
     // Print the string, leaving room for the terminating null.

 template <typename Arg, typename... Args>
 struct validate_format_parameters<Arg, Args...> {
   static_assert(std::is_scalar<Arg>::value,
                 "format can't be used with non fundamental / non pointer type");
   validate_format_parameters() { validate_format_parameters<Args...>(); }
 };
 template <> struct validate_format_parameters<> {};
 
-template <typename... Ts>
-class format_object final : public format_object_base {
-  std::tuple<Ts...> Vals;
-
-  template <std::size_t... Is>
-  int snprint_tuple(char *Buffer, unsigned BufferSize,
-                    std::index_sequence<Is...>) const {
+template <typename... Ts> auto format_capture(const char *Fmt, Ts... Vals) {
+  validate_format_parameters<Ts...>();
+  return [=](char *Buffer, unsigned BufferSize) {
 #ifdef _MSC_VER
-    return _snprintf(Buffer, BufferSize, Fmt, std::get<Is>(Vals)...);
+    return _snprintf(Buffer, BufferSize, Fmt, Vals...);
 #else
-    return snprintf(Buffer, BufferSize, Fmt, std::get<Is>(Vals)...);
+    return snprintf(Buffer, BufferSize, Fmt, Vals...);
 #endif
-  }
+  };
 
+template <typename... Ts>
+class format_object final : public format_object_base {
+  decltype(format_capture<Ts...>("", std::declval<Ts>()...)) Format;
+
 public:
-  format_object(const char *fmt, const Ts &... vals)
-      : format_object_base(fmt), Vals(vals...) {
-    validate_format_parameters<Ts...>();
-  }
+  format_object(const char *Fmt, const Ts &...vals)
+      : Format(format_capture(Fmt, vals...)) {}
 
   int snprint(char *Buffer, unsigned BufferSize) const override {
-    return snprint_tuple(Buffer, BufferSize, std::index_sequence_for<Ts...>());
+    return Format(Buffer, BufferSize);
   }
 };
 
 /// These are helper functions used to produce formatted output.  They use
 /// template type deduction to construct the appropriate instance of the
 /// format_object class to simplify their construction.
 ///
 /// This is typically used like:
 /// \code
 ///   OS << format("%0.4f", myfloat) << '\n';
 /// \endcode
 
 template <typename... Ts>
 inline format_object<Ts...> format(const char *Fmt, const Ts &... Vals) {
   return format_object<Ts...>(Fmt, Vals...);
 }
 
 /// This is a helper class for left_justify, right_justify, and center_justify.
 class FormattedString {
 public:
   enum Justification { JustifyNone, JustifyLeft, JustifyRight, JustifyCenter };
   FormattedString(StringRef S, unsigned W, Justification J)
       : Str(S), Width(W), Justify(J) {}
 
 private:

llvm/lib/Support/CMakeLists.txt

add_llvm_component_library(LLVMSupport
   ELFAttributes.cpp
   Error.cpp
   ErrorHandling.cpp
   ExtensibleRTTI.cpp
   FileCollector.cpp
   FileUtilities.cpp
   FileOutputBuffer.cpp
   FoldingSet.cpp
+  Format.cpp
   FormattedStream.cpp
   FormatVariadic.cpp
   GlobPattern.cpp
   GraphWriter.cpp
   Hashing.cpp
   InitLLVM.cpp
   InstructionCost.cpp
   IntEqClasses.cpp

llvm/lib/Support/Format.cpp

+//===- Format.cpp - Efficient printf-style formatting for streams -*- C++ -*-=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the non-template part of Format.h, which is used to
+// provide a type-safe-ish interface to printf-style formatting.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/Format.h"
+
+namespace {
+/// Enum representation of a printf-style length specifier.
+enum ArgLength : char {
+  /// Corresponds to 'hh' length specifier.
+  AL_ShortShort,
+  /// Corresponds to 'h' length specifier.
+  AL_Short,
+  /// Corresponds to default length specifier.
+  AL_Default,
+  /// Corresponds to 'l' length specifier.
+  AL_Long,
+  /// Corresponds to 'll' length specifier.
+  AL_LongLong,
+  /// Corresponds to 'j' length specifier.
+  AL_IntMax,
+  /// Corresponds to 'z' length specifier.
+  AL_Size,
+  /// Corresponds to 't' length specifier.
+  AL_Ptrdiff,
+  /// Corresponds to 'L' length specifier.
+  AL_LongDouble,
+  /// First invalid value of \p ArgLength.
+  AL_End,
+};
+
+/// Enum representation of a printf-style specifier.
+enum SpecifierChar : char {
+  /// Corresponds to any of 'd', 'i', 'u', 'o', 'x' or 'X' specifiers.
+  SC_Int,
+  /// Corresponds to any of 'f', 'F', 'e', 'E', 'g', 'G', 'a' or 'A' specifiers.
+  SC_Float,
+  /// Corresponds to 'c' specifier.
+  SC_Char,
+  /// Corresponds to 's' specifier.
+  SC_String,
+  /// Corresponds to 'p' specifier.
+  SC_VoidPointer,
+  /// Corresponds to 'n' specifier.
+  SC_Count,
+  /// First invalid value of \p SpecifierChar.
+  SC_End,
+};
+
+constexpr uint64_t SpecifierBit(char C) { return 1 << (C - 0x40); }
+
+template <size_t N>
+constexpr /* consteval */ uint64_t SpecifierMask(const char (&Specifiers)[N]) {
+  uint64_t Mask = 0;
+  for (const char *I = std::begin(Specifiers); I != std::end(Specifiers); ++I) {
+    if (*I == 0)
+      break;
+    Mask |= SpecifierBit(*I);
+  }
+  return Mask;
+}
+
+constexpr auto ST_Unknown = llvm::PrintfStyleFormatReader::ST_Unknown;
+constexpr auto ST_WideChar = llvm::PrintfStyleFormatReader::ST_WideChar;
+constexpr auto ST_Int = llvm::PrintfStyleFormatReader::ST_Int;
+constexpr auto ST_Long = llvm::PrintfStyleFormatReader::ST_Long;
+constexpr auto ST_LongLong = llvm::PrintfStyleFormatReader::ST_LongLong;
+constexpr auto ST_IntMax = llvm::PrintfStyleFormatReader::ST_IntMax;
+constexpr auto ST_Size = llvm::PrintfStyleFormatReader::ST_Size;
+constexpr auto ST_Ptrdiff = llvm::PrintfStyleFormatReader::ST_Ptrdiff;
+constexpr auto ST_Double = llvm::PrintfStyleFormatReader::ST_Double;
+constexpr auto ST_LongDouble = llvm::PrintfStyleFormatReader::ST_LongDouble;
+constexpr auto ST_CString = llvm::PrintfStyleFormatReader::ST_CString;
+constexpr auto ST_WideCString = llvm::PrintfStyleFormatReader::ST_WideCString;
+constexpr auto ST_VoidPointer = llvm::PrintfStyleFormatReader::ST_VoidPointer;
+constexpr auto ST_Count_Char = llvm::PrintfStyleFormatReader::ST_Count_Char;
+constexpr auto ST_Count_Short = llvm::PrintfStyleFormatReader::ST_Count_Short;
+constexpr auto ST_Count_Int = llvm::PrintfStyleFormatReader::ST_Count_Int;
+constexpr auto ST_Count_Long = llvm::PrintfStyleFormatReader::ST_Count_Long;
+constexpr auto ST_Count_LongLong =
+    llvm::PrintfStyleFormatReader::ST_Count_LongLong;
+constexpr auto ST_Count_IntMax = llvm::PrintfStyleFormatReader::ST_Count_IntMax;
+constexpr auto ST_Count_Size = llvm::PrintfStyleFormatReader::ST_Count_Size;
+constexpr auto ST_Count_Ptrdiff =
+    llvm::PrintfStyleFormatReader::ST_Count_Ptrdiff;
+
+llvm::PrintfStyleFormatReader::SpecifierType SpecifierTable[SC_End][AL_End] = {
+    {
+        // SC_Int
+        ST_Int,
+        ST_Int,
+        ST_Int,
+        ST_Long,
+        ST_LongLong,
+        ST_IntMax,
+        ST_Size,
+        ST_Ptrdiff,
+        ST_Unknown,
+    },
+    {
+        // SC_Float
+        ST_Unknown,
+        ST_Unknown,
+        ST_Double,
+        ST_Double,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_LongDouble,
+    },
+    {
+        // SC_Char
+        ST_Unknown,
+        ST_Unknown,
+        ST_Int,
+        ST_WideChar,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+    },
+    {
+        // SC_String
+        ST_Unknown,
+        ST_Unknown,
+        ST_CString,
+        ST_WideCString,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+    },
+    {
+        // SC_VoidPointer
+        ST_Unknown,
+        ST_Unknown,
+        ST_VoidPointer,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+        ST_Unknown,
+    },
+    {
+        // SC_Count
+        ST_Count_Char,
+        ST_Count_Short,
+        ST_Count_Int,
+        ST_Count_Long,
+        ST_Count_LongLong,
+        ST_Count_IntMax,
+        ST_Count_Size,
+        ST_Count_Ptrdiff,
+        ST_Unknown,
+    },
+};
+} // namespace
+
+namespace llvm {
+
+void PrintfStyleFormatReader::RefillSpecifierQueue() {
+  if (auto PercentPtr = strchr(Fmt, '%')) {
+    Fmt = PercentPtr;
+  } else {
+    SpecifierQueue.push_back(ST_EndOfFormatString);
+    return;
+  }
+
+  if (*++Fmt == '%') {
+    // %% case: skip and try again
+    Fmt++;
+    RefillSpecifierQueue();
+    return;
+  }
+
+  // Push ST_Unknown to SpecifierQueue. If we bail out early, this is what
+  // the caller gets. Fill in real specifiers to Specifiers: if we
+  // successfully get to the end, then swap Specifiers with SpecifierQueue.
+  SpecifierQueue.push_back(ST_Unknown);
+  llvm::SmallVector<SpecifierType, 3> Specifiers;
+
+  // Bitfield keeping track of which specifier characters are allowed given
+  // flags and precision settings. Each bit tells whether ascii character
+  // 0x40 + <bit index> is allowed as a specifier. '%', which has an ASCII value
+  // less than 0x40 and does not allow any customization, is handled by a check
+  // above. The starting value contains all standard specifiers.
+  uint64_t ValidSpecifiers = SpecifierMask("diuoxXfFeEgGaAcspn");
+
+  // update specifier mask based on flags
+  bool ReadAllFlags = false;
+  while (!ReadAllFlags) {
+    switch (*Fmt) {
+    case '+':
+    case '-':
+    case ' ':
+      // valid for all specifiers
+      ++Fmt;
+      break;
+    case '#':
+      ValidSpecifiers &= SpecifierMask("xXaAeEfFgG");
+      ++Fmt;
+      break;
+    case '0':
+      ValidSpecifiers &= SpecifierMask("diouxXaAeEfFgG");
+      ++Fmt;
+      break;
+    default:
+      ReadAllFlags = true;
+      break;
+    }
+  }
+
+  // skip width
+  if (*Fmt == '*') {
+    Specifiers.push_back(ST_Int);
+    Fmt++;
+  } else
+    while (*Fmt >= '0' && *Fmt <= '9')
+      Fmt++;
+
+  // test precision
+  if (*Fmt == '.') {
+    ValidSpecifiers &= SpecifierMask("diouxXaAeEfFgGs");
+    Fmt++;
+    if (*Fmt == '*') {
+      Specifiers.push_back(ST_Int);
+      Fmt++;
+    } else
+      while (*Fmt >= '0' && *Fmt <= '9')
+        Fmt++;
+  }
+
+  // parse length
+  bool FoundLength = false;
+  ArgLength AL = AL_Default;
+  while (!FoundLength) {
+    ArgLength NewAL;
+    switch (*Fmt) {
+    case 'h':
+      NewAL = AL_Short;
+      break;
+    case 'l':
+      NewAL = AL_Long;
+      break;
+    case 'j':
+      NewAL = AL_IntMax;
+      break;
+    case 'z':
+      NewAL = AL_Size;
+      break;
+    case 't':
+      NewAL = AL_Ptrdiff;
+      break;
+    case 'L':
+      NewAL = AL_LongDouble;
+      break;
+    default:
+      FoundLength = true;
+      continue;
+    }
+
+    if (NewAL == AL_Long && AL == AL_Long)
+      AL = AL_LongLong;
+    else if (NewAL == AL_Short && AL == AL_Short)
+      AL = AL_ShortShort;
+    else if (AL == AL_Default)
+      AL = NewAL;
+    else
+      return;
+    Fmt++;
+  }
+
+  // parse specifier; verify that the character is a valid specifier given
+  // restrictions imposed by by the use of flags and precision values
+  char Next = *Fmt++;
+  if (Next < 0x40 || (SpecifierBit(Next) & ValidSpecifiers) == 0)
+    return;
+
+  SpecifierChar SC;
+  switch (Next) {
+  case 'd':
+  case 'i':
+  case 'u':
+  case 'o':
+  case 'x':
+  case 'X':
+    SC = SC_Int;
+    break;
+
+  case 'a':
+  case 'A':
+  case 'e':
+  case 'E':
+  case 'f':
+  case 'F':
+  case 'g':
+  case 'G':
+    SC = SC_Float;
+    break;
+
+  case 'c':
+    SC = SC_Char;
+    break;
+
+  case 's':
+    SC = SC_String;
+    break;
+
+  case 'p':
+    SC = SC_VoidPointer;
+    break;
+
+  case 'n':
+    SC = SC_Count;
+    break;
+
+  default:
+    return;
+  }
+
+  auto Spec = SpecifierTable[SC][AL];
+  if (Spec == ST_Unknown)
+    return;
+
+  Specifiers.push_back(Spec);
+  std::reverse(Specifiers.begin(), Specifiers.end());
+  std::swap(Specifiers, SpecifierQueue);
+}
+
+const char *PrintfStyleFormatReader::EnsureCompatible(const char *Expected,
+                                                      const char *Fmt) {
+  PrintfStyleFormatReader EFR(Expected);
+  PrintfStyleFormatReader FFR(Fmt);
+  SpecifierType EST;
+  do {
+    EST = EFR.NextSpecifier();
+    if (EST != FFR.NextSpecifier())
+      return Expected;
+  } while (EST);
+  return Fmt;
+}
+
+} // namespace llvm

llvm/lib/TableGen/SetTheory.cpp

void apply(SetTheory &ST, DagInit *Expr, RecSet &Elts,
     if (IntInit *II = dyn_cast<IntInit>(Expr->arg_begin()[2]))
       To = II->getValue();
     else
       PrintFatalError(Loc, "To must be an integer: " + Expr->getAsString());
     if (To < 0 || To >= (1 << 30))
       PrintFatalError(Loc, "To out of range");
 
     RecordKeeper &Records =
         cast<DefInit>(Expr->getOperator())->getDef()->getRecords();
 
     Step *= From <= To ? 1 : -1;
+    const char FallbackFmt[] = "%u";
     while (true) {
       if (Step > 0 && From > To)
         break;
       else if (Step < 0 && From < To)
         break;
+      const char *const VerifiedFmt = PrintfStyleFormatReader::EnsureCompatible(
+          FallbackFmt, Format.c_str());
+      if (VerifiedFmt == FallbackFmt)
+        PrintFatalError(Loc, "Format string '" + Format +
+                                 "' is incompatible with '%u'!");
       std::string Name;
-      raw_string_ostream OS(Name);
-      OS << format(Format.c_str(), unsigned(From));
-      Record *Rec = Records.getDef(OS.str());
+      raw_string_ostream(Name) << format(VerifiedFmt, unsigned(From));
+      Record *Rec = Records.getDef(Name);
       if (!Rec)
-        PrintFatalError(Loc, "No def named '" + Name + "': " +
-          Expr->getAsString());
+        PrintFatalError(Loc,
+                        "No def named '" + Name + "': " + Expr->getAsString());
       // Try to reevaluate Rec in case it is a set.
       if (const RecVec *Result = ST.expand(Rec))
         Elts.insert(Result->begin(), Result->end());
       else
         Elts.insert(Rec);
 
       From += Step;
     }

llvm/unittests/Support/CMakeLists.txt

add_llvm_unittest(SupportTests
   ELFAttributeParserTest.cpp
   ErrnoTest.cpp
   ErrorOrTest.cpp
   ErrorTest.cpp
   ExtensibleRTTITest.cpp
   FileCollectorTest.cpp
   FileOutputBufferTest.cpp
   FileUtilitiesTest.cpp
+  FormatChkTest.cpp
   FormatVariadicTest.cpp
   FSUniqueIDTest.cpp
   GlobPatternTest.cpp
   HashBuilderTest.cpp
   Host.cpp
   IndexedAccessorTest.cpp
   InstructionCostTest.cpp
   ItaniumManglingCanonicalizerTest.cpp

llvm/unittests/Support/FormatChkTest.cpp

+//===- FormatChkTest.cpp - Unit tests for checked string formatting -------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/Format.h"
+#include "gtest/gtest.h"
+#include <vector>
+
+using namespace llvm;
+
+namespace {
+
+constexpr auto ST_Unknown = llvm::PrintfStyleFormatReader::ST_Unknown;
+constexpr auto ST_WideChar = llvm::PrintfStyleFormatReader::ST_WideChar;
+constexpr auto ST_Int = llvm::PrintfStyleFormatReader::ST_Int;
+constexpr auto ST_Long = llvm::PrintfStyleFormatReader::ST_Long;
+constexpr auto ST_LongLong = llvm::PrintfStyleFormatReader::ST_LongLong;
+constexpr auto ST_IntMax = llvm::PrintfStyleFormatReader::ST_IntMax;
+constexpr auto ST_Size = llvm::PrintfStyleFormatReader::ST_Size;
+constexpr auto ST_Ptrdiff = llvm::PrintfStyleFormatReader::ST_Ptrdiff;
+constexpr auto ST_Double = llvm::PrintfStyleFormatReader::ST_Double;
+constexpr auto ST_LongDouble = llvm::PrintfStyleFormatReader::ST_LongDouble;
+constexpr auto ST_CString = llvm::PrintfStyleFormatReader::ST_CString;
+constexpr auto ST_WideCString = llvm::PrintfStyleFormatReader::ST_WideCString;
+constexpr auto ST_VoidPointer = llvm::PrintfStyleFormatReader::ST_VoidPointer;
+constexpr auto ST_Count_Char = llvm::PrintfStyleFormatReader::ST_Count_Char;
+constexpr auto ST_Count_Short = llvm::PrintfStyleFormatReader::ST_Count_Short;
+constexpr auto ST_Count_Int = llvm::PrintfStyleFormatReader::ST_Count_Int;
+constexpr auto ST_Count_Long = llvm::PrintfStyleFormatReader::ST_Count_Long;
+constexpr auto ST_Count_LongLong =
+    llvm::PrintfStyleFormatReader::ST_Count_LongLong;
+constexpr auto ST_Count_IntMax = llvm::PrintfStyleFormatReader::ST_Count_IntMax;
+constexpr auto ST_Count_Size = llvm::PrintfStyleFormatReader::ST_Count_Size;
+constexpr auto ST_Count_Ptrdiff =
+    llvm::PrintfStyleFormatReader::ST_Count_Ptrdiff;
+
+using STVec = std::vector<PrintfStyleFormatReader::SpecifierType>;
+
+STVec ParseFormatString(const char *Fmt) {
+  STVec Result;
+  PrintfStyleFormatReader Reader(Fmt);
+  while (auto Spec = Reader.NextSpecifier()) {
+    Result.push_back(Spec);
+  }
+  return Result;
+}
+
+void ExpectEq(const char *Fmt, const STVec &RHS) {
+  EXPECT_EQ(ParseFormatString(Fmt), RHS);
+}
+
+} // namespace
+
+TEST(FormatReader, EmptyFormatString) {
+  EXPECT_EQ(ParseFormatString(""),
+            std::vector<PrintfStyleFormatReader::SpecifierType>());
+}
+
+TEST(FormatReader, PercentEscape) {
+  EXPECT_EQ(ParseFormatString("%%"),
+            std::vector<PrintfStyleFormatReader::SpecifierType>());
+}
+
+TEST(FormatReader, PercentAtEnd) { ExpectEq("%", {ST_Unknown}); }
+
+TEST(FormatReader, PercentWithWidth) { ExpectEq("%ll%", {ST_Unknown}); }
+
+TEST(FormatReader, OneFormat) {
+  ExpectEq("%i xx", {ST_Int});
+  ExpectEq("yy %i", {ST_Int});
+  ExpectEq("yy %i xx", {ST_Int});
+}
+
+TEST(FormatReader, TwoFormats) {
+  ExpectEq("%i yy %f xx", {ST_Int, ST_Double});
+  ExpectEq("zz %i yy %f", {ST_Int, ST_Double});
+  ExpectEq("zz %i yy %f xx", {ST_Int, ST_Double});
+}
+
+TEST(FormatReader, PoundFlagValid) {
+  ExpectEq("%#x", {ST_Int});
+  ExpectEq("%#X", {ST_Int});
+  ExpectEq("%#a", {ST_Double});
+  ExpectEq("%#A", {ST_Double});
+  ExpectEq("%#e", {ST_Double});
+  ExpectEq("%#E", {ST_Double});
+  ExpectEq("%#f", {ST_Double});
+  ExpectEq("%#F", {ST_Double});
+  ExpectEq("%#g", {ST_Double});
+  ExpectEq("%#G", {ST_Double});
+
+  ExpectEq("%#p", {ST_Unknown});
+  ExpectEq("%#i", {ST_Unknown});
+  ExpectEq("%#c", {ST_Unknown});
+  ExpectEq("%#s", {ST_Unknown});
+  ExpectEq("%#d", {ST_Unknown});
+  ExpectEq("%#u", {ST_Unknown});
+  ExpectEq("%#o", {ST_Unknown});
+  ExpectEq("%#n", {ST_Unknown});
+}
+
+TEST(FormatReader, ZeroFlagValid) {
+  ExpectEq("%0x", {ST_Int});
+  ExpectEq("%0X", {ST_Int});
+  ExpectEq("%0i", {ST_Int});
+  ExpectEq("%0d", {ST_Int});
+  ExpectEq("%0u", {ST_Int});
+  ExpectEq("%0o", {ST_Int});
+  ExpectEq("%0a", {ST_Double});
+  ExpectEq("%0A", {ST_Double});
+  ExpectEq("%0e", {ST_Double});
+  ExpectEq("%0E", {ST_Double});
+  ExpectEq("%0f", {ST_Double});
+  ExpectEq("%0F", {ST_Double});
+  ExpectEq("%0g", {ST_Double});
+  ExpectEq("%0G", {ST_Double});
+
+  ExpectEq("%0p", {ST_Unknown});
+  ExpectEq("%0n", {ST_Unknown});
+  ExpectEq("%0c", {ST_Unknown});
+  ExpectEq("%0s", {ST_Unknown});
+}
+
+TEST(FormatReader, PrecisionValid) {
+  ExpectEq("%.1x", {ST_Int});
+  ExpectEq("%.1X", {ST_Int});
+  ExpectEq("%.1i", {ST_Int});
+  ExpectEq("%.1d", {ST_Int});
+  ExpectEq("%.1u", {ST_Int});
+  ExpectEq("%.1o", {ST_Int});
+  ExpectEq("%.1a", {ST_Double});
+  ExpectEq("%.1A", {ST_Double});
+  ExpectEq("%.1e", {ST_Double});
+  ExpectEq("%.1E", {ST_Double});
+  ExpectEq("%.1f", {ST_Double});
+  ExpectEq("%.1F", {ST_Double});
+  ExpectEq("%.1g", {ST_Double});
+  ExpectEq("%.1G", {ST_Double});
+  ExpectEq("%.1s", {ST_CString});
+
+  ExpectEq("%.1p", {ST_Unknown});
+  ExpectEq("%.1n", {ST_Unknown});
+  ExpectEq("%.1c", {ST_Unknown});
+}
+
+TEST(FormatReader, LongWidth) {
+  ExpectEq("%1li", {ST_Long});
+  ExpectEq("%11li", {ST_Long});
+  ExpectEq("%1111li", {ST_Long});
+  ExpectEq("%10li", {ST_Long});
+  ExpectEq("%*li", {ST_Int, ST_Long});
+  ExpectEq("%*l!", {ST_Unknown});
+}
+
+TEST(FormatReader, LongPrecision) {
+  ExpectEq("%.1li", {ST_Long});
+  ExpectEq("%.11li", {ST_Long});
+  ExpectEq("%.1111li", {ST_Long});
+  ExpectEq("%.10li", {ST_Long});
+  ExpectEq("%.*li", {ST_Int, ST_Long});
+  ExpectEq("%.*l!", {ST_Unknown});
+
+  ExpectEq("%1.1li", {ST_Long});
+  ExpectEq("%11.11li", {ST_Long});
+  ExpectEq("%111.1111li", {ST_Long});
+  ExpectEq("%110.10li", {ST_Long});
+  ExpectEq("%1.*li", {ST_Int, ST_Long});
+  ExpectEq("%1.*l!", {ST_Unknown});
+
+  ExpectEq("%*.*li", {ST_Int, ST_Int, ST_Long});
+  ExpectEq("%*.*l!", {ST_Unknown});
+}
+
+TEST(FormatReader, IntSpecifiers) {
+  ExpectEq("%hhi", {ST_Int});
+  ExpectEq("%hhd", {ST_Int});
+  ExpectEq("%hi", {ST_Int});
+  ExpectEq("%hd", {ST_Int});
+  ExpectEq("%i", {ST_Int});
+  ExpectEq("%d", {ST_Int});
+  ExpectEq("%li", {ST_Long});
+  ExpectEq("%ld", {ST_Long});
+  ExpectEq("%lli", {ST_LongLong});
+  ExpectEq("%lld", {ST_LongLong});
+  ExpectEq("%ji", {ST_IntMax});
+  ExpectEq("%jd", {ST_IntMax});
+  ExpectEq("%zi", {ST_Size});
+  ExpectEq("%zd", {ST_Size});
+  ExpectEq("%ti", {ST_Ptrdiff});
+  ExpectEq("%td", {ST_Ptrdiff});
+
+  ExpectEq("%Li", {ST_Unknown});
+  ExpectEq("%Ld", {ST_Unknown});
+}
+
+TEST(FormatReader, UIntSpecifiers) {
+  ExpectEq("%hhu", {ST_Int});
+  ExpectEq("%hho", {ST_Int});
+  ExpectEq("%hhx", {ST_Int});
+  ExpectEq("%hhX", {ST_Int});
+  ExpectEq("%hu", {ST_Int});
+  ExpectEq("%ho", {ST_Int});
+  ExpectEq("%hx", {ST_Int});
+  ExpectEq("%hX", {ST_Int});
+  ExpectEq("%u", {ST_Int});
+  ExpectEq("%o", {ST_Int});
+  ExpectEq("%x", {ST_Int});
+  ExpectEq("%X", {ST_Int});
+  ExpectEq("%lu", {ST_Long});
+  ExpectEq("%lo", {ST_Long});
+  ExpectEq("%lx", {ST_Long});
+  ExpectEq("%lX", {ST_Long});
+  ExpectEq("%llu", {ST_LongLong});
+  ExpectEq("%llo", {ST_LongLong});
+  ExpectEq("%llx", {ST_LongLong});
+  ExpectEq("%llX", {ST_LongLong});
+  ExpectEq("%ju", {ST_IntMax});
+  ExpectEq("%jo", {ST_IntMax});
+  ExpectEq("%jx", {ST_IntMax});
+  ExpectEq("%jX", {ST_IntMax});
+  ExpectEq("%zu", {ST_Size});
+  ExpectEq("%zo", {ST_Size});
+  ExpectEq("%zx", {ST_Size});
+  ExpectEq("%zX", {ST_Size});
+  ExpectEq("%tu", {ST_Ptrdiff});
+  ExpectEq("%to", {ST_Ptrdiff});
+  ExpectEq("%tx", {ST_Ptrdiff});
+  ExpectEq("%tX", {ST_Ptrdiff});
+
+  ExpectEq("%Lu", {ST_Unknown});
+  ExpectEq("%Lo", {ST_Unknown});
+  ExpectEq("%Lx", {ST_Unknown});
+  ExpectEq("%LX", {ST_Unknown});
+}
+
+TEST(FormatReader, FloatSpecifiers) {
+  ExpectEq("%a", {ST_Double});
+  ExpectEq("%e", {ST_Double});
+  ExpectEq("%f", {ST_Double});
+  ExpectEq("%g", {ST_Double});
+  ExpectEq("%la", {ST_Double});
+  ExpectEq("%le", {ST_Double});
+  ExpectEq("%lf", {ST_Double});
+  ExpectEq("%lg", {ST_Double});
+
+  ExpectEq("%La", {ST_LongDouble});
+  ExpectEq("%Le", {ST_LongDouble});
+  ExpectEq("%Lf", {ST_LongDouble});
+  ExpectEq("%Lg", {ST_LongDouble});
+
+  ExpectEq("%ha", {ST_Unknown});
+  ExpectEq("%he", {ST_Unknown});
+  ExpectEq("%hf", {ST_Unknown});
+  ExpectEq("%hg", {ST_Unknown});
+  ExpectEq("%hha", {ST_Unknown});
+  ExpectEq("%hhe", {ST_Unknown});
+  ExpectEq("%hhf", {ST_Unknown});
+  ExpectEq("%hhg", {ST_Unknown});
+  ExpectEq("%lla", {ST_Unknown});
+  ExpectEq("%lle", {ST_Unknown});
+  ExpectEq("%llf", {ST_Unknown});
+  ExpectEq("%llg", {ST_Unknown});
+}
+
+TEST(FormatReader, CharSpecifiers) {
+  ExpectEq("%hhc", {ST_Unknown});
+  ExpectEq("%hc", {ST_Unknown});
+  ExpectEq("%c", {ST_Int});
+  ExpectEq("%lc", {ST_WideChar});
+  ExpectEq("%llc", {ST_Unknown});
+  ExpectEq("%jc", {ST_Unknown});
+  ExpectEq("%zc", {ST_Unknown});
+  ExpectEq("%tc", {ST_Unknown});
+  ExpectEq("%Lc", {ST_Unknown});
+}
+
+TEST(FormatReader, StringSpecifiers) {
+  ExpectEq("%hhs", {ST_Unknown});
+  ExpectEq("%hs", {ST_Unknown});
+  ExpectEq("%s", {ST_CString});
+  ExpectEq("%ls", {ST_WideCString});
+  ExpectEq("%lls", {ST_Unknown});
+  ExpectEq("%js", {ST_Unknown});
+  ExpectEq("%zs", {ST_Unknown});
+  ExpectEq("%ts", {ST_Unknown});
+  ExpectEq("%Ls", {ST_Unknown});
+}
+
+TEST(FormatReader, VoidPointerSpecifiers) {
+  ExpectEq("%hhp", {ST_Unknown});
+  ExpectEq("%hp", {ST_Unknown});
+  ExpectEq("%p", {ST_VoidPointer});
+  ExpectEq("%lp", {ST_Unknown});
+  ExpectEq("%llp", {ST_Unknown});
+  ExpectEq("%jp", {ST_Unknown});
+  ExpectEq("%zp", {ST_Unknown});
+  ExpectEq("%tp", {ST_Unknown});
+  ExpectEq("%Lp", {ST_Unknown});
+}
+
+TEST(FormatReader, CountSpecifiers) {
+  ExpectEq("%hhn", {ST_Count_Char});
+  ExpectEq("%hn", {ST_Count_Short});
+  ExpectEq("%n", {ST_Count_Int});
+  ExpectEq("%ln", {ST_Count_Long});
+  ExpectEq("%lln", {ST_Count_LongLong});
+  ExpectEq("%jn", {ST_Count_IntMax});
+  ExpectEq("%zn", {ST_Count_Size});
+  ExpectEq("%tn", {ST_Count_Ptrdiff});
+  ExpectEq("%Ln", {ST_Unknown});
+}

llvm/utils/TableGen/GlobalISel/GIMatchDag.cpp

for (const auto &N : InstrNodes) {
     OS << "]\n";
   }
 
   for (const auto &E : Edges) {
     const char *FromFmt = "Node%p:s%d:n";
     const char *ToFmt = "Node%p:d%d:s";
     if (E->getFromMO()->isDef() && !E->getToMO()->isDef())
       std::swap(FromFmt, ToFmt);
-    auto From = format(FromFmt, E->getFromMI(), E->getFromMO()->getIdx());
-    auto To = format(ToFmt, E->getToMI(), E->getToMO()->getIdx());
-    if (E->getFromMO()->isDef() && !E->getToMO()->isDef())
-      std::swap(From, To);
+    auto FromF = format(FromFmt, E->getFromMI(), E->getFromMO()->getIdx());
+    auto ToF = format(ToFmt, E->getToMI(), E->getToMO()->getIdx());
+    bool Swap = E->getFromMO()->isDef() && !E->getToMO()->isDef();
+    auto &From = Swap ? ToF : FromF;
+    auto &To = Swap ? FromF : ToF;
 
     OS << "  " << From << " -> " << To << " [label=\"$" << E->getName();
     if (E->getFromMO()->isDef() == E->getToMO()->isDef())
       OS << " INVALID EDGE!";
     OS << "\"";
     if (E->getFromMO()->isDef() == E->getToMO()->isDef())
       OS << ",color=red";
     else if (E->getFromMO()->isDef() && !E->getToMO()->isDef())