[clang] dcc04e0 - [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 30 01:38:49 PDT 2020
Author: Balázs Kéri
Date: 2020-03-30T10:33:14+02:00
New Revision: dcc04e09cf6e36c6d2c47057a8f201f2c0784c69
URL: https://github.com/llvm/llvm-project/commit/dcc04e09cf6e36c6d2c47057a8f201f2c0784c69
DIFF: https://github.com/llvm/llvm-project/commit/dcc04e09cf6e36c6d2c47057a8f201f2c0784c69.diff
LOG: [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
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
clang/test/Analysis/kmalloc-linux.c
clang/test/Analysis/malloc.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index baf2c48de3b2..32a500ffd102 100644
--- 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 @@ class MallocChecker
// 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 @@ class MallocChecker
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 e43b172d018d..4b63ebc40ede 100644
--- 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 6b17ef2d5a79..4cca5712b096 100644
--- 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 6e5a0e4d5971..cc7fefe23d75 100644
--- 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)}}
+}
More information about the cfe-commits
mailing list