Remove ScanfArgType and bake that logic into ArgType.

This is useful for example for %n in printf, which expects
a pointer to int with the same logic for checking as %d
would have in scanf.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161407 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/Analyses/FormatString.h

@@ -209,15 +209,24 @@ private:
   const Kind K;
   QualType T;
   const char *Name;
+  bool Ptr;
 public:
-  ArgType(Kind k = UnknownTy, const char *n = 0) : K(k), Name(n) {}
-  ArgType(QualType t, const char *n = 0) : K(SpecificTy), T(t), Name(n)  {}
-  ArgType(CanQualType t) : K(SpecificTy), T(t), Name(0) {}
+  ArgType(Kind k = UnknownTy, const char *n = 0) : K(k), Name(n), Ptr(false) {}
+  ArgType(QualType t, const char *n = 0)
+      : K(SpecificTy), T(t), Name(n), Ptr(false) {}
+  ArgType(CanQualType t) : K(SpecificTy), T(t), Name(0), Ptr(false) {}
 
   static ArgType Invalid() { return ArgType(InvalidTy); }
-
   bool isValid() const { return K != InvalidTy; }
 
+  /// Create an ArgType which corresponds to the type pointer to A.
+  static ArgType PtrTo(const ArgType& A) {
+    assert(A.K >= InvalidTy && "ArgType cannot be pointer to invalid/unknown");
+    ArgType Res = A;
+    Res.Ptr = true;
+    return Res;
+  }
+
   bool matchesType(ASTContext &C, QualType argTy) const;
 
   QualType getRepresentativeType(ASTContext &C) const;
@@ -517,30 +526,6 @@ using analyze_format_string::LengthModifier;
 using analyze_format_string::OptionalAmount;
 using analyze_format_string::OptionalFlag;
 
-class ScanfArgType : public ArgType {
-public:
-  enum Kind { UnknownTy, InvalidTy, CStrTy, WCStrTy, PtrToArgTypeTy };
-private:
-  Kind K;
-  ArgType A;
-  const char *Name;
-  QualType getRepresentativeType(ASTContext &C) const;
-public:
-  ScanfArgType(Kind k = UnknownTy, const char* n = 0) : K(k), Name(n) {}
-  ScanfArgType(ArgType a, const char *n = 0)
-      : K(PtrToArgTypeTy), A(a), Name(n) {
-    assert(A.isValid());
-  }
-
-  static ScanfArgType Invalid() { return ScanfArgType(InvalidTy); }
-
-  bool isValid() const { return K != InvalidTy; }
-
-  bool matchesType(ASTContext& C, QualType argTy) const;
-
-  std::string getRepresentativeTypeName(ASTContext& C) const;
-};
-
 class ScanfSpecifier : public analyze_format_string::FormatSpecifier {
   OptionalFlag SuppressAssignment; // '*'
 public:
@@ -569,7 +554,7 @@ public:
     return CS.consumesDataArgument() && !SuppressAssignment;
   }
 
-  ScanfArgType getArgType(ASTContext &Ctx) const;
+  ArgType getArgType(ASTContext &Ctx) const;
 
   bool fixType(QualType QT, const LangOptions &LangOpt, ASTContext &Ctx);
 

lib/Analysis/FormatString.cpp

