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

[fcloutier]

Summary

Second take on https://reviews.llvm.org/rGfb1e90ef07fec0d64a05c0b6d41117a5ea3e8344, which was pulled out because it broke GCC-based builds. The offending code was:

constexpr uint64_t specifierBit(char C) { return 1 << (C - 0x40); }

The issue is that the shift expression has type int, but the shift value has range 0..<0x40. Shifting (int)1 by more than 30 is undefined behavior.

With GCC, this manifested with test failures, and when sanitizers were enabled, with sanitizer traps. However, because the function is constexpr, Clang chose to evaluate it at compile-time even when when its execution was undefined. (Both Clang and GCC reject calls to specifierBit and its specifierMask caller when using constinit.) Unfortunately, somehow, the result was good enough to pass all tests when built with clang; and since the undefined behavior was evaluated at build time, UBSan wouldn't catch it.

I have reproduced the problem with a GCC build of LLVM and verified that this change fixes it:

constexpr uint64_t specifierBit(char C) { return (uint64_t)1 << (C - 0x40); }

[ahatanak]

Probably not NFC according to https://llvm.org/docs/Lexicon.html, but LGTM.

llvm/lib/Support/Format.cpp

60

Can you add an explanation for what the number 0x40 means or why it was chosen?

[fcloutier]

Add comment explaining what specifierBit does.

[efriedma]

since the undefined behavior was evaluated at build time, UBSan wouldn't catch it.

Can you file a bug for this? Opportunistic constant evaluation like that shouldn't bypass ubsan.

[fcloutier]

Fix failing test when parsing "%" as a format string.

[fcloutier]

In D136805#3887165, @efriedma wrote:

since the undefined behavior was evaluated at build time, UBSan wouldn't catch it.

Can you file a bug for this? Opportunistic constant evaluation like that shouldn't bypass ubsan.

I'm not able to reproduce the problem and my setup was janky, so I must have jumped to conclusions too fast after a tangential Godbolt test shows that Clang will evaluate some UB at compile-time (with sanitizers disabled) where GCC won't. This example is probably happening in the back-end instead of the front-end. Sorry for the bad signal.

[fcloutier]

Fix whitespace difference that clang-format didn't like.

[efriedma]

In D136805#3897947, @fcloutier wrote:

In D136805#3887165, @efriedma wrote:

since the undefined behavior was evaluated at build time, UBSan wouldn't catch it.

Can you file a bug for this? Opportunistic constant evaluation like that shouldn't bypass ubsan.

I'm not able to reproduce the problem and my setup was janky, so I must have jumped to conclusions too fast after a tangential Godbolt test shows that Clang will evaluate some UB at compile-time (with sanitizers disabled) where GCC won't. This example is probably happening in the back-end instead of the front-end. Sorry for the bad signal.

I managed to come up with a case similar to what you described: https://github.com/llvm/llvm-project/issues/58735 . I think whether the issue triggers depends on how the result of the call is used.

[fcloutier]

Merged to mainline; sadly forgot to update the commit message to include the differential ID, so manually closing. I'll see if I can find existing git hooks that check for that in the future.

