[cfe-commits] r133136 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-memset-bad-sizeof.cpp

[Nico Weber]

On Wed, Jun 15, 2011 at 7:27 PM, Nico Weber <thakis at chromium.org> wrote:
> On Wed, Jun 15, 2011 at 7:00 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>> Author: chandlerc
>> Date: Wed Jun 15 21:00:04 2011
>> New Revision: 133136
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=133136&view=rev
>> Log:
>> Skip both character pointers and void pointers when diagnosing bad
>> argument types for mem{set,cpy,move}. Character pointers, much like void
>> pointers, often point to generic "memory", so trying to check whether
>> they match the type of the argument to 'sizeof' (or other checks) is
>> unproductive and often results in false positives.
>>
>> Nico, please review; does this miss any of the bugs you were trying to
>> find with this warning? The array test case you had should be caught by
>> the array-specific sizeof warning I think.
>
> Yes, now it doesn't find http://codereview.chromium.org/7167009/

And depending on how typedefs work, it probably misses
http://codereview.chromium.org/7003140/diff/1001/media/video/capture/fake_video_capture_device.cc
as well.

>
> Can you provide any examples where this warns when it shouldn't? (…and
> maybe revert this in the meantime?)
>
>>
>> Modified:
>>    cfe/trunk/lib/Sema/SemaChecking.cpp
>>    cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=133136&r1=133135&r2=133136&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Jun 15 21:00:04 2011
>> @@ -1866,7 +1866,9 @@
>>     if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
>>       QualType PointeeTy = DestPtrTy->getPointeeType();
>>
>> -      if (PointeeTy->isVoidType())
>> +      // Don't warn about void pointers or char pointers as both are often used
>> +      // for directly representing memory, regardless of its underlying type.
>> +      if (PointeeTy->isVoidType() || PointeeTy->isCharType())
>>         continue;
>>
>>       // Catch "memset(p, 0, sizeof(p))" -- needs to be sizeof(*p).
>>
>> Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp?rev=133136&r1=133135&r2=133136&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Wed Jun 15 21:00:04 2011
>> @@ -23,10 +23,11 @@
>>  }
>>
>>  // http://www.lysator.liu.se/c/c-faq/c-2.html#2-6
>> -void f(char fake_array[8], Mat m, const Foo& const_foo) {
>> +void f(Mat m, const Foo& const_foo) {
>>   S s;
>>   S* ps = &s;
>>   PS ps2 = &s;
>> +  char c = 42;
>>   char arr[5];
>>   char* parr[5];
>>   Foo foo;
>> @@ -42,8 +43,6 @@
>>       // expected-warning {{the argument to sizeof is pointer type 'typeof (ps2)' (aka 'S *'), expected 'S' to match first argument to 'memset'}}
>>   memset(ps2, 0, sizeof(PS));  // \
>>       // expected-warning {{the argument to sizeof is pointer type 'PS' (aka 'S *'), expected 'S' to match first argument to 'memset'}}
>> -  memset(fake_array, 0, sizeof(fake_array));  // \
>> -      // expected-warning {{the argument to sizeof is pointer type 'char *', expected 'char' to match first argument to 'memset'}}
>>
>>   memcpy(&s, 0, sizeof(&s));  // \
>>       // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memcpy'}}
>> @@ -69,6 +68,8 @@
>>   memcpy(&foo, &const_foo, sizeof(Foo));
>>   memcpy((void*)&s, 0, sizeof(&s));
>>   memcpy(0, (void*)&s, sizeof(&s));
>> +  memcpy(&parr[3], &c, sizeof(&c));
>> +  memcpy((char*)&parr[3], &c, sizeof(&c));
>>
>>   CFooRef cfoo = foo;
>>   memcpy(&foo, &cfoo, sizeof(Foo));
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>