r277787 - [Sema] Add sizeof diagnostics for bzero

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 5 09:50:16 PDT 2016


Hi Gabor,

Thanks for the PR, I reverted it in r277830 until I can manage to fix it.

On Fri, Aug 5, 2016 at 6:44 AM, Gabor Ballabas <gaborb at inf.u-szeged.hu> wrote:
> Hi Bruno,
>
> My name is Gabor Ballabas, I work at the University of Szeged, Hungary.
> We run daily code size benchmarks testing trunk Clang. After your patch
> landed
> we started to get a Clang crash when compiling one of the files in our
> benchmark system.
>
> I created a bugreport with the details:
> https://llvm.org/bugs/show_bug.cgi?id=28870
>
> I hope you can take a look at this issue.
>
>
> Best regards,
> Gabor Ballabas
>
>
> On 08/05/2016 01:55 AM, Bruno Cardoso Lopes via cfe-commits wrote:
>>
>> Author: bruno
>> Date: Thu Aug  4 18:55:22 2016
>> New Revision: 277787
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=277787&view=rev
>> Log:
>> [Sema] Add sizeof diagnostics for bzero
>>
>> For memset (and others) we can get diagnostics like:
>>
>>    struct stat { int x; };
>>    void foo(struct stat *stamps) {
>>      bzero(stamps, sizeof(stamps));
>>      memset(stamps, 0, sizeof(stamps));
>>    }
>>
>>    t.c:7:28: warning: 'memset' call operates on objects of type 'struct
>> stat' while the size is based on a different type 'struct stat *'
>> [-Wsizeof-pointer-memaccess]
>>      memset(stamps, 0, sizeof(stamps));
>>             ~~~~~~            ^~~~~~
>>    t.c:7:28: note: did you mean to dereference the argument to 'sizeof'
>> (and multiply it by the number of elements)?
>>      memset(stamps, 0, sizeof(stamps));
>>                               ^~~~~~
>>
>> This patch implements the same class of warnings for bzero.
>>
>> Differential Revision: https://reviews.llvm.org/D22525
>>
>> rdar://problem/18963514
>>
>> Modified:
>>      cfe/trunk/lib/AST/Decl.cpp
>>      cfe/trunk/lib/Sema/SemaChecking.cpp
>>      cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
>>
>> Modified: cfe/trunk/lib/AST/Decl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=277787&r1=277786&r2=277787&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/Decl.cpp (original)
>> +++ cfe/trunk/lib/AST/Decl.cpp Thu Aug  4 18:55:22 2016
>> @@ -3408,6 +3408,10 @@ unsigned FunctionDecl::getMemoryFunction
>>     case Builtin::BIstrlen:
>>       return Builtin::BIstrlen;
>>   +  case Builtin::BI__builtin_bzero:
>> +  case Builtin::BIbzero:
>> +    return Builtin::BIbzero;
>> +
>>     default:
>>       if (isExternC()) {
>>         if (FnInfo->isStr("memset"))
>> @@ -3430,6 +3434,8 @@ unsigned FunctionDecl::getMemoryFunction
>>           return Builtin::BIstrndup;
>>         else if (FnInfo->isStr("strlen"))
>>           return Builtin::BIstrlen;
>> +      else if (FnInfo->isStr("bzero"))
>> +        return Builtin::BIbzero;
>>       }
>>       break;
>>     }
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277787&r1=277786&r2=277787&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug  4 18:55:22 2016
>> @@ -6179,13 +6179,15 @@ void Sema::CheckMemaccessArguments(const
>>       // It is possible to have a non-standard definition of memset.
>> Validate
>>     // we have enough arguments, and if not, abort further checking.
>> -  unsigned ExpectedNumArgs = (BId == Builtin::BIstrndup ? 2 : 3);
>> +  unsigned ExpectedNumArgs =
>> +      (BId == Builtin::BIstrndup || Builtin::BIbzero ? 2 : 3);
>>     if (Call->getNumArgs() < ExpectedNumArgs)
>>       return;
>>   -  unsigned LastArg = (BId == Builtin::BImemset ||
>> +  unsigned LastArg = (BId == Builtin::BImemset || BId == Builtin::BIbzero
>> ||
>>                         BId == Builtin::BIstrndup ? 1 : 2);
>> -  unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
>> +  unsigned LenArg =
>> +      (BId == Builtin::BIbzero || BId == Builtin::BIstrndup ? 1 : 2);
>>     const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
>>       if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,
>>
>> 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=277787&r1=277786&r2=277787&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Thu Aug  4 18:55:22
>> 2016
>> @@ -1,5 +1,6 @@
>>   // RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s
>>   //
>> +extern "C" void *bzero(void *, unsigned);
>>   extern "C" void *memset(void *, int, unsigned);
>>   extern "C" void *memmove(void *s1, const void *s2, unsigned n);
>>   extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
>> @@ -47,6 +48,19 @@ void f(Mat m, const Foo& const_foo, char
>>     memset(heap_buffer, 0, sizeof(heap_buffer));  // \
>>         // expected-warning {{'memset' call operates on objects of type
>> 'char' while the size is based on a different type 'char *'}}
>> expected-note{{did you mean to provide an explicit length?}}
>>   +  bzero(&s, sizeof(&s));  // \
>> +      // expected-warning {{'bzero' call operates on objects of type 'S'
>> while the size is based on a different type 'S *'}} expected-note{{did you
>> mean to remove the addressof in the argument to 'sizeof' (and multiply it by
>> the number of elements)?}}
>> +  bzero(ps, sizeof(ps));  // \
>> +      // expected-warning {{'bzero' call operates on objects of type 'S'
>> while the size is based on a different type 'S *'}} expected-note{{did you
>> mean to dereference the argument to 'sizeof' (and multiply it by the number
>> of elements)?}}
>> +  bzero(ps2, sizeof(ps2));  // \
>> +      // expected-warning {{'bzero' call operates on objects of type 'S'
>> while the size is based on a different type 'PS' (aka 'S *')}}
>> expected-note{{did you mean to dereference the argument to 'sizeof' (and
>> multiply it by the number of elements)?}}
>> +  bzero(ps2, sizeof(typeof(ps2)));  // \
>> +      // expected-warning {{argument to 'sizeof' in 'bzero' call is the
>> same pointer type}}
>> +  bzero(ps2, sizeof(PS));  // \
>> +      // expected-warning {{argument to 'sizeof' in 'bzero' call is the
>> same pointer type}}
>> +  bzero(heap_buffer, sizeof(heap_buffer));  // \
>> +      // expected-warning {{'bzero' call operates on objects of type
>> 'char' while the size is based on a different type 'char *'}}
>> expected-note{{did you mean to provide an explicit length?}}
>> +
>>     memcpy(&s, 0, sizeof(&s));  // \
>>         // expected-warning {{'memcpy' call operates on objects of type
>> 'S' while the size is based on a different type 'S *'}} expected-note{{did
>> you mean to remove the addressof in the argument to 'sizeof' (and multiply
>> it by the number of elements)?}}
>>     memcpy(0, &s, sizeof(&s));  // \
>> @@ -73,6 +87,21 @@ void f(Mat m, const Foo& const_foo, char
>>     memset(arr, 0, sizeof(arr));
>>     memset(parr, 0, sizeof(parr));
>>   +  bzero((void*)&s, sizeof(&s));
>> +  bzero(&s, sizeof(s));
>> +  bzero(&s, sizeof(S));
>> +  bzero(&s, sizeof(const S));
>> +  bzero(&s, sizeof(volatile S));
>> +  bzero(&s, sizeof(volatile const S));
>> +  bzero(&foo, sizeof(CFoo));
>> +  bzero(&foo, sizeof(VFoo));
>> +  bzero(&foo, sizeof(CVFoo));
>> +  bzero(ps, sizeof(*ps));
>> +  bzero(ps2, sizeof(*ps2));
>> +  bzero(ps2, sizeof(typeof(*ps2)));
>> +  bzero(arr, sizeof(arr));
>> +  bzero(parr, sizeof(parr));
>> +
>>     memcpy(&foo, &const_foo, sizeof(Foo));
>>     memcpy((void*)&s, 0, sizeof(&s));
>>     memcpy(0, (void*)&s, sizeof(&s));
>> @@ -96,12 +125,17 @@ void f(Mat m, const Foo& const_foo, char
>>     int iarr[14];
>>     memset(&iarr[0], 0, sizeof iarr);
>>     memset(iarr, 0, sizeof iarr);
>> +  bzero(&iarr[0], sizeof iarr);
>> +  bzero(iarr, sizeof iarr);
>>       int* iparr[14];
>>     memset(&iparr[0], 0, sizeof iparr);
>>     memset(iparr, 0, sizeof iparr);
>> +  bzero(&iparr[0], sizeof iparr);
>> +  bzero(iparr, sizeof iparr);
>>       memset(m, 0, sizeof(Mat));
>> +  bzero(m, sizeof(Mat));
>>       // Copy to raw buffer shouldn't warn either
>>     memcpy(&foo, &arr, sizeof(Foo));
>> @@ -114,12 +148,21 @@ void f(Mat m, const Foo& const_foo, char
>>       for (;;) {}
>>       &s;
>>     }), 0, sizeof(s));
>> +
>> +  bzero(({
>> +    if (0) {}
>> +    while (0) {}
>> +    for (;;) {}
>> +    &s;
>> +  }), sizeof(s));
>>   }
>>     namespace ns {
>>   void memset(void* s, char c, int n);
>> +void bzero(void* s, int n);
>>   void f(int* i) {
>>     memset(i, 0, sizeof(i));
>> +  bzero(i, sizeof(i));
>>   }
>>   }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc


More information about the cfe-commits mailing list