Sema: Warn on sizeof on binary ops on decayed arrays.

The array will decay into a pointer, creating an unexpected result.
sizeof(array + int) is an easy to make typo for sizeof(array) + int.

This was motivated by a NetBSD security bug, used sizeof(key - r) instead of
sizeof(key) - r, reducing entropy in a random number generator.
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/subr_cprng.c.diff?r1=1.14&r2=1.15&only_with_tag=MAIN&f=h

Differential Revision: http://llvm-reviews.chandlerc.com/D571

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178371 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 0e1baebe9ad1e8069b54afcc1f4a9aa06b05d62c..3114a1fb221cf81d9c28d3a3de362410e93e61c7 100644 (file)
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -215,6 +215,7 @@ def : DiagGroup<"stack-protector">;
 def : DiagGroup<"switch-default">;
 def : DiagGroup<"synth">;
 def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">;
+def SizeofArrayDecay : DiagGroup<"sizeof-array-decay">;
 def SizeofPointerMemaccess : DiagGroup<"sizeof-pointer-memaccess">;
 def StaticInInline : DiagGroup<"static-in-inline">;
 def GNUStaticFloatInit : DiagGroup<"gnu-static-float-init">;
@@ -421,6 +422,7 @@ def Most : DiagGroup<"most", [
     ReturnType,
     SelfAssignment,
     SizeofArrayArgument,
+    SizeofArrayDecay,
     StringPlusInt,
     Trigraphs,
     Uninitialized,

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 2a71b17293b0d3f0aa21fb749028121468930af5..4bb4fba853714097153393a758987d4195d1f9d8 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4033,6 +4033,10 @@ def warn_sizeof_array_param : Warning<
   "sizeof on array function parameter will return size of %0 instead of %1">,
   InGroup<SizeofArrayArgument>;
 
+def warn_sizeof_array_decay : Warning<
+  "sizeof on pointer operation will return size of %0 instead of %1">,
+  InGroup<SizeofArrayDecay>;
+
 def err_sizeof_nonfragile_interface : Error<
   "application of '%select{alignof|sizeof}1' to interface %0 is "
   "not supported on this architecture and platform">;

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index c6882908c2400deb759a228df2af30e9d0e3a3e5..62bfa3c709d57f47af3b3b92cf5344439b466523 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3089,6 +3089,24 @@ static bool CheckObjCTraitOperandConstraints(Sema &S, QualType T,
   return false;
 }
 
+/// \brief Check whether E is a pointer from a decayed array type (the decayed
+/// pointer type is equal to T) and emit a warning if it is.
+static void warnOnSizeofOnArrayDecay(Sema &S, SourceLocation Loc, QualType T,
+                                     Expr *E) {
+  // Don't warn if the operation changed the type.
+  if (T != E->getType())
+    return;
+
+  // Now look for array decays.
+  ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E);
+  if (!ICE || ICE->getCastKind() != CK_ArrayToPointerDecay)
+    return;
+
+  S.Diag(Loc, diag::warn_sizeof_array_decay) << ICE->getSourceRange()
+                                             << ICE->getType()
+                                             << ICE->getSubExpr()->getType();
+}
+
 /// \brief Check the constrains on expression operands to unary type expression
 /// and type traits.
 ///
@@ -3142,6 +3160,16 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
         }
       }
     }
+
+    // Warn on "sizeof(array op x)" and "sizeof(x op array)", where the array
+    // decays into a pointer and returns an unintended result. This is most
+    // likely a typo for "sizeof(array) op x".
+    if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E->IgnoreParens())) {
+      warnOnSizeofOnArrayDecay(*this, BO->getOperatorLoc(), BO->getType(),
+                               BO->getLHS());
+      warnOnSizeofOnArrayDecay(*this, BO->getOperatorLoc(), BO->getType(),
+                               BO->getRHS());
+    }
   }
 
   return false;

diff --git a/test/Sema/expr-comma-c99.c b/test/Sema/expr-comma-c99.c
index 6e97a4fc49570caadb375b0d72adaacb742d4dce..02886bff053f1850deb3008dfd5223309e34f23f 100644 (file)
--- a/test/Sema/expr-comma-c99.c
+++ b/test/Sema/expr-comma-c99.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c99
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c99 -Wno-sizeof-array-decay
 // expected-no-diagnostics
 // rdar://6095180
 

diff --git a/test/Sema/expr-comma.c b/test/Sema/expr-comma.c
index 7902715915a2b5e65bf7ba94c025c1d5135b441a..e2beafe236c2a30e5138504ae76ae82cc316d7c7 100644 (file)
--- a/test/Sema/expr-comma.c
+++ b/test/Sema/expr-comma.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c89
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c89 -Wno-sizeof-array-decay
 // expected-no-diagnostics
 // rdar://6095180
 

diff --git a/test/Sema/warn-sizeof-array-decay.c b/test/Sema/warn-sizeof-array-decay.c
new file mode 100644 (file)
index 0000000..cc3ee1d
--- /dev/null
+++ b/test/Sema/warn-sizeof-array-decay.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f(int x) {
+  char foo[10];
+  int bar[20];
+  char qux[30];
+
+  (void)sizeof(bar + 10); // expected-warning{{sizeof on pointer operation will return size of 'int *' instead of 'int [20]'}}
+  (void)sizeof(foo - 20); // expected-warning{{sizeof on pointer operation will return size of 'char *' instead of 'char [10]'}}
+  (void)sizeof(bar - x); // expected-warning{{sizeof on pointer operation will return size of 'int *' instead of 'int [20]'}}
+  (void)sizeof(foo + x); // expected-warning{{sizeof on pointer operation will return size of 'char *' instead of 'char [10]'}}
+
+  // This is ptrdiff_t.
+  (void)sizeof(foo - qux); // no-warning
+
+  (void)sizeof(foo, x); // no-warning
+  (void)sizeof(x, foo); // expected-warning{{sizeof on pointer operation will return size of 'char *' instead of 'char [10]'}}
+}