[clang] Add getUnsignedPointerDiffType method

[alexander-shaposhnikov]

Summary

C11 standard refers to the unsigned counterpart of the type ptrdiff_t
in the paragraph 7.21.6.1p7 where it defines the format specifier %tu.
In Clang (in PrintfFormatString.cpp, lines 508-510) there is a FIXME for this case,
in particular, right now Clang doesn't diagnose %tu issues at all, i.e.
it doesn't emit any warnings on the code printf("%tu", 3.14).
In this diff we add a method getUnsignedPointerDiffType for getting the corresponding type
similarly to how it's already done in the other analogous cases (size_t, ssize_t, ptrdiff_t etc)
and fix -Wformat diagnostics for %tu plus the emitted fix-it as well.

[alexander-shaposhnikov]

fix typo

[aaron.ballman]

The code for printf diagnostics + new tests are supposed to be added by a separate diff.

I think you should combine that diff and this one into a single patch, otherwise this patch is untestable.

include/clang/AST/ASTContext.h

1492–1493 ↗

(On Diff #116635)

Wrap the comment, and you might want to refer to C11 7.21.6.1p7 as a justification for why this is needed.

[alexander-shaposhnikov]

Address the comments

[aaron.ballman]

test/FixIt/format.m

256 ↗

(On Diff #116894)

Can you add some positive tests that show "%t" and "%tu" working properly without warnings?

[alexander-shaposhnikov]

Add positive tests

[aaron.ballman]

LGTM!

cfe/trunk/include/clang/AST/ASTContext.h

public:
   /// \brief Return a type compatible with "uintptr_t" (C99 7.18.1.4),
   /// as defined by the target.
   QualType getUIntPtrType() const;
 
   /// \brief Return the unique type for "ptrdiff_t" (C99 7.17) defined in
   /// <stddef.h>. Pointer - pointer requires this (C99 6.5.6p9).
   QualType getPointerDiffType() const;
 
+  /// \brief Return the unique unsigned counterpart of "ptrdiff_t"
+  /// integer type. The standard (C11 7.21.6.1p7) refers to this type
+  /// in the definition of %tu format specifier.
+  QualType getUnsignedPointerDiffType() const;
+
   /// \brief Return the unique type for "pid_t" defined in
   /// <sys/types.h>. We need this to compute the correct type for vfork().
   QualType getProcessIDType() const;
 
   /// \brief Return the C structure type used to represent constant CFStrings.
   QualType getCFConstantStringType() const;
 
   /// \brief Returns the C struct type for objc_super

cfe/trunk/include/clang/Basic/TargetInfo.h

public:
   }
   IntType getIntMaxType() const { return IntMaxType; }
   IntType getUIntMaxType() const {
     return getCorrespondingUnsignedType(IntMaxType);
   }
   IntType getPtrDiffType(unsigned AddrSpace) const {
     return AddrSpace == 0 ? PtrDiffType : getPtrDiffTypeV(AddrSpace);
   }
+  IntType getUnsignedPtrDiffType(unsigned AddrSpace) const {
+    return getCorrespondingUnsignedType(getPtrDiffType(AddrSpace));
+  }
   IntType getIntPtrType() const { return IntPtrType; }
   IntType getUIntPtrType() const {
     return getCorrespondingUnsignedType(IntPtrType);
   }
   IntType getWCharType() const { return WCharType; }
   IntType getWIntType() const { return WIntType; }
   IntType getChar16Type() const { return Char16Type; }
   IntType getChar32Type() const { return Char32Type; }

cfe/trunk/lib/AST/ASTContext.cpp

 }
 
 /// getPointerDiffType - Return the unique type for "ptrdiff_t" (C99 7.17)
 /// defined in <stddef.h>. Pointer - pointer requires this (C99 6.5.6p9).
 QualType ASTContext::getPointerDiffType() const {
   return getFromTargetType(Target->getPtrDiffType(0));
 }
 
