[abhina-sree]

In many places in the format string parsing, there are direct character comparisons of string literals to hardcoded characters in UTF-8.
This patch uses the FromSystemEncodingConverter to perform the conversion from SystemEncoding -> UTF-8 in order to compare already converted literals to those hardcoded UTF-8 characters.

[llvmorg-github-actions]

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-llvm-support

Author: Abhina Sree (abhina-sree)

Changes

---

Patch is 42.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/196568.diff

18 Files Affected:

• (modified) clang/include/clang/AST/Expr.h (+6)
• (modified) clang/include/clang/AST/FormatString.h (+7-5)
• (modified) clang/include/clang/Basic/TargetInfo.h (+3)
• (modified) clang/include/clang/Lex/TextEncodingConfig.h (+2-1)
• (modified) clang/include/clang/Sema/Sema.h (+1-1)
• (modified) clang/lib/AST/Expr.cpp (+14)
• (modified) clang/lib/AST/FormatString.cpp (+46-40)
• (modified) clang/lib/AST/FormatStringParsing.h (+25-11)
• (modified) clang/lib/AST/PrintfFormatString.cpp (+58-31)
• (modified) clang/lib/AST/ScanfFormatString.cpp (+15-8)
• (modified) clang/lib/Basic/TargetInfo.cpp (+3)
• (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-1)
• (modified) clang/lib/Lex/TextEncodingConfig.cpp (+10-1)
• (modified) clang/lib/Sema/SemaChecking.cpp (+33-21)
• (modified) clang/lib/Sema/SemaExpr.cpp (+3-2)
• (modified) clang/test/CodeGen/systemz-charset.c (+2)
• (modified) llvm/include/llvm/Support/TextEncoding.h (+10)
• (modified) llvm/lib/Support/TextEncoding.cpp (+19)

diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 393fe275c6269..d01afcff4095d 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -28,6 +28,7 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SyncScope.h"
 #include "clang/Basic/TypeTraits.h"
+#include "clang/Lex/TextEncodingConfig.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
@@ -2066,6 +2067,11 @@ class PredefinedExpr final
     return getIdentKindName(getIdentKind());
   }
 
+  static std::string
+  ComputeNameAndTranslate(PredefinedIdentKind IK, const Decl *CurrentDecl,
+                          TextEncodingConfig &TEC,
+                          bool ForceElaboratedPrinting = false);
+
   static std::string ComputeName(PredefinedIdentKind IK,
                                  const Decl *CurrentDecl,
                                  bool ForceElaboratedPrinting = false);
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index a3382e1a1d007..a24ade2d71ee9 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -19,6 +19,7 @@
 #define LLVM_CLANG_AST_FORMATSTRING_H
 
 #include "clang/AST/CanonicalType.h"
