r201795 - Sema: Emit a warning for non-null terminated format strings and other pathological cases.

[Richard Smith]

This caused PR18939. Please fix or revert, thanks!

Looks like the bug is in Sema::ParseObjCStringLiteral -- it synthesizes a
string literal from a token concatenation and gives it the type "char*"


On Thu, Feb 20, 2014 at 9:05 AM, Benjamin Kramer
<benny.kra at googlemail.com>wrote:

> Author: d0k
> Date: Thu Feb 20 11:05:38 2014
> New Revision: 201795
>
> URL: http://llvm.org/viewvc/llvm-project?rev=201795&view=rev
> Log:
> Sema: Emit a warning for non-null terminated format strings and other
> pathological cases.
>
> PR18905.
>
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/lib/Sema/SemaChecking.cpp
>     cfe/trunk/test/Sema/format-strings.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=201795&r1=201794&r2=201795&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb 20
> 11:05:38 2014
> @@ -6223,6 +6223,8 @@ def warn_format_string_is_wide_literal :
>    "format string should not be a wide string">, InGroup<Format>;
>  def warn_printf_format_string_contains_null_char : Warning<
>    "format string contains '\\0' within the string body">, InGroup<Format>;
> +def warn_printf_format_string_not_null_terminated : Warning<
> +  "format string is not null-terminated">, InGroup<Format>;
>  def warn_printf_asterisk_missing_arg : Warning<
>    "'%select{*|.*}0' specified field %select{width|precision}0 is missing
> a matching 'int' argument">,
>    InGroup<Format>;
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=201795&r1=201794&r2=201795&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Feb 20 11:05:38 2014
> @@ -3493,9 +3493,25 @@ void Sema::CheckFormatString(const Strin
>    // Str - The format string.  NOTE: this is NOT null-terminated!
>    StringRef StrRef = FExpr->getString();
>    const char *Str = StrRef.data();
> -  unsigned StrLen = StrRef.size();
> +  // Account for cases where the string literal is truncated in a
> declaration.
> +  const ConstantArrayType *T =
> Context.getAsConstantArrayType(FExpr->getType());
> +  assert(T && "String literal not of constant array type!");
> +  size_t TypeSize = T->getSize().getZExtValue();
> +  size_t StrLen = std::min(std::max(TypeSize, size_t(1)) - 1,
> StrRef.size());
>    const unsigned numDataArgs = Args.size() - firstDataArg;
> -
> +
> +  // Emit a warning if the string literal is truncated and does not
> contain an
> +  // embedded null character.
> +  if (TypeSize <= StrRef.size() &&
> +      StrRef.substr(0, TypeSize).find('\0') == StringRef::npos) {
> +    CheckFormatHandler::EmitFormatDiagnostic(
> +        *this, inFunctionCall, Args[format_idx],
> +        PDiag(diag::warn_printf_format_string_not_null_terminated),
> +        FExpr->getLocStart(),
> +        /*IsStringLocation=*/true, OrigFormatExpr->getSourceRange());
> +    return;
> +  }
> +
>    // CHECK: empty format string?
>    if (StrLen == 0 && numDataArgs > 0) {
>      CheckFormatHandler::EmitFormatDiagnostic(
>
> Modified: cfe/trunk/test/Sema/format-strings.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=201795&r1=201794&r2=201795&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Sema/format-strings.c (original)
> +++ cfe/trunk/test/Sema/format-strings.c Thu Feb 20 11:05:38 2014
> @@ -535,6 +535,21 @@ void pr9751() {
>           0.0); // expected-warning{{format specifies}}
>  }
>
> +void pr18905() {
> +  const char s1[] = "s\0%s"; // expected-note{{format string is defined
> here}}
> +  const char s2[1] = "s"; // expected-note{{format string is defined
> here}}
> +  const char s3[2] = "s\0%s"; // expected-warning{{initializer-string for
> char array is too long}}
> +  const char s4[10] = "s";
> +  const char s5[0] = "%s"; // expected-warning{{initializer-string for
> char array is too long}}
> +                           // expected-note at -1{{format string is defined
> here}}
> +
> +  printf(s1); // expected-warning{{format string contains '\0' within the
> string body}}
> +  printf(s2); // expected-warning{{format string is not null-terminated}}
> +  printf(s3); // no-warning
> +  printf(s4); // no-warning
> +  printf(s5); // expected-warning{{format string is not null-terminated}}
> +}
> +
>  void __attribute__((format(strfmon,1,2))) monformat(const char *fmt, ...);
>  void __attribute__((format(strftime,1,0))) dateformat(const char *fmt);
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140224/b6729b3f/attachment.html>