<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto">D68526 should fix it. Take a look please.<br><br><div dir="ltr"><br><blockquote type="cite">Dňa 7. 10. 2019 o 17:09 užívateľ Nico Weber <thakis@chromium.org> napísal:<br><br></blockquote></div><blockquote type="cite"><div dir="ltr"><div dir="ltr">I gave this another try now that we have a compiler with rL372600. Another thing the warning currently warns on is code like this:<div><br></div><div>  char memory[kOpcodeMemory];<br>  OpcodeFactory opcode_maker(memory, sizeof(memory));<br>  size_t count = sizeof(memory) / sizeof(PolicyOpcode);<br></div><div><br></div><div>or</div><div><br></div><div>  int32_t fds[sizeof(buffer->data) / sizeof(int32_t)], i, count;<br>  size_t size;<br></div><div><br></div><div>(the latter from wayland).</div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 23, 2019 at 9:11 AM Dávid Bolvanský <<a href="mailto:david.bolvansky@gmail.com">david.bolvansky@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Yeah, this needs to be handled a bit differently (if we want so).</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">po 23. 9. 2019 o 15:07 Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> napísal(a):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">It still warns if the inner array is in a struct. That's probably ok though.<div><br></div><div>struct Point {<br>  int xy[2];<br>};<br><br>void f() {<br>  Point points[3];<br>  for (int i = 0; i < sizeof(points) / sizeof(int); ++i)<br>    (&points[0].xy[0])[i] = 0;<br>}<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 23, 2019 at 8:54 AM Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">That was fast. Thanks much! :)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 23, 2019 at 8:52 AM Dávid Bolvanský <<a href="mailto:david.bolvansky@gmail.com" target="_blank">david.bolvansky@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hello,<div><br></div><div>Thanks for the proposed idea, implemented in rL372600.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">po 23. 9. 2019 o 14:23 Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> napísal(a):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">We're looking at turning this one.<div><br></div><div>One thing that this warns about that's a false positive where we've seen it is this code for nested arrays:</div><div><br></div><div>  float m[4][4];<br>  for (int i = 0; i < sizeof(m) / sizeof(**m); ++i) (&**m)[i] = 0;<br></div><div><br></div><div>(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.)</div><div><br>Can we make the warning not fire when dividing the size of a nested array by the size of the deepest base type?</div><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 11, 2019 at 6:58 AM David Bolvansky via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: xbolva00<br>
Date: Wed Sep 11 03:59:47 2019<br>
New Revision: 371605<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=371605&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=371605&view=rev</a><br>
Log:<br>
[Diagnostics] Add -Wsizeof-array-div<br>
<br>
Summary: Clang version of <a href="https://www.viva64.com/en/examples/v706/" rel="noreferrer" target="_blank">https://www.viva64.com/en/examples/v706/</a><br>
<br>
Reviewers: rsmith<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D67287" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67287</a><br>
<br>
Added:<br>
    cfe/trunk/test/Sema/div-sizeof-array.cpp<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
    cfe/trunk/lib/Sema/SemaExpr.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11 03:59:47 2019<br>
@@ -3406,6 +3406,10 @@ def note_pointer_declared_here : Note<<br>
 def warn_division_sizeof_ptr : Warning<<br>
   "'%0' will return the size of the pointer, not the array itself">,<br>
   InGroup<DiagGroup<"sizeof-pointer-div">>;<br>
+def warn_division_sizeof_array : Warning<<br>
+  "expression does not compute the number of elements in this array; element "<br>
+  "type is %0, not %1">,<br>
+  InGroup<DiagGroup<"sizeof-array-div">>;<br>
<br>
 def note_function_warning_silence : Note<<br>
     "prefix with the address-of operator to silence this warning">;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019<br>
@@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe<br>
   else<br>
     RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();<br>
<br>
-  if (!LHSTy->isPointerType() || RHSTy->isPointerType())<br>
-    return;<br>
-  if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() !=<br>
-      RHSTy.getCanonicalType().getUnqualifiedType())<br>
-    return;<br>
+  if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {<br>
+    if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), RHSTy))<br>
+      return;<br>
<br>
-  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();<br>
-  if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {<br>
-    if (const ValueDecl *LHSArgDecl = DRE->getDecl())<br>
-      S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)<br>
-          << LHSArgDecl;<br>
+    S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();<br>
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {<br>
+      if (const ValueDecl *LHSArgDecl = DRE->getDecl())<br>
+        S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)<br>
+            << LHSArgDecl;<br>
+    }<br>
+  } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {<br>
+    QualType ArrayElemTy = ArrayTy->getElementType();<br>
+    if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||<br>
+        S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))<br>
+      return;<br>
+    S.Diag(Loc, diag::warn_division_sizeof_array)<br>
+        << LHSArg->getSourceRange() << ArrayElemTy << RHSTy;<br>
+    if (const auto *DRE = dyn_cast<DeclRefExpr>(LHSArg)) {<br>
+      if (const ValueDecl *LHSArgDecl = DRE->getDecl())<br>
+        S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here)<br>
+            << LHSArgDecl;<br>
+    }<br>
   }<br>
 }<br>
<br>
<br>
Added: cfe/trunk/test/Sema/div-sizeof-array.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)<br>
+++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019<br>
@@ -0,0 +1,28 @@<br>
+// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only<br>
+<br>
+template <typename Ty, int N><br>
+int f(Ty (&Array)[N]) {<br>
+  return sizeof(Array) / sizeof(Ty); // Should not warn<br>
+}<br>
+<br>
+typedef int int32;<br>
+<br>
+void test(void) {<br>
+  int arr[12];                // expected-note 2 {{array 'arr' declared here}}<br>
+  unsigned long long arr2[4];<br>
+  int *p = &arr[0];<br>
+  int a1 = sizeof(arr) / sizeof(*arr);<br>
+  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 *'}}<br>
+  int a4 = sizeof arr2 / sizeof p;<br>
+  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'}}<br>
+  int a6 = sizeof(arr) / sizeof(int32);<br>
+  int a7 = sizeof(arr) / sizeof(int);<br>
+  int a9 = sizeof(arr) / sizeof(unsigned int);<br>
+  const char arr3[2] = "A";<br>
+  int a10 = sizeof(arr3) / sizeof(char);<br>
+<br>
+  int arr4[10][12];                         // expected-note 3 {{array 'arr4' declared here}}<br>
+  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'}}<br>
+  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 *'}}<br>
+  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 *'}}<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>
</blockquote></div>
</div></blockquote></body></html>