Hi,
New one with comments handled.
________________________________________
Från: Jordan Rose [[email protected]]
Skickat: den 20 december 2013 19:15
Till: Anders Rönnholm
Cc: [email protected]; Daniel Marjamäki; Anna Zaks; David Blaikie;
Richard Smith; Matt Calabrese
Ämne: Re: [PATCH] [StaticAnalyzer] New checker Sizeof on expression
On Dec 10, 2013, at 4:38 , Anders Rönnholm
<[email protected]<mailto:[email protected]>> wrote:
Are you OK to commit this patch or do you see more issues?
I'm not sure if anyone else has ideological concerns. There's always a flag to
turn this off, I suppose.
+ if (S.isSFINAEContext())
+ return;
Code style: extra indent?
+ if(E->HasSideEffects(S.getASTContext()))
+ return;
sizeof doesn't evaluate its argument, so I'm not sure why you wouldn't want to
warn here.
+ const FunctionDecl *FD = S.getCurFunctionDecl();
+ if(FD && FD->isFunctionTemplateSpecialization())
+ return;
Code style: space after if. (Above too, actually.)
+ if (Binop->getLHS()->getType()->isArrayType() ||
+ Binop->getLHS()->getType()->isAnyPointerType() ||
+ Binop->getRHS()->getType()->isArrayType() ||
+ Binop->getRHS()->getType()->isAnyPointerType())
+ return;
I don't think this is correct...the user is only trying to get ptrdiff_t if
both the LHS and RHS are pointer-ish.
+def warn_sizeof_bin_op : Warning<
+ "using sizeof() on an expression with an operator may yield unexpected
results">,
+ InGroup<SizeofOnExpression>;
+
+def warn_sizeof_sizeof : Warning<
+ "using sizeof() on sizeof() may yield unexpected results.">,
+ InGroup<SizeofOnExpression>;
+
sizeof doesn't actually require parens, so we shouldn't put the parens in the
diagnostics.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 199892)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -4367,6 +4367,14 @@
"sizeof on pointer operation will return size of %0 instead of %1">,
InGroup<SizeofArrayDecay>;
+def warn_sizeof_bin_op : Warning<
+ "using sizeof on an expression with an operator may yield unexpected results">,
+ InGroup<SizeofOnExpression>;
+
+def warn_sizeof_sizeof : Warning<
+ "using sizeof on sizeof may yield unexpected results.">,
+ InGroup<SizeofOnExpression>;
+
def err_sizeof_nonfragile_interface : Error<
"application of '%select{alignof|sizeof}1' to interface %0 is "
"not supported on this architecture and platform">;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 199892)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -273,6 +273,8 @@
def : DiagGroup<"synth">;
def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">;
def SizeofArrayDecay : DiagGroup<"sizeof-array-decay">;
+def SizeofOnExpression : DiagGroup<"sizeof-on-expression">;
+
def SizeofPointerMemaccess : DiagGroup<"sizeof-pointer-memaccess">;
def StaticInInline : DiagGroup<"static-in-inline">;
def StaticLocalInInline : DiagGroup<"static-local-in-inline">;
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 199892)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -3287,6 +3287,47 @@
<< ICE->getSubExpr()->getType();
}
+/// \brief Warns if sizeof expression has a binary operation or contains a
+/// sizeof.
+static void warnOnSizeofOnExpression(Sema &S, Expr *E) {
+ if (S.isSFINAEContext())
+ return;
+
+ const FunctionDecl *FD = S.getCurFunctionDecl();
+ if (FD && FD->isFunctionTemplateSpecialization())
+ return;
+
+ if (const BinaryOperator *Binop =
+ dyn_cast<BinaryOperator>(E->IgnoreParens())) {
+ if (!Binop->isMultiplicativeOp() &&
+ !Binop->isAdditiveOp())
+ return;
+
+ if ((Binop->getLHS()->getType()->isArrayType() ||
+ Binop->getLHS()->getType()->isAnyPointerType()) &&
+ (Binop->getRHS()->getType()->isArrayType() ||
+ Binop->getRHS()->getType()->isAnyPointerType()))
+ return;
+
+ // Don't warn if expression is based on macro.
+ if ((Binop->getLHS()->getExprLoc().isMacroID()) ||
+ (Binop->getRHS()->getExprLoc().isMacroID()))
+ return;
+
+ SourceRange DiagRange(Binop->getLHS()->getLocStart(),
+ Binop->getRHS()->getLocEnd());
+ S.Diag(Binop->getOperatorLoc(), diag::warn_sizeof_bin_op) << DiagRange;
+ } else if (const UnaryExprOrTypeTraitExpr *UnaryExpr =
+ dyn_cast<UnaryExprOrTypeTraitExpr>(E->IgnoreParens())) {
+ if (UnaryExpr->getKind() != UETT_SizeOf)
+ return;
+
+ SourceRange DiagRange(UnaryExpr->getLocStart(),
+ UnaryExpr->getLocEnd());
+ S.Diag(UnaryExpr->getOperatorLoc(), diag::warn_sizeof_sizeof) << DiagRange;
+ }
+}
+
/// \brief Check the constraints on expression operands to unary type expression
/// and type traits.
///
@@ -3349,6 +3390,8 @@
warnOnSizeofOnArrayDecay(*this, BO->getOperatorLoc(), BO->getType(),
BO->getRHS());
}
+
+ warnOnSizeofOnExpression(*this, E);
}
return false;
Index: test/SemaCXX/decl-expr-ambiguity.cpp
===================================================================
--- test/SemaCXX/decl-expr-ambiguity.cpp (revision 199892)
+++ test/SemaCXX/decl-expr-ambiguity.cpp (working copy)
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -Wno-int-to-pointer-cast -fsyntax-only -verify -pedantic-errors %s
-// RUN: %clang_cc1 -Wno-int-to-pointer-cast -fsyntax-only -verify -pedantic-errors -x objective-c++ %s
+// RUN: %clang_cc1 -Wno-sizeof-on-expression -Wno-int-to-pointer-cast -fsyntax-only -verify -pedantic-errors %s
+// RUN: %clang_cc1 -Wno-sizeof-on-expression -Wno-int-to-pointer-cast -fsyntax-only -verify -pedantic-errors -x objective-c++ %s
void f() {
int a;
Index: test/SemaCXX/sizeofonexpression.cpp
===================================================================
--- test/SemaCXX/sizeofonexpression.cpp (revision 0)
+++ test/SemaCXX/sizeofonexpression.cpp (working copy)
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+template<typename T, typename RESULT,
+ typename ARG1, typename ARG2, typename ARG3>
+class DoesDebug
+{
+ template <typename U, RESULT (U::*)(ARG1, ARG2, ARG3)> struct Check;
+ template <typename U> static char checkFn(Check<U, &U::Debug> *);
+ template <typename U> static int checkFn(...);
+public:
+ enum { Exist = sizeof(checkFn<T>(0)) == sizeof(char) }; // no-warning
+
+ int binOp() {
+ return sizeof(checkFn<T>(0)+1);
+ } // no-warning
+
+ int sizeofsizeof() {
+ return sizeof(sizeof(checkFn<T>(0)));
+ } // no-warning
+};
+class A {
+public:
+ A(): a(1) { }
+ A(int p) : a(sizeof(p + 1)) { } // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+ int a;
+};
+
+void f()
+{
+ int bb = 0;
+ int b = sizeof(bb + 1); // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
Index: test/Sema/exprs.c
===================================================================
--- test/Sema/exprs.c (revision 199892)
+++ test/Sema/exprs.c (working copy)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+// RUN: %clang_cc1 %s -Wno-sizeof-on-expression -verify -pedantic -fsyntax-only
// PR 8876 - don't warn about trivially unreachable null derefs. Note that
// we put this here because the reachability analysis only kicks in for
Index: test/Sema/sizeofonexpression.c
===================================================================
--- test/Sema/sizeofonexpression.c (revision 0)
+++ test/Sema/sizeofonexpression.c (working copy)
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define DEF (1+2)
+
+struct s {
+ int xx;
+ int yy;
+};
+
+int SizeofStructExpression(struct s p) {
+ return sizeof(sizeof(p) + 1); // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
+
+int SizeofStructMemberExpression(struct s p) {
+ return sizeof(p.xx * p.yy); // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
+
+int SizeofLiteralExpression() {
+ return sizeof(2 + 1); // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
+
+int SizeofRefExpression() {
+ int x = 1, y = 2;
+ return sizeof(x / y); // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
+
+int SizeofDefine() {
+ return sizeof(DEF);
+} // no-warning
+
+int SizeofDefineExpression() {
+ return sizeof(DEF + 2);
+} // no-warning
+
+int SizeofDefineExpression2() {
+ return sizeof(2 - DEF);
+} // no-warning
+
+int SizeofFunctionCallExpression() {
+ return sizeof(SizeofDefine() - 1);
+} // no-warning
+
+int SizeofSizeof() {
+ return sizeof(sizeof(1)); // expected-warning{{using sizeof() on sizeof() may yield unexpected results}}
+}
+
+int SizeofComparison() {
+ return sizeof(1+1) == 1; // expected-warning{{using sizeof() on an expression with an operator may yield unexpected results}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