+#include "llvm/Support/TextEncoding.h"
 #include <optional>
 
 namespace clang {
@@ -728,7 +729,8 @@ class FormatStringHandler {
 
   virtual bool HandleInvalidPrintfConversionSpecifier(
       const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier,
-      unsigned specifierLen) {
+      unsigned specifierLen,
+      const llvm::TextEncodingConverter &FormatStrConverter) {
     return true;
   }
 
@@ -744,10 +746,10 @@ class FormatStringHandler {
 
   // Scanf-specific handlers.
 
-  virtual bool
-  HandleInvalidScanfConversionSpecifier(const analyze_scanf::ScanfSpecifier &FS,
-                                        const char *startSpecifier,
-                                        unsigned specifierLen) {
+  virtual bool HandleInvalidScanfConversionSpecifier(
+      const analyze_scanf::ScanfSpecifier &FS, const char *startSpecifier,
+      unsigned specifierLen,
+      const llvm::TextEncodingConverter &FormatStrConverter) {
     return true;
   }
 
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 9f7d2a17a0f8a..ec7d4fcd4d8e3 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -38,6 +38,7 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/TextEncoding.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/TargetParser/Triple.h"
 #include <cassert>
@@ -323,6 +324,8 @@ class TargetInfo : public TransferrableTargetInfo,
 
   virtual ~TargetInfo();
 
+  llvm::TextEncodingConverter *FormatStrConverter;
+
   /// Retrieve the target options.
   TargetOptions &getTargetOpts() const {
     assert(TargetOpts && "Missing target options");
diff --git a/clang/include/clang/Lex/TextEncodingConfig.h b/clang/include/clang/Lex/TextEncodingConfig.h
index 09967a81beeed..f4ef578eb2991 100644
--- a/clang/include/clang/Lex/TextEncodingConfig.h
+++ b/clang/include/clang/Lex/TextEncodingConfig.h
@@ -26,7 +26,8 @@ class TextEncodingConfig {
   llvm::TextEncodingConverter *getConverter(ConversionAction Action) const;
   static std::error_code
   setConvertersFromOptions(TextEncodingConfig &TEC,
-                           const clang::LangOptions &Opts);
+                           const clang::LangOptions &Opts,
+                           clang::TargetInfo &TInfo);
 
   llvm::StringRef getExecEncoding() { return ExecEncoding; }
 };
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e2bc5593efa97..8ac5cc175fd2f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -55,7 +55,7 @@
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Basic/TypeTraits.h"
-#include "clang/Lex/LiteralConverter.h"
+#include "clang/Lex/TextEncodingConfig.h"
 #include "clang/Sema/AnalysisBasedWarnings.h"
 #include "clang/Sema/Attr.h"
 #include "clang/Sema/CleanupInfo.h"
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 64d61dbc3d128..e067df4cefd7b 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -668,6 +668,20 @@ StringRef PredefinedExpr::getIdentKindName(PredefinedIdentKind IK) {
   llvm_unreachable("Unknown ident kind for PredefinedExpr");
 }
 
+std::string PredefinedExpr::ComputeNameAndTranslate(
+    PredefinedIdentKind IK, const Decl *CurrentDecl, TextEncodingConfig &TEC,
+    bool ForceElaboratedPrinting) {
+  using namespace clang::charinfo;
+  std::string Result = ComputeName(IK, CurrentDecl, ForceElaboratedPrinting);
+  llvm::TextEncodingConverter *Converter = TEC.getConverter(CA_ToExecEncoding);
+  if (Converter) {
+    SmallString<128> Converted;
+    Converter->convert(Result, Converted);
+    Result = std::string(Converted);
+  }
+  return Result;
+}
+
 // FIXME: Maybe this should use DeclPrinter with a special "print predefined
 // expr" policy instead.
 std::string PredefinedExpr::ComputeName(PredefinedIdentKind IK,
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index 7e1ac0de6dcaf..0d449fb5f0904 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -33,8 +33,9 @@ FormatStringHandler::~FormatStringHandler() {}
 // scanf format strings.
 //===----------------------------------------------------------------------===//
 
-OptionalAmount clang::analyze_format_string::ParseAmount(const char *&Beg,
-                                                         const char *E) {
+OptionalAmount clang::analyze_format_string::ParseAmount(
+    const char *&Beg, const char *E,
+    const llvm::TextEncodingConverter &FormatStrConverter) {
   const char *I = Beg;
   UpdateOnReturn<const char *> UpdateBeg(Beg, I);
 
@@ -42,7 +43,7 @@ OptionalAmount clang::analyze_format_string::ParseAmount(const char *&Beg,
   bool hasDigits = false;
 
   for (; I != E; ++I) {
-    char c = *I;
+    char c = FormatStrConverter.convert(*I);
     if (c >= '0' && c <= '9') {
       hasDigits = true;
       accumulator = (accumulator * 10) + (c - '0');
@@ -60,21 +61,22 @@ OptionalAmount clang::analyze_format_string::ParseAmount(const char *&Beg,
 }
 
 OptionalAmount clang::analyze_format_string::ParseNonPositionAmount(
-    const char *&Beg, const char *E, unsigned &argIndex) {
-  if (*Beg == '*') {
+    const char *&Beg, const char *E, unsigned &argIndex,
+    const llvm::TextEncodingConverter &FormatStrConverter) {
+  if (FormatStrConverter.convert(*Beg) == '*') {
     ++Beg;
     return OptionalAmount(OptionalAmount::Arg, argIndex++, Beg, 0, false);
   }
 
-  return ParseAmount(Beg, E);
+  return ParseAmount(Beg, E, FormatStrConverter);
 }
 
 OptionalAmount clang::analyze_format_string::ParsePositionAmount(
     FormatStringHandler &H, const char *Start, const char *&Beg, const char *E,
-    PositionContext p) {
-  if (*Beg == '*') {
+    PositionContext p, const llvm::TextEncodingConverter &FormatStrConverter) {
+  if (FormatStrConverter.convert(*Beg) == '*') {
     const char *I = Beg + 1;
-    const OptionalAmount &Amt = ParseAmount(I, E);
+    const OptionalAmount &Amt = ParseAmount(I, E, FormatStrConverter);
 
     if (Amt.getHowSpecified() == OptionalAmount::NotSpecified) {
       H.HandleInvalidPosition(Beg, I - Beg, p);
@@ -89,7 +91,7 @@ OptionalAmount clang::analyze_format_string::ParsePositionAmount(
 
     assert(Amt.getHowSpecified() == OptionalAmount::Constant);
 
-    if (*I == '$') {
+    if (FormatStrConverter.convert(*I) == '$') {
       // Handle positional arguments
 
       // Special case: '*0$', since this is an easy mistake.
@@ -109,18 +111,21 @@ OptionalAmount clang::analyze_format_string::ParsePositionAmount(
     return OptionalAmount(false);
   }
 
-  return ParseAmount(Beg, E);
+  return ParseAmount(Beg, E, FormatStrConverter);
 }
 
 bool clang::analyze_format_string::ParseFieldWidth(
     FormatStringHandler &H, FormatSpecifier &CS, const char *Start,
-    const char *&Beg, const char *E, unsigned *argIndex) {
+    const char *&Beg, const char *E, unsigned *argIndex,
+    const llvm::TextEncodingConverter &FormatStrConverter) {
   // FIXME: Support negative field widths.
   if (argIndex) {
-    CS.setFieldWidth(ParseNonPositionAmount(Beg, E, *argIndex));
+    CS.setFieldWidth(
+        ParseNonPositionAmount(Beg, E, *argIndex, FormatStrConverter));
   } else {
     const OptionalAmount Amt = ParsePositionAmount(
-        H, Start, Beg, E, analyze_format_string::FieldWidthPos);
+        H, Start, Beg, E, analyze_format_string::FieldWidthPos,
+        FormatStrConverter);
 
     if (Amt.isInvalid())
       return true;
@@ -129,14 +134,13 @@ bool clang::analyze_format_string::ParseFieldWidth(
   return false;
 }
 
-bool clang::analyze_format_string::ParseArgPosition(FormatStringHandler &H,
-                                                    FormatSpecifier &FS,
-                                                    const char *Start,
-                                                    const char *&Beg,
-                                                    const char *E) {
+bool clang::analyze_format_string::ParseArgPosition(
+    FormatStringHandler &H, FormatSpecifier &FS, const char *Start,
+    const char *&Beg, const char *E,
+    const llvm::TextEncodingConverter &FormatStrConverter) {
   const char *I = Beg;
 
-  const OptionalAmount &Amt = ParseAmount(I, E);
+  const OptionalAmount &Amt = ParseAmount(I, E, FormatStrConverter);
 
   if (I == E) {
     // No more characters left?
@@ -144,7 +148,8 @@ bool clang::analyze_format_string::ParseArgPosition(FormatStringHandler &H,
     return true;
   }
 
-  if (Amt.getHowSpecified() == OptionalAmount::Constant && *(I++) == '$') {
+  if (Amt.getHowSpecified() == OptionalAmount::Constant &&
+      FormatStrConverter.convert(*(I++)) == '$') {
     // Warn that positional arguments are non-standard.
     H.HandlePosition(Start, I - Start);
 
@@ -165,16 +170,15 @@ bool clang::analyze_format_string::ParseArgPosition(FormatStringHandler &H,
   return false;
 }
 
-bool clang::analyze_format_string::ParseVectorModifier(FormatStringHandler &H,
-                                                       FormatSpecifier &FS,
-                                                       const char *&I,
-                                                       const char *E,
-                                                       const LangOptions &LO) {
+bool clang::analyze_format_string::ParseVectorModifier(
+    FormatStringHandler &H, FormatSpecifier &FS, const char *&I, const char *E,
+    const LangOptions &LO,
+    const llvm::TextEncodingConverter &FormatStrConverter) {
   if (!LO.OpenCL)
     return false;
 
   const char *Start = I;
-  if (*I == 'v') {
+  if (FormatStrConverter.convert(*I) == 'v') {
     ++I;
 
     if (I == E) {
@@ -182,7 +186,7 @@ bool clang::analyze_format_string::ParseVectorModifier(FormatStringHandler &H,
       return true;
     }
 
-    OptionalAmount NumElts = ParseAmount(I, E);
+    OptionalAmount NumElts = ParseAmount(I, E, FormatStrConverter);
     if (NumElts.getHowSpecified() != OptionalAmount::Constant) {
       H.HandleIncompleteSpecifier(Start, E - Start);
       return true;
@@ -194,22 +198,20 @@ bool clang::analyze_format_string::ParseVectorModifier(FormatStringHandler &H,
   return false;
 }
 
-bool clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
-                                                       const char *&I,
-                                                       const char *E,
-                                                       const LangOptions &LO,
-                                                       bool IsScanf) {
+bool clang::analyze_format_string::ParseLengthModifier(
+    FormatSpecifier &FS, const char *&I, const char *E, const LangOptions &LO,
+    const llvm::TextEncodingConverter &FormatStrConverter, bool IsScanf) {
   LengthModifier::Kind lmKind = LengthModifier::None;
   const char *lmPosition = I;
-  switch (*I) {
+  switch (FormatStrConverter.convert(*I)) {
   default:
     return false;
   case 'h':
     ++I;
-    if (I != E && *I == 'h') {
+    if (I != E && FormatStrConverter.convert(*I) == 'h') {
       ++I;
       lmKind = LengthModifier::AsChar;
-    } else if (I != E && *I == 'l' && LO.OpenCL) {
+    } else if (I != E && FormatStrConverter.convert(*I) == 'l' && LO.OpenCL) {
       ++I;
       lmKind = LengthModifier::AsShortLong;
     } else {
@@ -218,7 +220,7 @@ bool clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
     break;
   case 'l':
     ++I;
-    if (I != E && *I == 'l') {
+    if (I != E && FormatStrConverter.convert(*I) == 'l') {
       ++I;
       lmKind = LengthModifier::AsLongLong;
     } else {
@@ -251,7 +253,9 @@ bool clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
       // be parsed as the GNU extension 'a' length modifier. If not, this
       // will be parsed as a conversion specifier.
       ++I;
-      if (I != E && (*I == 's' || *I == 'S' || *I == '[')) {
+      if (I != E && (FormatStrConverter.convert(*I) == 's' ||
+                     FormatStrConverter.convert(*I) == 'S' ||
+                     FormatStrConverter.convert(*I) == '[')) {
         lmKind = LengthModifier::AsAllocate;
         break;
       }
@@ -269,7 +273,8 @@ bool clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
   // scanf:  AsInt64
   case 'I':
     if (I + 1 != E && I + 2 != E) {
-      if (I[1] == '6' && I[2] == '4') {
+      if (FormatStrConverter.convert(I[1]) == '6' &&
+          FormatStrConverter.convert(I[2]) == '4') {
         I += 3;
         lmKind = LengthModifier::AsInt64;
         break;
@@ -277,7 +282,8 @@ bool clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
       if (IsScanf)
         return false;
 
-      if (I[1] == '3' && I[2] == '2') {
+      if (FormatStrConverter.convert(I[1]) == '3' &&
+          FormatStrConverter.convert(I[2]) == '2') {
         I += 3;
         lmKind = LengthModifier::AsInt32;
         break;
diff --git a/clang/lib/AST/FormatStringParsing.h b/clang/lib/AST/FormatStringParsing.h
index 401528481a9d6..531bc291e0b5b 100644
--- a/clang/lib/AST/FormatStringParsing.h
+++ b/clang/lib/AST/FormatStringParsing.h
@@ -35,29 +35,43 @@ template <typename T> class UpdateOnReturn {
 
 namespace analyze_format_string {
 
-OptionalAmount ParseAmount(const char *&Beg, const char *E);
-OptionalAmount ParseNonPositionAmount(const char *&Beg, const char *E,
-                                      unsigned &argIndex);
+OptionalAmount
+ParseAmount(const char *&Beg, const char *E,
+            const llvm::TextEncodingConverter &FormatStrConverter);
 
-OptionalAmount ParsePositionAmount(FormatStringHandler &H, const char *Start,
-                                   const char *&Beg, const char *E,
-                                   PositionContext p);
+OptionalAmount
+ParseNonPositionAmount(const char *&Beg, const char *E, unsigned &argIndex,
+                       const llvm::TextEncodingConverter &FormatStrConverter);
+
+OptionalAmount
+ParsePositionAmount(FormatStringHandler &H, const char *Start, const char *&Beg,
+                    const char *E, PositionContext p,
+                    const llvm::TextEncodingConverter &FormatStrConverter);
+
+OptionalAmount
+ParsePositionAmount(FormatStringHandler &H, const char *Start, const char *&Beg,
+                    const char *E, PositionContext p,
+                    const llvm::TextEncodingConverter &FormatStrConverter);
 
 bool ParseFieldWidth(FormatStringHandler &H, FormatSpecifier &CS,
                      const char *Start, const char *&Beg, const char *E,
-                     unsigned *argIndex);
+                     unsigned *argIndex,
+                     const llvm::TextEncodingConverter &FormatStrConverter);
 
 bool ParseArgPosition(FormatStringHandler &H, FormatSpecifier &CS,
-                      const char *Start, const char *&Beg, const char *E);
+                      const char *Start, const char *&Beg, const char *E,
+                      const llvm::TextEncodingConverter &FormatStrConverter);
 
 bool ParseVectorModifier(FormatStringHandler &H, FormatSpecifier &FS,
-                         const char *&Beg, const char *E,
-                         const LangOptions &LO);
+                         const char *&Beg, const char *E, const LangOptions &LO,
+                         const llvm::TextEncodingConverter &FormatStrConverter);
 
 /// Returns true if a LengthModifier was parsed and installed in the
 /// FormatSpecifier& argument, and false otherwise.
 bool ParseLengthModifier(FormatSpecifier &FS, const char *&Beg, const char *E,
-                         const LangOptions &LO, bool IsScanf = false);
+                         const LangOptions &LO,
+                         const llvm::TextEncodingConverter &FormatStrConverter,
+                         bool IsScanf = false);
 
 /// Returns true if the invalid specifier in \p SpecifierBegin is a UTF-8
 /// string; check that it won't go further than \p FmtStrEnd and write
diff --git a/clang/lib/AST/PrintfFormatString.cpp b/clang/lib/AST/PrintfFormatString.cpp
index 6610a2de9e083..7efcc554ec136 100644
--- a/clang/lib/AST/PrintfFormatString.cpp
+++ b/clang/lib/AST/PrintfFormatString.cpp
@@ -35,14 +35,17 @@ typedef clang::analyze_format_string::SpecifierResult<PrintfSpecifier>
 
 using analyze_format_string::ParseNonPositionAmount;
 
-static bool ParsePrecision(FormatStringHandler &H, PrintfSpecifier &FS,
-                           const char *Start, const char *&Beg, const char *E,
-                           unsigned *argIndex) {
+static bool
+ParsePrecision(FormatStringHandler &H, PrintfSpecifier &FS, const char *Start,
+               const char *&Beg, const char *E, unsigned *argIndex,
+               const llvm::TextEncodingConverter &FormatStrConverter) {
   if (argIndex) {
-    FS.setPrecision(ParseNonPositionAmount(Beg, E, *argIndex));
+    FS.setPrecision(
+        ParseNonPositionAmount(Beg, E, *argIndex, FormatStrConverter));
   } else {
     const OptionalAmount Amt = ParsePositionAmount(
-        H, Start, Beg, E, analyze_format_string::PrecisionPos);
+        H, Start, Beg, E, analyze_format_string::PrecisionPos,
+        FormatStrConverter);
     if (Amt.isInvalid())
       return true;
     FS.setPrecision(Amt);
@@ -50,11 +53,14 @@ static bool ParsePrecision(FormatStringHandler &H, PrintfSpecifier &FS,
   return false;
 }
 
-static bool ParseObjCFlags(FormatStringHandler &H, PrintfSpecifier &FS,
-                           const char *FlagBeg, const char *E, bool Warn) {
+static bool
+ParseObjCFlags(FormatStringHandler &H, PrintfSpecifier &FS, const char *FlagBeg,
+               const char *E, bool Warn,
+               const llvm::TextEncodingConverter &FormatStrConverter) {
   StringRef Flag(FlagBeg, E - FlagBeg);
   // Currently there is only one flag.
-  if (Flag == "tt") {
+  if (Flag.size() == 2 && FormatStrConverter.convert(FlagBeg[0]) == 't' &&
+      FormatStrConverter.convert(FlagBeg[1]) == 't') {
     FS.setHasObjCTechnicalTerm(FlagBeg);
     return false;
   }
@@ -81,6 +87,8 @@ ParsePrintfSpecifier(FormatStringHandler &H, const char *&Beg, const char *E,
   const char *Start = nullptr;
   UpdateOnReturn<const char *> UpdateBeg(Beg, I);
 
+  const llvm::TextEncodingConverter &FormatStrConverter =
+      *Target.FormatStrConverter;
   // Look for a '%' character that indicates the start of a format specifier.
   for (; I != E; ++I) {
     char c = *I;
@@ -89,7 +97,7 @@ ParsePrintfSpecifier(FormatStringHandler &H, const char *&Beg, const char *E,
       H.HandleNullChar(I);
       return true;
     }
-    if (c == '%') {
+    if (FormatStrConverter.convert(c) == '%') {
       Start = I++; // Record the start of the format specifier.
       break;
     }
@@ -107,7 +115,7 @@ ParsePrintfSpecifier(FormatStringHandler &H, const char *&Beg, const char *E,
   }
 
   PrintfSpecifier FS;
-  if (ParseArgPosition(H, FS, Start, I, E))
+  if (ParseArgPosition(H, FS, Start, I, E, FormatStrConverter))
     return true;
 
   if (I == E) {
@@ -117,13 +125,17 @@ ParsePrintfSpecifier(FormatStrin...
[truncated]

[abhina-sree]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• Enable driver changes for fexec-charset #195890
• Convert to exec-charset inside getPredefinedStringLiteralFromCache #196569
• Add format string handling #196568 👈 (View in Graphite)
• [clang] Fix encoding handling of asm strings #169803
• Enable fexec-charset option  #138895
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

🐧 Linux x64 Test Results

• 190161 tests passed
• 5861 tests skipped

✅ The build succeeded and all tests passed.

[github-actions]

🪟 Windows x64 Test Results

• 148251 tests passed
• 3705 tests skipped

✅ The build succeeded and all tests passed.

[abhina-sree]

Copying the comment from the last PR from @s-barannikov

May it make sense to introduce a strong typedef for a "target" character and use it in format string parsing? This would help find all the places where we need to convert the "target" character to "internal" encoding. It would also make the intent more clear.

I'm not sure if this works. I tried implementing this, but it seems to offer little benefit, because although we are doing comparisons with the reverse-translated character, we are still using the original character after those comparisons. So it would not catch any missing conversions, unless there is a way to implement this that does catch the missing conversions

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions h,cpp -- clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp clang-tools-extra/clang-tidy/utils/FormatStringConverter.h clang/include/clang/AST/FormatString.h clang/include/clang/Lex/TextEncoding.h clang/lib/AST/FormatString.cpp clang/lib/AST/FormatStringParsing.h clang/lib/AST/OSLog.cpp clang/lib/AST/PrintfFormatString.cpp clang/lib/AST/ScanfFormatString.cpp clang/lib/Analysis/UnsafeBufferUsage.cpp clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaStmtAsm.cpp llvm/include/llvm/Support/TextEncoding.h llvm/lib/Support/TextEncoding.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index fdd40208a..796cc3d94 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -35,8 +35,7 @@ FormatStringHandler::~FormatStringHandler() {}
 //===----------------------------------------------------------------------===//
 
 OptionalAmount clang::analyze_format_string::ParseAmount(
-    const char *&Beg, const char *E,
-    const llvm::TextEncodingConverter &Conv) {
+    const char *&Beg, const char *E, const llvm::TextEncodingConverter &Conv) {
   const char *I = Beg;
   UpdateOnReturn<const char *> UpdateBeg(Beg, I);
 
@@ -89,8 +88,7 @@ OptionalAmount clang::analyze_format_string::ParseNonPositionAmount(
 
 OptionalAmount clang::analyze_format_string::ParsePositionAmount(
     FormatStringHandler &H, const char *Start, const char *&Beg, const char *E,
-    PositionContext p,
-    const llvm::TextEncodingConverter &Conv) {
+    PositionContext p, const llvm::TextEncodingConverter &Conv) {
   if (Conv.convertBasicChar(*Beg) == u8'*') {
     const char *I = Beg + 1;
     const OptionalAmount &Amt = ParseAmount(I, E, Conv);
@@ -137,12 +135,10 @@ bool clang::analyze_format_string::ParseFieldWidth(
     const llvm::TextEncodingConverter &Conv) {
   // FIXME: Support negative field widths.
   if (argIndex) {
-    CS.setFieldWidth(
-        ParseNonPositionAmount(Beg, E, *argIndex, Conv));
+    CS.setFieldWidth(ParseNonPositionAmount(Beg, E, *argIndex, Conv));
   } else {
     const OptionalAmount Amt = ParsePositionAmount(
-        H, Start, Beg, E, analyze_format_string::FieldWidthPos,
-        Conv);
+        H, Start, Beg, E, analyze_format_string::FieldWidthPos, Conv);
 
     if (Amt.isInvalid())
       return true;
@@ -153,8 +149,7 @@ bool clang::analyze_format_string::ParseFieldWidth(
 
 bool clang::analyze_format_string::ParseArgPosition(
     FormatStringHandler &H, FormatSpecifier &FS, const char *Start,
-    const char *&Beg, const char *E,
-    const llvm::TextEncodingConverter &Conv) {
+    const char *&Beg, const char *E, const llvm::TextEncodingConverter &Conv) {
   const char *I = Beg;
 
   const OptionalAmount &Amt = ParseAmount(I, E, Conv);
@@ -189,8 +184,7 @@ bool clang::analyze_format_string::ParseArgPosition(
 
 bool clang::analyze_format_string::ParseVectorModifier(
     FormatStringHandler &H, FormatSpecifier &FS, const char *&I, const char *E,
-    const LangOptions &LO,
-    const llvm::TextEncodingConverter &Conv) {
+    const LangOptions &LO, const llvm::TextEncodingConverter &Conv) {
   if (!LO.OpenCL)
     return false;
 
@@ -217,8 +211,7 @@ bool clang::analyze_format_string::ParseVectorModifier(
 
 bool clang::analyze_format_string::ParseLengthModifier(
     FormatSpecifier &FS, const char *&I, const char *E, const LangOptions &LO,
-    const llvm::TextEncodingConverter &Conv,
-    bool IsScanf) {
+    const llvm::TextEncodingConverter &Conv, bool IsScanf) {
   LengthModifier::Kind lmKind = LengthModifier::None;
   const char *lmPosition = I;
   switch (Conv.convertBasicChar(*I)) {
@@ -229,9 +222,7 @@ bool clang::analyze_format_string::ParseLengthModifier(
     if (I != E && Conv.convertBasicChar(*I) == u8'h') {
       ++I;
       lmKind = LengthModifier::AsChar;
-    } else if (I != E &&
-               Conv.convertBasicChar(*I) == u8'l' &&
-               LO.OpenCL) {
+    } else if (I != E && Conv.convertBasicChar(*I) == u8'l' && LO.OpenCL) {
       ++I;
       lmKind = LengthModifier::AsShortLong;
     } else {
@@ -273,10 +264,9 @@ bool clang::analyze_format_string::ParseLengthModifier(
       // be parsed as the GNU extension 'a' length modifier. If not, this
       // will be parsed as a conversion specifier.
       ++I;
-      if (I != E &&
-          (Conv.convertBasicChar(*I) == u8's' ||
-           Conv.convertBasicChar(*I) == u8'S' ||
-           Conv.convertBasicChar(*I) == u8'[')) {
+      if (I != E && (Conv.convertBasicChar(*I) == u8's' ||
+                     Conv.convertBasicChar(*I) == u8'S' ||
+                     Conv.convertBasicChar(*I) == u8'[')) {
         lmKind = LengthModifier::AsAllocate;
         break;
       }
diff --git a/clang/lib/AST/FormatStringParsing.h b/clang/lib/AST/FormatStringParsing.h
index 52915387c..8857a617b 100644
--- a/clang/lib/AST/FormatStringParsing.h
+++ b/clang/lib/AST/FormatStringParsing.h
@@ -34,7 +34,7 @@ makeFormatStrConverter(const TargetInfo &Target) {
   auto MaybeConverter =
       Target.getTriple().isOSzOS()
           ? llvm::TextEncodingConverter::create(llvm::TextEncoding::IBM1047,
-                                               llvm::TextEncoding::UTF8)
+                                                llvm::TextEncoding::UTF8)
           : llvm::TextEncodingConverter::createNoopConverter();
   assert(MaybeConverter && "Failed to create format string converter");
   return std::move(*MaybeConverter);
diff --git a/clang/lib/AST/ScanfFormatString.cpp b/clang/lib/AST/ScanfFormatString.cpp
index 9c5132e5f..454372110 100644
--- a/clang/lib/AST/ScanfFormatString.cpp
+++ b/clang/lib/AST/ScanfFormatString.cpp
@@ -612,10 +612,10 @@ void ScanfSpecifier::toString(raw_ostream &os) const {
 }
 
 bool clang::analyze_format_string::ParseScanfString(FormatStringHandler &H,
-                                                     const char *I,
-                                                     const char *E,
-                                                     const LangOptions &LO,
-                                                     const TargetInfo &Target) {
+                                                    const char *I,
+                                                    const char *E,
+                                                    const LangOptions &LO,
+                                                    const TargetInfo &Target) {
   unsigned argIndex = 0;
   llvm::TextEncodingConverter Conv = makeFormatStrConverter(Target);
 
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 72dd33e31..fae9585f1 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1056,8 +1056,7 @@ hasUnsafeFormatOrSArg(ASTContext &Ctx, const CallExpr *Call,
 
     bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
                                const char *startSpecifier,
-                               unsigned specifierLen,
-                               const TargetInfo &Target,
+                               unsigned specifierLen, const TargetInfo &Target,
                                const llvm::TextEncodingConverter &) override {
       if (FS.getConversionSpecifier().getKind() !=
           analyze_printf::PrintfConversionSpecifier::sArg)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5dddb89b9..7bdd5f8f2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -8314,10 +8314,11 @@ public:
                        ArrayRef<FixItHint> Fixit = {});
 
 protected:
-  bool HandleInvalidConversionSpecifier(
-      unsigned argIndex, SourceLocation Loc, const char *startSpec,
-      unsigned specifierLen, const char *csStart, unsigned csLen,
-      const llvm::TextEncodingConverter &Conv);
+  bool
+  HandleInvalidConversionSpecifier(unsigned argIndex, SourceLocation Loc,
+                                   const char *startSpec, unsigned specifierLen,
+                                   const char *csStart, unsigned csLen,
+                                   const llvm::TextEncodingConverter &Conv);
 
   void HandlePositionalNonpositionalArgs(SourceLocation Loc,
                                          const char *startSpec,
@@ -8742,8 +8743,7 @@ public:
 
   bool HandleInvalidPrintfConversionSpecifier(
       const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier,
-      unsigned specifierLen,
-      const llvm::TextEncodingConverter &Conv) override;
+      unsigned specifierLen, const llvm::TextEncodingConverter &Conv) override;
 
   void handleInvalidMaskType(StringRef MaskType) override;
 
@@ -8885,8 +8885,7 @@ public:
 
 bool CheckPrintfHandler::HandleInvalidPrintfConversionSpecifier(
     const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier,
-    unsigned specifierLen,
-    const llvm::TextEncodingConverter &Conv) {
+    unsigned specifierLen, const llvm::TextEncodingConverter &Conv) {
   const analyze_printf::PrintfConversionSpecifier &CS =
       FS.getConversionSpecifier();
 
@@ -10108,8 +10107,7 @@ public:
 
   bool HandleInvalidScanfConversionSpecifier(
       const analyze_scanf::ScanfSpecifier &FS, const char *startSpecifier,
-      unsigned specifierLen,
-      const llvm::TextEncodingConverter &Conv) override;
+      unsigned specifierLen, const llvm::TextEncodingConverter &Conv) override;
 
   void HandleIncompleteScanList(const char *start, const char *end) override;
 };
@@ -10125,8 +10123,7 @@ void CheckScanfHandler::HandleIncompleteScanList(const char *start,
 
 bool CheckScanfHandler::HandleInvalidScanfConversionSpecifier(
     const analyze_scanf::ScanfSpecifier &FS, const char *startSpecifier,
-    unsigned specifierLen,
-    const llvm::TextEncodingConverter &Conv) {
+    unsigned specifierLen, const llvm::TextEncodingConverter &Conv) {
 
   const analyze_scanf::ScanfConversionSpecifier &CS =
       FS.getConversionSpecifier();

[hubert-reinterpretcast]

z/OS format strings are locale-dependent as with other platforms. In particular, "\u{e29b}\n" in IBM-939 appears as the byte sequence corresponding to "\x0E%s\x0F\n" but, under the appropriate locale, merely prints as that sequence of bytes.

This PR is making changes in the expectations users can have with respect to the interpretation of format strings and requires a documentation update. In particular, it appears that interpretation is done without regard to the literal encoding.

Note that the documentation should also account for the treatment of format strings in llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp.