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/ 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
