[cfe-commits] r161440 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td test/Analysis/cstring-syntax.c

[Anna Zaks]

On Aug 8, 2012, at 10:39 AM, Matt Beaumont-Gay wrote:

> On Wed, Aug 8, 2012 at 10:34 AM, Anna Zaks <ganna at apple.com> wrote:
>> 
>> On Aug 7, 2012, at 12:01 PM, Matt Beaumont-Gay wrote:
>> 
>>> Should we have similar treatment for strncpy?
>>> 
>>> Also, a nit:
>>> void f(char* s, char* d) {
>>> strncat(d, s, sizeof(d));
>>> }
>>> 
>>> test.cc:5:17: warning: the value of the size argument in 'strncat' is
>>> too large, might lead to a buffer overflow [-Wstrncat-size]
>>> strncat(d, s, sizeof(d));
>>>               ^~~~~~~~~
>>> 
>>> Here, sizeof(d) is not necessarily too large, just wrong.
>> 
>> It is too large in this case - it should be at least 1 less than sizeof(d), even if d is empty.
> 
> My point is that d is a pointer here, not an array.
> 

I've added a more generic message for this case in r161527.

Thanks for reviewing!
Anna.

>> 
>> Anna.
>>> 
>>> On Tue, Aug 7, 2012 at 11:36 AM, Anna Zaks <ganna at apple.com> wrote:
>>>> Author: zaks
>>>> Date: Tue Aug  7 13:36:58 2012
>>>> New Revision: 161440
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=161440&view=rev
>>>> Log:
>>>> Turn on strncat-size warning implemented a while ago.
>>>> 
>>>> Warns on anti-patterns/typos in the 'size' argument to strncat. The
>>>> correct size argument should look like the following:
>>>> - strncat(dst, src, sizeof(dst) - strlen(dest) - 1);
>>>> 
>>>> We warn on:
>>>> - sizeof(dst)
>>>> - sizeof(src)
>>>> - sizeof(dst) - strlen(dst)
>>>> - sizeof(src) - anything
>>>> 
>>>> (This has been implemented in void Sema::CheckStrncatArguments().)
>>>> 
>>>> Modified:
>>>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>   cfe/trunk/test/Analysis/cstring-syntax.c
>>>> 
>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=161440&r1=161439&r2=161440&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug  7 13:36:58 2012
>>>> @@ -373,9 +373,9 @@
>>>> 
>>>> def warn_strncat_large_size : Warning<
>>>>  "the value of the size argument in 'strncat' is too large, might lead to a "
>>>> -  "buffer overflow">, InGroup<StrncatSize>, DefaultIgnore;
>>>> +  "buffer overflow">, InGroup<StrncatSize>, DefaultWarnNoWerror;
>>>> def warn_strncat_src_size : Warning<"size argument in 'strncat' call appears "
>>>> -  "to be size of the source">, InGroup<StrncatSize>, DefaultIgnore;
>>>> +  "to be size of the source">, InGroup<StrncatSize>, DefaultWarnNoWerror;
>>>> def note_strncat_wrong_size : Note<
>>>>  "change the argument to be the free space in the destination buffer minus "
>>>>  "the terminating null byte">;
>>>> 
>>>> Modified: cfe/trunk/test/Analysis/cstring-syntax.c
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=161440&r1=161439&r2=161440&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/test/Analysis/cstring-syntax.c (original)
>>>> +++ cfe/trunk/test/Analysis/cstring-syntax.c Tue Aug  7 13:36:58 2012
>>>> @@ -1,4 +1,4 @@
>>>> -// RUN: %clang_cc1 -analyze -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
>>>> +// RUN: %clang_cc1 -analyze -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
>>>> 
>>>> typedef __SIZE_TYPE__ size_t;
>>>> char  *strncat(char *, const char *, size_t);
>>>> 
>>>> 
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>