[cfe-commits] r111965 - in /cfe/trunk: include/clang/Analysis/Analyses/PseudoConstantAnalysis.h lib/Analysis/PseudoConstantAnalysis.cpp lib/Checker/IdempotentOperationChecker.cpp test/Analysis/dead-stores.c test/Analysis/idempotent-operations.c test/Analysis/idempotent-operations.cpp test/Analysis/rdar-6541136-region.c test/Analysis/rdar-6541136.c
Tom Care
tcare at apple.com
Tue Aug 24 14:09:07 PDT 2010
Author: tcare
Date: Tue Aug 24 16:09:07 2010
New Revision: 111965
URL: http://llvm.org/viewvc/llvm-project?rev=111965&view=rev
Log:
Improvements to IdempotentOperationChecker and its use of PseudoConstantAnalysis
- Added wasReferenced function to PseudoConstantAnalysis to determine if a variable was ever referenced in a function (outside of a self-assignment)
- BlockDeclRefExpr referenced variables are now explicitly added to the non-constant list
- Remove unnecessary ignore of implicit casts
- Generalized parameter self-assign detection to detect deliberate self-assigns of variables to avoid unused variable warnings
- Updated test cases with deliberate self-assignments
- Fixed bug with C++ references and pseudoconstants
- Added test case for C++ references and pseudoconstants
Added:
cfe/trunk/test/Analysis/idempotent-operations.cpp
Modified:
cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
cfe/trunk/test/Analysis/dead-stores.c
cfe/trunk/test/Analysis/idempotent-operations.c
cfe/trunk/test/Analysis/rdar-6541136-region.c
cfe/trunk/test/Analysis/rdar-6541136.c
Modified: cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/PseudoConstantAnalysis.h Tue Aug 24 16:09:07 2010
@@ -26,12 +26,14 @@
~PseudoConstantAnalysis();
bool isPseudoConstant(const VarDecl *VD);
+ bool wasReferenced(const VarDecl *VD);
private:
void RunAnalysis();
// for storing the result of analyzed ValueDecls
void *NonConstantsImpl;
+ void *UsedVarsImpl;
const Stmt *DeclBody;
bool Analyzed;
Modified: cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp (original)
+++ cfe/trunk/lib/Analysis/PseudoConstantAnalysis.cpp Tue Aug 24 16:09:07 2010
@@ -28,10 +28,12 @@
PseudoConstantAnalysis::PseudoConstantAnalysis(const Stmt *DeclBody) :
DeclBody(DeclBody), Analyzed(false) {
NonConstantsImpl = new VarDeclSet;
+ UsedVarsImpl = new VarDeclSet;
}
PseudoConstantAnalysis::~PseudoConstantAnalysis() {
delete (VarDeclSet*)NonConstantsImpl;
+ delete (VarDeclSet*)UsedVarsImpl;
}
// Returns true if the given ValueDecl is never written to in the given DeclBody
@@ -50,9 +52,22 @@
return !NonConstants->count(VD);
}
+// Returns true if the variable was used (self assignments don't count)
+bool PseudoConstantAnalysis::wasReferenced(const VarDecl *VD) {
+ if (!Analyzed) {
+ RunAnalysis();
+ Analyzed = true;
+ }
+
+ VarDeclSet *UsedVars = (VarDeclSet*)UsedVarsImpl;
+
+ return UsedVars->count(VD);
+}
+
void PseudoConstantAnalysis::RunAnalysis() {
std::deque<const Stmt *> WorkList;
VarDeclSet *NonConstants = (VarDeclSet*)NonConstantsImpl;
+ VarDeclSet *UsedVars = (VarDeclSet*)UsedVarsImpl;
// Start with the top level statement of the function
WorkList.push_back(DeclBody);
@@ -65,7 +80,7 @@
// Case 1: Assignment operators modifying ValueDecl
case Stmt::BinaryOperatorClass: {
const BinaryOperator *BO = cast<BinaryOperator>(Head);
- const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts();
+ const Expr *LHS = BO->getLHS()->IgnoreParenCasts();
const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS);
// We only care about DeclRefExprs on the LHS
@@ -76,7 +91,16 @@
// for any of the assignment operators, implying that this Decl is being
// written to.
switch (BO->getOpcode()) {
- case BinaryOperator::Assign:
+ case BinaryOperator::Assign: {
+ const Expr *RHS = BO->getRHS()->IgnoreParenCasts();
+ if (const DeclRefExpr *RHSDecl = dyn_cast<DeclRefExpr>(RHS)) {
+ // Self-assignments don't count as use of a variable
+ if (DR->getDecl() == RHSDecl->getDecl())
+ // Do not visit the children
+ continue;
+ }
+
+ }
case BinaryOperator::AddAssign:
case BinaryOperator::SubAssign:
case BinaryOperator::MulAssign:
@@ -148,16 +172,37 @@
if (!VD->getType().getTypePtr()->isReferenceType())
continue;
- // Ignore VarDecls without a body
- if (!VD->getBody())
- continue;
-
// If the reference is to another var, add the var to the non-constant
// list
- if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(VD->getBody()))
- if (const VarDecl *RefVD = dyn_cast<VarDecl>(DR->getDecl()))
+ if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(VD->getInit()))
+ if (const VarDecl *RefVD = dyn_cast<VarDecl>(DR->getDecl())) {
NonConstants->insert(RefVD);
+ continue;
+ }
+ }
+ break;
+ }
+
+ // Case 4: Block variable references
+ case Stmt::BlockDeclRefExprClass: {
+ // Any block variables are assumed to be non-constant
+ const BlockDeclRefExpr *BDR = cast<BlockDeclRefExpr>(Head);
+ if (const VarDecl *VD = dyn_cast<VarDecl>(BDR->getDecl())) {
+ NonConstants->insert(VD);
+ UsedVars->insert(VD);
+ continue;
}
+ break;
+ }
+
+ // Case 5: Variable references
+ case Stmt::DeclRefExprClass: {
+ const DeclRefExpr *DR = cast<DeclRefExpr>(Head);
+ if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+ UsedVars->insert(VD);
+ continue;
+ }
+ break;
}
default:
Modified: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Tue Aug 24 16:09:07 2010
@@ -74,7 +74,9 @@
void UpdateAssumption(Assumption &A, const Assumption &New);
// False positive reduction methods
- static bool isParameterSelfAssign(const Expr *LHS, const Expr *RHS);
+ static bool isUnusedSelfAssign(const Expr *LHS,
+ const Expr *RHS,
+ AnalysisContext *AC);
static bool isTruncationExtensionAssignment(const Expr *LHS,
const Expr *RHS);
static bool PathWasCompletelyAnalyzed(const CFG *C,
@@ -182,12 +184,19 @@
// Fall through intentional
case BinaryOperator::Assign:
- // x Assign x has a few more false positives we can check for
- if (isParameterSelfAssign(RHS, LHS)
- || isTruncationExtensionAssignment(RHS, LHS)) {
+ // 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;
}
+ if (LHSVal != RHSVal)
+ break;
+ UpdateAssumption(A, Equal);
+ return;
case BinaryOperator::SubAssign:
case BinaryOperator::DivAssign:
@@ -397,10 +406,12 @@
}
}
-// Check for a statement were a parameter is self assigned (to avoid an unused
-// variable warning)
-bool IdempotentOperationChecker::isParameterSelfAssign(const Expr *LHS,
- const Expr *RHS) {
+// 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) {
LHS = LHS->IgnoreParenCasts();
RHS = RHS->IgnoreParenCasts();
@@ -408,15 +419,22 @@
if (!LHS_DR)
return false;
- const ParmVarDecl *PD = dyn_cast<ParmVarDecl>(LHS_DR->getDecl());
- if (!PD)
+ const VarDecl *VD = dyn_cast<VarDecl>(LHS_DR->getDecl());
+ if (!VD)
return false;
const DeclRefExpr *RHS_DR = dyn_cast<DeclRefExpr>(RHS);
if (!RHS_DR)
return false;
- return PD == RHS_DR->getDecl();
+ if (VD != RHS_DR->getDecl())
+ return false;
+
+ // If the var was used outside of a selfassign, then we should still report
+ if (AC->getPseudoConstantAnalysis()->wasReferenced(VD))
+ return false;
+
+ return true;
}
// Check for self casts truncating/extending a variable
Modified: cfe/trunk/test/Analysis/dead-stores.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.c?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.c (original)
+++ cfe/trunk/test/Analysis/dead-stores.c Tue Aug 24 16:09:07 2010
@@ -158,7 +158,7 @@
// Self-assignments should not be flagged as dead stores.
void f17() {
int x = 1;
- x = x; // expected-warning{{Assigned value is always the same as the existing value}}
+ x = x;
}
// <rdar://problem/6506065>
Modified: cfe/trunk/test/Analysis/idempotent-operations.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.c (original)
+++ cfe/trunk/test/Analysis/idempotent-operations.c Tue Aug 24 16:09:07 2010
@@ -91,15 +91,18 @@
return enum1 + a; // no-warning
}
-// Self assignments of parameters are common false positives
-unsigned false3(int param) {
+// Self assignments of unused variables are common false positives
+unsigned false3(int param, int param2) {
param = param; // no-warning
+ // if a self assigned variable is used later, then it should be reported still
+ param2 = param2; // expected-warning{{Assigned value is always the same as the existing value}}
+
unsigned nonparam = 5;
nonparam = nonparam; // expected-warning{{Assigned value is always the same as the existing value}}
- return nonparam;
+ return param2 + nonparam;
}
// Pseudo-constants (vars only read) and constants should not be reported
Added: cfe/trunk/test/Analysis/idempotent-operations.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.cpp?rev=111965&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/idempotent-operations.cpp (added)
+++ cfe/trunk/test/Analysis/idempotent-operations.cpp Tue Aug 24 16:09:07 2010
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -analyzer-check-idempotent-operations -verify %s
+
+// C++ specific false positives
+
+extern void test(int i);
+extern void test_ref(int &i);
+
+// Test references affecting pseudoconstants
+void false1() {
+ int a = 0;
+ int five = 5;
+ int &b = a;
+ test(five * a); // expected-warning {{The right operand to '*' is always 0}}
+ b = 4;
+}
Modified: cfe/trunk/test/Analysis/rdar-6541136-region.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/rdar-6541136-region.c?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/rdar-6541136-region.c (original)
+++ cfe/trunk/test/Analysis/rdar-6541136-region.c Tue Aug 24 16:09:07 2010
@@ -9,7 +9,7 @@
void test1( void ) {
kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
+ cmd = cmd;
char *p = (void*) &wonky[1];
kernel_tea_cheese_t *q = &wonky[1];
// This test case tests both the RegionStore logic (doesn't crash) and
@@ -21,7 +21,7 @@
void test1_b( void ) {
kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
+ cmd = cmd;
char *p = (void*) &wonky[1];
*p = 1; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
}
Modified: cfe/trunk/test/Analysis/rdar-6541136.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/rdar-6541136.c?rev=111965&r1=111964&r2=111965&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/rdar-6541136.c (original)
+++ cfe/trunk/test/Analysis/rdar-6541136.c Tue Aug 24 16:09:07 2010
@@ -12,7 +12,7 @@
{
kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd; // expected-warning{{Assigned value is always the same as the existing value}}
+ cmd = cmd;
char *p = (void*) &wonky[1];
*p = 1;
kernel_tea_cheese_t *q = &wonky[1];
More information about the cfe-commits
mailing list