[Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

Summary:
The kernel kmalloc function may return a constant value ZERO_SIZE_PTR
if a zero-sized block is allocated. This special value is allowed to
be passed to kfree and should produce no warning.

This is a simple version but should be no problem. The macro is always
detected independent of if this is a kernel source code or any other
code.

Reviewers: Szelethus, martong

Reviewed By: Szelethus, martong

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D76830

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index baf2c48..32a500f 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -58,6 +58,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
@@ -389,6 +390,13 @@ private:
   // TODO: Remove mutable by moving the initializtaion to the registry function.
   mutable Optional<uint64_t> KernelZeroFlagVal;
 
+  using KernelZeroSizePtrValueTy = Optional<int>;
+  /// Store the value of macro called `ZERO_SIZE_PTR`.
+  /// The value is initialized at first use, before first use the outer
+  /// Optional is empty, afterwards it contains another Optional that indicates
+  /// if the macro value could be determined, and if yes the value itself.
+  mutable Optional<KernelZeroSizePtrValueTy> KernelZeroSizePtrValue;
+
   /// Process C++ operator new()'s allocation, which is the part of C++
   /// new-expression that goes before the constructor.
   void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C,
@@ -658,6 +666,10 @@ private:
                                     CheckerContext &C);
 
   void reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const;
+
+  /// Test if value in ArgVal equals to value in macro `ZERO_SIZE_PTR`.
+  bool isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext &C,
+                          SVal ArgVal) const;
 };
 
 //===----------------------------------------------------------------------===//
@@ -1677,7 +1689,13 @@ ProgramStateRef MallocChecker::FreeMemAux(
   // Nonlocs can't be freed, of course.
   // Non-region locations (labels and fixed addresses) also shouldn't be freed.
   if (!R) {
-    ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
+    // Exception:
+    // If the macro ZERO_SIZE_PTR is defined, this could be a kernel source
+    // code. In that case, the ZERO_SIZE_PTR defines a special value used for a
+    // zero-sized memory block which is allowed to be freed, despite not being a
+    // null pointer.
+    if (Family != AF_Malloc || !isArgZERO_SIZE_PTR(State, C, ArgVal))
+      ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
     return nullptr;
   }
 
@@ -3023,6 +3041,18 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(
   return State;
 }
 
+bool MallocChecker::isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext &C,
+                                       SVal ArgVal) const {
+  if (!KernelZeroSizePtrValue)
+    KernelZeroSizePtrValue =
+        tryExpandAsInteger("ZERO_SIZE_PTR", C.getPreprocessor());
+
+  const llvm::APSInt *ArgValKnown =
+      C.getSValBuilder().getKnownValue(State, ArgVal);
+  return ArgValKnown && *KernelZeroSizePtrValue &&
+         ArgValKnown->getSExtValue() == **KernelZeroSizePtrValue;
+}
+
 static SymbolRef findFailedReallocSymbol(ProgramStateRef currState,
                                          ProgramStateRef prevState) {
   ReallocPairsTy currMap = currState->get<ReallocPairs>();

diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index e43b172..4b63ebc 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -126,9 +126,6 @@ llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
     if (!T.isOneOf(tok::l_paren, tok::r_paren))
       FilteredTokens.push_back(T);
 
-  if (FilteredTokens.size() > 2)
-    return llvm::None;
-
   // Parse an integer at the end of the macro definition.
   const Token &T = FilteredTokens.back();
   if (!T.isLiteral())
@@ -140,11 +137,10 @@ llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
     return llvm::None;
 
   // Parse an optional minus sign.
-  if (FilteredTokens.size() == 2) {
-    if (FilteredTokens.front().is(tok::minus))
+  size_t Size = FilteredTokens.size();
+  if (Size >= 2) {
+    if (FilteredTokens[Size - 2].is(tok::minus))
       IntValue = -IntValue;
-    else
-      return llvm::None;
   }
 
   return IntValue.getSExtValue();

diff --git a/clang/test/Analysis/kmalloc-linux.c b/clang/test/Analysis/kmalloc-linux.c
index 6b17ef2..4cca571 100644 (file)
--- a/clang/test/Analysis/kmalloc-linux.c
+++ b/clang/test/Analysis/kmalloc-linux.c
@@ -121,3 +121,17 @@ void test_3arg_malloc_leak(struct malloc_type *mtp, int flags) {
   if (list == NULL)
     return;
 } // expected-warning{{Potential leak of memory pointed to by 'list'}}
+
+// kmalloc can return a constant value defined in ZERO_SIZE_PTR
+// if a block of size 0 is requested
+#define ZERO_SIZE_PTR ((void *)16)
+
+void test_kfree_ZERO_SIZE_PTR() {
+  void *ptr = ZERO_SIZE_PTR;
+  kfree(ptr); // no warning about freeing this value
+}
+
+void test_kfree_other_constant_value() {
+  void *ptr = (void *)1;
+  kfree(ptr); // expected-warning{{Argument to kfree() is a constant address (1)}}
+}

diff --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 6e5a0e4..cc7fefe 100644 (file)
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -164,3 +164,11 @@ void test(A a) {
   (void)a.getName();
 }
 } // namespace argument_leak
+
+#define ZERO_SIZE_PTR ((void *)16)
+
+void test_delete_ZERO_SIZE_PTR() {
+  int *Ptr = (int *)ZERO_SIZE_PTR;
+  // ZERO_SIZE_PTR is specially handled but only for malloc family
+  delete Ptr; // expected-warning{{Argument to 'delete' is a constant address (16)}}
+}