On Wed, Jun 15, 2011 at 7:27 PM, Nico Weber <[email protected]> wrote: > On Wed, Jun 15, 2011 at 7:00 PM, Chandler Carruth <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
