[clang] [clang] Update -Wformat warnings for fixed-point format specifiers (PR #82855)

via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 27 11:09:06 PST 2024


https://github.com/PiJoules updated https://github.com/llvm/llvm-project/pull/82855

>From 466aa42532902df2c13b539b44a4b31e80b13056 Mon Sep 17 00:00:00 2001
From: Leonard Chan <leonardchan at google.com>
Date: Fri, 23 Feb 2024 16:57:58 -0800
Subject: [PATCH] [clang] Update -Wformat warnings for fixed-point format
 specifiers

ISO/IEC TR 18037 defines %r, %R, %k, and %K for fixed point format
specifiers. -Wformat should not warn on these when they are provided.
---
 clang/include/clang/AST/ASTContext.h   |   4 +
 clang/include/clang/AST/FormatString.h |  11 ++
 clang/lib/AST/ASTContext.cpp           |  36 ++++++
 clang/lib/AST/FormatString.cpp         |  25 +++++
 clang/lib/AST/PrintfFormatString.cpp   |  85 +++++++++++++-
 clang/test/Sema/format-fixed-point.c   | 148 +++++++++++++++++++++++++
 6 files changed, 306 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Sema/format-fixed-point.c

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 12ce9af1e53f63..ff6b64c7f72d57 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2981,6 +2981,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   // corresponding saturated type for a given fixed point type.
   QualType getCorrespondingSaturatedType(QualType Ty) const;
 
+  // Per ISO N1169, this method accepts fixed point types and returns the
+  // corresponding non-saturated type for a given fixed point type.
+  QualType getCorrespondingUnsaturatedType(QualType Ty) const;
+
   // This method accepts fixed point types and returns the corresponding signed
   // type. Unlike getCorrespondingUnsignedType(), this only accepts unsigned
   // fixed point types because there are unsigned integer types like bool and
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index 5c4ad9baaef608..e2232fb4a47153 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -171,6 +171,14 @@ class ConversionSpecifier {
 
     ZArg, // MS extension
 
+    // ISO/IEC TR 18037 (fixed-point) specific specifiers.
+    kArg, // %k for signed accum types
+    KArg, // %K for unsigned accum types
+    rArg, // %r for signed fract types
+    RArg, // %R for unsigned fract types
+    FixedPointArgBeg = kArg,
+    FixedPointArgEnd = RArg,
+
     // Objective-C specific specifiers.
     ObjCObjArg, // '@'
     ObjCBeg = ObjCObjArg,
@@ -237,6 +245,9 @@ class ConversionSpecifier {
   bool isDoubleArg() const {
     return kind >= DoubleArgBeg && kind <= DoubleArgEnd;
   }
+  bool isFixedPointArg() const {
+    return kind >= FixedPointArgBeg && kind <= FixedPointArgEnd;
+  }
 
   const char *toString() const;
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index c475c841233c59..5a8fae76a43a4d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13314,6 +13314,42 @@ QualType ASTContext::getCommonSugaredType(QualType X, QualType Y,
   return R;
 }
 
+QualType ASTContext::getCorrespondingUnsaturatedType(QualType Ty) const {
+  assert(Ty->isFixedPointType());
+
+  if (Ty->isUnsaturatedFixedPointType())
+    return Ty;
+
+  switch (Ty->castAs<BuiltinType>()->getKind()) {
+  default:
+    llvm_unreachable("Not a saturated fixed point type!");
+  case BuiltinType::SatShortAccum:
+    return ShortAccumTy;
+  case BuiltinType::SatAccum:
+    return AccumTy;
+  case BuiltinType::SatLongAccum:
+    return LongAccumTy;
+  case BuiltinType::SatUShortAccum:
+    return UnsignedShortAccumTy;
+  case BuiltinType::SatUAccum:
+    return UnsignedAccumTy;
+  case BuiltinType::SatULongAccum:
+    return UnsignedLongAccumTy;
+  case BuiltinType::SatShortFract:
+    return ShortFractTy;
+  case BuiltinType::SatFract:
+    return FractTy;
+  case BuiltinType::SatLongFract:
+    return LongFractTy;
+  case BuiltinType::SatUShortFract:
+    return UnsignedShortFractTy;
+  case BuiltinType::SatUFract:
+    return UnsignedFractTy;
+  case BuiltinType::SatULongFract:
+    return UnsignedLongFractTy;
+  }
+}
+
 QualType ASTContext::getCorrespondingSaturatedType(QualType Ty) const {
   assert(Ty->isFixedPointType());
 
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index c5d14b4af7ff15..0c80ad109ccbb2 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -403,6 +403,10 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
         else if (ETy->isUnscopedEnumerationType())
           argTy = ETy->getDecl()->getIntegerType();
       }
+
+      if (argTy->isSaturatedFixedPointType())
+        argTy = C.getCorrespondingUnsaturatedType(argTy);
+
       argTy = C.getCanonicalType(argTy).getUnqualifiedType();
 
       if (T == argTy)
@@ -761,6 +765,16 @@ const char *ConversionSpecifier::toString() const {
 
   // MS specific specifiers.
   case ZArg: return "Z";
+
+  // ISO/IEC TR 18037 (fixed-point) specific specifiers.
+  case rArg:
+    return "r";
+  case RArg:
+    return "R";
+  case kArg:
+    return "k";
+  case KArg:
+    return "K";
   }
   return nullptr;
 }
@@ -825,6 +839,9 @@ bool FormatSpecifier::hasValidLengthModifier(const TargetInfo &Target,
       if (LO.OpenCL && CS.isDoubleArg())
         return !VectorNumElts.isInvalid();
 
+      if (CS.isFixedPointArg())
+        return true;
+
       if (Target.getTriple().isOSMSVCRT()) {
         switch (CS.getKind()) {
           case ConversionSpecifier::cArg:
@@ -877,6 +894,9 @@ bool FormatSpecifier::hasValidLengthModifier(const TargetInfo &Target,
         return true;
       }
 
+      if (CS.isFixedPointArg())
+        return true;
+
       switch (CS.getKind()) {
         case ConversionSpecifier::bArg:
         case ConversionSpecifier::BArg:
@@ -1043,6 +1063,11 @@ bool FormatSpecifier::hasStandardConversionSpecifier(
     case ConversionSpecifier::UArg:
     case ConversionSpecifier::ZArg:
       return false;
+    case ConversionSpecifier::rArg:
+    case ConversionSpecifier::RArg:
+    case ConversionSpecifier::kArg:
+    case ConversionSpecifier::KArg:
+      return LangOpt.FixedPoint;
   }
   llvm_unreachable("Invalid ConversionSpecifier Kind!");
 }
diff --git a/clang/lib/AST/PrintfFormatString.cpp b/clang/lib/AST/PrintfFormatString.cpp
index 3b09ca40bd2a53..fec8ce13e8c447 100644
--- a/clang/lib/AST/PrintfFormatString.cpp
+++ b/clang/lib/AST/PrintfFormatString.cpp
@@ -348,6 +348,8 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H,
     case 'r':
       if (isFreeBSDKPrintf)
         k = ConversionSpecifier::FreeBSDrArg; // int
+      else if (LO.FixedPoint)
+        k = ConversionSpecifier::rArg;
       break;
     case 'y':
       if (isFreeBSDKPrintf)
@@ -373,6 +375,20 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H,
       if (Target.getTriple().isOSMSVCRT())
         k = ConversionSpecifier::ZArg;
       break;
+    // ISO/IEC TR 18037 (fixed-point) specific.
+    // NOTE: 'r' is handled up above since FreeBSD also supports %r.
+    case 'k':
+      if (LO.FixedPoint)
+        k = ConversionSpecifier::kArg;
+      break;
+    case 'K':
+      if (LO.FixedPoint)
+        k = ConversionSpecifier::KArg;
+      break;
+    case 'R':
+      if (LO.FixedPoint)
+        k = ConversionSpecifier::RArg;
+      break;
   }
 
   // Check to see if we used the Objective-C modifier flags with
@@ -627,6 +643,9 @@ ArgType PrintfSpecifier::getScalarArgType(ASTContext &Ctx,
     }
   }
 
+  if (CS.isFixedPointArg() && !Ctx.getLangOpts().FixedPoint)
+    return ArgType::Invalid();
+
   switch (CS.getKind()) {
     case ConversionSpecifier::sArg:
       if (LM.getKind() == LengthModifier::AsWideChar) {
@@ -658,6 +677,50 @@ ArgType PrintfSpecifier::getScalarArgType(ASTContext &Ctx,
       return ArgType::CPointerTy;
     case ConversionSpecifier::ObjCObjArg:
       return ArgType::ObjCPointerTy;
+    case ConversionSpecifier::kArg:
+      switch (LM.getKind()) {
+      case LengthModifier::None:
+        return Ctx.AccumTy;
+      case LengthModifier::AsShort:
+        return Ctx.ShortAccumTy;
+      case LengthModifier::AsLong:
+        return Ctx.LongAccumTy;
+      default:
+        return ArgType::Invalid();
+      }
+    case ConversionSpecifier::KArg:
+      switch (LM.getKind()) {
+      case LengthModifier::None:
+        return Ctx.UnsignedAccumTy;
+      case LengthModifier::AsShort:
+        return Ctx.UnsignedShortAccumTy;
+      case LengthModifier::AsLong:
+        return Ctx.UnsignedLongAccumTy;
+      default:
+        return ArgType::Invalid();
+      }
+    case ConversionSpecifier::rArg:
+      switch (LM.getKind()) {
+      case LengthModifier::None:
+        return Ctx.FractTy;
+      case LengthModifier::AsShort:
+        return Ctx.ShortFractTy;
+      case LengthModifier::AsLong:
+        return Ctx.LongFractTy;
+      default:
+        return ArgType::Invalid();
+      }
+    case ConversionSpecifier::RArg:
+      switch (LM.getKind()) {
+      case LengthModifier::None:
+        return Ctx.UnsignedFractTy;
+      case LengthModifier::AsShort:
+        return Ctx.UnsignedShortFractTy;
+      case LengthModifier::AsLong:
+        return Ctx.UnsignedLongFractTy;
+      default:
+        return ArgType::Invalid();
+      }
     default:
       break;
   }
@@ -955,6 +1018,8 @@ bool PrintfSpecifier::hasValidPlusPrefix() const {
   case ConversionSpecifier::AArg:
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
+  case ConversionSpecifier::rArg:
+  case ConversionSpecifier::kArg:
     return true;
 
   default:
@@ -966,7 +1031,7 @@ bool PrintfSpecifier::hasValidAlternativeForm() const {
   if (!HasAlternativeForm)
     return true;
 
-  // Alternate form flag only valid with the bBoxXaAeEfFgG conversions
+  // Alternate form flag only valid with the bBoxXaAeEfFgGrRkK conversions
   switch (CS.getKind()) {
   case ConversionSpecifier::bArg:
   case ConversionSpecifier::BArg:
@@ -984,6 +1049,10 @@ bool PrintfSpecifier::hasValidAlternativeForm() const {
   case ConversionSpecifier::GArg:
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
+  case ConversionSpecifier::rArg:
+  case ConversionSpecifier::RArg:
+  case ConversionSpecifier::kArg:
+  case ConversionSpecifier::KArg:
     return true;
 
   default:
@@ -995,7 +1064,7 @@ bool PrintfSpecifier::hasValidLeadingZeros() const {
   if (!HasLeadingZeroes)
     return true;
 
-  // Leading zeroes flag only valid with the bBdiouxXaAeEfFgG conversions
+  // Leading zeroes flag only valid with the bBdiouxXaAeEfFgGrRkK conversions
   switch (CS.getKind()) {
   case ConversionSpecifier::bArg:
   case ConversionSpecifier::BArg:
@@ -1018,6 +1087,10 @@ bool PrintfSpecifier::hasValidLeadingZeros() const {
   case ConversionSpecifier::GArg:
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
+  case ConversionSpecifier::rArg:
+  case ConversionSpecifier::RArg:
+  case ConversionSpecifier::kArg:
+  case ConversionSpecifier::KArg:
     return true;
 
   default:
@@ -1044,6 +1117,8 @@ bool PrintfSpecifier::hasValidSpacePrefix() const {
   case ConversionSpecifier::AArg:
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
+  case ConversionSpecifier::rArg:
+  case ConversionSpecifier::kArg:
     return true;
 
   default:
@@ -1089,7 +1164,7 @@ bool PrintfSpecifier::hasValidPrecision() const {
   if (Precision.getHowSpecified() == OptionalAmount::NotSpecified)
     return true;
 
-  // Precision is only valid with the bBdiouxXaAeEfFgGsP conversions
+  // Precision is only valid with the bBdiouxXaAeEfFgGsPrRkK conversions
   switch (CS.getKind()) {
   case ConversionSpecifier::bArg:
   case ConversionSpecifier::BArg:
@@ -1114,6 +1189,10 @@ bool PrintfSpecifier::hasValidPrecision() const {
   case ConversionSpecifier::FreeBSDrArg:
   case ConversionSpecifier::FreeBSDyArg:
   case ConversionSpecifier::PArg:
+  case ConversionSpecifier::rArg:
+  case ConversionSpecifier::RArg:
+  case ConversionSpecifier::kArg:
+  case ConversionSpecifier::KArg:
     return true;
 
   default:
diff --git a/clang/test/Sema/format-fixed-point.c b/clang/test/Sema/format-fixed-point.c
new file mode 100644
index 00000000000000..47b22f1a7a5f30
--- /dev/null
+++ b/clang/test/Sema/format-fixed-point.c
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -ffixed-point -fsyntax-only -verify -Wformat -isystem %S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -isystem %S/Inputs %s -DWITHOUT_FIXED_POINT
+
+int printf(const char *restrict, ...);
+
+short s;
+unsigned short us;
+int i;
+unsigned int ui;
+long l;
+unsigned long ul;
+float fl;
+double d;
+char c;
+unsigned char uc;
+
+#ifndef WITHOUT_FIXED_POINT
+short _Fract sf;
+_Fract f;
+long _Fract lf;
+unsigned short _Fract usf;
+unsigned _Fract uf;
+unsigned long _Fract ulf;
+short _Accum sa;
+_Accum a;
+long _Accum la;
+unsigned short _Accum usa;
+unsigned _Accum ua;
+unsigned long _Accum ula;
+_Sat short _Fract sat_sf;
+_Sat _Fract sat_f;
+_Sat long _Fract sat_lf;
+_Sat unsigned short _Fract sat_usf;
+_Sat unsigned _Fract sat_uf;
+_Sat unsigned long _Fract sat_ulf;
+_Sat short _Accum sat_sa;
+_Sat _Accum sat_a;
+_Sat long _Accum sat_la;
+_Sat unsigned short _Accum sat_usa;
+_Sat unsigned _Accum sat_ua;
+_Sat unsigned long _Accum sat_ula;
+
+void test_invalid_args(void) {
+  /// None of these should match against a fixed point type.
+  printf("%r", s);   // expected-warning{{format specifies type '_Fract' but the argument has type 'short'}}
+  printf("%r", us);  // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned short'}}
+  printf("%r", i);   // expected-warning{{format specifies type '_Fract' but the argument has type 'int'}}
+  printf("%r", ui);  // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned int'}}
+  printf("%r", l);   // expected-warning{{format specifies type '_Fract' but the argument has type 'long'}}
+  printf("%r", ul);  // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned long'}}
+  printf("%r", fl);  // expected-warning{{format specifies type '_Fract' but the argument has type 'float'}}
+  printf("%r", d);   // expected-warning{{format specifies type '_Fract' but the argument has type 'double'}}
+  printf("%r", c);   // expected-warning{{format specifies type '_Fract' but the argument has type 'char'}}
+  printf("%r", uc);  // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned char'}}
+}
+
+void test_fixed_point_specifiers(void) {
+  printf("%r", f);
+  printf("%R", uf);
+  printf("%k", a);
+  printf("%K", ua);
+
+  /// Test different sizes.
+  printf("%r", sf);   // expected-warning{{format specifies type '_Fract' but the argument has type 'short _Fract'}}
+  printf("%r", lf);   // expected-warning{{format specifies type '_Fract' but the argument has type 'long _Fract'}}
+  printf("%R", usf);  // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned short _Fract'}}
+  printf("%R", ulf);  // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned long _Fract'}}
+  printf("%k", sa);   // expected-warning{{format specifies type '_Accum' but the argument has type 'short _Accum'}}
+  printf("%k", la);   // expected-warning{{format specifies type '_Accum' but the argument has type 'long _Accum'}}
+  printf("%K", usa);  // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned short _Accum'}}
+  printf("%K", ula);  // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned long _Accum'}}
+
+  /// Test signs.
+  printf("%r", uf);  // expected-warning{{format specifies type '_Fract' but the argument has type 'unsigned _Fract'}}
+  printf("%R", f);   // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type '_Fract'}}
+  printf("%k", ua);  // expected-warning{{format specifies type '_Accum' but the argument has type 'unsigned _Accum'}}
+  printf("%K", a);   // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type '_Accum'}}
+
+  /// Test between types.
+  printf("%r", a);   // expected-warning{{format specifies type '_Fract' but the argument has type '_Accum'}}
+  printf("%R", ua);  // expected-warning{{format specifies type 'unsigned _Fract' but the argument has type 'unsigned _Accum'}}
+  printf("%k", f);   // expected-warning{{format specifies type '_Accum' but the argument has type '_Fract'}}
+  printf("%K", uf);  // expected-warning{{format specifies type 'unsigned _Accum' but the argument has type 'unsigned _Fract'}}
+
+  /// Test saturated types.
+  printf("%r", sat_f);
+  printf("%R", sat_uf);
+  printf("%k", sat_a);
+  printf("%K", sat_ua);
+}
+
+void test_length_modifiers_and_flags(void) {
+  printf("%hr", sf);
+  printf("%lr", lf);
+  printf("%hR", usf);
+  printf("%lR", ulf);
+  printf("%hk", sa);
+  printf("%lk", la);
+  printf("%hK", usa);
+  printf("%lK", ula);
+
+  printf("%hr", sat_sf);
+  printf("%lr", sat_lf);
+  printf("%hR", sat_usf);
+  printf("%lR", sat_ulf);
+  printf("%hk", sat_sa);
+  printf("%lk", sat_la);
+  printf("%hK", sat_usa);
+  printf("%lK", sat_ula);
+
+  printf("%10r", f);
+  printf("%10.10r", f);
+  printf("%010r", f);
+  printf("%-10r", f);
+  printf("%.10r", f);
+  printf("%+r", f);
+  printf("% r", f);
+  printf("%#r", f);
+  printf("%#.r", f);
+  printf("%#.0r", f);
+
+  /// Test some invalid length modifiers.
+  printf("%zr", f);   // expected-warning{{length modifier 'z' results in undefined behavior or no effect with 'r' conversion specifier}}
+  printf("%llr", f);  // expected-warning{{length modifier 'll' results in undefined behavior or no effect with 'r' conversion specifier}}
+  printf("%hhr", f);  // expected-warning{{length modifier 'hh' results in undefined behavior or no effect with 'r' conversion specifier}}
+
+  // + on an unsigned fixed point type.
+  printf("%+hR", usf);  // expected-warning{{flag '+' results in undefined behavior with 'R' conversion specifier}}
+  printf("%+R", uf);    // expected-warning{{flag '+' results in undefined behavior with 'R' conversion specifier}}
+  printf("%+lR", ulf);  // expected-warning{{flag '+' results in undefined behavior with 'R' conversion specifier}}
+  printf("%+hK", usa);  // expected-warning{{flag '+' results in undefined behavior with 'K' conversion specifier}}
+  printf("%+K", ua);    // expected-warning{{flag '+' results in undefined behavior with 'K' conversion specifier}}
+  printf("%+lK", ula);  // expected-warning{{flag '+' results in undefined behavior with 'K' conversion specifier}}
+  printf("% hR", usf);  // expected-warning{{flag ' ' results in undefined behavior with 'R' conversion specifier}}
+  printf("% R", uf);    // expected-warning{{flag ' ' results in undefined behavior with 'R' conversion specifier}}
+  printf("% lR", ulf);  // expected-warning{{flag ' ' results in undefined behavior with 'R' conversion specifier}}
+  printf("% hK", usa);  // expected-warning{{flag ' ' results in undefined behavior with 'K' conversion specifier}}
+  printf("% K", ua);    // expected-warning{{flag ' ' results in undefined behavior with 'K' conversion specifier}}
+  printf("% lK", ula);  // expected-warning{{flag ' ' results in undefined behavior with 'K' conversion specifier}}
+}
+#else
+void test_fixed_point_specifiers_no_printf() {
+  printf("%k", i);  // expected-warning{{invalid conversion specifier 'k'}}  
+  printf("%K", i);  // expected-warning{{invalid conversion specifier 'K'}}
+  printf("%r", i);  // expected-warning{{invalid conversion specifier 'r'}}
+  printf("%R", i);  // expected-warning{{invalid conversion specifier 'R'}}
+}
+#endif  // WITHOUT_FIXED_POINT



More information about the cfe-commits mailing list