balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske added a comment.
balazske marked an inline comment as done.
FIXME: There is a test file "kmalloc-linux.c" but it seems to be non-maintained
and buggy (has no //-verify// option so it passes always but produces multiple
warnings).
================
Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:146
return IntValue.getSExtValue();
}
----------------
The function was changed to get the numeric value from the end of the macro in
any case. This way it recognizes a `(void *)16` as 16 (but maybe `16+16` too as
16).
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. And it is recognized in any kind of free function, not only kfree.
(These functions are used already intermixed, at least in the tests.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76830
Files:
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
clang/test/Analysis/kmalloc-linux-1.c
Index: clang/test/Analysis/kmalloc-linux-1.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/kmalloc-linux-1.c
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=unix.Malloc -verify %s
+
+#define ZERO_SIZE_PTR ((void *)16)
+
+void *__kmalloc(int size, int flags);
+
+// Linux kmalloc looks like this.
+// (simplified)
+void *kmalloc(int size, int flags) {
+ if (size == 0)
+ return ZERO_SIZE_PTR;
+ return __kmalloc(size, flags);
+}
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)
+void kfree(void *, int);
+
+void test_kmalloc_zero_sized_block_fixed_value_address() {
+ void *ptr = kmalloc(0, 0); // kmalloc returns a constant address
+ kfree(ptr, 0); // no warning about freeing a constant value
+}
+
+void test_kfree_constant_value() {
+ void *ptr = (void *)1;
+ kfree(ptr, 0); // expected-warning{{Argument to kfree() is a constant address (1)}}
+}
Index: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -126,9 +126,6 @@
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 @@
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();
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ 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,11 @@
// TODO: Remove mutable by moving the initializtaion to the registry function.
mutable Optional<uint64_t> KernelZeroFlagVal;
+ /// Store value of "ZERO_SIZE_PTR" macro if found.
+ /// It requires two-level Optional: Was lazy initialization done yet, and if
+ /// yes was the value obtained or not.
+ mutable Optional<Optional<int>> 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,
@@ -1672,6 +1678,20 @@
if (ArgVal.isUnknownOrUndef())
return nullptr;
+ // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
+ // and this value indicates a special value used for a zero-sized memory
+ // block. It is a constant value that is allowed to be freed.
+ const llvm::APSInt *ArgValKnown =
+ C.getSValBuilder().getKnownValue(State, ArgVal);
+ if (ArgValKnown) {
+ if (!KernelZeroSizePtrValue)
+ KernelZeroSizePtrValue =
+ tryExpandAsInteger("ZERO_SIZE_PTR", C.getPreprocessor());
+ if (*KernelZeroSizePtrValue &&
+ ArgValKnown->getSExtValue() == **KernelZeroSizePtrValue)
+ return nullptr;
+ }
+
const MemRegion *R = ArgVal.getAsRegion();
// Nonlocs can't be freed, of course.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
