[cfe-commits] r112501 - in /cfe/trunk: lib/Checker/IdempotentOperationChecker.cpp test/Analysis/func.c test/Analysis/idempotent-operations.c test/Analysis/misc-ps-region-store.m test/Analysis/misc-ps.m
Tom Care
tcare at apple.com
Mon Aug 30 12:25:43 PDT 2010
Author: tcare
Date: Mon Aug 30 14:25:43 2010
New Revision: 112501
URL: http://llvm.org/viewvc/llvm-project?rev=112501&view=rev
Log:
Adjusted the semantics of assign checking in IdempotentOperationChecker
- Fixed a regression where assigning '0' would be reported
- Changed the way self assignments are filtered to allow constant testing
- Added a test case for assign ops
- Fixed one test case where a function pointer was not considered constant
- Fixed test cases relating to 0 assignment
Modified:
cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
cfe/trunk/test/Analysis/func.c
cfe/trunk/test/Analysis/idempotent-operations.c
cfe/trunk/test/Analysis/misc-ps-region-store.m
cfe/trunk/test/Analysis/misc-ps.m
Modified: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=112501&r1=112500&r2=112501&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Mon Aug 30 14:25:43 2010
@@ -74,15 +74,15 @@
void UpdateAssumption(Assumption &A, const Assumption &New);
// False positive reduction methods
- static bool isUnusedSelfAssign(const Expr *LHS,
- const Expr *RHS,
- AnalysisContext *AC);
+ static bool isSelfAssign(const Expr *LHS, const Expr *RHS);
+ static bool isUnused(const Expr *E, AnalysisContext *AC);
static bool isTruncationExtensionAssignment(const Expr *LHS,
const Expr *RHS);
static bool PathWasCompletelyAnalyzed(const CFG *C,
const CFGBlock *CB,
const GRCoreEngine &CE);
- static bool CanVary(const Expr *Ex, AnalysisContext *AC);
+ static bool CanVary(const Expr *Ex,
+ AnalysisContext *AC);
static bool isConstantOrPseudoConstant(const DeclRefExpr *DR,
AnalysisContext *AC);
static bool containsNonLocalVarDecl(const Stmt *S);
@@ -184,19 +184,19 @@
// Fall through intentional
case BO_Assign:
- // x Assign x can be used to silence unused variable warnings intentionally,
- // and has a slightly different definition for false positives.
- if (isUnusedSelfAssign(RHS, LHS, AC)
- || isTruncationExtensionAssignment(RHS, LHS)
- || containsNonLocalVarDecl(RHS)
- || containsNonLocalVarDecl(LHS)) {
- A = Impossible;
- return;
+ // x Assign x can be used to silence unused variable warnings intentionally.
+ // If this is a self assignment and the variable is referenced elsewhere,
+ // then it is a false positive.
+ if (isSelfAssign(LHS, RHS)) {
+ if (!isUnused(LHS, AC)) {
+ UpdateAssumption(A, Equal);
+ return;
+ }
+ else {
+ A = Impossible;
+ return;
+ }
}
- if (LHSVal != RHSVal)
- break;
- UpdateAssumption(A, Equal);
- return;
case BO_SubAssign:
case BO_DivAssign:
@@ -412,12 +412,9 @@
}
}
-// Check for a statement where a variable is self assigned to avoid an unused
-// variable warning. We still report if the variable is used after the self
-// assignment.
-bool IdempotentOperationChecker::isUnusedSelfAssign(const Expr *LHS,
- const Expr *RHS,
- AnalysisContext *AC) {
+// Check for a statement where a variable is self assigned to possibly avoid an
+// unused variable warning.
+bool IdempotentOperationChecker::isSelfAssign(const Expr *LHS, const Expr *RHS) {
LHS = LHS->IgnoreParenCasts();
RHS = RHS->IgnoreParenCasts();
@@ -436,7 +433,24 @@
if (VD != RHS_DR->getDecl())
return false;
- // If the var was used outside of a selfassign, then we should still report
+ return true;
+}
+
+// Returns true if the Expr points to a VarDecl that is not read anywhere
+// outside of self-assignments.
+bool IdempotentOperationChecker::isUnused(const Expr *E,
+ AnalysisContext *AC) {
+ if (!E)
+ return false;
+
+ const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts());
+ if (!DR)
+ return false;
+
+ const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+ if (!VD)
+ return false;
+
if (AC->getPseudoConstantAnalysis()->wasReferenced(VD))
return false;
@@ -571,6 +585,7 @@
return SE->getTypeOfArgument()->isVariableArrayType();
}
case Stmt::DeclRefExprClass:
+ // Check for constants/pseudoconstants
return !isConstantOrPseudoConstant(cast<DeclRefExpr>(Ex), AC);
// The next cases require recursion for subexpressions
@@ -593,14 +608,14 @@
return CanVary(cast<const ChooseExpr>(Ex)->getChosenSubExpr(
AC->getASTContext()), AC);
case Stmt::ConditionalOperatorClass:
- return CanVary(cast<const ConditionalOperator>(Ex)->getCond(), AC);
+ return CanVary(cast<const ConditionalOperator>(Ex)->getCond(), AC);
}
}
// Returns true if a DeclRefExpr is or behaves like a constant.
bool IdempotentOperationChecker::isConstantOrPseudoConstant(
- const DeclRefExpr *DR,
- AnalysisContext *AC) {
+ const DeclRefExpr *DR,
+ AnalysisContext *AC) {
// Check if the type of the Decl is const-qualified
if (DR->getType().isConstQualified())
return true;
Modified: cfe/trunk/test/Analysis/func.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/func.c?rev=112501&r1=112500&r2=112501&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/func.c (original)
+++ cfe/trunk/test/Analysis/func.c Mon Aug 30 14:25:43 2010
@@ -4,7 +4,7 @@
void f(void) {
void (*p)(void);
p = f;
- p = &f; // expected-warning{{Assigned value is always the same as the existing value}}
+ p = &f;
p();
(*p)();
}
Modified: cfe/trunk/test/Analysis/idempotent-operations.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=112501&r1=112500&r2=112501&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.c (original)
+++ cfe/trunk/test/Analysis/idempotent-operations.c Mon Aug 30 14:25:43 2010
@@ -172,3 +172,18 @@
return localInt;
}
+
+// Check that assignments filter out false positives correctly
+int false7() {
+ int zero = 0; // psuedo-constant
+ int one = 1;
+
+ int a = 55;
+ a = a; // expected-warning{{Assigned value is always the same as the existing value}}
+ a = enum1 * a; // no-warning
+
+ int b = 123;
+ b = b; // no-warning
+
+ return a;
+}
Modified: cfe/trunk/test/Analysis/misc-ps-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.m?rev=112501&r1=112500&r2=112501&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Mon Aug 30 14:25:43 2010
@@ -708,7 +708,7 @@
long long *z = (long long *) (intptr_t) src;
long long w = 0;
int n = 0;
- for (n = 0; n < y; ++n) { // expected-warning{{Assigned value is always the same as the existing value}}
+ for (n = 0; n < y; ++n) {
// Previously we crashed analyzing this statement.
w = *z++;
}
Modified: cfe/trunk/test/Analysis/misc-ps.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=112501&r1=112500&r2=112501&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.m (original)
+++ cfe/trunk/test/Analysis/misc-ps.m Mon Aug 30 14:25:43 2010
@@ -458,7 +458,7 @@
// ElementRegion is created.
unsigned char test_array_index_bitwidth(const unsigned char *p) {
unsigned short i = 0;
- for (i = 0; i < 2; i++) p = &p[i]; // expected-warning{{Assigned value is always the same as the existing value}}
+ for (i = 0; i < 2; i++) p = &p[i];
return p[i+1];
}
More information about the cfe-commits
mailing list