+/// \brief Return the unique unsigned counterpart of "ptrdiff_t"
+/// integer type. The standard (C11 7.21.6.1p7) refers to this type
+/// in the definition of %tu format specifier.
+QualType ASTContext::getUnsignedPointerDiffType() const {
+  return getFromTargetType(Target->getUnsignedPtrDiffType(0));
+}
+
 /// \brief Return the unique type for "pid_t" defined in
 /// <sys/types.h>. We need this to compute the correct type for vfork().
 QualType ASTContext::getProcessIDType() const {
   return getFromTargetType(Target->getProcessIDType());
 }
 
 //===----------------------------------------------------------------------===//
 //                              Type Operators

cfe/trunk/lib/Analysis/PrintfFormatString.cpp

switch (LM.getKind()) {
         return ArgType(Ctx.getUIntMaxType(), "uintmax_t");
       case LengthModifier::AsSizeT:
         return ArgType(Ctx.getSizeType(), "size_t");
       case LengthModifier::AsInt3264:
         return Ctx.getTargetInfo().getTriple().isArch64Bit()
                    ? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64")
                    : ArgType(Ctx.UnsignedIntTy, "unsigned __int32");
       case LengthModifier::AsPtrDiff:
-        // FIXME: How to get the corresponding unsigned
-        // version of ptrdiff_t?
-        return ArgType();
+        return ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t");
       case LengthModifier::AsAllocate:
       case LengthModifier::AsMAllocate:
       case LengthModifier::AsWide:
         return ArgType::Invalid();
     }
 
   if (CS.isDoubleArg()) {
     if (LM.getKind() == LengthModifier::AsLongDouble)

cfe/trunk/lib/Analysis/ScanfFormatString.cpp

case ConversionSpecifier::XArg:
           return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
         case LengthModifier::AsInt64:
           return ArgType::PtrTo(ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64"));
         case LengthModifier::AsIntMax:
           return ArgType::PtrTo(ArgType(Ctx.getUIntMaxType(), "uintmax_t"));
         case LengthModifier::AsSizeT:
           return ArgType::PtrTo(ArgType(Ctx.getSizeType(), "size_t"));
         case LengthModifier::AsPtrDiff:
-          // FIXME: Unsigned version of ptrdiff_t?
-          return ArgType();
+          return ArgType::PtrTo(
+              ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t"));
         case LengthModifier::AsLongDouble:
           // GNU extension.
           return ArgType::PtrTo(Ctx.UnsignedLongLongTy);
         case LengthModifier::AsAllocate:
         case LengthModifier::AsMAllocate:
         case LengthModifier::AsInt32:
         case LengthModifier::AsInt3264:
         case LengthModifier::AsWide:

cfe/trunk/test/FixIt/format.m

void testSizeTypes() {
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%f"
   
   short x;
   printf("%zn", &x); // expected-warning-re{{format specifies type 'ssize_t *' (aka '{{.+}}') but the argument has type 'short *'}}
   // PrintfSpecifier::fixType doesn't handle %n, so a fix-it is not emitted, 
   // see the comment in PrintfSpecifier::fixType in PrintfFormatString.cpp.
 }
 
+typedef __PTRDIFF_TYPE__ ptrdiff_t;
+#define __UNSIGNED_PTRDIFF_TYPE__                                              \
+  __typeof__(_Generic((__PTRDIFF_TYPE__)0,                                     \
+                      long long int : (unsigned long long int)0,               \
+                      long int : (unsigned long int)0,                         \
+                      int : (unsigned int)0,                                   \
+                      short : (unsigned short)0,                               \
+                      signed char : (unsigned char)0))
+
+void testPtrDiffTypes() {
+  __UNSIGNED_PTRDIFF_TYPE__ p1 = 0;
+  printf("%tu", p1);  // No warning.
+
+  printf("%tu", 0.f); // expected-warning-re{{format specifies type 'unsigned ptrdiff_t' (aka '{{.+}}') but the argument has type 'float'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%f"
+  
+  ptrdiff_t p2 = 0;
+  printf("%td", p2);  // No warning.
+
+  printf("%td", 0.f); // expected-warning-re{{format specifies type 'ptrdiff_t' (aka '{{.+}}') but the argument has type 'float'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%f"
+
+  ptrdiff_t p3 = 0;
+  printf("%tn", &p3); // No warning.
+
+  short x;
+  printf("%tn", &x); // expected-warning-re{{format specifies type 'ptrdiff_t *' (aka '{{.+}}') but the argument has type 'short *'}}
+  // PrintfSpecifier::fixType doesn't handle %n, so a fix-it is not emitted,
+  // see the comment in PrintfSpecifier::fixType in PrintfFormatString.cpp.
+}
+
 void testEnum() {
   typedef enum {
     ImplicitA = 1,
     ImplicitB = 2
   } Implicit;
 
   typedef enum {
     ImplicitLLA = 0,

cfe/trunk/test/Sema/format-strings-scanf.c

 // RUN: %clang_cc1 -std=c11 -fsyntax-only -verify -Wformat-nonliteral %s
 
 // Test that -Wformat=0 works:
 // RUN: %clang_cc1 -std=c11 -fsyntax-only -Werror -Wformat=0 %s
 
 #include <stdarg.h>
 typedef __SIZE_TYPE__ size_t;
 #define __SSIZE_TYPE__                                                         \
   __typeof__(_Generic((__SIZE_TYPE__)0,                                        \
                       unsigned long long int : (long long int)0,               \
                       unsigned long int : (long int)0,                         \
                       unsigned int : (int)0,                                   \
                       unsigned short : (short)0,                               \
                       unsigned char : (signed char)0))
 typedef __SSIZE_TYPE__ ssize_t;                         
+
+typedef __PTRDIFF_TYPE__ ptrdiff_t;
+#define __UNSIGNED_PTRDIFF_TYPE__                                              \
+  __typeof__(_Generic((__PTRDIFF_TYPE__)0,                                     \
+                      long long int : (unsigned long long int)0,               \
+                      long int : (unsigned long int)0,                         \
+                      int : (unsigned int)0,                                   \
+                      short : (unsigned short)0,                               \
+                      signed char : (unsigned char)0))
+
 typedef struct _FILE FILE;
 typedef __WCHAR_TYPE__ wchar_t;
 
 int fscanf(FILE * restrict, const char * restrict, ...) ;
 int scanf(const char * restrict, ...) ;
 int sscanf(const char * restrict, const char * restrict, ...) ;
 int my_scanf(const char * restrict, ...) __attribute__((__format__(__scanf__, 1, 2)));
 
void test_size_types() {
 
   ssize_t sn = 0;
   scanf("%zn", &sn); // No warning.
 
   double d3 = 0.;
   scanf("%zn", &d3); // expected-warning-re{{format specifies type 'ssize_t *' (aka '{{.+}}') but the argument has type 'double *'}}
 }
 
+void test_ptrdiff_t_types() {
+  __UNSIGNED_PTRDIFF_TYPE__ p1 = 0;
+  scanf("%tu", &p1); // No warning.
+
+  double d1 = 0.;
+  scanf("%tu", &d1); // expected-warning-re{{format specifies type 'unsigned ptrdiff_t *' (aka '{{.+}}') but the argument has type 'double *'}}
+
+  ptrdiff_t p2 = 0;
+  scanf("%td", &p2); // No warning.
+  
+  double d2 = 0.;
+  scanf("%td", &d2); // expected-warning-re{{format specifies type 'ptrdiff_t *' (aka '{{.+}}') but the argument has type 'double *'}}
+
+  ptrdiff_t p3 = 0;
+  scanf("%tn", &p3); // No warning.
+  
+  double d3 = 0.;
+  scanf("%tn", &d3); // expected-warning-re{{format specifies type 'ptrdiff_t *' (aka '{{.+}}') but the argument has type 'double *'}}
+}
+
 void check_conditional_literal(char *s, int *i) {
   scanf(0 ? "%s" : "%d", i); // no warning
   scanf(1 ? "%s" : "%d", i); // expected-warning{{format specifies type 'char *'}}
   scanf(0 ? "%d %d" : "%d", i); // no warning
   scanf(1 ? "%d %d" : "%d", i); // expected-warning{{more '%' conversions than data arguments}}
   scanf(0 ? "%d %d" : "%d", i, s); // expected-warning{{data argument not used}}
   scanf(1 ? "%d %s" : "%d", i, s); // no warning
   scanf(i ? "%d %s" : "%d", i, s); // no warning
   scanf(i ? "%d" : "%d", i, s); // expected-warning{{data argument not used}}
   scanf(i ? "%s" : "%d", s); // expected-warning{{format specifies type 'int *'}}
 }