https://reviews.llvm.org/rGcf239c2f1777

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:

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) {
+  // specifierMask builds a bit map where each set bit indicates that the
+  // character whose ASCII value is 64 + <bit index> would be legal to use
+  // as a format specifier in the current parsing context.
+  // To cover all ASCII characters, we would need 128 bits; however, the only
+  // character with an ASCII value less than 64 that can be used as a specifier
+  // is % (as in %%), so we save some space and complexity by dropping the
+  // lower half of the bit map, which is going to be all zeroes anyway.
+  // % is handled as a special case.
+  return (uint64_t)1 << (C - 64);
+}
+
+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 == 0)
+    return;
+
+  ++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;
+}
+
+#define EXPECT_FMT_EQ(FMT, ...)                                                \
+  EXPECT_EQ(ParseFormatString(FMT), STVec({__VA_ARGS__}))
+
+} // namespace
+
+TEST(FormatReader, EmptyFormatString) {
+  EXPECT_EQ(ParseFormatString(""),
+            std::vector<PrintfStyleFormatReader::SpecifierType>());
+}
+
+TEST(FormatReader, PercentEscape) {
+  EXPECT_EQ(ParseFormatString("%%"),
+            std::vector<PrintfStyleFormatReader::SpecifierType>());
+}
+
+TEST(FormatReader, PercentAtEnd) { EXPECT_FMT_EQ("%", ST_Unknown); }
+
+TEST(FormatReader, PercentWithWidth) { EXPECT_FMT_EQ("%ll%", ST_Unknown); }
+
+TEST(FormatReader, OneFormat) {
+  EXPECT_FMT_EQ("%i xx", ST_Int);
+  EXPECT_FMT_EQ("yy %i", ST_Int);
+  EXPECT_FMT_EQ("yy %i xx", ST_Int);
+}
+
+TEST(FormatReader, TwoFormats) {
+  EXPECT_FMT_EQ("%i yy %f xx", ST_Int, ST_Double);
+  EXPECT_FMT_EQ("zz %i yy %f", ST_Int, ST_Double);
+  EXPECT_FMT_EQ("zz %i yy %f xx", ST_Int, ST_Double);
+}
+
+TEST(FormatReader, PoundFlagValid) {
+  EXPECT_FMT_EQ("%#x", ST_Int);
+  EXPECT_FMT_EQ("%#X", ST_Int);
+  EXPECT_FMT_EQ("%#a", ST_Double);
+  EXPECT_FMT_EQ("%#A", ST_Double);
+  EXPECT_FMT_EQ("%#e", ST_Double);
+  EXPECT_FMT_EQ("%#E", ST_Double);
+  EXPECT_FMT_EQ("%#f", ST_Double);
+  EXPECT_FMT_EQ("%#F", ST_Double);
+  EXPECT_FMT_EQ("%#g", ST_Double);
+  EXPECT_FMT_EQ("%#G", ST_Double);
+
+  EXPECT_FMT_EQ("%#p", ST_Unknown);
+  EXPECT_FMT_EQ("%#i", ST_Unknown);
+  EXPECT_FMT_EQ("%#c", ST_Unknown);
+  EXPECT_FMT_EQ("%#s", ST_Unknown);
+  EXPECT_FMT_EQ("%#d", ST_Unknown);
+  EXPECT_FMT_EQ("%#u", ST_Unknown);
+  EXPECT_FMT_EQ("%#o", ST_Unknown);
+  EXPECT_FMT_EQ("%#n", ST_Unknown);
+}
+
+TEST(FormatReader, ZeroFlagValid) {
+  EXPECT_FMT_EQ("%0x", ST_Int);
+  EXPECT_FMT_EQ("%0X", ST_Int);
+  EXPECT_FMT_EQ("%0i", ST_Int);
+  EXPECT_FMT_EQ("%0d", ST_Int);
+  EXPECT_FMT_EQ("%0u", ST_Int);
+  EXPECT_FMT_EQ("%0o", ST_Int);
+  EXPECT_FMT_EQ("%0a", ST_Double);
+  EXPECT_FMT_EQ("%0A", ST_Double);
+  EXPECT_FMT_EQ("%0e", ST_Double);
+  EXPECT_FMT_EQ("%0E", ST_Double);
+  EXPECT_FMT_EQ("%0f", ST_Double);
+  EXPECT_FMT_EQ("%0F", ST_Double);
+  EXPECT_FMT_EQ("%0g", ST_Double);
+  EXPECT_FMT_EQ("%0G", ST_Double);
+
+  EXPECT_FMT_EQ("%0p", ST_Unknown);
+  EXPECT_FMT_EQ("%0n", ST_Unknown);
+  EXPECT_FMT_EQ("%0c", ST_Unknown);
+  EXPECT_FMT_EQ("%0s", ST_Unknown);
+}
+
+TEST(FormatReader, PrecisionValid) {
+  EXPECT_FMT_EQ("%.1x", ST_Int);
+  EXPECT_FMT_EQ("%.1X", ST_Int);
+  EXPECT_FMT_EQ("%.1i", ST_Int);
+  EXPECT_FMT_EQ("%.1d", ST_Int);
+  EXPECT_FMT_EQ("%.1u", ST_Int);
+  EXPECT_FMT_EQ("%.1o", ST_Int);
+  EXPECT_FMT_EQ("%.1a", ST_Double);
+  EXPECT_FMT_EQ("%.1A", ST_Double);
+  EXPECT_FMT_EQ("%.1e", ST_Double);
+  EXPECT_FMT_EQ("%.1E", ST_Double);
+  EXPECT_FMT_EQ("%.1f", ST_Double);
+  EXPECT_FMT_EQ("%.1F", ST_Double);
+  EXPECT_FMT_EQ("%.1g", ST_Double);
+  EXPECT_FMT_EQ("%.1G", ST_Double);
+  EXPECT_FMT_EQ("%.1s", ST_CString);
+
+  EXPECT_FMT_EQ("%.1p", ST_Unknown);
+  EXPECT_FMT_EQ("%.1n", ST_Unknown);
+  EXPECT_FMT_EQ("%.1c", ST_Unknown);
+}
+
+TEST(FormatReader, LongWidth) {
+  EXPECT_FMT_EQ("%1li", ST_Long);
+  EXPECT_FMT_EQ("%11li", ST_Long);
+  EXPECT_FMT_EQ("%1111li", ST_Long);
+  EXPECT_FMT_EQ("%10li", ST_Long);
+  EXPECT_FMT_EQ("%*li", ST_Int, ST_Long);
+  EXPECT_FMT_EQ("%*l!", ST_Unknown);
+}
+
+TEST(FormatReader, LongPrecision) {
+  EXPECT_FMT_EQ("%.1li", ST_Long);
+  EXPECT_FMT_EQ("%.11li", ST_Long);
+  EXPECT_FMT_EQ("%.1111li", ST_Long);
+  EXPECT_FMT_EQ("%.10li", ST_Long);
+  EXPECT_FMT_EQ("%.*li", ST_Int, ST_Long);
+  EXPECT_FMT_EQ("%.*l!", ST_Unknown);
+
+  EXPECT_FMT_EQ("%1.1li", ST_Long);
+  EXPECT_FMT_EQ("%11.11li", ST_Long);
+  EXPECT_FMT_EQ("%111.1111li", ST_Long);
+  EXPECT_FMT_EQ("%110.10li", ST_Long);
+  EXPECT_FMT_EQ("%1.*li", ST_Int, ST_Long);
+  EXPECT_FMT_EQ("%1.*l!", ST_Unknown);
+
+  EXPECT_FMT_EQ("%*.*li", ST_Int, ST_Int, ST_Long);
+  EXPECT_FMT_EQ("%*.*l!", ST_Unknown);
+}
+
+TEST(FormatReader, IntSpecifiers) {
+  EXPECT_FMT_EQ("%hhi", ST_Int);
+  EXPECT_FMT_EQ("%hhd", ST_Int);
+  EXPECT_FMT_EQ("%hi", ST_Int);
+  EXPECT_FMT_EQ("%hd", ST_Int);
+  EXPECT_FMT_EQ("%i", ST_Int);
+  EXPECT_FMT_EQ("%d", ST_Int);
+  EXPECT_FMT_EQ("%li", ST_Long);
+  EXPECT_FMT_EQ("%ld", ST_Long);
+  EXPECT_FMT_EQ("%lli", ST_LongLong);
+  EXPECT_FMT_EQ("%lld", ST_LongLong);
+  EXPECT_FMT_EQ("%ji", ST_IntMax);
+  EXPECT_FMT_EQ("%jd", ST_IntMax);
+  EXPECT_FMT_EQ("%zi", ST_Size);
+  EXPECT_FMT_EQ("%zd", ST_Size);
+  EXPECT_FMT_EQ("%ti", ST_Ptrdiff);
+  EXPECT_FMT_EQ("%td", ST_Ptrdiff);
+
+  EXPECT_FMT_EQ("%Li", ST_Unknown);
+  EXPECT_FMT_EQ("%Ld", ST_Unknown);
+}
+
+TEST(FormatReader, UIntSpecifiers) {
+  EXPECT_FMT_EQ("%hhu", ST_Int);
+  EXPECT_FMT_EQ("%hho", ST_Int);
+  EXPECT_FMT_EQ("%hhx", ST_Int);
+  EXPECT_FMT_EQ("%hhX", ST_Int);
+  EXPECT_FMT_EQ("%hu", ST_Int);
+  EXPECT_FMT_EQ("%ho", ST_Int);
+  EXPECT_FMT_EQ("%hx", ST_Int);
+  EXPECT_FMT_EQ("%hX", ST_Int);
+  EXPECT_FMT_EQ("%u", ST_Int);
+  EXPECT_FMT_EQ("%o", ST_Int);
+  EXPECT_FMT_EQ("%x", ST_Int);
+  EXPECT_FMT_EQ("%X", ST_Int);
+  EXPECT_FMT_EQ("%lu", ST_Long);
+  EXPECT_FMT_EQ("%lo", ST_Long);
+  EXPECT_FMT_EQ("%lx", ST_Long);
+  EXPECT_FMT_EQ("%lX", ST_Long);
+  EXPECT_FMT_EQ("%llu", ST_LongLong);
+  EXPECT_FMT_EQ("%llo", ST_LongLong);
+  EXPECT_FMT_EQ("%llx", ST_LongLong);
+  EXPECT_FMT_EQ("%llX", ST_LongLong);
+  EXPECT_FMT_EQ("%ju", ST_IntMax);
+  EXPECT_FMT_EQ("%jo", ST_IntMax);
+  EXPECT_FMT_EQ("%jx", ST_IntMax);
+  EXPECT_FMT_EQ("%jX", ST_IntMax);
+  EXPECT_FMT_EQ("%zu", ST_Size);
+  EXPECT_FMT_EQ("%zo", ST_Size);
+  EXPECT_FMT_EQ("%zx", ST_Size);
+  EXPECT_FMT_EQ("%zX", ST_Size);
+  EXPECT_FMT_EQ("%tu", ST_Ptrdiff);
+  EXPECT_FMT_EQ("%to", ST_Ptrdiff);
+  EXPECT_FMT_EQ("%tx", ST_Ptrdiff);
+  EXPECT_FMT_EQ("%tX", ST_Ptrdiff);
+
+  EXPECT_FMT_EQ("%Lu", ST_Unknown);
+  EXPECT_FMT_EQ("%Lo", ST_Unknown);
+  EXPECT_FMT_EQ("%Lx", ST_Unknown);
+  EXPECT_FMT_EQ("%LX", ST_Unknown);
+}
+
+TEST(FormatReader, FloatSpecifiers) {
+  EXPECT_FMT_EQ("%a", ST_Double);
+  EXPECT_FMT_EQ("%e", ST_Double);
+  EXPECT_FMT_EQ("%f", ST_Double);
+  EXPECT_FMT_EQ("%g", ST_Double);
+  EXPECT_FMT_EQ("%la", ST_Double);
+  EXPECT_FMT_EQ("%le", ST_Double);
+  EXPECT_FMT_EQ("%lf", ST_Double);
+  EXPECT_FMT_EQ("%lg", ST_Double);
+
+  EXPECT_FMT_EQ("%La", ST_LongDouble);
+  EXPECT_FMT_EQ("%Le", ST_LongDouble);
+  EXPECT_FMT_EQ("%Lf", ST_LongDouble);
+  EXPECT_FMT_EQ("%Lg", ST_LongDouble);
+
+  EXPECT_FMT_EQ("%ha", ST_Unknown);
+  EXPECT_FMT_EQ("%he", ST_Unknown);
+  EXPECT_FMT_EQ("%hf", ST_Unknown);
+  EXPECT_FMT_EQ("%hg", ST_Unknown);
+  EXPECT_FMT_EQ("%hha", ST_Unknown);
+  EXPECT_FMT_EQ("%hhe", ST_Unknown);
+  EXPECT_FMT_EQ("%hhf", ST_Unknown);
+  EXPECT_FMT_EQ("%hhg", ST_Unknown);
+  EXPECT_FMT_EQ("%lla", ST_Unknown);
+  EXPECT_FMT_EQ("%lle", ST_Unknown);
+  EXPECT_FMT_EQ("%llf", ST_Unknown);
+  EXPECT_FMT_EQ("%llg", ST_Unknown);
+}
+
+TEST(FormatReader, CharSpecifiers) {
+  EXPECT_FMT_EQ("%hhc", ST_Unknown);
+  EXPECT_FMT_EQ("%hc", ST_Unknown);
+  EXPECT_FMT_EQ("%c", ST_Int);
+  EXPECT_FMT_EQ("%lc", ST_WideChar);
+  EXPECT_FMT_EQ("%llc", ST_Unknown);
+  EXPECT_FMT_EQ("%jc", ST_Unknown);
+  EXPECT_FMT_EQ("%zc", ST_Unknown);
+  EXPECT_FMT_EQ("%tc", ST_Unknown);
+  EXPECT_FMT_EQ("%Lc", ST_Unknown);
+}
+
+TEST(FormatReader, StringSpecifiers) {
+  EXPECT_FMT_EQ("%hhs", ST_Unknown);
+  EXPECT_FMT_EQ("%hs", ST_Unknown);
+  EXPECT_FMT_EQ("%s", ST_CString);
+  EXPECT_FMT_EQ("%ls", ST_WideCString);
+  EXPECT_FMT_EQ("%lls", ST_Unknown);
+  EXPECT_FMT_EQ("%js", ST_Unknown);
+  EXPECT_FMT_EQ("%zs", ST_Unknown);
+  EXPECT_FMT_EQ("%ts", ST_Unknown);
+  EXPECT_FMT_EQ("%Ls", ST_Unknown);
+}
+
+TEST(FormatReader, VoidPointerSpecifiers) {
+  EXPECT_FMT_EQ("%hhp", ST_Unknown);
+  EXPECT_FMT_EQ("%hp", ST_Unknown);
+  EXPECT_FMT_EQ("%p", ST_VoidPointer);
+  EXPECT_FMT_EQ("%lp", ST_Unknown);
+  EXPECT_FMT_EQ("%llp", ST_Unknown);
+  EXPECT_FMT_EQ("%jp", ST_Unknown);
+  EXPECT_FMT_EQ("%zp", ST_Unknown);
+  EXPECT_FMT_EQ("%tp", ST_Unknown);
+  EXPECT_FMT_EQ("%Lp", ST_Unknown);
+}
+
+TEST(FormatReader, CountSpecifiers) {
+  EXPECT_FMT_EQ("%hhn", ST_Count_Char);
+  EXPECT_FMT_EQ("%hn", ST_Count_Short);
+  EXPECT_FMT_EQ("%n", ST_Count_Int);
+  EXPECT_FMT_EQ("%ln", ST_Count_Long);
+  EXPECT_FMT_EQ("%lln", ST_Count_LongLong);
+  EXPECT_FMT_EQ("%jn", ST_Count_IntMax);
+  EXPECT_FMT_EQ("%zn", ST_Count_Size);
+  EXPECT_FMT_EQ("%tn", ST_Count_Ptrdiff);
+  EXPECT_FMT_EQ("%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())