[clang] 9fd7ce7 - [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters
Kristóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 25 02:17:41 PST 2020
Author: Kristóf Umann
Date: 2020-02-25T11:17:32+01:00
New Revision: 9fd7ce7f4449619bc85ab4d2643e656836a2d5e2
URL: https://github.com/llvm/llvm-project/commit/9fd7ce7f4449619bc85ab4d2643e656836a2d5e2
DIFF: https://github.com/llvm/llvm-project/commit/9fd7ce7f4449619bc85ab4d2643e656836a2d5e2.diff
LOG: [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters
The following series of refactoring patches aim to fix the horrible mess that MallocChecker.cpp is.
I genuinely hate this file. It goes completely against how most of the checkers
are implemented, its by far the biggest headache regarding checker dependencies,
checker options, or anything you can imagine. On top of all that, its just bad
code. Its seriously everything that you shouldn't do in C++, or any other
language really. Bad variable/class names, in/out parameters... Apologies, rant
over.
So: there are a variety of memory manipulating function this checker models. One
aspect of these functions is their AllocationFamily, which we use to distinguish
between allocation kinds, like using free() on an object allocated by operator
new. However, since we always know which function we're actually modeling, in
fact we know it compile time, there is no need to use tricks to retrieve this
information out of thin air n+1 function calls down the line. This patch changes
many methods of MallocChecker to take a non-optional AllocationFamily template
parameter (which also makes stack dumps a bit nicer!), and removes some no
longer needed auxiliary functions.
Differential Revision: https://reviews.llvm.org/D68162
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
index 9642588d6a41..99731d6044a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
+++ b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
@@ -11,13 +11,19 @@
#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
-namespace clang {
-class CheckerManager;
+// FIXME: This file goes against how a checker should be implemented either in
+// a single file, or be exposed in a header file. Let's try to get rid of it!
+
+namespace clang {
namespace ento {
+class CheckerManager;
+
/// Register the part of MallocChecker connected to InnerPointerChecker.
void registerInnerPointerCheckerAux(CheckerManager &Mgr);
-}}
+} // namespace ento
+} // namespace clang
+
#endif /* INTERCHECKERAPI_H_ */
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 1d365bb727fd..fd3ce6cf3b9b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -44,13 +44,14 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "AllocationState.h"
#include "InterCheckerAPI.h"
#include "clang/AST/Attr.h"
#include "clang/AST/ParentMap.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -64,7 +65,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
-#include "AllocationState.h"
+#include "llvm/Support/ErrorHandling.h"
#include <climits>
#include <utility>
@@ -72,7 +73,10 @@ using namespace clang;
using namespace ento;
//===----------------------------------------------------------------------===//
-// The types of allocation we're modeling.
+// The types of allocation we're modeling. This is used to check whether a
+// dynamically allocated object is deallocated with the correct function, like
+// not using operator delete on an object created by malloc(), or alloca regions
+// aren't ever deallocated manually.
//===----------------------------------------------------------------------===//
namespace {
@@ -92,22 +96,14 @@ struct MemFunctionInfoTy;
} // end of anonymous namespace
-/// Determine family of a deallocation expression.
-static AllocationFamily
-getAllocationFamily(const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C,
- const Stmt *S);
-
/// Print names of allocators and deallocators.
///
/// \returns true on success.
-static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
- const Expr *E);
+static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E);
-/// Print expected name of an allocator based on the deallocator's
-/// family derived from the DeallocExpr.
-static void printExpectedAllocName(raw_ostream &os,
- const MemFunctionInfoTy &MemFunctionInfo,
- CheckerContext &C, const Expr *E);
+/// Print expected name of an allocator based on the deallocator's family
+/// derived from the DeallocExpr.
+static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family);
/// Print expected name of a deallocator based on the allocator's
/// family.
@@ -208,7 +204,7 @@ static bool isReleased(SymbolRef Sym, CheckerContext &C);
/// value; if unspecified, the value of expression \p E is used.
static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
ProgramStateRef State,
- AllocationFamily Family = AF_Malloc,
+ AllocationFamily Family,
Optional<SVal> RetVal = None);
//===----------------------------------------------------------------------===//
@@ -402,7 +398,7 @@ class MallocChecker
/// 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,
- SVal Target) const;
+ SVal Target, AllocationFamily Family) const;
/// Perform a zero-allocation check.
///
@@ -450,7 +446,7 @@ class MallocChecker
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
const Expr *SizeEx, SVal Init,
ProgramStateRef State,
- AllocationFamily Family = AF_Malloc);
+ AllocationFamily Family);
/// Models memory allocation.
///
@@ -464,7 +460,7 @@ class MallocChecker
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
SVal Size, SVal Init,
ProgramStateRef State,
- AllocationFamily Family = AF_Malloc);
+ AllocationFamily Family);
static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE,
ProgramStateRef State, SVal Target);
@@ -518,6 +514,7 @@ class MallocChecker
ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State, unsigned Num, bool Hold,
bool &IsKnownToBeAllocated,
+ AllocationFamily Family,
bool ReturnsNullOnFailure = false) const;
/// Models memory deallocation.
@@ -542,6 +539,7 @@ class MallocChecker
ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
const Expr *ParentExpr, ProgramStateRef State,
bool Hold, bool &IsKnownToBeAllocated,
+ AllocationFamily Family,
bool ReturnsNullOnFailure = false) const;
// TODO: Needs some refactoring, as all other deallocation modeling
@@ -559,6 +557,7 @@ class MallocChecker
/// \returns The ProgramState right after reallocation.
ProgramStateRef ReallocMemAux(CheckerContext &C, const CallExpr *CE,
bool ShouldFreeOnFail, ProgramStateRef State,
+ AllocationFamily Family,
bool SuffixWithN = false) const;
/// Evaluates the buffer size that needs to be allocated.
@@ -623,9 +622,7 @@ class MallocChecker
/// family/call/symbol.
Optional<CheckKind> getCheckIfTracked(AllocationFamily Family,
bool IsALeakCheck = false) const;
- Optional<CheckKind> getCheckIfTracked(CheckerContext &C,
- const Stmt *AllocDeallocStmt,
- bool IsALeakCheck = false) const;
+
Optional<CheckKind> getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
bool IsALeakCheck = false) const;
///@}
@@ -633,17 +630,22 @@ class MallocChecker
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
- const Expr *DeallocExpr) const;
+ const Expr *DeallocExpr, AllocationFamily Family) const;
+
void ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
SourceRange Range) const;
+
void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range,
const Expr *DeallocExpr, const RefState *RS,
SymbolRef Sym, bool OwnershipTransferred) const;
+
void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
- const Expr *DeallocExpr,
+ const Expr *DeallocExpr, AllocationFamily Family,
const Expr *AllocExpr = nullptr) const;
+
void ReportUseAfterFree(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const;
+
void ReportDoubleFree(CheckerContext &C, SourceRange Range, bool Released,
SymbolRef Sym, SymbolRef PrevSym) const;
@@ -653,7 +655,8 @@ class MallocChecker
SymbolRef Sym) const;
void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
- SourceRange Range, const Expr *FreeExpr) const;
+ SourceRange Range, const Expr *FreeExpr,
+ AllocationFamily Family) const;
/// Find the location of the allocation for Sym on the path leading to the
/// exploded node N.
@@ -1036,7 +1039,7 @@ llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc(
// If M_ZERO is set, treat this like calloc (initialized).
if (TrueState && !FalseState) {
SVal ZeroVal = C.getSValBuilder().makeZeroVal(Ctx.CharTy);
- return MallocMemAux(C, CE, CE->getArg(0), ZeroVal, TrueState);
+ return MallocMemAux(C, CE, CE->getArg(0), ZeroVal, TrueState, AF_Malloc);
}
return None;
@@ -1075,11 +1078,13 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
default:
return;
case 1:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 0, State);
break;
case 2:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_Malloc);
break;
case 3:
llvm::Optional<ProgramStateRef> MaybeState =
@@ -1087,7 +1092,8 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (MaybeState.hasValue())
State = MaybeState.getValue();
else
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_Malloc);
break;
}
} else if (FunI == MemFunctionInfo.II_kmalloc) {
@@ -1098,19 +1104,22 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (MaybeState.hasValue())
State = MaybeState.getValue();
else
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_Malloc);
} else if (FunI == MemFunctionInfo.II_valloc) {
if (CE->getNumArgs() < 1)
return;
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State =
+ MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 0, State);
} else if (FunI == MemFunctionInfo.II_realloc ||
FunI == MemFunctionInfo.II_g_realloc ||
FunI == MemFunctionInfo.II_g_try_realloc) {
- State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State);
+ State =
+ ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_reallocf) {
- State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ true, State);
+ State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ true, State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_calloc) {
State = CallocMem(C, CE, State);
@@ -1122,20 +1131,21 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (suppressDeallocationsInSuspiciousContexts(CE, C))
return;
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory,
+ AF_Malloc);
} else if (FunI == MemFunctionInfo.II_strdup ||
FunI == MemFunctionInfo.II_win_strdup ||
FunI == MemFunctionInfo.II_wcsdup ||
FunI == MemFunctionInfo.II_win_wcsdup) {
- State = MallocUpdateRefState(C, CE, State);
+ State = MallocUpdateRefState(C, CE, State, AF_Malloc);
} else if (FunI == MemFunctionInfo.II_strndup) {
- State = MallocUpdateRefState(C, CE, State);
+ State = MallocUpdateRefState(C, CE, State, AF_Malloc);
} else if (FunI == MemFunctionInfo.II_alloca ||
FunI == MemFunctionInfo.II_win_alloca) {
if (CE->getNumArgs() < 1)
return;
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
- AF_Alloca);
+ State =
+ MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Alloca);
State = ProcessZeroAllocCheck(C, CE, 0, State);
} else if (MemFunctionInfo.isStandardNewDelete(FD, C.getASTContext())) {
// Process direct calls to operator new/new[]/delete/delete[] functions
@@ -1154,8 +1164,12 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
State = ProcessZeroAllocCheck(C, CE, 0, State);
break;
case OO_Delete:
+ State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory,
+ AF_CXXNew);
+ break;
case OO_Array_Delete:
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory,
+ AF_CXXNewArray);
break;
default:
llvm_unreachable("not a new/delete operator");
@@ -1166,19 +1180,21 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
State = MallocMemAux(C, CE, UnknownVal(), UnknownVal(), State,
AF_IfNameIndex);
} else if (FunI == MemFunctionInfo.II_if_freenameindex) {
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory,
+ AF_IfNameIndex);
} else if (FunI == MemFunctionInfo.II_g_malloc0 ||
FunI == MemFunctionInfo.II_g_try_malloc0) {
if (CE->getNumArgs() < 1)
return;
SValBuilder &svalBuilder = C.getSValBuilder();
SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
- State = MallocMemAux(C, CE, CE->getArg(0), zeroVal, State);
+ State = MallocMemAux(C, CE, CE->getArg(0), zeroVal, State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 0, State);
} else if (FunI == MemFunctionInfo.II_g_memdup) {
if (CE->getNumArgs() < 2)
return;
- State = MallocMemAux(C, CE, CE->getArg(1), UndefinedVal(), State);
+ State =
+ MallocMemAux(C, CE, CE->getArg(1), UndefinedVal(), State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_g_malloc_n ||
FunI == MemFunctionInfo.II_g_try_malloc_n ||
@@ -1193,14 +1209,14 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
Init = SB.makeZeroVal(SB.getContext().CharTy);
}
SVal TotalSize = evalMulForBufferSize(C, CE->getArg(0), CE->getArg(1));
- State = MallocMemAux(C, CE, TotalSize, Init, State);
+ State = MallocMemAux(C, CE, TotalSize, Init, State, AF_Malloc);
State = ProcessZeroAllocCheck(C, CE, 0, State);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_g_realloc_n ||
FunI == MemFunctionInfo.II_g_try_realloc_n) {
if (CE->getNumArgs() < 3)
return;
- State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State,
+ State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State, AF_Malloc,
/*SuffixWithN*/ true);
State = ProcessZeroAllocCheck(C, CE, 1, State);
State = ProcessZeroAllocCheck(C, CE, 2, State);
@@ -1332,8 +1348,8 @@ static bool hasNonTrivialConstructorCall(const CXXNewExpr *NE) {
}
void MallocChecker::processNewAllocation(const CXXNewExpr *NE,
- CheckerContext &C,
- SVal Target) const {
+ CheckerContext &C, SVal Target,
+ AllocationFamily Family) const {
if (!MemFunctionInfo.isStandardNewDelete(NE->getOperatorNew(),
C.getASTContext()))
return;
@@ -1352,8 +1368,7 @@ void MallocChecker::processNewAllocation(const CXXNewExpr *NE,
// value (if any) and we don't want to loose this value. So we call
// MallocUpdateRefState() instead of MallocMemAux() which breaks the
// existing binding.
- State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray
- : AF_CXXNew, Target);
+ State = MallocUpdateRefState(C, NE, State, Family, Target);
State = addExtentSize(C, NE, State, Target);
State = ProcessZeroAllocCheck(C, NE, 0, State, Target);
C.addTransition(State);
@@ -1361,14 +1376,19 @@ void MallocChecker::processNewAllocation(const CXXNewExpr *NE,
void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
CheckerContext &C) const {
- if (!C.getAnalysisManager().getAnalyzerOptions().MayInlineCXXAllocator)
- processNewAllocation(NE, C, C.getSVal(NE));
+ if (!C.getAnalysisManager().getAnalyzerOptions().MayInlineCXXAllocator) {
+ if (NE->isArray())
+ processNewAllocation(NE, C, C.getSVal(NE),
+ (NE->isArray() ? AF_CXXNewArray : AF_CXXNew));
+ }
}
void MallocChecker::checkNewAllocator(const CXXNewExpr *NE, SVal Target,
CheckerContext &C) const {
- if (!C.wasInlined)
- processNewAllocation(NE, C, Target);
+ if (!C.wasInlined) {
+ processNewAllocation(NE, C, Target,
+ (NE->isArray() ? AF_CXXNewArray : AF_CXXNew));
+ }
}
// Sets the extent value of the MemRegion allocated by
@@ -1431,7 +1451,8 @@ void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
ProgramStateRef State = C.getState();
bool IsKnownToBeAllocated;
State = FreeMemAux(C, DE->getArgument(), DE, State,
- /*Hold*/ false, IsKnownToBeAllocated);
+ /*Hold*/ false, IsKnownToBeAllocated,
+ (DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew));
C.addTransition(State);
}
@@ -1477,7 +1498,7 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
bool IsKnownToBeAllocatedMemory;
ProgramStateRef State =
FreeMemAux(C, Call.getArgExpr(0), Call.getOriginExpr(), C.getState(),
- /*Hold=*/true, IsKnownToBeAllocatedMemory,
+ /*Hold=*/true, IsKnownToBeAllocatedMemory, AF_Malloc,
/*RetNullOnFailure=*/true);
C.addTransition(State);
@@ -1496,9 +1517,9 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
if (I != E) {
return MallocMemAux(C, CE, CE->getArg(I->getASTIndex()), UndefinedVal(),
- State);
+ State, AF_Malloc);
}
- return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), State);
+ return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), State, AF_Malloc);
}
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
@@ -1513,10 +1534,9 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
}
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
- const CallExpr *CE,
- SVal Size, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family) {
+ const CallExpr *CE, SVal Size,
+ SVal Init, ProgramStateRef State,
+ AllocationFamily Family) {
if (!State)
return nullptr;
@@ -1593,9 +1613,10 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
bool IsKnownToBeAllocated = false;
for (const auto &Arg : Att->args()) {
- ProgramStateRef StateI = FreeMemAux(
- C, CE, State, Arg.getASTIndex(),
- Att->getOwnKind() == OwnershipAttr::Holds, IsKnownToBeAllocated);
+ ProgramStateRef StateI =
+ FreeMemAux(C, CE, State, Arg.getASTIndex(),
+ Att->getOwnKind() == OwnershipAttr::Holds,
+ IsKnownToBeAllocated, AF_Malloc);
if (StateI)
State = StateI;
}
@@ -1605,6 +1626,7 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State, unsigned Num,
bool Hold, bool &IsKnownToBeAllocated,
+ AllocationFamily Family,
bool ReturnsNullOnFailure) const {
if (!State)
return nullptr;
@@ -1613,7 +1635,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
return nullptr;
return FreeMemAux(C, CE->getArg(Num), CE, State, Hold, IsKnownToBeAllocated,
- ReturnsNullOnFailure);
+ Family, ReturnsNullOnFailure);
}
/// Checks if the previous call to free on the given symbol failed - if free
@@ -1631,58 +1653,7 @@ static bool didPreviousFreeFail(ProgramStateRef State,
return false;
}
-static AllocationFamily
-getAllocationFamily(const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C,
- const Stmt *S) {
-
- if (!S)
- return AF_None;
-
- if (const CallExpr *CE = dyn_cast<CallExpr>(S)) {
- const FunctionDecl *FD = C.getCalleeDecl(CE);
-
- if (!FD)
- FD = dyn_cast<FunctionDecl>(CE->getCalleeDecl());
-
- ASTContext &Ctx = C.getASTContext();
-
- if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_Malloc,
- MemoryOperationKind::MOK_Any))
- return AF_Malloc;
-
- if (MemFunctionInfo.isStandardNewDelete(FD, Ctx)) {
- OverloadedOperatorKind Kind = FD->getOverloadedOperator();
- if (Kind == OO_New || Kind == OO_Delete)
- return AF_CXXNew;
- else if (Kind == OO_Array_New || Kind == OO_Array_Delete)
- return AF_CXXNewArray;
- }
-
- if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_IfNameIndex,
- MemoryOperationKind::MOK_Any))
- return AF_IfNameIndex;
-
- if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_Alloca,
- MemoryOperationKind::MOK_Any))
- return AF_Alloca;
-
- return AF_None;
- }
-
- if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(S))
- return NE->isArray() ? AF_CXXNewArray : AF_CXXNew;
-
- if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(S))
- return DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew;
-
- if (isa<ObjCMessageExpr>(S))
- return AF_Malloc;
-
- return AF_None;
-}
-
-static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
- const Expr *E) {
+static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) {
if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
// FIXME: This doesn't handle indirect calls.
const FunctionDecl *FD = CE->getDirectCallee();
@@ -1721,10 +1692,7 @@ static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
return false;
}
-static void printExpectedAllocName(raw_ostream &os,
- const MemFunctionInfoTy &MemFunctionInfo,
- CheckerContext &C, const Expr *E) {
- AllocationFamily Family = getAllocationFamily(MemFunctionInfo, C, E);
+static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) {
switch(Family) {
case AF_Malloc: os << "malloc()"; return;
@@ -1749,12 +1717,10 @@ static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) {
}
}
-ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
- const Expr *ArgExpr,
- const Expr *ParentExpr,
- ProgramStateRef State, bool Hold,
- bool &IsKnownToBeAllocated,
- bool ReturnsNullOnFailure) const {
+ProgramStateRef MallocChecker::FreeMemAux(
+ CheckerContext &C, const Expr *ArgExpr, const Expr *ParentExpr,
+ ProgramStateRef State, bool Hold, bool &IsKnownToBeAllocated,
+ AllocationFamily Family, bool ReturnsNullOnFailure) const {
if (!State)
return nullptr;
@@ -1784,7 +1750,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
// 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);
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
return nullptr;
}
@@ -1792,7 +1758,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
// Blocks might show up as heap data, but should not be free()d
if (isa<BlockDataRegion>(R)) {
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
return nullptr;
}
@@ -1812,7 +1778,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
if (isa<AllocaRegion>(R))
ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
else
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
return nullptr;
}
@@ -1851,9 +1817,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
RsBase->isEscaped()) {
// Check if an expected deallocation function matches the real one.
- bool DeallocMatchesAlloc =
- RsBase->getAllocationFamily() ==
- getAllocationFamily(MemFunctionInfo, C, ParentExpr);
+ bool DeallocMatchesAlloc = RsBase->getAllocationFamily() == Family;
if (!DeallocMatchesAlloc) {
ReportMismatchedDealloc(C, ArgExpr->getSourceRange(),
ParentExpr, RsBase, SymBase, Hold);
@@ -1868,14 +1832,15 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
Offset.getOffset() != 0) {
const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
- AllocExpr);
+ Family, AllocExpr);
return nullptr;
}
}
}
if (SymBase->getType()->isFunctionPointerType()) {
- ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+ Family);
return nullptr;
}
@@ -1893,9 +1858,12 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
}
}
- AllocationFamily Family =
- RsBase ? RsBase->getAllocationFamily()
- : getAllocationFamily(MemFunctionInfo, C, ParentExpr);
+ // If we don't know anything about this symbol, a free on it may be totally
+ // valid. If this is the case, lets assume that the allocation family of the
+ // freeing function is the same as the symbols allocation family, and go with
+ // that.
+ assert(!RsBase || (RsBase && RsBase->getAllocationFamily() == Family));
+
// Normal free.
if (Hold)
return State->set<RegionState>(SymBase,
@@ -1941,14 +1909,6 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family,
llvm_unreachable("unhandled family");
}
-Optional<MallocChecker::CheckKind>
-MallocChecker::getCheckIfTracked(CheckerContext &C,
- const Stmt *AllocDeallocStmt,
- bool IsALeakCheck) const {
- return getCheckIfTracked(
- getAllocationFamily(MemFunctionInfo, C, AllocDeallocStmt), IsALeakCheck);
-}
-
Optional<MallocChecker::CheckKind>
MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
bool IsALeakCheck) const {
@@ -2048,15 +2008,14 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os,
}
void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
- SourceRange Range,
- const Expr *DeallocExpr) const {
+ SourceRange Range, const Expr *DeallocExpr,
+ AllocationFamily Family) const {
if (!ChecksEnabled[CK_MallocChecker] &&
!ChecksEnabled[CK_NewDeleteChecker])
return;
- Optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(C, DeallocExpr);
+ Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
if (!CheckKind.hasValue())
return;
@@ -2073,7 +2032,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
MR = ER->getSuperRegion();
os << "Argument to ";
- if (!printAllocDeallocName(os, C, DeallocExpr))
+ if (!printMemFnName(os, C, DeallocExpr))
os << "deallocator";
os << " is ";
@@ -2084,7 +2043,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
else
os << "not memory allocated by ";
- printExpectedAllocName(os, MemFunctionInfo, C, DeallocExpr);
+ printExpectedAllocName(os, Family);
auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind],
os.str(), N);
@@ -2146,25 +2105,25 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
llvm::raw_svector_ostream DeallocOs(DeallocBuf);
if (OwnershipTransferred) {
- if (printAllocDeallocName(DeallocOs, C, DeallocExpr))
+ if (printMemFnName(DeallocOs, C, DeallocExpr))
os << DeallocOs.str() << " cannot";
else
os << "Cannot";
os << " take ownership of memory";
- if (printAllocDeallocName(AllocOs, C, AllocExpr))
+ if (printMemFnName(AllocOs, C, AllocExpr))
os << " allocated by " << AllocOs.str();
} else {
os << "Memory";
- if (printAllocDeallocName(AllocOs, C, AllocExpr))
+ if (printMemFnName(AllocOs, C, AllocExpr))
os << " allocated by " << AllocOs.str();
os << " should be deallocated by ";
printExpectedDeallocName(os, RS->getAllocationFamily());
- if (printAllocDeallocName(DeallocOs, C, DeallocExpr))
- os << ", not " << DeallocOs.str();
+ if (printMemFnName(DeallocOs, C, DeallocExpr))
+ os << ", not " << DeallocOs.str();
}
auto R = std::make_unique<PathSensitiveBugReport>(*BT_MismatchedDealloc,
@@ -2178,15 +2137,14 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
SourceRange Range, const Expr *DeallocExpr,
+ AllocationFamily Family,
const Expr *AllocExpr) const {
-
if (!ChecksEnabled[CK_MallocChecker] &&
!ChecksEnabled[CK_NewDeleteChecker])
return;
- Optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(C, AllocExpr);
+ Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
if (!CheckKind.hasValue())
return;
@@ -2215,14 +2173,14 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
int offsetBytes = Offset.getOffset() / C.getASTContext().getCharWidth();
os << "Argument to ";
- if (!printAllocDeallocName(os, C, DeallocExpr))
+ if (!printMemFnName(os, C, DeallocExpr))
os << "deallocator";
os << " is offset by "
<< offsetBytes
<< " "
<< ((abs(offsetBytes) > 1) ? "bytes" : "byte")
<< " from the start of ";
- if (AllocExpr && printAllocDeallocName(AllocNameOs, C, AllocExpr))
+ if (AllocExpr && printMemFnName(AllocNameOs, C, AllocExpr))
os << "memory allocated by " << AllocNameOs.str();
else
os << "allocated memory";
@@ -2360,11 +2318,12 @@ void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
SourceRange Range,
- const Expr *FreeExpr) const {
+ const Expr *FreeExpr,
+ AllocationFamily Family) const {
if (!ChecksEnabled[CK_MallocChecker])
return;
- Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, FreeExpr);
+ Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family);
if (!CheckKind.hasValue())
return;
@@ -2381,7 +2340,7 @@ void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
MR = ER->getSuperRegion();
Os << "Argument to ";
- if (!printAllocDeallocName(Os, C, FreeExpr))
+ if (!printMemFnName(Os, C, FreeExpr))
Os << "deallocator";
Os << " is a function pointer";
@@ -2394,11 +2353,10 @@ void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
}
}
-ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
- const CallExpr *CE,
- bool ShouldFreeOnFail,
- ProgramStateRef State,
- bool SuffixWithN) const {
+ProgramStateRef
+MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE,
+ bool ShouldFreeOnFail, ProgramStateRef State,
+ AllocationFamily Family, bool SuffixWithN) const {
if (!State)
return nullptr;
@@ -2445,8 +2403,8 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
// If the ptr is NULL and the size is not 0, the call is equivalent to
// malloc(size).
if (PrtIsNull && !SizeIsZero) {
- ProgramStateRef stateMalloc = MallocMemAux(C, CE, TotalSize,
- UndefinedVal(), StatePtrIsNull);
+ ProgramStateRef stateMalloc =
+ MallocMemAux(C, CE, TotalSize, UndefinedVal(), StatePtrIsNull, Family);
return stateMalloc;
}
@@ -2469,16 +2427,16 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
// If size was equal to 0, either NULL or a pointer suitable to be passed
// to free() is returned. We just free the input pointer and do not add
// any constrains on the output pointer.
- if (ProgramStateRef stateFree =
- FreeMemAux(C, CE, StateSizeIsZero, 0, false, IsKnownToBeAllocated))
+ if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero, 0, false,
+ IsKnownToBeAllocated, Family))
return stateFree;
// Default behavior.
if (ProgramStateRef stateFree =
- FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocated)) {
+ FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocated, Family)) {
- ProgramStateRef stateRealloc = MallocMemAux(C, CE, TotalSize,
- UnknownVal(), stateFree);
+ ProgramStateRef stateRealloc =
+ MallocMemAux(C, CE, TotalSize, UnknownVal(), stateFree, Family);
if (!stateRealloc)
return nullptr;
@@ -2511,7 +2469,7 @@ ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE,
SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
SVal TotalSize = evalMulForBufferSize(C, CE->getArg(0), CE->getArg(1));
- return MallocMemAux(C, CE, TotalSize, zeroVal, State);
+ return MallocMemAux(C, CE, TotalSize, zeroVal, State, AF_Malloc);
}
MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
More information about the cfe-commits
mailing list