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

[Jordan Rose]

That would indeed be more appropriate. I'm not actually familiar with this part of the code, so I can't say whether or not it will work without further changes, but it certainly ought to be correct. +John in case there's something obvious I'm missing.

Jordan


On Feb 24, 2014, at 20:07 , Richard Smith <richard at metafoo.co.uk> wrote:

> The more appropriate thing to do would be to give them array-of-const-char types. Is that also OK?
> 
> On 24 Feb 2014 19:53, "Jordan Rose" <jordan_rose at apple.com> wrote:
> We can certainly give those buffers type const char * without harming anything.
> 
> Jordan
> 
> 
> On Feb 24, 2014, at 19:24 , Richard Smith <richard at metafoo.co.uk> wrote:
> 
>> 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
>> 
>> _______________________________________________
>> 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/e165ff62/attachment.html>