Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, Charusso, dcoughlin,
baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, whisperity.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D68162
Files:
clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ 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"
@@ -63,7 +64,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>
@@ -71,7 +72,10 @@
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 {
@@ -91,22 +95,14 @@
} // 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.
@@ -205,9 +201,9 @@
/// Update the RefState to reflect the new memory allocation.
/// The optional \p RetVal parameter specifies the newly allocated pointer
/// value; if unspecified, the value of expression \p E is used.
+template <AllocationFamily Family>
static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
ProgramStateRef State,
- AllocationFamily Family = AF_Malloc,
Optional<SVal> RetVal = None);
//===----------------------------------------------------------------------===//
@@ -400,6 +396,7 @@
/// Process C++ operator new()'s allocation, which is the part of C++
/// new-expression that goes before the constructor.
+ template <AllocationFamily Family>
void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C,
SVal Target) const;
@@ -446,10 +443,10 @@
/// malloc leaves it undefined.
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
+ template <AllocationFamily Family>
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
const Expr *SizeEx, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family = AF_Malloc);
+ ProgramStateRef State);
/// Models memory allocation.
///
@@ -460,10 +457,10 @@
/// malloc leaves it undefined.
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
+ template <AllocationFamily Family>
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
SVal Size, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family = AF_Malloc);
+ ProgramStateRef State);
static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE,
ProgramStateRef State, SVal Target);
@@ -514,6 +511,7 @@
/// \param [in] ReturnsNullOnFailure Whether the memory deallocation function
/// we're modeling returns with Null on failure.
/// \returns The ProgramState right after deallocation.
+ template <AllocationFamily Family>
ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State, unsigned Num, bool Hold,
bool &IsKnownToBeAllocated,
@@ -538,6 +536,7 @@
/// \param [in] ReturnsNullOnFailure Whether the memory deallocation function
/// we're modeling returns with Null on failure.
/// \returns The ProgramState right after deallocation.
+ template <AllocationFamily Family>
ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
const Expr *ParentExpr, ProgramStateRef State,
bool Hold, bool &IsKnownToBeAllocated,
@@ -556,6 +555,7 @@
/// \param [in] SuffixWithN Whether the reallocation function we're modeling
/// has an '_n' suffix, such as g_realloc_n.
/// \returns The ProgramState right after reallocation.
+ template <AllocationFamily Family>
ProgramStateRef ReallocMemAux(CheckerContext &C, const CallExpr *CE,
bool ShouldFreeOnFail, ProgramStateRef State,
bool SuffixWithN = false) const;
@@ -622,27 +622,37 @@
/// family/call/symbol.
Optional<CheckKind> getCheckIfTracked(AllocationFamily Family,
bool IsALeakCheck = false) const;
+
+ template <AllocationFamily Family>
Optional<CheckKind> getCheckIfTracked(CheckerContext &C,
const Stmt *AllocDeallocStmt,
bool IsALeakCheck = false) const;
+
Optional<CheckKind> getCheckIfTracked(CheckerContext &C, SymbolRef Sym,
bool IsALeakCheck = false) const;
///@}
static bool SummarizeValue(raw_ostream &os, SVal V);
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
+ template <AllocationFamily Family>
void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
const Expr *DeallocExpr) 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;
+
+ template <AllocationFamily Family>
void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
const Expr *DeallocExpr,
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;
@@ -651,6 +661,7 @@
void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
SymbolRef Sym) const;
+ template <AllocationFamily Family>
void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
SourceRange Range, const Expr *FreeExpr) const;
@@ -1035,7 +1046,7 @@
// 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<AF_Malloc>(C, CE, CE->getArg(0), ZeroVal, TrueState);
}
return None;
@@ -1074,11 +1085,13 @@
default:
return;
case 1:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(),
+ State);
State = ProcessZeroAllocCheck(C, CE, 0, State);
break;
case 2:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(),
+ State);
break;
case 3:
llvm::Optional<ProgramStateRef> MaybeState =
@@ -1086,7 +1099,8 @@
if (MaybeState.hasValue())
State = MaybeState.getValue();
else
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(),
+ State);
break;
}
} else if (FunI == MemFunctionInfo.II_kmalloc) {
@@ -1097,19 +1111,22 @@
if (MaybeState.hasValue())
State = MaybeState.getValue();
else
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(),
+ State);
} else if (FunI == MemFunctionInfo.II_valloc) {
if (CE->getNumArgs() < 1)
return;
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State =
+ MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
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<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ false, State);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_reallocf) {
- State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ true, State);
+ State = ReallocMemAux<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ true, State);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_calloc) {
State = CallocMem(C, CE, State);
@@ -1121,20 +1138,21 @@
if (suppressDeallocationsInSuspiciousContexts(CE, C))
return;
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux<AF_Malloc>(C, CE, State, 0, false,
+ IsKnownToBeAllocatedMemory);
} 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<AF_Malloc>(C, CE, State);
} else if (FunI == MemFunctionInfo.II_strndup) {
- State = MallocUpdateRefState(C, CE, State);
+ State = MallocUpdateRefState<AF_Malloc>(C, CE, State);
} 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<AF_Alloca>(C, CE, CE->getArg(0), UndefinedVal(), State);
State = ProcessZeroAllocCheck(C, CE, 0, State);
} else if (MemFunctionInfo.isStandardNewDelete(FD, C.getASTContext())) {
// Process direct calls to operator new/new[]/delete/delete[] functions
@@ -1143,18 +1161,22 @@
// CXXDeleteExpr.
switch (FD->getOverloadedOperator()) {
case OO_New:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
- AF_CXXNew);
+ State = MallocMemAux<AF_CXXNew>(C, CE, CE->getArg(0), UndefinedVal(),
+ State);
State = ProcessZeroAllocCheck(C, CE, 0, State);
break;
case OO_Array_New:
- State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
- AF_CXXNewArray);
+ State = MallocMemAux<AF_CXXNewArray>(C, CE, CE->getArg(0),
+ UndefinedVal(), State);
State = ProcessZeroAllocCheck(C, CE, 0, State);
break;
case OO_Delete:
+ State = FreeMemAux<AF_CXXNew>(C, CE, State, 0, false,
+ IsKnownToBeAllocatedMemory);
+ break;
case OO_Array_Delete:
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux<AF_CXXNewArray>(C, CE, State, 0, false,
+ IsKnownToBeAllocatedMemory);
break;
default:
llvm_unreachable("not a new/delete operator");
@@ -1162,22 +1184,24 @@
} else if (FunI == MemFunctionInfo.II_if_nameindex) {
// Should we model this differently? We can allocate a fixed number of
// elements with zeros in the last one.
- State = MallocMemAux(C, CE, UnknownVal(), UnknownVal(), State,
- AF_IfNameIndex);
+ State = MallocMemAux<AF_IfNameIndex>(C, CE, UnknownVal(), UnknownVal(),
+ State);
} else if (FunI == MemFunctionInfo.II_if_freenameindex) {
- State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+ State = FreeMemAux<AF_IfNameIndex>(C, CE, State, 0, false,
+ IsKnownToBeAllocatedMemory);
} 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<AF_Malloc>(C, CE, CE->getArg(0), zeroVal, State);
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<AF_Malloc>(C, CE, CE->getArg(1), UndefinedVal(), State);
State = ProcessZeroAllocCheck(C, CE, 1, State);
} else if (FunI == MemFunctionInfo.II_g_malloc_n ||
FunI == MemFunctionInfo.II_g_try_malloc_n ||
@@ -1192,15 +1216,15 @@
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<AF_Malloc>(C, CE, TotalSize, Init, State);
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,
- /*SuffixWithN*/ true);
+ State = ReallocMemAux<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ false, State,
+ /*SuffixWithN*/ true);
State = ProcessZeroAllocCheck(C, CE, 1, State);
State = ProcessZeroAllocCheck(C, CE, 2, State);
}
@@ -1330,9 +1354,9 @@
return false;
}
+template <AllocationFamily Family>
void MallocChecker::processNewAllocation(const CXXNewExpr *NE,
- CheckerContext &C,
- SVal Target) const {
+ CheckerContext &C, SVal Target) const {
if (!MemFunctionInfo.isStandardNewDelete(NE->getOperatorNew(),
C.getASTContext()))
return;
@@ -1351,8 +1375,7 @@
// 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<Family>(C, NE, State, Target);
State = addExtentSize(C, NE, State, Target);
State = ProcessZeroAllocCheck(C, NE, 0, State, Target);
C.addTransition(State);
@@ -1360,14 +1383,22 @@
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<AF_CXXNewArray>(NE, C, C.getSVal(NE));
+ else
+ processNewAllocation<AF_CXXNew>(NE, C, C.getSVal(NE));
+ }
}
void MallocChecker::checkNewAllocator(const CXXNewExpr *NE, SVal Target,
CheckerContext &C) const {
- if (!C.wasInlined)
- processNewAllocation(NE, C, Target);
+ if (!C.wasInlined) {
+ if (NE->isArray())
+ processNewAllocation<AF_CXXNewArray>(NE, C, Target);
+ else
+ processNewAllocation<AF_CXXNew>(NE, C, Target);
+ }
}
// Sets the extent value of the MemRegion allocated by
@@ -1428,8 +1459,12 @@
ProgramStateRef State = C.getState();
bool IsKnownToBeAllocated;
- State = FreeMemAux(C, DE->getArgument(), DE, State,
- /*Hold*/ false, IsKnownToBeAllocated);
+ if (DE->isArrayForm())
+ State = FreeMemAux<AF_CXXNewArray>(C, DE->getArgument(), DE, State,
+ /*Hold*/ false, IsKnownToBeAllocated);
+ else
+ State = FreeMemAux<AF_CXXNew>(C, DE->getArgument(), DE, State,
+ /*Hold*/ false, IsKnownToBeAllocated);
C.addTransition(State);
}
@@ -1470,10 +1505,10 @@
return;
bool IsKnownToBeAllocatedMemory;
- ProgramStateRef State =
- FreeMemAux(C, Call.getArgExpr(0), Call.getOriginExpr(), C.getState(),
- /*Hold=*/true, IsKnownToBeAllocatedMemory,
- /*RetNullOnFailure=*/true);
+ ProgramStateRef State = FreeMemAux<AF_Malloc>(
+ C, Call.getArgExpr(0), Call.getOriginExpr(), C.getState(),
+ /*Hold=*/true, IsKnownToBeAllocatedMemory,
+ /*RetNullOnFailure=*/true);
C.addTransition(State);
}
@@ -1490,28 +1525,27 @@
OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
if (I != E) {
- return MallocMemAux(C, CE, CE->getArg(I->getASTIndex()), UndefinedVal(),
- State);
+ return MallocMemAux<AF_Malloc>(C, CE, CE->getArg(I->getASTIndex()),
+ UndefinedVal(), State);
}
- return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), State);
+ return MallocMemAux<AF_Malloc>(C, CE, UnknownVal(), UndefinedVal(), State);
}
+template <AllocationFamily Family>
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
const CallExpr *CE,
const Expr *SizeEx, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family) {
+ ProgramStateRef State) {
if (!State)
return nullptr;
- return MallocMemAux(C, CE, C.getSVal(SizeEx), Init, State, Family);
+ return MallocMemAux<Family>(C, CE, C.getSVal(SizeEx), Init, State);
}
+template <AllocationFamily Family>
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) {
if (!State)
return nullptr;
@@ -1548,12 +1582,12 @@
assert(State);
}
- return MallocUpdateRefState(C, CE, State, Family);
+ return MallocUpdateRefState<Family>(C, CE, State);
}
+template <AllocationFamily Family>
static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
ProgramStateRef State,
- AllocationFamily Family,
Optional<SVal> RetVal) {
if (!State)
return nullptr;
@@ -1588,7 +1622,7 @@
bool IsKnownToBeAllocated = false;
for (const auto &Arg : Att->args()) {
- ProgramStateRef StateI = FreeMemAux(
+ ProgramStateRef StateI = FreeMemAux<AF_Malloc>(
C, CE, State, Arg.getASTIndex(),
Att->getOwnKind() == OwnershipAttr::Holds, IsKnownToBeAllocated);
if (StateI)
@@ -1597,6 +1631,7 @@
return State;
}
+template <AllocationFamily Family>
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State, unsigned Num,
bool Hold, bool &IsKnownToBeAllocated,
@@ -1607,8 +1642,8 @@
if (CE->getNumArgs() < (Num + 1))
return nullptr;
- return FreeMemAux(C, CE->getArg(Num), CE, State, Hold, IsKnownToBeAllocated,
- ReturnsNullOnFailure);
+ return FreeMemAux<Family>(C, CE->getArg(Num), CE, State, Hold,
+ IsKnownToBeAllocated, ReturnsNullOnFailure);
}
/// Checks if the previous call to free on the given symbol failed - if free
@@ -1626,58 +1661,7 @@
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();
@@ -1716,10 +1700,7 @@
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;
@@ -1744,12 +1725,12 @@
}
}
-ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
- const Expr *ArgExpr,
- const Expr *ParentExpr,
- ProgramStateRef State, bool Hold,
- bool &IsKnownToBeAllocated,
- bool ReturnsNullOnFailure) const {
+template <AllocationFamily Family>
+ProgramStateRef
+MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr,
+ const Expr *ParentExpr, ProgramStateRef State,
+ bool Hold, bool &IsKnownToBeAllocated,
+ bool ReturnsNullOnFailure) const {
if (!State)
return nullptr;
@@ -1779,7 +1760,7 @@
// 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<Family>(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return nullptr;
}
@@ -1787,7 +1768,7 @@
// Blocks might show up as heap data, but should not be free()d
if (isa<BlockDataRegion>(R)) {
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportBadFree<Family>(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return nullptr;
}
@@ -1807,7 +1788,7 @@
if (isa<AllocaRegion>(R))
ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange());
else
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportBadFree<Family>(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return nullptr;
}
@@ -1846,9 +1827,7 @@
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);
@@ -1862,15 +1841,16 @@
!Offset.hasSymbolicOffset() &&
Offset.getOffset() != 0) {
const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
- ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
- AllocExpr);
+ ReportOffsetFree<Family>(C, ArgVal, ArgExpr->getSourceRange(),
+ ParentExpr, AllocExpr);
return nullptr;
}
}
}
if (SymBase->getType()->isFunctionPointerType()) {
- ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+ ReportFunctionPointerFree<Family>(C, ArgVal, ArgExpr->getSourceRange(),
+ ParentExpr);
return nullptr;
}
@@ -1888,9 +1868,12 @@
}
}
- 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,
@@ -1936,12 +1919,10 @@
llvm_unreachable("unhandled family");
}
-Optional<MallocChecker::CheckKind>
-MallocChecker::getCheckIfTracked(CheckerContext &C,
- const Stmt *AllocDeallocStmt,
- bool IsALeakCheck) const {
- return getCheckIfTracked(
- getAllocationFamily(MemFunctionInfo, C, AllocDeallocStmt), IsALeakCheck);
+template <AllocationFamily Family>
+Optional<MallocChecker::CheckKind> MallocChecker::getCheckIfTracked(
+ CheckerContext &C, const Stmt *AllocDeallocStmt, bool IsALeakCheck) const {
+ return getCheckIfTracked(Family, IsALeakCheck);
}
Optional<MallocChecker::CheckKind>
@@ -2042,6 +2023,7 @@
}
}
+template <AllocationFamily Family>
void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
SourceRange Range,
const Expr *DeallocExpr) const {
@@ -2051,7 +2033,7 @@
return;
Optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(C, DeallocExpr);
+ getCheckIfTracked<Family>(C, DeallocExpr);
if (!CheckKind.hasValue())
return;
@@ -2068,7 +2050,7 @@
MR = ER->getSuperRegion();
os << "Argument to ";
- if (!printAllocDeallocName(os, C, DeallocExpr))
+ if (!printMemFnName(os, C, DeallocExpr))
os << "deallocator";
os << " is ";
@@ -2079,7 +2061,7 @@
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);
@@ -2141,25 +2123,25 @@
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,
@@ -2171,17 +2153,17 @@
}
}
+template <AllocationFamily Family>
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
SourceRange Range, const Expr *DeallocExpr,
const Expr *AllocExpr) const {
-
if (!ChecksEnabled[CK_MallocChecker] &&
!ChecksEnabled[CK_NewDeleteChecker])
return;
Optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(C, AllocExpr);
+ getCheckIfTracked<Family>(C, AllocExpr);
if (!CheckKind.hasValue())
return;
@@ -2210,14 +2192,14 @@
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";
@@ -2353,13 +2335,15 @@
}
}
+template <AllocationFamily Family>
void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
SourceRange Range,
const Expr *FreeExpr) const {
if (!ChecksEnabled[CK_MallocChecker])
return;
- Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, FreeExpr);
+ Optional<MallocChecker::CheckKind> CheckKind =
+ getCheckIfTracked<Family>(C, FreeExpr);
if (!CheckKind.hasValue())
return;
@@ -2376,7 +2360,7 @@
MR = ER->getSuperRegion();
Os << "Argument to ";
- if (!printAllocDeallocName(Os, C, FreeExpr))
+ if (!printMemFnName(Os, C, FreeExpr))
Os << "deallocator";
Os << " is a function pointer";
@@ -2389,11 +2373,11 @@
}
}
-ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
- const CallExpr *CE,
- bool ShouldFreeOnFail,
- ProgramStateRef State,
- bool SuffixWithN) const {
+template <AllocationFamily Family>
+ProgramStateRef
+MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE,
+ bool ShouldFreeOnFail, ProgramStateRef State,
+ bool SuffixWithN) const {
if (!State)
return nullptr;
@@ -2440,8 +2424,8 @@
// 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<Family>(C, CE, TotalSize, UndefinedVal(), StatePtrIsNull);
return stateMalloc;
}
@@ -2464,16 +2448,16 @@
// 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<Family>(
+ C, CE, StateSizeIsZero, 0, false, IsKnownToBeAllocated))
return stateFree;
// Default behavior.
if (ProgramStateRef stateFree =
- FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocated)) {
+ FreeMemAux<Family>(C, CE, State, 0, false, IsKnownToBeAllocated)) {
- ProgramStateRef stateRealloc = MallocMemAux(C, CE, TotalSize,
- UnknownVal(), stateFree);
+ ProgramStateRef stateRealloc =
+ MallocMemAux<Family>(C, CE, TotalSize, UnknownVal(), stateFree);
if (!stateRealloc)
return nullptr;
@@ -2506,7 +2490,7 @@
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<AF_Malloc>(C, CE, TotalSize, zeroVal, State);
}
MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
Index: clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
+++ 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_ */
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
