<div class="gmail_quote">On Tue, Jul 6, 2010 at 2:43 PM, Tom Care <span dir="ltr"><<a href="mailto:tcare@apple.com">tcare@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Author: tcare<br>
Date: Tue Jul 6 16:43:29 2010<br>
New Revision: 107706<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=107706&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=107706&view=rev</a><br>
Log:<br>
Added a path-sensitive idempotent operation checker (-analyzer-idempotent-operation). Finds idempotent and/or tautological operations in a path sensitive context, flagging operations that have no effect or a predictable effect.<br>
<br>
Example:<br>
{<br>
int a = 1;<br>
int b = 5;<br>
int c = b / a; // a is 1 on all paths<br>
}<br>
<br>
- New IdempotentOperationChecker class<br>
- Moved recursive Stmt functions in r107675 to IdempotentOperationChecker<br>
- Minor refactoring of SVal to allow checking for any integer<br>
- Added command line option for check<br>
- Added basic test cases<br>
<br>
Added:<br>
cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp<br>
cfe/trunk/test/Analysis/idempotent-operations.c<br>
Modified:<br>
cfe/trunk/include/clang/AST/Stmt.h<br>
cfe/trunk/include/clang/Checker/PathSensitive/SVals.h<br>
cfe/trunk/include/clang/Driver/CC1Options.td<br>
cfe/trunk/include/clang/Frontend/AnalyzerOptions.h<br>
cfe/trunk/lib/AST/Stmt.cpp<br>
cfe/trunk/lib/Checker/AnalysisConsumer.cpp<br>
cfe/trunk/lib/Checker/CMakeLists.txt<br>
cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h<br>
cfe/trunk/lib/Checker/SVals.cpp<br>
cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Stmt.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Stmt.h (original)<br>
+++ cfe/trunk/include/clang/AST/Stmt.h Tue Jul 6 16:43:29 2010<br>
@@ -253,25 +253,6 @@<br>
/// within CFGs.<br>
bool hasImplicitControlFlow() const;<br>
<br>
- /// contains* - Useful recursive methods to see if a statement contains an<br>
- /// element somewhere. Used in static analysis to reduce false positives.<br>
- static bool containsMacro(const Stmt *S);<br>
- static bool containsEnum(const Stmt *S);<br>
- static bool containsZeroConstant(const Stmt *S);<br>
- static bool containsOneConstant(const Stmt *S);<br>
- template <class T> static bool containsStmt(const Stmt *S) {<br>
- if (isa<T>(S))<br>
- return true;<br>
-<br>
- for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end(); ++I)<br>
- if (const Stmt *child = *I)<br>
- if (containsStmt<T>(child))<br>
- return true;<br>
-<br>
- return false;<br>
- }<br>
-<br>
-<br>
/// Child Iterators: All subclasses must implement child_begin and child_end<br>
/// to permit easy iteration over the substatements/subexpessions of an<br>
/// AST node. This permits easy iteration over all nodes in the AST.<br>
<br>
Modified: cfe/trunk/include/clang/Checker/PathSensitive/SVals.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/PathSensitive/SVals.h?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/PathSensitive/SVals.h?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Checker/PathSensitive/SVals.h (original)<br>
+++ cfe/trunk/include/clang/Checker/PathSensitive/SVals.h Tue Jul 6 16:43:29 2010<br>
@@ -98,6 +98,8 @@<br>
<br>
bool isConstant() const;<br>
<br>
+ bool isConstant(int I) const;<br>
+<br>
bool isZeroConstant() const;<br>
<br>
/// hasConjuredSymbol - If this SVal wraps a conjured symbol, return true;<br>
<br>
Modified: cfe/trunk/include/clang/Driver/CC1Options.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)<br>
+++ cfe/trunk/include/clang/Driver/CC1Options.td Tue Jul 6 16:43:29 2010<br>
@@ -81,6 +81,8 @@<br>
HelpText<"Emit verbose output about the analyzer's progress">;<br>
def analyzer_experimental_checks : Flag<"-analyzer-experimental-checks">,<br>
HelpText<"Use experimental path-sensitive checks">;<br>
+def analyzer_idempotent_operation : Flag<"-analyzer-idempotent-operation">,<br>
+ HelpText<"Use experimental path-sensitive idempotent operation checker">;<br>
def analyzer_experimental_internal_checks :<br>
Flag<"-analyzer-experimental-internal-checks">,<br>
HelpText<"Use new default path-sensitive checks currently in testing">;<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/AnalyzerOptions.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/AnalyzerOptions.h?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/AnalyzerOptions.h?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/AnalyzerOptions.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/AnalyzerOptions.h Tue Jul 6 16:43:29 2010<br>
@@ -72,6 +72,7 @@<br>
unsigned VisualizeEGUbi : 1;<br>
unsigned EnableExperimentalChecks : 1;<br>
unsigned EnableExperimentalInternalChecks : 1;<br>
+ unsigned EnableIdempotentOperationChecker : 1;<br>
unsigned InlineCall : 1;<br>
<br>
public:<br>
<br>
Modified: cfe/trunk/lib/AST/Stmt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/Stmt.cpp (original)<br>
+++ cfe/trunk/lib/AST/Stmt.cpp Tue Jul 6 16:43:29 2010<br>
@@ -127,72 +127,6 @@<br>
}<br>
}<br>
<br>
-// Recursively find any substatements containing macros<br>
-bool Stmt::containsMacro(const Stmt *S) {<br>
- if (S->getLocStart().isMacroID())<br>
- return true;<br>
-<br>
- if (S->getLocEnd().isMacroID())<br>
- return true;<br>
-<br>
- for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end(); ++I)<br>
- if (const Stmt *child = *I)<br>
- if (containsMacro(child))<br>
- return true;<br>
-<br>
- return false;<br>
-}<br>
-<br>
-// Recursively find any substatements containing enum constants<br>
-bool Stmt::containsEnum(const Stmt *S) {<br>
- const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);<br>
-<br>
- if (DR && isa<EnumConstantDecl>(DR->getDecl()))<br>
- return true;<br>
-<br>
- for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end(); ++I)<br>
- if (const Stmt *child = *I)<br>
- if (containsEnum(child))<br>
- return true;<br>
-<br>
- return false;<br>
-}<br>
-<br>
-bool Stmt::containsZeroConstant(const Stmt *S) {<br>
- const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);<br>
- if (IL && IL->getValue() == 0)<br>
- return true;<br>
-<br>
- const FloatingLiteral *FL = dyn_cast<FloatingLiteral>(S);<br>
- if (FL && FL->getValue().isZero())<br>
- return true;<br>
-<br>
- for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end(); ++I)<br>
- if (const Stmt *child = *I)<br>
- if (containsZeroConstant(child))<br>
- return true;<br>
-<br>
- return false;<br>
-}<br>
-<br>
-bool Stmt::containsOneConstant(const Stmt *S) {<br>
- const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);<br>
- if (IL && IL->getValue() == 1)<br>
- return true;<br>
-<br>
- const FloatingLiteral *FL = dyn_cast<FloatingLiteral>(S);<br>
- const llvm::APFloat one(1.0);<br>
- if (FL && FL->getValue().compare(one) == llvm::APFloat::cmpEqual)<br>
- return true;<br>
-<br>
- for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end(); ++I)<br>
- if (const Stmt *child = *I)<br>
- if (containsOneConstant(child))<br>
- return true;<br>
-<br>
- return false;<br>
-}<br>
-<br>
Expr *AsmStmt::getOutputExpr(unsigned i) {<br>
return cast<Expr>(Exprs[i]);<br>
}<br>
<br>
Modified: cfe/trunk/lib/Checker/AnalysisConsumer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/AnalysisConsumer.cpp?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/AnalysisConsumer.cpp?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Checker/AnalysisConsumer.cpp (original)<br>
+++ cfe/trunk/lib/Checker/AnalysisConsumer.cpp Tue Jul 6 16:43:29 2010<br>
@@ -28,6 +28,7 @@<br>
#include "clang/Checker/PathSensitive/GRExprEngine.h"<br>
#include "clang/Checker/PathSensitive/GRTransferFuncs.h"<br>
#include "clang/Checker/PathDiagnosticClients.h"<br>
+#include "GRExprEngineExperimentalChecks.h"<br>
#include "clang/Basic/FileManager.h"<br>
#include "clang/Basic/SourceManager.h"<br>
#include "clang/Frontend/AnalyzerOptions.h"<br>
@@ -340,6 +341,9 @@<br>
if (C.Opts.EnableExperimentalChecks)<br>
RegisterExperimentalChecks(Eng);<br>
<br>
+ if (C.Opts.EnableIdempotentOperationChecker)<br>
+ RegisterIdempotentOperationChecker(Eng);<br>
+<br>
// Set the graph auditor.<br>
llvm::OwningPtr<ExplodedNode::Auditor> Auditor;<br>
if (mgr.shouldVisualizeUbigraph()) {<br>
<br>
Modified: cfe/trunk/lib/Checker/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/CMakeLists.txt?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/CMakeLists.txt?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Checker/CMakeLists.txt (original)<br>
+++ cfe/trunk/lib/Checker/CMakeLists.txt Tue Jul 6 16:43:29 2010<br>
@@ -39,6 +39,7 @@<br>
GRExprEngineExperimentalChecks.cpp<br>
GRState.cpp<br>
HTMLDiagnostics.cpp<br>
+ IdempotentOperationChecker.cpp<br>
LLVMConventionsChecker.cpp<br>
MacOSXAPIChecker.cpp<br>
MallocChecker.cpp<br>
<br>
Modified: cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h (original)<br>
+++ cfe/trunk/lib/Checker/GRExprEngineExperimentalChecks.h Tue Jul 6 16:43:29 2010<br>
@@ -22,6 +22,7 @@<br>
void RegisterPthreadLockChecker(GRExprEngine &Eng);<br>
void RegisterMallocChecker(GRExprEngine &Eng);<br>
void RegisterStreamChecker(GRExprEngine &Eng);<br>
+void RegisterIdempotentOperationChecker(GRExprEngine &Eng);<br>
<br>
} // end clang namespace<br>
#endif<br>
<br>
Added: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=107706&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=107706&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (added)<br>
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Tue Jul 6 16:43:29 2010<br>
@@ -0,0 +1,453 @@<br>
+//==- IdempotentOperationChecker.cpp - Idempotent Operations ----*- C++ -*-==//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// This file defines a set of path-sensitive checks for idempotent and/or<br>
+// tautological operations. Each potential operation is checked along all paths<br>
+// to see if every path results in a pointless operation.<br>
+// +-------------------------------------------+<br>
+// |Table of idempotent/tautological operations|<br>
+// +-------------------------------------------+<br>
+//+--------------------------------------------------------------------------+<br>
+//|Operator | x op x | x op 1 | 1 op x | x op 0 | 0 op x | x op ~0 | ~0 op x |<br>
+//+--------------------------------------------------------------------------+<br>
+// +, += | | | | x | x | |<br>
+// -, -= | | | | x | -x | |<br>
+// *, *= | | x | x | 0 | 0 | |<br>
+// /, /= | 1 | x | | N/A | 0 | |<br>
+// &, &= | x | | | 0 | 0 | x | x<br>
+// |, |= | x | | | x | x | ~0 | ~0<br>
+// ^, ^= | 0 | | | x | x | |<br>
+// <<, <<= | | | | x | 0 | |<br>
+// >>, >>= | | | | x | 0 | |<br>
+// || | 1 | 1 | 1 | x | x | 1 | 1<br>
+// && | 1 | x | x | 0 | 0 | x | x<br>
+// = | x | | | | | |<br>
+// == | 1 | | | | | |<br>
+// >= | 1 | | | | | |<br>
+// <= | 1 | | | | | |<br>
+// > | 0 | | | | | |<br>
+// < | 0 | | | | | |<br>
+// != | 0 | | | | | |<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// Ways to reduce false positives (that need to be implemented):<br>
+// - Don't flag downsizing casts<br>
+// - Improved handling of static/global variables<br>
+// - Per-block marking of incomplete analysis<br>
+// - Handling ~0 values<br>
+// - False positives involving silencing unused variable warnings<br>
+//<br>
+// Other things TODO:<br>
+// - Improved error messages<br>
+// - Handle mixed assumptions (which assumptions can belong together?)<br>
+// - Finer grained false positive control (levels)<br>
+<br>
+#include "GRExprEngineExperimentalChecks.h"<br>
+#include "clang/Checker/BugReporter/BugType.h"<br>
+#include "clang/Checker/PathSensitive/CheckerVisitor.h"<br>
+#include "clang/Checker/PathSensitive/SVals.h"<br>
+#include "clang/AST/Stmt.h"<br>
+#include "llvm/ADT/DenseMap.h"<br>
+<br>
+using namespace clang;<br>
+<br>
+namespace {<br>
+class IdempotentOperationChecker<br>
+ : public CheckerVisitor<IdempotentOperationChecker> {<br>
+ public:<br>
+ static void *getTag();<br>
+ void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B);<br>
+ void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B,<br>
+ bool hasWorkRemaining);<br>
+<br>
+ private:<br>
+ // Our assumption about a particular operation.<br>
+ enum Assumption { Possible, Impossible, Equal, LHSis1, RHSis1, LHSis0,<br>
+ RHSis0 };<br>
+<br>
+ void UpdateAssumption(Assumption &A, const Assumption &New);<br>
+<br>
+ /// contains* - Useful recursive methods to see if a statement contains an<br>
+ /// element somewhere. Used in static analysis to reduce false positives.<br>
+ static bool containsMacro(const Stmt *S);<br>
+ static bool containsEnum(const Stmt *S);<br>
+ static bool containsBuiltinOffsetOf(const Stmt *S);<br>
+ static bool containsZeroConstant(const Stmt *S);<br>
+ static bool containsOneConstant(const Stmt *S);<br>
+ template <class T> static bool containsStmt(const Stmt *S) {<br>
+ if (isa<T>(S))<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsStmt<T>(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+ }<br>
+<br>
+ // Hash table<br>
+ typedef llvm::DenseMap<const BinaryOperator *, Assumption> AssumptionMap;<br>
+ AssumptionMap hash;<br>
+};<br>
+}<br>
+<br>
+void *IdempotentOperationChecker::getTag() {<br>
+ static int x = 0;<br>
+ return &x;<br>
+}<br>
+<br>
+void clang::RegisterIdempotentOperationChecker(GRExprEngine &Eng) {<br>
+ Eng.registerCheck(new IdempotentOperationChecker());<br>
+}<br>
+<br>
+void IdempotentOperationChecker::PreVisitBinaryOperator(<br>
+ CheckerContext &C,<br>
+ const BinaryOperator *B) {<br>
+ // Find or create an entry in the hash for this BinaryOperator instance<br>
+ AssumptionMap::iterator i = hash.find(B);<br>
+ Assumption &A = i == hash.end() ? hash[B] : i->second;<br>
+<br>
+ // If we had to create an entry, initialise the value to Possible<br>
+ if (i == hash.end())<br>
+ A = Possible;<br>
+<br>
+ // If we already have visited this node on a path that does not contain an<br>
+ // idempotent operation, return immediately.<br>
+ if (A == Impossible)<br>
+ return;<br>
+<br>
+ // Skip binary operators containing common false positives<br>
+ if (containsMacro(B) || containsEnum(B) || containsStmt<SizeOfAlignOfExpr>(B)<br>
+ || containsZeroConstant(B) || containsOneConstant(B)<br>
+ || containsBuiltinOffsetOf(B)) {<br>
+ A = Impossible;<br>
+ return;<br>
+ }<br>
+<br>
+ const Expr *LHS = B->getLHS();<br>
+ const Expr *RHS = B->getRHS();<br>
+<br>
+ const GRState *state = C.getState();<br>
+<br>
+ SVal LHSVal = state->getSVal(LHS);<br>
+ SVal RHSVal = state->getSVal(RHS);<br>
+<br>
+ // If either value is unknown, we can't be 100% sure of all paths.<br>
+ if (LHSVal.isUnknownOrUndef() || RHSVal.isUnknownOrUndef()) {<br>
+ A = Impossible;<br>
+ return;<br>
+ }<br>
+ BinaryOperator::Opcode Op = B->getOpcode();<br>
+<br>
+ // Dereference the LHS SVal if this is an assign operation<br>
+ switch (Op) {<br>
+ default:<br>
+ break;<br>
+<br>
+ // Fall through intentional<br>
+ case BinaryOperator::AddAssign:<br>
+ case BinaryOperator::SubAssign:<br>
+ case BinaryOperator::MulAssign:<br>
+ case BinaryOperator::DivAssign:<br>
+ case BinaryOperator::AndAssign:<br>
+ case BinaryOperator::OrAssign:<br>
+ case BinaryOperator::XorAssign:<br>
+ case BinaryOperator::ShlAssign:<br>
+ case BinaryOperator::ShrAssign:<br>
+ case BinaryOperator::Assign:<br>
+ // Assign statements have one extra level of indirection<br>
+ if (!isa<Loc>(LHSVal)) {<br>
+ A = Impossible;<br>
+ return;<br>
+ }<br>
+ LHSVal = state->getSVal(cast<Loc>(LHSVal));<br>
+ }<br>
+<br>
+<br>
+ // We now check for various cases which result in an idempotent operation.<br>
+<br>
+ // x op x<br>
+ switch (Op) {<br>
+ default:<br>
+ break; // We don't care about any other operators.<br>
+<br>
+ // Fall through intentional<br>
+ case BinaryOperator::SubAssign:<br>
+ case BinaryOperator::DivAssign:<br>
+ case BinaryOperator::AndAssign:<br>
+ case BinaryOperator::OrAssign:<br>
+ case BinaryOperator::XorAssign:<br>
+ case BinaryOperator::Assign:<br>
+ case BinaryOperator::Sub:<br>
+ case BinaryOperator::Div:<br>
+ case BinaryOperator::And:<br>
+ case BinaryOperator::Or:<br>
+ case BinaryOperator::Xor:<br>
+ case BinaryOperator::LOr:<br>
+ case BinaryOperator::LAnd:<br>
+ if (LHSVal != RHSVal)<br>
+ break;<br>
+ UpdateAssumption(A, Equal);<br>
+ return;<br>
+ }<br>
+<br>
+ // x op 1<br>
+ switch (Op) {<br>
+ default:<br>
+ break; // We don't care about any other operators.<br>
+<br>
+ // Fall through intentional<br>
+ case BinaryOperator::MulAssign:<br>
+ case BinaryOperator::DivAssign:<br>
+ case BinaryOperator::Mul:<br>
+ case BinaryOperator::Div:<br>
+ case BinaryOperator::LOr:<br>
+ case BinaryOperator::LAnd:<br>
+ if (!RHSVal.isConstant(1))<br>
+ break;<br>
+ UpdateAssumption(A, RHSis1);<br>
+ return;<br>
+ }<br>
+<br>
+ // 1 op x<br>
+ switch (Op) {<br>
+ default:<br>
+ break; // We don't care about any other operators.<br>
+<br>
+ // Fall through intentional<br>
+ case BinaryOperator::MulAssign:<br>
+ case BinaryOperator::Mul:<br>
+ case BinaryOperator::LOr:<br>
+ case BinaryOperator::LAnd:<br>
+ if (!LHSVal.isConstant(1))<br>
+ break;<br>
+ UpdateAssumption(A, LHSis1);<br>
+ return;<br>
+ }<br>
+<br>
+ // x op 0<br>
+ switch (Op) {<br>
+ default:<br>
+ break; // We don't care about any other operators.<br>
+<br>
+ // Fall through intentional<br>
+ case BinaryOperator::AddAssign:<br>
+ case BinaryOperator::SubAssign:<br>
+ case BinaryOperator::MulAssign:<br>
+ case BinaryOperator::AndAssign:<br>
+ case BinaryOperator::OrAssign:<br>
+ case BinaryOperator::XorAssign:<br>
+ case BinaryOperator::Add:<br>
+ case BinaryOperator::Sub:<br>
+ case BinaryOperator::Mul:<br>
+ case BinaryOperator::And:<br>
+ case BinaryOperator::Or:<br>
+ case BinaryOperator::Xor:<br>
+ case BinaryOperator::Shl:<br>
+ case BinaryOperator::Shr:<br>
+ case BinaryOperator::LOr:<br>
+ case BinaryOperator::LAnd:<br>
+ if (!RHSVal.isConstant(0))<br>
+ break;<br>
+ UpdateAssumption(A, RHSis0);<br>
+ return;<br>
+ }<br>
+<br>
+ // 0 op x<br>
+ switch (Op) {<br>
+ default:<br>
+ break; // We don't care about any other operators.<br>
+<br>
+ // Fall through intentional<br>
+ //case BinaryOperator::AddAssign: // Common false positive<br>
+ case BinaryOperator::SubAssign: // Check only if unsigned<br>
+ case BinaryOperator::MulAssign:<br>
+ case BinaryOperator::DivAssign:<br>
+ case BinaryOperator::AndAssign:<br>
+ //case BinaryOperator::OrAssign: // Common false positive<br>
+ //case BinaryOperator::XorAssign: // Common false positive<br>
+ case BinaryOperator::ShlAssign:<br>
+ case BinaryOperator::ShrAssign:<br>
+ case BinaryOperator::Add:<br>
+ case BinaryOperator::Sub:<br>
+ case BinaryOperator::Mul:<br>
+ case BinaryOperator::Div:<br>
+ case BinaryOperator::And:<br>
+ case BinaryOperator::Or:<br>
+ case BinaryOperator::Xor:<br>
+ case BinaryOperator::Shl:<br>
+ case BinaryOperator::Shr:<br>
+ case BinaryOperator::LOr:<br>
+ case BinaryOperator::LAnd:<br>
+ if (!LHSVal.isConstant(0))<br>
+ break;<br>
+ UpdateAssumption(A, LHSis0);<br>
+ return;<br>
+ }<br>
+<br>
+ // If we get to this point, there has been a valid use of this operation.<br>
+ A = Impossible;<br>
+}<br>
+<br>
+void IdempotentOperationChecker::VisitEndAnalysis(ExplodedGraph &G,<br>
+ BugReporter &B,<br>
+ bool hasWorkRemaining) {<br>
+ // If there is any work remaining we cannot be 100% sure about our warnings<br>
+ if (hasWorkRemaining)<br>
+ return;<br>
+<br>
+ // Iterate over the hash to see if we have any paths with definite<br>
+ // idempotent operations.<br>
+ for (AssumptionMap::const_iterator i =<br>
+ hash.begin(); i != hash.end(); ++i) {<br>
+ if (i->second != Impossible) {<br></blockquote><div><br></div><div>Any reason not to just 'continue' in the case below? At the very least 'break' there seems strange, llvm_unreachable would make more sense.</div>
<div><br></div><div>Also, I tweaked this code a bit in r107741 and 107745 to make GCC happier about the msg variable. Check and see if that looks OK?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+ // Select the error message.<br>
+ const char *msg;<br>
+ switch (i->second) {<br>
+ case Equal:<br>
+ msg = "idempotent operation; both operands are always equal in value";<br>
+ break;<br>
+ case LHSis1:<br>
+ msg = "idempotent operation; the left operand is always 1";<br>
+ break;<br>
+ case RHSis1:<br>
+ msg = "idempotent operation; the right operand is always 1";<br>
+ break;<br>
+ case LHSis0:<br>
+ msg = "idempotent operation; the left operand is always 0";<br>
+ break;<br>
+ case RHSis0:<br>
+ msg = "idempotent operation; the right operand is always 0";<br>
+ break;<br>
+ case Impossible:<br>
+ break;<br>
+ case Possible:<br>
+ assert(0 && "Operation was never marked with an assumption");<br>
+ }<br>
+<br>
+ // Create the SourceRange Arrays<br>
+ SourceRange S[2] = { i->first->getLHS()->getSourceRange(),<br>
+ i->first->getRHS()->getSourceRange() };<br>
+ B.EmitBasicReport("Idempotent operation", msg, i->first->getOperatorLoc(),<br>
+ S, 2);<br>
+ }<br>
+ }<br>
+}<br>
+<br>
+// Updates the current assumption given the new assumption<br>
+inline void IdempotentOperationChecker::UpdateAssumption(Assumption &A,<br>
+ const Assumption &New) {<br>
+ switch (A) {<br>
+ // If we don't currently have an assumption, set it<br>
+ case Possible:<br>
+ A = New;<br>
+ return;<br>
+<br>
+ // If we have determined that a valid state happened, ignore the new<br>
+ // assumption.<br>
+ case Impossible:<br>
+ return;<br>
+<br>
+ // Any other case means that we had a different assumption last time. We don't<br>
+ // currently support mixing assumptions for diagnostic reasons, so we set<br>
+ // our assumption to be impossible.<br>
+ default:<br>
+ A = Impossible;<br>
+ return;<br>
+ }<br>
+}<br>
+<br>
+// Recursively find any substatements containing macros<br>
+bool IdempotentOperationChecker::containsMacro(const Stmt *S) {<br>
+ if (S->getLocStart().isMacroID())<br>
+ return true;<br>
+<br>
+ if (S->getLocEnd().isMacroID())<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsMacro(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
+// Recursively find any substatements containing enum constants<br>
+bool IdempotentOperationChecker::containsEnum(const Stmt *S) {<br>
+ const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(S);<br>
+<br>
+ if (DR && isa<EnumConstantDecl>(DR->getDecl()))<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsEnum(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
+// Recursively find any substatements containing __builtin_offset_of<br>
+bool IdempotentOperationChecker::containsBuiltinOffsetOf(const Stmt *S) {<br>
+ const UnaryOperator *UO = dyn_cast<UnaryOperator>(S);<br>
+<br>
+ if (UO && UO->getOpcode() == UnaryOperator::OffsetOf)<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsBuiltinOffsetOf(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
+bool IdempotentOperationChecker::containsZeroConstant(const Stmt *S) {<br>
+ const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);<br>
+ if (IL && IL->getValue() == 0)<br>
+ return true;<br>
+<br>
+ const FloatingLiteral *FL = dyn_cast<FloatingLiteral>(S);<br>
+ if (FL && FL->getValue().isZero())<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsZeroConstant(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
+bool IdempotentOperationChecker::containsOneConstant(const Stmt *S) {<br>
+ const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(S);<br>
+ if (IL && IL->getValue() == 1)<br>
+ return true;<br>
+<br>
+ const FloatingLiteral *FL = dyn_cast<FloatingLiteral>(S);<br>
+ const llvm::APFloat one(1.0);<br>
+ if (FL && FL->getValue().compare(one) == llvm::APFloat::cmpEqual)<br>
+ return true;<br>
+<br>
+ for (Stmt::const_child_iterator I = S->child_begin(); I != S->child_end();<br>
+ ++I)<br>
+ if (const Stmt *child = *I)<br>
+ if (containsOneConstant(child))<br>
+ return true;<br>
+<br>
+ return false;<br>
+}<br>
+<br>
<br>
Modified: cfe/trunk/lib/Checker/SVals.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/SVals.cpp?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/SVals.cpp?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Checker/SVals.cpp (original)<br>
+++ cfe/trunk/lib/Checker/SVals.cpp Tue Jul 6 16:43:29 2010<br>
@@ -200,15 +200,19 @@<br>
return isa<nonloc::ConcreteInt>(this) || isa<loc::ConcreteInt>(this);<br>
}<br>
<br>
-bool SVal::isZeroConstant() const {<br>
+bool SVal::isConstant(int I) const {<br>
if (isa<loc::ConcreteInt>(*this))<br>
- return cast<loc::ConcreteInt>(*this).getValue() == 0;<br>
+ return cast<loc::ConcreteInt>(*this).getValue() == I;<br>
else if (isa<nonloc::ConcreteInt>(*this))<br>
- return cast<nonloc::ConcreteInt>(*this).getValue() == 0;<br>
+ return cast<nonloc::ConcreteInt>(*this).getValue() == I;<br>
else<br>
return false;<br>
}<br>
<br>
+bool SVal::isZeroConstant() const {<br>
+ return isConstant(0);<br>
+}<br>
+<br>
<br>
//===----------------------------------------------------------------------===//<br>
// Transfer function dispatch for Non-Locs.<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=107706&r1=107705&r2=107706&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=107706&r1=107705&r2=107706&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Jul 6 16:43:29 2010<br>
@@ -112,6 +112,8 @@<br>
Res.push_back("-analyzer-experimental-checks");<br>
if (Opts.EnableExperimentalInternalChecks)<br>
Res.push_back("-analyzer-experimental-internal-checks");<br>
+ if (Opts.EnableIdempotentOperationChecker)<br>
+ Res.push_back("-analyzer-idempotent-operation");<br>
}<br>
<br>
static void CodeGenOptsToArgs(const CodeGenOptions &Opts,<br>
@@ -788,6 +790,8 @@<br>
Opts.EnableExperimentalChecks = Args.hasArg(OPT_analyzer_experimental_checks);<br>
Opts.EnableExperimentalInternalChecks =<br>
Args.hasArg(OPT_analyzer_experimental_internal_checks);<br>
+ Opts.EnableIdempotentOperationChecker =<br>
+ Args.hasArg(OPT_analyzer_idempotent_operation);<br>
Opts.TrimGraph = Args.hasArg(OPT_trim_egraph);<br>
Opts.MaxNodes = Args.getLastArgIntValue(OPT_analyzer_max_nodes, 150000,Diags);<br>
Opts.MaxLoop = Args.getLastArgIntValue(OPT_analyzer_max_loop, 3, Diags);<br>
<br>
Added: cfe/trunk/test/Analysis/idempotent-operations.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=107706&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/idempotent-operations.c?rev=107706&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Analysis/idempotent-operations.c (added)<br>
+++ cfe/trunk/test/Analysis/idempotent-operations.c Tue Jul 6 16:43:29 2010<br>
@@ -0,0 +1,52 @@<br>
+// RUN: %clang_cc1 -Wunused-variable -analyze -analyzer-idempotent-operation -analyzer-store=basic -analyzer-constraints=range<br>
+<br>
+// Basic tests<br>
+<br>
+extern void test(int i);<br>
+<br>
+void basic() {<br>
+ int x = 10, zero = 0, one = 1;<br>
+<br>
+ // x op x<br>
+ x = x; // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ test(x - x); // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x -= x; // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x = 10; // no-warning<br>
+ test(x / x); // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x /= x; // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x = 10; // no-warning<br>
+ test(x & x); // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x &= x; // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ test(x | x); // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+ x |= x; // expected-warning {{idempotent operation; both operands are always equal in value}}<br>
+<br>
+ // x op 1<br>
+ test(x * one); // expected-warning {{idempotent operation; the right operand is always 1}}<br>
+ x *= one; // expected-warning {{idempotent operation; the right operand is always 1}}<br>
+ test(x / one); // expected-warning {{idempotent operation; the right operand is always 1}}<br>
+ x /= one; // expected-warning {{idempotent operation; the right operand is always 1}}<br>
+<br>
+ // 1 op x<br>
+ test(one * x); // expected-warning {{idempotent operation; the left operand is always 1}}<br>
+<br>
+ // x op 0<br>
+ test(x + zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x - zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x * zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x & zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x | zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x ^ zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x << zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+ test(x >> zero); // expected-warning {{idempotent operation; the right operand is always 0}}<br>
+<br>
+ // 0 op x<br>
+ test(zero + x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero - x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero / x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero * x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero & x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero | x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero ^ x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero << x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+ test(zero >> x); // expected-warning {{idempotent operation; the left operand is always 0}}<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>