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

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
>