[cfe-commits] r160966 - in /cfe/trunk: lib/Analysis/PrintfFormatString.cpp lib/Analysis/ScanfFormatString.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-scanf.c test/Sema/format-strings.c

[Alexey Samsonov]

Hello Hans,

After this patch one of our tests failed with the following error:

error: format specifies type 'int *' but the argument has type
'volatile int *' [-Werror,-Wformat]
  fprintf(stdout, "printing a string: %s%n\n", s, &a);
                                        ~~        ^~

Do you think that we should fix the code, or Clang shouldn't produce the
warning in this case?

On Mon, Jul 30, 2012 at 9:11 PM, Hans Wennborg <hans at hanshq.net> wrote:

> Author: hans
> Date: Mon Jul 30 12:11:32 2012
> New Revision: 160966
>
> URL: http://llvm.org/viewvc/llvm-project?rev=160966&view=rev
> Log:
> Make -Wformat check the argument type for %n.
>
> This makes Clang check that the corresponding argument for "%n" in a
> format string is a pointer to int.
>
> Modified:
>     cfe/trunk/lib/Analysis/PrintfFormatString.cpp
>     cfe/trunk/lib/Analysis/ScanfFormatString.cpp
>     cfe/trunk/lib/Sema/SemaChecking.cpp
>     cfe/trunk/test/Sema/format-strings-scanf.c
>     cfe/trunk/test/Sema/format-strings.c
>
> Modified: cfe/trunk/lib/Analysis/PrintfFormatString.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PrintfFormatString.cpp?rev=160966&r1=160965&r2=160966&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/PrintfFormatString.cpp (original)
> +++ cfe/trunk/lib/Analysis/PrintfFormatString.cpp Mon Jul 30 12:11:32 2012
> @@ -330,6 +330,8 @@
>        return ArgTypeResult(Ctx.WCharTy, "wchar_t");
>      case ConversionSpecifier::pArg:
>        return ArgTypeResult::CPointerTy;
> +    case ConversionSpecifier::nArg:
> +      return Ctx.getPointerType(Ctx.IntTy);
>      case ConversionSpecifier::ObjCObjArg:
>        return ArgTypeResult::ObjCPointerTy;
>      default:
> @@ -342,6 +344,10 @@
>
>  bool PrintfSpecifier::fixType(QualType QT, const LangOptions &LangOpt,
>                                ASTContext &Ctx, bool IsObjCLiteral) {
> +  // %n is different from other conversion specifiers; don't try to fix
> it.
> +  if (CS.getKind() == ConversionSpecifier::nArg)
> +    return false;
> +
>    // Handle Objective-C objects first. Note that while the '%@' specifier
> will
>    // not warn for structure pointer or void pointer arguments (because
> that's
>    // how CoreFoundation objects are implemented), we only show a fixit
> for '%@'
>
> Modified: cfe/trunk/lib/Analysis/ScanfFormatString.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ScanfFormatString.cpp?rev=160966&r1=160965&r2=160966&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ScanfFormatString.cpp (original)
> +++ cfe/trunk/lib/Analysis/ScanfFormatString.cpp Mon Jul 30 12:11:32 2012
> @@ -303,6 +303,9 @@
>      case ConversionSpecifier::pArg:
>        return ScanfArgTypeResult(ArgTypeResult(ArgTypeResult::CPointerTy));
>
> +    case ConversionSpecifier::nArg:
> +      return ArgTypeResult(Ctx.IntTy);
> +
>      default:
>        break;
>    }
> @@ -315,6 +318,10 @@
>    if (!QT->isPointerType())
>      return false;
>
> +  // %n is different from other conversion specifiers; don't try to fix
> it.
> +  if (CS.getKind() == ConversionSpecifier::nArg)
> +    return false;
> +
>    QualType PT = QT->getPointeeType();
>
>    // If it's an enum, get its underlying type.
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=160966&r1=160965&r2=160966&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jul 30 12:11:32 2012
> @@ -2568,8 +2568,6 @@
>                           getLocationOfByte(CS.getStart()),
>                           /*IsStringLocation*/true,
>                           getSpecifierRange(startSpecifier, specifierLen));
> -    // Continue checking the other format specifiers.
> -    return true;
>    }
>
>    // The remaining checks depend on the data arguments.
>
> Modified: cfe/trunk/test/Sema/format-strings-scanf.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-scanf.c?rev=160966&r1=160965&r2=160966&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/format-strings-scanf.c (original)
> +++ cfe/trunk/test/Sema/format-strings-scanf.c Mon Jul 30 12:11:32 2012
> @@ -121,3 +121,8 @@
>    scanf("%qd", x); // expected-warning{{format specifies type 'long long
> *' but the argument has type 'int *'}}
>    scanf("%qd", llx); // no-warning
>  }
> +
> +void test_writeback(int *x) {
> +  scanf("%n", (void*)0); // expected-warning{{format specifies type 'int
> *' but the argument has type 'void *'}}
> +  scanf("%n %c", x, x); // expected-warning{{format specifies type 'char
> *' but the argument has type 'int *'}}
> +}
>
> Modified: cfe/trunk/test/Sema/format-strings.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=160966&r1=160965&r2=160966&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/format-strings.c (original)
> +++ cfe/trunk/test/Sema/format-strings.c Mon Jul 30 12:11:32 2012
> @@ -91,6 +91,7 @@
>
>    printf("%n",&x); // expected-warning {{'%n' in format string
> discouraged}}
>    sprintf(b,"%d%%%n",1, &x); // expected-warning {{'%n' in format string
> dis}}
> +  printf("%n",b); // expected-warning {{'%n' in format string
> discouraged}} expected-warning{{format specifies type 'int *' but the
> argument has type 'char *'}}
>  }
>
>  void check_invalid_specifier(FILE* fp, char *buf)
> @@ -316,14 +317,14 @@
>    // Bad flag usage
>    printf("%#p", (void *) 0); // expected-warning{{flag '#' results in
> undefined behavior with 'p' conversion specifier}}
>    printf("%0d", -1); // no-warning
> -  printf("%#n", (void *) 0); // expected-warning{{flag '#' results in
> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
> '%n' in format string discouraged (potentially insecure)}}
> -  printf("%-n", (void *) 0); // expected-warning{{flag '-' results in
> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
> '%n' in format string discouraged (potentially insecure)}}
> +  printf("%#n", (int *) 0); // expected-warning{{flag '#' results in
> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
> '%n' in format string discouraged (potentially insecure)}}
> +  printf("%-n", (int *) 0); // expected-warning{{flag '-' results in
> undefined behavior with 'n' conversion specifier}} expected-warning{{use of
> '%n' in format string discouraged (potentially insecure)}}
>    printf("%-p", (void *) 0); // no-warning
>
>    // Bad optional amount use
>    printf("%.2c", 'a'); // expected-warning{{precision used with 'c'
> conversion specifier, resulting in undefined behavior}}
> -  printf("%1n", (void *) 0); // expected-warning{{field width used with
> 'n' conversion specifier, resulting in undefined behavior}}
> expected-warning{{use of '%n' in format string discouraged (potentially
> insecure)}}
> -  printf("%.9n", (void *) 0); // expected-warning{{precision used with
> 'n' conversion specifier, resulting in undefined behavior}}
> expected-warning{{use of '%n' in format string discouraged (potentially
> insecure)}}
> +  printf("%1n", (int *) 0); // expected-warning{{field width used with
> 'n' conversion specifier, resulting in undefined behavior}}
> expected-warning{{use of '%n' in format string discouraged (potentially
> insecure)}}
> +  printf("%.9n", (int *) 0); // expected-warning{{precision used with 'n'
> conversion specifier, resulting in undefined behavior}}
> expected-warning{{use of '%n' in format string discouraged (potentially
> insecure)}}
>
>    // Ignored flags
>    printf("% +f", 1.23); // expected-warning{{flag ' ' is ignored when
> flag '+' is present}}
> @@ -436,8 +437,9 @@
>    printf("%18$s\n", 1, "foo"); // expected-warning{{data argument
> position '18' exceeds the number of data arguments (2)}}
>
>    const char kFormat3[] = "%n"; // expected-note{{format string is
> defined here}}
> -  printf(kFormat3, "as"); // expected-warning{{use of '%n' in format
> string discouraged}}
> -  printf("%n", "as"); // expected-warning{{use of '%n' in format string
> discouraged}}
> +  printf(kFormat3, (int*)NULL); // expected-warning{{use of '%n' in
> format string discouraged}}
> +  printf("%n", (int*)NULL); // expected-warning{{use of '%n' in format
> string discouraged}}
> +
>
>    const char kFormat4[] = "%y"; // expected-note{{format string is
> defined here}}
>    printf(kFormat4, 5); // expected-warning{{invalid conversion specifier
> 'y'}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120731/ab313001/attachment.html>