[cfe-commits] r76900 - in /cfe/trunk: lib/Analysis/CheckSecuritySyntaxOnly.cpp test/Analysis/security-syntax-checks.m
Ted Kremenek
kremenek at apple.com
Thu Jul 23 14:34:50 PDT 2009
Author: kremenek
Date: Thu Jul 23 16:34:35 2009
New Revision: 76900
URL: http://llvm.org/viewvc/llvm-project?rev=76900&view=rev
Log:
Refine checking and diagnostics for use of floating point variable as a counter.
This implements <rdar://problem/6336718> and checks for CERT secure coding
advisory FLP30-C.
Added:
cfe/trunk/test/Analysis/security-syntax-checks.m
Modified:
cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
Modified: cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp?rev=76900&r1=76899&r2=76900&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/Analysis/CheckSecuritySyntaxOnly.cpp Thu Jul 23 16:34:35 2009
@@ -15,6 +15,7 @@
#include "clang/Analysis/LocalCheckers.h"
#include "clang/AST/StmtVisitor.h"
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/raw_ostream.h"
using namespace clang;
@@ -25,15 +26,13 @@
WalkAST(BugReporter &br) : BR(br) {}
// Statement visitor methods.
- void VisitDoStmt(DoStmt *S);
- void VisitWhileStmt(WhileStmt *S);
void VisitForStmt(ForStmt *S);
+ void VisitStmt(Stmt *S) { VisitChildren(S); }
void VisitChildren(Stmt *S);
- void VisitStmt(Stmt *S) { VisitChildren(S); }
// Checker-specific methods.
- void CheckLoopConditionForFloat(Stmt *Loop, Expr *Condition);
+ void CheckLoopConditionForFloat(const ForStmt *FS);
};
} // end anonymous namespace
@@ -47,69 +46,117 @@
Visit(child);
}
-void WalkAST::VisitDoStmt(DoStmt *S) {
- CheckLoopConditionForFloat(S, S->getCond());
- VisitChildren(S);
-}
-
-void WalkAST::VisitForStmt(ForStmt *S) {
- if (Expr *Cond = S->getCond())
- CheckLoopConditionForFloat(S, Cond);
+void WalkAST::VisitForStmt(ForStmt *FS) {
+ CheckLoopConditionForFloat(FS);
- VisitChildren(S);
-}
-
-void WalkAST::VisitWhileStmt(WhileStmt *S) {
- CheckLoopConditionForFloat(S, S->getCond());
- VisitChildren(S);
+ // Recurse and check children.
+ VisitChildren(FS);
}
//===----------------------------------------------------------------------===//
-// Checking logic.
+// Check: floating poing variable used as loop counter.
//===----------------------------------------------------------------------===//
-static Expr* IsFloatCondition(Expr *Condition) {
- while (Condition) {
- Condition = Condition->IgnoreParenCasts();
-
- if (Condition->getType()->isFloatingType())
- return Condition;
-
- switch (Condition->getStmtClass()) {
- case Stmt::BinaryOperatorClass: {
- BinaryOperator *B = cast<BinaryOperator>(Condition);
-
- Expr *LHS = B->getLHS();
- if (LHS->getType()->isFloatingType())
- return LHS;
-
- Expr *RHS = B->getRHS();
- if (RHS->getType()->isFloatingType())
- return RHS;
-
- return NULL;
- }
- case Stmt::UnaryOperatorClass: {
- UnaryOperator *U = cast<UnaryOperator>(Condition);
- if (U->isArithmeticOp()) {
- Condition = U->getSubExpr();
- continue;
- }
- return NULL;
- }
- default:
- break;
- }
+static const DeclRefExpr*
+GetIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
+ expr = expr->IgnoreParenCasts();
+
+ if (const BinaryOperator *B = dyn_cast<BinaryOperator>(expr)) {
+ if (!(B->isAssignmentOp() || B->isCompoundAssignmentOp() ||
+ B->getOpcode() == BinaryOperator::Comma))
+ return NULL;
+
+ if (const DeclRefExpr *lhs = GetIncrementedVar(B->getLHS(), x, y))
+ return lhs;
+
+ if (const DeclRefExpr *rhs = GetIncrementedVar(B->getRHS(), x, y))
+ return rhs;
+
+ return NULL;
}
+
+ if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(expr)) {
+ const NamedDecl *ND = DR->getDecl();
+ return ND == x || ND == y ? DR : NULL;
+ }
+
+ if (const UnaryOperator *U = dyn_cast<UnaryOperator>(expr))
+ return U->isIncrementDecrementOp()
+ ? GetIncrementedVar(U->getSubExpr(), x, y) : NULL;
+
return NULL;
}
-void WalkAST::CheckLoopConditionForFloat(Stmt *Loop, Expr *Condition) {
- if ((Condition = IsFloatCondition(Condition))) {
- const char *bugType = "Floating point value used in loop condition";
- SourceRange R = Condition->getSourceRange();
- BR.EmitBasicReport(bugType, "Security", bugType, Loop->getLocStart(),&R, 1);
- }
+/// CheckLoopConditionForFloat - This check looks for 'for' statements that
+/// use a floating point variable as a loop counter.
+/// CERT: FLP30-C, FLP30-CPP.
+///
+void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
+ // Does the loop have a condition?
+ const Expr *condition = FS->getCond();
+
+ if (!condition)
+ return;
+
+ // Does the loop have an increment?
+ const Expr *increment = FS->getInc();
+
+ if (!increment)
+ return;
+
+ // Strip away '()' and casts.
+ condition = condition->IgnoreParenCasts();
+ increment = increment->IgnoreParenCasts();
+
+ // Is the loop condition a comparison?
+ const BinaryOperator *B = dyn_cast<BinaryOperator>(condition);
+
+ if (!B)
+ return;
+
+ // The actual error condition.
+ if (!((B->isRelationalOp() || B->isEqualityOp()) &&
+ ((B->getLHS()->getType()->isFloatingType() ||
+ B->getRHS()->getType()->isFloatingType()))))
+ return;
+
+ // Are we comparing variables?
+ const DeclRefExpr *drLHS = dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreParens());
+ const DeclRefExpr *drRHS = dyn_cast<DeclRefExpr>(B->getRHS()->IgnoreParens());
+
+ if (!drLHS && !drRHS)
+ return;
+
+ const VarDecl *vdLHS = drLHS ? dyn_cast<VarDecl>(drLHS->getDecl()) : NULL;
+ const VarDecl *vdRHS = drRHS ? dyn_cast<VarDecl>(drRHS->getDecl()) : NULL;
+
+ if (!vdLHS && !vdRHS)
+ return;
+
+ // Does either variable appear in increment?
+ const DeclRefExpr *drInc = GetIncrementedVar(increment, vdLHS, vdRHS);
+
+ if (!drInc)
+ return;
+
+ // Emit the error. First figure out which DeclRefExpr in the condition
+ // referenced the compared variable.
+ const DeclRefExpr *drCond = vdLHS == drInc->getDecl() ? drLHS : drRHS;
+
+ llvm::SmallVector<SourceRange, 2> ranges;
+ std::string sbuf;
+ llvm::raw_string_ostream os(sbuf);
+
+ os << "Variable '" << drCond->getDecl()->getNameAsCString()
+ << "' with floating point type '" << drCond->getType().getAsString()
+ << "' should not be used as a loop counter";
+
+ ranges.push_back(drCond->getSourceRange());
+ ranges.push_back(drInc->getSourceRange());
+
+ const char *bugType = "Floating point variable used as loop counter";
+ BR.EmitBasicReport(bugType, "Security", os.str().c_str(),
+ FS->getLocStart(), ranges.data(), ranges.size());
}
//===----------------------------------------------------------------------===//
Added: cfe/trunk/test/Analysis/security-syntax-checks.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/security-syntax-checks.m?rev=76900&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/security-syntax-checks.m (added)
+++ cfe/trunk/test/Analysis/security-syntax-checks.m Thu Jul 23 16:34:35 2009
@@ -0,0 +1,23 @@
+// RUN: clang-cc -analyze -warn-security-syntactic %s -verify
+
+// <rdar://problem/6336718> rule request: floating point used as loop
+// condition (FLP30-C, FLP-30-CPP)
+//
+// For reference: https://www.securecoding.cert.org/confluence/display/seccode/FLP30-C.+Do+not+use+floating+point+variables+as+loop+counters
+//
+void test_float_condition() {
+ for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // expected-warning{{Variable 'x' with floating point type 'float'}}
+ for (float x = 100000001.0f; x <= 100000010.0f; x += 1.0f) {} // expected-warning{{Variable 'x' with floating point type 'float'}}
+ for (float x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'float'}}
+ for (double x = 100000001.0; x <= 100000010.0; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'double'}}
+ for (double x = 100000001.0; ((x)) <= 100000010.0; ((x))++ ) {} // expected-warning{{Variable 'x' with floating point type 'double'}}
+
+ for (double x = 100000001.0; 100000010.0 >= x; x = x + 1.0 ) {} // expected-warning{{Variable 'x' with floating point type 'double'}}
+
+ int i = 0;
+ for (double x = 100000001.0; ((x)) <= 100000010.0; ((x))++, ++i ) {} // expected-warning{{Variable 'x' with floating point type 'double'}}
+
+ typedef float FooType;
+ for (FooType x = 100000001.0f; x <= 100000010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}}
+}
+
More information about the cfe-commits
mailing list