D68526 should fix it. Take a look please.

> Dňa 7. 10. 2019 o 17:09 užívateľ Nico Weber <tha...@chromium.org> napísal:
> 
> 
> I gave this another try now that we have a compiler with rL372600. Another 
> thing the warning currently warns on is code like this:
> 
>   char memory[kOpcodeMemory];
>   OpcodeFactory opcode_maker(memory, sizeof(memory));
>   size_t count = sizeof(memory) / sizeof(PolicyOpcode);
> 
> or
> 
>   int32_t fds[sizeof(buffer->data) / sizeof(int32_t)], i, count;
>   size_t size;
> 
> (the latter from wayland).
> 
> What do you think about also not emitting the warning if the lhs sizeof is an 
> array of signed or unsigned char? The warning wants the rhs sizeof to be 
> sizeof(char) which is 1, and dividing by that doesn't really make sense. So 
> this might be a change that improves false negative rate while probably not 
> hurting true positive rate.
> 
>> On Mon, Sep 23, 2019 at 9:11 AM Dávid Bolvanský <david.bolvan...@gmail.com> 
>> wrote:
>> Yeah, this needs to be handled a bit differently (if we want so).
>> 
>> po 23. 9. 2019 o 15:07 Nico Weber <tha...@chromium.org> napísal(a):
>>> It still warns if the inner array is in a struct. That's probably ok though.
>>> 
>>> struct Point {
>>>   int xy[2];
>>> };
>>> 
>>> void f() {
>>>   Point points[3];
>>>   for (int i = 0; i < sizeof(points) / sizeof(int); ++i)
>>>     (&points[0].xy[0])[i] = 0;
>>> }
>>> 
>>>> On Mon, Sep 23, 2019 at 8:54 AM Nico Weber <tha...@chromium.org> wrote:
>>>> That was fast. Thanks much! :)
>>>> 
>>>>> On Mon, Sep 23, 2019 at 8:52 AM Dávid Bolvanský 
>>>>> <david.bolvan...@gmail.com> wrote:
>>>>> Hello,
>>>>> 
>>>>> Thanks for the proposed idea, implemented in rL372600.
>>>>> 
>>>>> po 23. 9. 2019 o 14:23 Nico Weber <tha...@chromium.org> napísal(a):
>>>>>> We're looking at turning this one.
>>>>>> 
>>>>>> One thing that this warns about that's a false positive where we've seen 
>>>>>> it is this code for nested arrays:
>>>>>> 
>>>>>>   float m[4][4];
>>>>>>   for (int i = 0; i < sizeof(m) / sizeof(**m); ++i) (&**m)[i] = 0;
>>>>>> 
>>>>>> (Why would anyone write code like this? It's a reduced example; consider 
>>>>>> e.g. wanting to call std::generate_n() on all elements of a nested 
>>>>>> array.)
>>>>>> 
>>>>>> Can we make the warning not fire when dividing the size of a nested 
>>>>>> array by the size of the deepest base type?
>>>>>> 
>>>>>>> On Wed, Sep 11, 2019 at 6:58 AM David Bolvansky via cfe-commits 
>>>>>>> <cfe-commits@lists.llvm.org> wrote:
>>>>>>> Author: xbolva00
>>>>>>> Date: Wed Sep 11 03:59:47 2019
>>>>>>> New Revision: 371605
>>>>>>> 
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=371605&view=rev
>>>>>>> Log:
>>>>>>> [Diagnostics] Add -Wsizeof-array-div
>>>>>>> 
>>>>>>> Summary: Clang version of https://www.viva64.com/en/examples/v706/
>>>>>>> 
>>>>>>> Reviewers: rsmith
>>>>>>> 
>>>>>>> Differential Revision: https://reviews.llvm.org/D67287
>>>>>>> 
>>>>>>> Added:
>>>>>>>     cfe/trunk/test/Sema/div-sizeof-array.cpp
>>>>>>> Modified:
>>>>>>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>>>>     cfe/trunk/lib/Sema/SemaExpr.cpp
>>>>>>> 
>>>>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>>>> URL: 
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>>>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11 
>>>>>>> 03:59:47 2019
>>>>>>> @@ -3406,6 +3406,10 @@ def note_pointer_declared_here : Note<
>>>>>>>  def warn_division_sizeof_ptr : Warning<
>>>>>>>    "'%0' will return the size of the pointer, not the array itself">,
>>>>>>>    InGroup<DiagGroup<"sizeof-pointer-div">>;
>>>>>>> +def warn_division_sizeof_array : Warning<
>>>>>>> +  "expression does not compute the number of elements in this array; 
>>>>>>> element "
>>>>>>> +  "type is %0, not %1">,
>>>>>>> +  InGroup<DiagGroup<"sizeof-array-div">>;
>>>>>>> 
>>>>>>>  def note_function_warning_silence : Note<
>>>>>>>      "prefix with the address-of operator to silence this warning">;
>>>>>>> 
>>>>>>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>>>>>>> URL: 
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>>>>>>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019
>>>>>>> @@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe
>>>>>>>    else
>>>>>>>      RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>>>>>>> 
>>>>>>> -  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
>>>>>>> -    return;
>>>>>>> -  if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() 
>>>>>>> !=
>>>>>>> -      RHSTy.getCanonicalType().getUnqualifiedType())
>>>>>>> -    return;
>>>>>>> +  if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
>>>>>>> +    if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), 
>>>>>>> RHSTy))
>>>>>>> +      return;
>>>>>>> 
>>>>>>> -  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
>>>>>>> LHS->getSourceRange();
>>>>>>> -  if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
>>>>>>> -    if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>>>>>>> -      S.Diag(LHSArgDecl->getLocation(), 
>>>>>>> diag::note_pointer_declared_here)
>>>>>>> -          << LHSArgDecl;
>>>>>>> +    S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
>>>>>>> LHS->getSourceRange();
>>>>>>> +    if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
>>>>>>> +      if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>>>>>>> +        S.Diag(LHSArgDecl->getLocation(), 
>>>>>>> diag::note_pointer_declared_here)
>>>>>>> +            << LHSArgDecl;
>>>>>>> +    }
>>>>>>> +  } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
>>>>>>> +    QualType ArrayElemTy = ArrayTy->getElementType();
>>>>>>> +    if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
>>>>>>> +        S.Context.getTypeSize(ArrayElemTy) == 
>>>>>>> S.Context.getTypeSize(RHSTy))
>>>>>>> +      return;
>>>>>>> +    S.Diag(Loc, diag::warn_division_sizeof_array)
>>>>>>> +        << LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
>>>>>>> +    if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {
>>>>>>> +      if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>>>>>>> +        S.Diag(LHSArgDecl->getLocation(), 
>>>>>>> diag::note_array_declared_here)
>>>>>>> +            << LHSArgDecl;
>>>>>>> +    }
>>>>>>>    }
>>>>>>>  }
>>>>>>> 
>>>>>>> 
>>>>>>> Added: cfe/trunk/test/Sema/div-sizeof-array.cpp
>>>>>>> URL: 
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)
>>>>>>> +++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019
>>>>>>> @@ -0,0 +1,28 @@
>>>>>>> +// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
>>>>>>> +
>>>>>>> +template <typename Ty, int N>
>>>>>>> +int f(Ty (&Array)[N]) {
>>>>>>> +  return sizeof(Array) / sizeof(Ty); // Should not warn
>>>>>>> +}
>>>>>>> +
>>>>>>> +typedef int int32;
>>>>>>> +
>>>>>>> +void test(void) {
>>>>>>> +  int arr[12];                // expected-note 2 {{array 'arr' 
>>>>>>> declared here}}
>>>>>>> +  unsigned long long arr2[4];
>>>>>>> +  int *p = &arr[0];
>>>>>>> +  int a1 = sizeof(arr) / sizeof(*arr);
>>>>>>> +  int a2 = sizeof arr / sizeof p; // expected-warning {{expression 
>>>>>>> does not compute the number of elements in this array; element type is 
>>>>>>> 'int', not 'int *'}}
>>>>>>> +  int a4 = sizeof arr2 / sizeof p;
>>>>>>> +  int a5 = sizeof(arr) / sizeof(short); // expected-warning 
>>>>>>> {{expression does not compute the number of elements in this array; 
>>>>>>> element type is 'int', not 'short'}}
>>>>>>> +  int a6 = sizeof(arr) / sizeof(int32);
>>>>>>> +  int a7 = sizeof(arr) / sizeof(int);
>>>>>>> +  int a9 = sizeof(arr) / sizeof(unsigned int);
>>>>>>> +  const char arr3[2] = "A";
>>>>>>> +  int a10 = sizeof(arr3) / sizeof(char);
>>>>>>> +
>>>>>>> +  int arr4[10][12];                         // expected-note 3 {{array 
>>>>>>> 'arr4' declared here}}
>>>>>>> +  int b1 = sizeof(arr4) / sizeof(arr2[12]); // expected-warning 
>>>>>>> {{expression does not compute the number of elements in this array; 
>>>>>>> element type is 'int [12]', not 'unsigned long long'}}
>>>>>>> +  int b2 = sizeof(arr4) / sizeof(int *);    // expected-warning 
>>>>>>> {{expression does not compute the number of elements in this array; 
>>>>>>> element type is 'int [12]', not 'int *'}}
>>>>>>> +  int b3 = sizeof(arr4) / sizeof(short *);  // expected-warning 
>>>>>>> {{expression does not compute the number of elements in this array; 
>>>>>>> element type is 'int [12]', not 'short *'}}
>>>>>>> +}
>>>>>>> 
>>>>>>> 
>>>>>>> _______________________________________________
>>>>>>> cfe-commits mailing list
>>>>>>> cfe-commits@lists.llvm.org
>>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to