[PATCH v2] Bug 14526 - Make malloc() static analysis M_ZERO-aware

Meyer, Conrad conrad.meyer at isilon.com
Fri Mar 14 09:53:11 PDT 2014


Add M_ZERO awareness to malloc() static analysis in Clang for FreeBSD,
NetBSD, and OpenBSD in a similar fashion to O_CREAT for open(2). There
is some opportunity for code-sharing here.

This should reduce the number of false positives when running static
analysis on FreeBSD.

Future work involves a better (symbolic) method of checking for named
flags without hardcoding values.

Add Analysis/malloc-three-arg.c tests for M_ZERO-alike malloc.

Sponsored by: EMC/Isilon storage division
Signed-off-by: Conrad Meyer <conrad.meyer at isilon.com>
---
 lib/StaticAnalyzer/Checkers/MallocChecker.cpp  | 69 +++++++++++++++++++++++++-
 lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp |  1 +
 test/Analysis/malloc-three-arg.c               | 58 ++++++++++++++++++++++
 3 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 test/Analysis/malloc-three-arg.c

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index ca40beb..274ae5a 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -16,6 +16,7 @@
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -208,6 +209,7 @@ private:
   mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
   mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc,
                          *II_valloc, *II_reallocf, *II_strndup, *II_strdup;
+  mutable Optional<uint64_t> Val_M_ZERO;
 
   void initIdentifierInfo(ASTContext &C) const;
 
@@ -586,7 +588,72 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
     initIdentifierInfo(C.getASTContext());
     IdentifierInfo *FunI = FD->getIdentifier();
 
-    if (FunI == II_malloc || FunI == II_valloc) {
+    if (FunI == II_malloc) {
+      if (CE->getNumArgs() < 1)
+        return;
+      if (CE->getNumArgs() < 2) {
+        State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+      } else if (CE->getNumArgs() == 3) {
+        // 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels:
+        //
+        // void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
+        //
+        // One of the possible flags is M_ZERO, which means 'give me back an
+        // allocation which is already zeroed', like calloc.
+
+        // This logic is largely cloned from O_CREAT in UnixAPIChecker, maybe some
+        // code could be shared.
+
+        if (!Val_M_ZERO.hasValue()) {
+          if (C.getASTContext().getTargetInfo().getTriple().getOS()
+                                                            == llvm::Triple::FreeBSD)
+            Val_M_ZERO = 0x0100;
+          else if (C.getASTContext().getTargetInfo().getTriple().getOS()
+                                                                 == llvm::Triple::NetBSD)
+            Val_M_ZERO = 0x0002;
+          else if (C.getASTContext().getTargetInfo().getTriple().getOS()
+                                                                 == llvm::Triple::OpenBSD)
+            Val_M_ZERO = 0x0008;
+          else
+            // FIXME: We need a more general way of getting the M_ZERO value.
+            // See also: O_CREAT in UnixAPIChecker.cpp.
+
+            // Fall back to normal malloc behavior on platforms where we don't
+            // know M_ZERO.
+            State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+        }
+
+        if (Val_M_ZERO.hasValue()) {
+          const Expr *FlagsEx = CE->getArg(2);
+          const SVal V = State->getSVal(FlagsEx, C.getLocationContext());
+          if (!V.getAs<NonLoc>()) {
+            // The case where 'V' can be a location can only be due to a bad header,
+            // so in this case bail out.
+            return;
+          }
+
+          NonLoc Flags = V.castAs<NonLoc>();
+          NonLoc ZeroFlag = C.getSValBuilder()
+              .makeIntVal(Val_M_ZERO.getValue(), FlagsEx->getType()).castAs<NonLoc>();
+          SVal MaskedFlagsUC = C.getSValBuilder().evalBinOpNN(State, BO_And,
+                                                              Flags, ZeroFlag,
+                                                              FlagsEx->getType());
+          if (MaskedFlagsUC.isUnknownOrUndef())
+            return;
+          DefinedSVal MaskedFlags = MaskedFlagsUC.castAs<DefinedSVal>();
+
+          // Check if maskedFlags is non-zero.
+          ProgramStateRef TrueState, FalseState;
+          std::tie(TrueState, FalseState) = State->assume(MaskedFlags);
+
+          // If M_ZERO is set, treat this like calloc (initialized).
+          if (TrueState && !FalseState)
+            State = CallocMem(C, CE);
+          else
+            State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+        }
+      }
+    } else if (FunI == II_valloc) {
       if (CE->getNumArgs() < 1)
         return;
       State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 8869654..aa7e25bc 100644
--- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -80,6 +80,7 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
       // FIXME: We need a more general way of getting the O_CREAT value.
       // We could possibly grovel through the preprocessor state, but
       // that would require passing the Preprocessor object to the ExprEngine.
+      // See also: MallocChecker.cpp / M_ZERO.
       return;
     }
   }
diff --git a/test/Analysis/malloc-three-arg.c b/test/Analysis/malloc-three-arg.c
new file mode 100644
index 0000000..01b08ae
--- /dev/null
+++ b/test/Analysis/malloc-three-arg.c
@@ -0,0 +1,58 @@
+// RUN: %clang -target x86_64-unknown-freebsd --analyze %s
+
+#include "Inputs/system-header-simulator.h"
+
+#define M_ZERO 0x0100
+#define NULL ((void *)0)
+
+void *malloc(size_t, void *, int);
+
+struct test {
+};
+
+void foo(struct test *);
+
+void test_zeroed() {
+  struct test **list, *t;
+  int i;
+
+  list = malloc(sizeof(*list) * 10, NULL, M_ZERO);
+  if (list == NULL)
+    return;
+
+  for (i = 0; i < 10; i++) {
+    t = list[i];
+    foo(t);
+  }
+  free(list); // no-warning
+}
+
+void test_nonzero() {
+  struct test **list, *t;
+  int i;
+
+  list = malloc(sizeof(*list) * 10, NULL, 0);
+  if (list == NULL)
+    return;
+
+  for (i = 0; i < 10; i++) {
+    t = list[i]; // expected-warning{{undefined}}
+    foo(t);
+  }
+  free(list);
+}
+
+void test_indeterminate(int flags) {
+  struct test **list, *t;
+  int i;
+
+  list = malloc(sizeof(*list) * 10, NULL, flags);
+  if (list == NULL)
+    return;
+
+  for (i = 0; i < 10; i++) {
+    t = list[i]; // expected-warning{{undefined}}
+    foo(t);
+  }
+  free(list);
+}
-- 
1.8.5.3




More information about the cfe-commits mailing list