@@ -233,6 +233,19 @@ clang::analyze_format_string::ParseLengthModifier(FormatSpecifier &FS,
 //===----------------------------------------------------------------------===//
 
 bool ArgType::matchesType(ASTContext &C, QualType argTy) const {
+  if (Ptr) {
+    // It has to be a pointer.
+    const PointerType *PT = argTy->getAs<PointerType>();
+    if (!PT)
+      return false;
+
+    // We cannot write through a const qualified pointer.
+    if (PT->getPointeeType().isConstQualified())
+      return false;
+
+    argTy = PT->getPointeeType();
+  }
+
   switch (K) {
     case InvalidTy:
       llvm_unreachable("ArgType must be valid");
@@ -262,13 +275,6 @@ bool ArgType::matchesType(ASTContext &C, QualType argTy) const {
         argTy = ETy->getDecl()->getIntegerType();
       argTy = C.getCanonicalType(argTy).getUnqualifiedType();
 
-      if (const PointerType *PTy = argTy->getAs<PointerType>()) {
-        // Strip volatile qualifier from pointee type.
-        QualType Pointee = PTy->getPointeeType();
-        Pointee.removeLocalVolatile();
-        argTy = C.getPointerType(Pointee);
-      }
-
       if (T == argTy)
         return true;
       // Check for "compatible types".
@@ -375,35 +381,59 @@ bool ArgType::matchesType(ASTContext &C, QualType argTy) const {
 }
 
 QualType ArgType::getRepresentativeType(ASTContext &C) const {
+  QualType Res;
   switch (K) {
     case InvalidTy:
       llvm_unreachable("No representative type for Invalid ArgType");
     case UnknownTy:
-      return QualType();
+      llvm_unreachable("No representative type for Unknown ArgType");
     case AnyCharTy:
-      return C.CharTy;
+      Res = C.CharTy;
+      break;
     case SpecificTy:
-      return T;
+      Res = T;
+      break;
     case CStrTy:
-      return C.getPointerType(C.CharTy);
+      Res = C.getPointerType(C.CharTy);
+      break;
     case WCStrTy:
-      return C.getPointerType(C.getWCharType());
+      Res = C.getPointerType(C.getWCharType());
+      break;
     case ObjCPointerTy:
-      return C.ObjCBuiltinIdTy;
+      Res = C.ObjCBuiltinIdTy;
+      break;
     case CPointerTy:
-      return C.VoidPtrTy;
+      Res = C.VoidPtrTy;
+      break;
     case WIntTy: {
-      return C.getWIntType();
+      Res = C.getWIntType();
+      break;
     }
   }
 
-  llvm_unreachable("Invalid ArgType Kind!");
+  if (Ptr)
+    Res = C.getPointerType(Res);
+  return Res;
 }
 
 std::string ArgType::getRepresentativeTypeName(ASTContext &C) const {
   std::string S = getRepresentativeType(C).getAsString();
-  if (Name && S != Name)
-    return std::string("'") + Name + "' (aka '" + S + "')";
+
+  std::string Alias;
+  if (Name) {
+    // Use a specific name for this type, e.g. "size_t".
+    Alias = Name;
+    if (Ptr) {
+      // If ArgType is actually a pointer to T, append an asterisk.
+      Alias += (Alias[Alias.size()-1] == '*') ? "*" : " *";
+    }
+    // If Alias is the same as the underlying type, e.g. wchar_t, then drop it.
+    if (S == Alias)
+      Alias.clear();
+  }
+
+  if (!Alias.empty())
+    return std::string("'") + Alias + "' (aka '" + S + "')";
   return std::string("'") + S + "'";
 }
 

lib/Analysis/PrintfFormatString.cpp

@@ -331,7 +331,7 @@ ArgType PrintfSpecifier::getArgType(ASTContext &Ctx,
     case ConversionSpecifier::pArg:
       return ArgType::CPointerTy;
     case ConversionSpecifier::nArg:
-      return Ctx.getPointerType(Ctx.IntTy);
+      return ArgType::PtrTo(Ctx.IntTy);
     case ConversionSpecifier::ObjCObjArg:
       return ArgType::ObjCPointerTy;
     default:

lib/Analysis/ScanfFormatString.cpp

@@ -20,7 +20,6 @@ using clang::analyze_format_string::FormatStringHandler;
 using clang::analyze_format_string::LengthModifier;
 using clang::analyze_format_string::OptionalAmount;
 using clang::analyze_format_string::ConversionSpecifier;
-using clang::analyze_scanf::ScanfArgType;
 using clang::analyze_scanf::ScanfConversionSpecifier;
 using clang::analyze_scanf::ScanfSpecifier;
 using clang::UpdateOnReturn;
@@ -194,37 +193,42 @@ static ScanfSpecifierResult ParseScanfSpecifier(FormatStringHandler &H,
   return ScanfSpecifierResult(Start, FS);
 }
 
-ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const {
+ArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const {
   const ScanfConversionSpecifier &CS = getConversionSpecifier();
 
   if (!CS.consumesDataArgument())
-    return ScanfArgType::Invalid();
+    return ArgType::Invalid();
 
   switch(CS.getKind()) {
     // Signed int.
     case ConversionSpecifier::dArg:
     case ConversionSpecifier::iArg:
       switch (LM.getKind()) {
-        case LengthModifier::None: return ArgType(Ctx.IntTy);
+        case LengthModifier::None:
+          return ArgType::PtrTo(Ctx.IntTy);
         case LengthModifier::AsChar:
-          return ArgType(ArgType::AnyCharTy);
-        case LengthModifier::AsShort: return ArgType(Ctx.ShortTy);
-        case LengthModifier::AsLong: return ArgType(Ctx.LongTy);
+          return ArgType::PtrTo(ArgType::AnyCharTy);
+        case LengthModifier::AsShort:
+          return ArgType::PtrTo(Ctx.ShortTy);
+        case LengthModifier::AsLong:
+          return ArgType::PtrTo(Ctx.LongTy);
         case LengthModifier::AsLongLong:
         case LengthModifier::AsQuad:
-          return ArgType(Ctx.LongLongTy);
+          return ArgType::PtrTo(Ctx.LongLongTy);
         case LengthModifier::AsIntMax:
-          return ScanfArgType(Ctx.getIntMaxType(), "intmax_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getIntMaxType(), "intmax_t"));
         case LengthModifier::AsSizeT:
           // FIXME: ssize_t.
-          return ScanfArgType();
+          return ArgType();
         case LengthModifier::AsPtrDiff:
-          return ScanfArgType(Ctx.getPointerDiffType(), "ptrdiff_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
         case LengthModifier::AsLongDouble:
           // GNU extension.
-          return ArgType(Ctx.LongLongTy);
-        case LengthModifier::AsAllocate: return ScanfArgType::Invalid();
-        case LengthModifier::AsMAllocate: return ScanfArgType::Invalid();
+          return ArgType::PtrTo(Ctx.LongLongTy);
+        case LengthModifier::AsAllocate:
+          return ArgType::Invalid();
+        case LengthModifier::AsMAllocate:
+          return ArgType::Invalid();
       }
 
     // Unsigned int.
@@ -233,25 +237,31 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const {
     case ConversionSpecifier::xArg:
     case ConversionSpecifier::XArg:
       switch (LM.getKind()) {
-        case LengthModifier::None: return ArgType(Ctx.UnsignedIntTy);
-        case LengthModifier::AsChar: return ArgType(Ctx.UnsignedCharTy);
-        case LengthModifier::AsShort: return ArgType(Ctx.UnsignedShortTy);
-        case LengthModifier::AsLong: return ArgType(Ctx.UnsignedLongTy);
+        case LengthModifier::None:
+          return ArgType::PtrTo(Ctx.UnsignedIntTy);
+        case LengthModifier::AsChar:
+          return ArgType::PtrTo(Ctx.UnsignedCharTy);
+        case LengthModifier::AsShort:
+          return ArgType::PtrTo(Ctx.UnsignedShortTy);
+        case LengthModifier::AsLong:
+          return ArgType::PtrTo(Ctx.UnsignedLongTy);
         case LengthModifier::AsLongLong:
         case LengthModifier::AsQuad:
-          return ArgType(Ctx.UnsignedLongLongTy);
+          return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
         case LengthModifier::AsIntMax:
-          return ScanfArgType(Ctx.getUIntMaxType(), "uintmax_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getUIntMaxType(), "uintmax_t"));
         case LengthModifier::AsSizeT:
-          return ScanfArgType(Ctx.getSizeType(), "size_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getSizeType(), "size_t"));
         case LengthModifier::AsPtrDiff:
           // FIXME: Unsigned version of ptrdiff_t?
-          return ScanfArgType();
+          return ArgType();
         case LengthModifier::AsLongDouble:
           // GNU extension.
-          return ArgType(Ctx.UnsignedLongLongTy);
-        case LengthModifier::AsAllocate: return ScanfArgType::Invalid();
-        case LengthModifier::AsMAllocate: return ScanfArgType::Invalid();
+          return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
+        case LengthModifier::AsAllocate:
+          return ArgType::Invalid();
+        case LengthModifier::AsMAllocate:
+          return ArgType::Invalid();
       }
 
     // Float.
@@ -264,12 +274,14 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const {
     case ConversionSpecifier::gArg:
     case ConversionSpecifier::GArg:
       switch (LM.getKind()) {
-        case LengthModifier::None: return ArgType(Ctx.FloatTy);
-        case LengthModifier::AsLong: return ArgType(Ctx.DoubleTy);
+        case LengthModifier::None:
+          return ArgType::PtrTo(Ctx.FloatTy);
+        case LengthModifier::AsLong:
+          return ArgType::PtrTo(Ctx.DoubleTy);
         case LengthModifier::AsLongDouble:
-          return ArgType(Ctx.LongDoubleTy);
+          return ArgType::PtrTo(Ctx.LongDoubleTy);
         default:
-          return ScanfArgType::Invalid();
+          return ArgType::Invalid();
       }
 
     // Char, string and scanlist.
@@ -277,40 +289,42 @@ ScanfArgType ScanfSpecifier::getArgType(ASTContext &Ctx) const {
     case ConversionSpecifier::sArg:
     case ConversionSpecifier::ScanListArg:
       switch (LM.getKind()) {
-        case LengthModifier::None: return ScanfArgType::CStrTy;
+        case LengthModifier::None:
+          return ArgType::PtrTo(ArgType::AnyCharTy);
         case LengthModifier::AsLong:
-          return ScanfArgType(ScanfArgType::WCStrTy, "wchar_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getWCharType(), "wchar_t"));
         case LengthModifier::AsAllocate:
         case LengthModifier::AsMAllocate:
-          return ScanfArgType(ArgType::CStrTy);
+          return ArgType::PtrTo(ArgType::CStrTy);
         default:
-          return ScanfArgType::Invalid();
+          return ArgType::Invalid();
       }
     case ConversionSpecifier::CArg:
     case ConversionSpecifier::SArg:
       // FIXME: Mac OS X specific?
       switch (LM.getKind()) {
         case LengthModifier::None:
-          return ScanfArgType(ScanfArgType::WCStrTy, "wchar_t *");
+          return ArgType::PtrTo(ArgType(Ctx.getWCharType(), "wchar_t"));
         case LengthModifier::AsAllocate:
         case LengthModifier::AsMAllocate:
-          return ScanfArgType(ArgType::WCStrTy, "wchar_t **");
+          return ArgType::PtrTo(ArgType(ArgType::WCStrTy, "wchar_t *"));
         default:
-          return ScanfArgType::Invalid();
+          return ArgType::Invalid();
       }
 
     // Pointer.
     case ConversionSpecifier::pArg:
-      return ScanfArgType(ArgType(ArgType::CPointerTy));
+      return ArgType::PtrTo(ArgType::CPointerTy);
 
+    // Write-back.
     case ConversionSpecifier::nArg:
-      return ArgType(Ctx.IntTy);
+      return ArgType::PtrTo(Ctx.IntTy);
 
     default:
       break;
   }
 
-  return ScanfArgType();
+  return ArgType();
 }
 
 bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
@@ -393,8 +407,8 @@ bool ScanfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
     namedTypeToLengthModifier(PT, LM);
 
   // If fixing the length modifier was enough, we are done.
-  const analyze_scanf::ScanfArgType &ATR = getArgType(Ctx);
-  if (hasValidLengthModifier() && ATR.isValid() && ATR.matchesType(Ctx, QT))
+  const analyze_scanf::ArgType &AT = getArgType(Ctx);
+  if (hasValidLengthModifier() && AT.isValid() && AT.matchesType(Ctx, QT))
     return true;
 
   // Figure out the conversion specifier.
@@ -451,54 +465,3 @@ bool clang::analyze_format_string::ParseScanfString(FormatStringHandler &H,
   assert(I == E && "Format string not exhausted");
   return false;
 }
-
-bool ScanfArgType::matchesType(ASTContext& C, QualType argTy) const {
-  // It has to be a pointer type.
-  const PointerType *PT = argTy->getAs<PointerType>();
-  if (!PT)
-    return false;
-
-  // We cannot write through a const qualified pointer.
-  if (PT->getPointeeType().isConstQualified())
-    return false;
-
-  switch (K) {
-    case InvalidTy:
-      llvm_unreachable("ArgType must be valid");
-    case UnknownTy:
-      return true;
-    case CStrTy:
-      return ArgType(ArgType::CStrTy).matchesType(C, argTy);
-    case WCStrTy:
-      return ArgType(ArgType::WCStrTy).matchesType(C, argTy);
-    case PtrToArgTypeTy: {
-      return A.matchesType(C, PT->getPointeeType());
-    }
-  }
-
-  llvm_unreachable("Invalid ScanfArgType Kind!");
-}
-
-QualType ScanfArgType::getRepresentativeType(ASTContext &C) const {
-  switch (K) {
-    case InvalidTy:
-      llvm_unreachable("No representative type for Invalid ArgType");
-    case UnknownTy:
-      return QualType();
-    case CStrTy:
-      return C.getPointerType(C.CharTy);
-    case WCStrTy:
-      return C.getPointerType(C.getWCharType());
-    case PtrToArgTypeTy:
-      return C.getPointerType(A.getRepresentativeType(C));
-  }
-
-  llvm_unreachable("Invalid ScanfArgType Kind!");
-}
-
-std::string ScanfArgType::getRepresentativeTypeName(ASTContext& C) const {
-  std::string S = getRepresentativeType(C).getAsString();
-  if (!Name)
-    return std::string("'") + S + "'";
-  return std::string("'") + Name + "' (aka '" + S + "')";
-}

lib/Sema/SemaChecking.cpp

@@ -2800,8 +2800,8 @@ bool CheckScanfHandler::HandleScanfSpecifier(
   if (!Ex)
     return true;
 
-  const analyze_scanf::ScanfArgType &ATR = FS.getArgType(S.Context);
-  if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
+  const analyze_format_string::ArgType &AT = FS.getArgType(S.Context);
+  if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) {
     ScanfSpecifier fixedFS = FS;
     bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(),
                                    S.Context);
@@ -2814,7 +2814,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
 
       EmitFormatDiagnostic(
         S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << AT.getRepresentativeTypeName(S.Context) << Ex->getType()
           << Ex->getSourceRange(),
         Ex->getLocStart(),
         /*IsStringLocation*/false,
@@ -2825,7 +2825,7 @@ bool CheckScanfHandler::HandleScanfSpecifier(
     } else {
       EmitFormatDiagnostic(
         S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
-          << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
+          << AT.getRepresentativeTypeName(S.Context) << Ex->getType()
           << Ex->getSourceRange(),
         Ex->getLocStart(),
         /*IsStringLocation*/false,

test/Sema/format-strings-scanf.c

@@ -33,6 +33,12 @@ void test(const char *s, int *i) {
   scanf("%*d", i); // // expected-warning{{data argument not used by format string}}
   scanf("%*d", i); // // expected-warning{{data argument not used by format string}}
   scanf("%*d%1$d", i); // no-warning
+
+  scanf("%s", (char*)0); // no-warning
+  scanf("%s", (volatile char*)0); // no-warning
+  scanf("%s", (signed char*)0); // no-warning
+  scanf("%s", (unsigned char*)0); // no-warning
+  scanf("%hhu", (signed char*)0); // no-warning
 }
 
 void bad_length_modifiers(char *s, void *p, wchar_t *ws, long double *ld) {