[StaticAnalyzer] Improve+Refactor MallocOverflowSecurityChecker

[Aditya K]

Hi,
Currently MallocOverflowSecurityChecker does not catch cases like: malloc(n * 0 
* sizeof(int));
This patch reject such cases. Please give feedback/suggestions for improvement.

Thanks,
-Aditya


commit 3b536433707152d745e03d1d07f67fc8c9848ecf
Author: hiraditya <hiradi...@msn.com>
Date:   Sat May 9 11:55:50 2015 -0500

    Reject multiplication by zero.

    Replaced isa with dyn_cast.

diff --git a/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp 
b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
index e913479..3ff2ae4 100644
--- a/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
@@ -23,11 +23,14 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"

 using namespace clang;
 using namespace ento;

+using llvm::APSInt;
+
 namespace {
 struct MallocOverflowCheck {
   const BinaryOperator *mulop;
@@ -54,6 +57,13 @@ public:
 };
 } // end anonymous namespace

+// Return true for redundant computations.
+static bool RedundantComputation(APSInt &Val, BinaryOperatorKind op) {
+  if (op == BO_Mul && Val == 0)
+    return true;
+  return false;
+}
+
 void MallocOverflowSecurityChecker::CheckMallocArgument(
   SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
   const Expr *TheArgument,
@@ -64,13 +74,12 @@ void MallocOverflowSecurityChecker::CheckMallocArgument(
    Reject anything that applies to the variable: an explicit cast,
    conditional expression, an operation that could reduce the range
    of the result, or anything too complicated :-).  */
-  const Expr * e = TheArgument;
+  const Expr *e = TheArgument;
   const BinaryOperator * mulop = nullptr;

   for (;;) {
     e = e->IgnoreParenImpCasts();
-    if (isa<BinaryOperator>(e)) {
-      const BinaryOperator * binop = dyn_cast<BinaryOperator>(e);
+    if (const BinaryOperator *binop = dyn_cast<BinaryOperator>(e)) {
       BinaryOperatorKind opc = binop->getOpcode();
       // TODO: ignore multiplications by 1, reject if multiplied by 0.
       if (mulop == nullptr && opc == BO_Mul)
@@ -80,11 +89,19 @@ void MallocOverflowSecurityChecker::CheckMallocArgument(

       const Expr *lhs = binop->getLHS();
       const Expr *rhs = binop->getRHS();
-      if (rhs->isEvaluatable(Context))
+      if (rhs->isEvaluatable(Context)) {
         e = lhs;
+        APSInt Val = rhs->EvaluateKnownConstInt(Context);
+        if (RedundantComputation(Val, opc))
+          return;
+      }
       else if ((opc == BO_Add || opc == BO_Mul)
-               && lhs->isEvaluatable(Context))
+               && lhs->isEvaluatable(Context)) {
+        APSInt Val = lhs->EvaluateKnownConstInt(Context);
+        if (RedundantComputation(Val, opc))
+          return;
         e = rhs;
+      }
       else
         return;
     }
@@ -133,7 +150,7 @@ private:
       theVecType::iterator e = toScanFor.begin();

       if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) {
-        const Decl * EdreD = DR->getDecl();
+        const Decl *EdreD = DR->getDecl();
         while (i != e) {
           --i;
           if (const DeclRefExpr *DR_i = dyn_cast<DeclRefExpr>(i->variable)) {
@@ -162,7 +179,7 @@ private:
         const Expr * rhs = E->getRHS();
         // Ignore comparisons against zero, since they generally don't
         // protect against an overflow.
-        if (!isIntZeroExpr(lhs) && ! isIntZeroExpr(rhs)) {
+        if (!isIntZeroExpr(lhs) && !isIntZeroExpr(rhs)) {
           CheckExpr(lhs);
           CheckExpr(rhs);
         }
@@ -243,12 +260,12 @@ void 
MallocOverflowSecurityChecker::checkASTCodeBody(const Decl *D,
           const FunctionDecl *FD = TheCall->getDirectCallee();

           if (!FD)
-            return;
+            continue;

           // Get the name of the callee. If it's a builtin, strip off the 
prefix.
           IdentifierInfo *FnInfo = FD->getIdentifier();
           if (!FnInfo)
-            return;
+            continue;

           if (FnInfo->isStr ("malloc") || FnInfo->isStr ("_MALLOC")) {
             if (TheCall->getNumArgs() == 1)
diff --git a/test/Analysis/malloc-overflow.c b/test/Analysis/malloc-overflow.c
index 2f443ca..dbe8ffb 100644
--- a/test/Analysis/malloc-overflow.c
+++ b/test/Analysis/malloc-overflow.c
@@ -111,3 +111,9 @@ void * f14(int n)
     return NULL;
   return malloc(n * sizeof(int));  // expected-warning {{the computation of 
the size of the memory allocation may overflow}}
 }
+
+void * f15(int n)
+{
+  return malloc(n * 0 * sizeof(int));  // no warning
+}
+
                                          
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits