Improved false positive rate for the idempotent operations checker and moved it into the default path-sensitive analysis options.
- Added checks for static local variables, self assigned parameters, and truncating/extending self assignments
- Removed command line option (now default with --analyze)
- Updated test cases to pass with idempotent operation warnings
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108550 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td
index 7076f30..4502401 100644
--- a/include/clang/Driver/CC1Options.td
+++ b/include/clang/Driver/CC1Options.td
@@ -81,8 +81,6 @@
HelpText<"Emit verbose output about the analyzer's progress">;
def analyzer_experimental_checks : Flag<"-analyzer-experimental-checks">,
HelpText<"Use experimental path-sensitive checks">;
-def analyzer_idempotent_operation : Flag<"-analyzer-idempotent-operation">,
- HelpText<"Use experimental path-sensitive idempotent operation checker">;
def analyzer_experimental_internal_checks :
Flag<"-analyzer-experimental-internal-checks">,
HelpText<"Use new default path-sensitive checks currently in testing">;
diff --git a/lib/Checker/AnalysisConsumer.cpp b/lib/Checker/AnalysisConsumer.cpp
index 524f37e..6c7a294 100644
--- a/lib/Checker/AnalysisConsumer.cpp
+++ b/lib/Checker/AnalysisConsumer.cpp
@@ -341,9 +341,6 @@
if (C.Opts.EnableExperimentalChecks)
RegisterExperimentalChecks(Eng);
- if (C.Opts.EnableIdempotentOperationChecker)
- RegisterIdempotentOperationChecker(Eng);
-
// Set the graph auditor.
llvm::OwningPtr<ExplodedNode::Auditor> Auditor;
if (mgr.shouldVisualizeUbigraph()) {
diff --git a/lib/Checker/GRExprEngine.cpp b/lib/Checker/GRExprEngine.cpp
index 07fee9d..1424820 100644
--- a/lib/Checker/GRExprEngine.cpp
+++ b/lib/Checker/GRExprEngine.cpp
@@ -361,6 +361,7 @@
RegisterDereferenceChecker(Eng);
RegisterVLASizeChecker(Eng);
RegisterDivZeroChecker(Eng);
+ RegisterIdempotentOperationChecker(Eng);
RegisterReturnUndefChecker(Eng);
RegisterUndefinedArraySubscriptChecker(Eng);
RegisterUndefinedAssignmentChecker(Eng);
diff --git a/lib/Checker/GRExprEngineExperimentalChecks.h b/lib/Checker/GRExprEngineExperimentalChecks.h
index 7d1eb77..3c1c95f 100644
--- a/lib/Checker/GRExprEngineExperimentalChecks.h
+++ b/lib/Checker/GRExprEngineExperimentalChecks.h
@@ -23,7 +23,6 @@
void RegisterPthreadLockChecker(GRExprEngine &Eng);
void RegisterMallocChecker(GRExprEngine &Eng);
void RegisterStreamChecker(GRExprEngine &Eng);
-void RegisterIdempotentOperationChecker(GRExprEngine &Eng);
} // end clang namespace
#endif
diff --git a/lib/Checker/GRExprEngineInternalChecks.h b/lib/Checker/GRExprEngineInternalChecks.h
index f91a759..9644eaf 100644
--- a/lib/Checker/GRExprEngineInternalChecks.h
+++ b/lib/Checker/GRExprEngineInternalChecks.h
@@ -30,6 +30,7 @@
void RegisterDereferenceChecker(GRExprEngine &Eng);
void RegisterDivZeroChecker(GRExprEngine &Eng);
void RegisterFixedAddressChecker(GRExprEngine &Eng);
+void RegisterIdempotentOperationChecker(GRExprEngine &Eng);
void RegisterNoReturnFunctionChecker(GRExprEngine &Eng);
void RegisterPointerArithChecker(GRExprEngine &Eng);
void RegisterPointerSubChecker(GRExprEngine &Eng);
diff --git a/lib/Checker/IdempotentOperationChecker.cpp b/lib/Checker/IdempotentOperationChecker.cpp
index 6ed1841..744fe2b 100644
--- a/lib/Checker/IdempotentOperationChecker.cpp
+++ b/lib/Checker/IdempotentOperationChecker.cpp
@@ -48,7 +48,7 @@
// - Handle mixed assumptions (which assumptions can belong together?)
// - Finer grained false positive control (levels)
-#include "GRExprEngineExperimentalChecks.h"
+#include "GRExprEngineInternalChecks.h"
#include "clang/Checker/BugReporter/BugType.h"
#include "clang/Checker/PathSensitive/CheckerVisitor.h"
#include "clang/Checker/PathSensitive/SVals.h"
@@ -78,6 +78,10 @@
/// element somewhere. Used in static analysis to reduce false positives.
static bool containsMacro(const Stmt *S);
static bool containsEnum(const Stmt *S);
+ static bool containsStaticLocal(const Stmt *S);
+ static bool isParameterSelfAssign(const Expr *LHS, const Expr *RHS);
+ static bool isTruncationExtensionAssignment(const Expr *LHS,
+ const Expr *RHS);
static bool containsBuiltinOffsetOf(const Stmt *S);
static bool containsZeroConstant(const Stmt *S);
static bool containsOneConstant(const Stmt *S);
@@ -128,7 +132,7 @@
// Skip binary operators containing common false positives
if (containsMacro(B) || containsEnum(B) || containsStmt<SizeOfAlignOfExpr>(B)
|| containsZeroConstant(B) || containsOneConstant(B)
- || containsBuiltinOffsetOf(B)) {
+ || containsBuiltinOffsetOf(B) || containsStaticLocal(B)) {
A = Impossible;
return;
}
@@ -181,12 +185,19 @@
break; // We don't care about any other operators.
// 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)) {
+ A = Impossible;
+ return;
+ }
+
case BinaryOperator::SubAssign:
case BinaryOperator::DivAssign:
case BinaryOperator::AndAssign:
case BinaryOperator::OrAssign:
case BinaryOperator::XorAssign:
- case BinaryOperator::Assign:
case BinaryOperator::Sub:
case BinaryOperator::Div:
case BinaryOperator::And:
@@ -399,6 +410,70 @@
return false;
}
+// Recursively find any substatements containing static vars
+bool IdempotentOperationChecker::containsStaticLocal(const Stmt *S) {
+ const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);
+
+ if (DR)
+ if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()))
+ if (VD->isStaticLocal())
+ return true;
+
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();
+ ++I)
+ if (const Stmt *child = *I)
+ if (containsStaticLocal(child))
+ return true;
+
+ return false;
+}
+
+// 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) {
+ LHS = LHS->IgnoreParenCasts();
+ RHS = RHS->IgnoreParenCasts();
+
+ const DeclRefExpr *LHS_DR = dyn_cast<DeclRefExpr>(LHS);
+ if (!LHS_DR)
+ return false;
+
+ const ParmVarDecl *PD = dyn_cast<ParmVarDecl>(LHS_DR->getDecl());
+ if (!PD)
+ return false;
+
+ const DeclRefExpr *RHS_DR = dyn_cast<DeclRefExpr>(RHS);
+ if (!RHS_DR)
+ return false;
+
+ return PD == RHS_DR->getDecl();
+}
+
+// Check for self casts truncating/extending a variable
+bool IdempotentOperationChecker::isTruncationExtensionAssignment(
+ const Expr *LHS,
+ const Expr *RHS) {
+
+ const DeclRefExpr *LHS_DR = dyn_cast<DeclRefExpr>(LHS->IgnoreParenCasts());
+ if (!LHS_DR)
+ return false;
+
+ const VarDecl *VD = dyn_cast<VarDecl>(LHS_DR->getDecl());
+ if (!VD)
+ return false;
+
+ const DeclRefExpr *RHS_DR = dyn_cast<DeclRefExpr>(RHS->IgnoreParenCasts());
+ if (!RHS_DR)
+ return false;
+
+ if (VD != RHS_DR->getDecl())
+ return false;
+
+ return dyn_cast<DeclRefExpr>(RHS->IgnoreParens()) == NULL;
+}
+
+
// Recursively find any substatements containing __builtin_offset_of
bool IdempotentOperationChecker::containsBuiltinOffsetOf(const Stmt *S) {
const UnaryOperator *UO = dyn_cast<UnaryOperator>(S);
@@ -415,6 +490,7 @@
return false;
}
+// Check for a integer or float constant of 0
bool IdempotentOperationChecker::containsZeroConstant(const Stmt *S) {
const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);
if (IL && IL->getValue() == 0)
@@ -433,6 +509,7 @@
return false;
}
+// Check for an integer or float constant of 1
bool IdempotentOperationChecker::containsOneConstant(const Stmt *S) {
const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);
if (IL && IL->getValue() == 1)
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
index 418d25b..00363d9 100644
--- a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -112,8 +112,6 @@
Res.push_back("-analyzer-experimental-checks");
if (Opts.EnableExperimentalInternalChecks)
Res.push_back("-analyzer-experimental-internal-checks");
- if (Opts.EnableIdempotentOperationChecker)
- Res.push_back("-analyzer-idempotent-operation");
}
static void CodeGenOptsToArgs(const CodeGenOptions &Opts,
@@ -792,8 +790,6 @@
Opts.EnableExperimentalChecks = Args.hasArg(OPT_analyzer_experimental_checks);
Opts.EnableExperimentalInternalChecks =
Args.hasArg(OPT_analyzer_experimental_internal_checks);
- Opts.EnableIdempotentOperationChecker =
- Args.hasArg(OPT_analyzer_idempotent_operation);
Opts.TrimGraph = Args.hasArg(OPT_trim_egraph);
Opts.MaxNodes = Args.getLastArgIntValue(OPT_analyzer_max_nodes, 150000,Diags);
Opts.MaxLoop = Args.getLastArgIntValue(OPT_analyzer_max_loop, 3, Diags);
diff --git a/test/Analysis/constant-folding.c b/test/Analysis/constant-folding.c
index 6ed2b39..5a7e6be 100644
--- a/test/Analysis/constant-folding.c
+++ b/test/Analysis/constant-folding.c
@@ -18,10 +18,10 @@
}
void testSelfOperations (int a) {
- if ((a|a) != a) WARN;
- if ((a&a) != a) WARN;
- if ((a^a) != 0) WARN;
- if ((a-a) != 0) WARN;
+ if ((a|a) != a) WARN; // expected-warning{{idempotent operation}}
+ if ((a&a) != a) WARN; // expected-warning{{idempotent operation}}
+ if ((a^a) != 0) WARN; // expected-warning{{idempotent operation}}
+ if ((a-a) != 0) WARN; // expected-warning{{idempotent operation}}
}
void testIdempotent (int a) {
@@ -68,5 +68,5 @@
if (b!=a) WARN;
if (b>a) WARN;
if (b<a) WARN;
- if (b-a) WARN;
+ if (b-a) WARN; // expected-warning{{idempotent operation}}
}
diff --git a/test/Analysis/dead-stores.c b/test/Analysis/dead-stores.c
index defd7e0..a295d96 100644
--- a/test/Analysis/dead-stores.c
+++ b/test/Analysis/dead-stores.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-experimental-internal-checks -analyzer-check-dead-stores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
+// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-check-dead-stores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=basic -analyzer-check-dead-stores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-store=basic -analyzer-constraints=range -analyzer-check-dead-stores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-experimental-internal-checks -analyzer-check-objc-mem -analyzer-store=region -analyzer-constraints=basic -analyzer-check-dead-stores -fblocks -verify -Wno-unreachable-code -analyzer-opt-analyze-nested-blocks %s
@@ -158,7 +158,7 @@
// Self-assignments should not be flagged as dead stores.
void f17() {
int x = 1;
- x = x; // no-warning
+ x = x; // expected-warning{{idempotent operation}}
}
// <rdar://problem/6506065>
@@ -458,7 +458,7 @@
// Note that the next value stored to 'i' is never executed
// because the next statement to be executed is the 'break'
// in the increment code of the first loop.
- i = i * 3; // expected-warning{{Value stored to 'i' is never read}}
+ i = i * 3; // expected-warning{{Value stored to 'i' is never read}} expected-warning{{idempotent operation}}
}
}
diff --git a/test/Analysis/idempotent-operations.c b/test/Analysis/idempotent-operations.c
index 9cef08e..72adb8e 100644
--- a/test/Analysis/idempotent-operations.c
+++ b/test/Analysis/idempotent-operations.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-idempotent-operation -analyzer-store=region -analyzer-constraints=range -fblocks -verify -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-store=region -analyzer-constraints=range -fblocks -verify -analyzer-opt-analyze-nested-blocks -analyzer-check-objc-mem -verify %s
// Basic tests
diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m
index b1d47e2..7de1305 100644
--- a/test/Analysis/misc-ps.m
+++ b/test/Analysis/misc-ps.m
@@ -86,11 +86,11 @@
void r6268365() {
unsigned x = 0;
- x &= r6268365Aux();
+ x &= r6268365Aux(); // expected-warning{{idempotent operation}}
unsigned j = 0;
if (x == 0) ++j;
- if (x == 0) x = x / j; // no-warning
+ if (x == 0) x = x / j; // expected-warning{{idempotent operation}} expected-warning{{idempotent operation}}
}
void divzeroassume(unsigned x, unsigned j) {
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index 7ca22ad..0b4153c 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -280,7 +280,7 @@
// Test handling of translating between integer "pointers" and back.
void f13() {
int *x = 0;
- if (((((int) x) << 2) + 1) >> 1) *x = 1; // no-warning
+ if (((((int) x) << 2) + 1) >> 1) *x = 1; // expected-warning{{idempotent operation}}
}
// PR 4759 - Attribute non-null checking by the analyzer was not correctly
diff --git a/test/Analysis/rdar-6541136-region.c b/test/Analysis/rdar-6541136-region.c
index 82232c6..c1734ee 100644
--- a/test/Analysis/rdar-6541136-region.c
+++ b/test/Analysis/rdar-6541136-region.c
@@ -9,7 +9,7 @@
void test1( void ) {
kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd;
+ cmd = cmd; // expected-warning{{idempotent operation}}
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;
+ cmd = cmd; // expected-warning{{idempotent operation}}
char *p = (void*) &wonky[1];
*p = 1; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
}
diff --git a/test/Analysis/rdar-6541136.c b/test/Analysis/rdar-6541136.c
index 844a936..c729cb5 100644
--- a/test/Analysis/rdar-6541136.c
+++ b/test/Analysis/rdar-6541136.c
@@ -12,7 +12,7 @@
{
kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese;
struct load_wine *cmd = (void*) &wonky[1];
- cmd = cmd;
+ cmd = cmd; // expected-warning{{idempotent operation}}
char *p = (void*) &wonky[1];
*p = 1;
kernel_tea_cheese_t *q = &wonky[1];