Hi zaks.anna,
Proposed patch implements undefbehavior.ZeroAllocDereference checker as part of
unix.Malloc and cplusplus.NewDelete checkers.
The patch doesn't handle zero-size realloc()s due to specifics of current
handling of realloc() by unix.Malloc that need to be discussed.
Please review!
http://reviews.llvm.org/D8273
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
test/Analysis/NewDelete-checker-test.cpp
test/Analysis/NewDelete-intersections.mm
test/Analysis/malloc.c
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -161,10 +161,10 @@
{
public:
MallocChecker()
- : II_alloca(nullptr), II_malloc(nullptr), II_free(nullptr),
- II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr),
- II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr),
- II_kmalloc(nullptr), II_if_nameindex(nullptr),
+ : II_alloca(nullptr), II_builtin_alloca(nullptr), II_malloc(nullptr),
+ II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr),
+ II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr),
+ II_strdup(nullptr), II_kmalloc(nullptr), II_if_nameindex(nullptr),
II_if_freenameindex(nullptr) {}
/// In pessimistic mode, the checker assumes that it does not know which
@@ -222,11 +222,15 @@
mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
- mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc,
- *II_calloc, *II_valloc, *II_reallocf, *II_strndup,
- *II_strdup, *II_kmalloc, *II_if_nameindex,
+ mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
+ mutable IdentifierInfo *II_alloca, *II_builtin_alloca, *II_malloc, *II_free,
+ *II_realloc, *II_calloc, *II_valloc, *II_reallocf,
+ *II_strndup, *II_strdup, *II_kmalloc, *II_if_nameindex,
*II_if_freenameindex;
mutable Optional<uint64_t> KernelZeroFlagVal;
+ // A set of allocation statements allocating zero bytes. Used to detect
+ // manipulations with zero-allocated memory.
+ mutable llvm::DenseSet<const Stmt*> ZeroAllocated;
void initIdentifierInfo(ASTContext &C) const;
@@ -257,6 +261,12 @@
MemoryOperationKind MemKind) const;
bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const;
///@}
+
+ /// \brief Perform a zero-allocation check.
+ ProgramStateRef ZeroAllocationCheck(CheckerContext &C, const Expr *E,
+ const unsigned AllocationSizeArg,
+ ProgramStateRef State) const;
+
ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
const CallExpr *CE,
const OwnershipAttr* Att,
@@ -307,6 +317,9 @@
bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const;
+ void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
+ const Stmt *S) const;
+
bool checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const;
/// Check if the function is known free memory, or if it is
@@ -361,6 +374,9 @@
void ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const;
+ void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
+ SymbolRef Sym) const;
+
/// Find the location of the allocation for Sym on the path leading to the
/// exploded node N.
LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
@@ -507,6 +523,7 @@
if (II_malloc)
return;
II_alloca = &Ctx.Idents.get("alloca");
+ II_builtin_alloca = &Ctx.Idents.get("__builtin_alloca");
II_malloc = &Ctx.Idents.get("malloc");
II_free = &Ctx.Idents.get("free");
II_realloc = &Ctx.Idents.get("realloc");
@@ -728,6 +745,9 @@
if (FunI == II_malloc) {
if (CE->getNumArgs() < 1)
return;
+ else if (CE->getNumArgs() == 1)
+ State = ZeroAllocationCheck(C, CE, 0, State);
+
if (CE->getNumArgs() < 3) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
} else if (CE->getNumArgs() == 3) {
@@ -749,12 +769,17 @@
if (CE->getNumArgs() < 1)
return;
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+ State = ZeroAllocationCheck(C, CE, 0, State);
} else if (FunI == II_realloc) {
State = ReallocMem(C, CE, false, State);
+ State = ZeroAllocationCheck(C, CE, 1, State);
} else if (FunI == II_reallocf) {
State = ReallocMem(C, CE, true, State);
+ State = ZeroAllocationCheck(C, CE, 1, State);
} else if (FunI == II_calloc) {
State = CallocMem(C, CE, State);
+ State = ZeroAllocationCheck(C, CE, 0, State);
+ State = ZeroAllocationCheck(C, CE, 1, State);
} else if (FunI == II_free) {
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
} else if (FunI == II_strdup) {
@@ -764,18 +789,26 @@
} else if (FunI == II_alloca) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
+ State = ZeroAllocationCheck(C, CE, 0, State);
+ }
+ else if (FunI == II_builtin_alloca) {
+ State = ZeroAllocationCheck(C, CE, 0, State);
} else if (isStandardNewDelete(FD, C.getASTContext())) {
// Process direct calls to operator new/new[]/delete/delete[] functions
// as distinct from new/new[]/delete/delete[] expressions that are
// processed by the checkPostStmt callbacks for CXXNewExpr and
// CXXDeleteExpr.
OverloadedOperatorKind K = FD->getOverloadedOperator();
- if (K == OO_New)
+ if (K == OO_New) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_CXXNew);
- else if (K == OO_Array_New)
+ State = ZeroAllocationCheck(C, CE, 0, State);
+ }
+ else if (K == OO_Array_New) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_CXXNewArray);
+ State = ZeroAllocationCheck(C, CE, 0, State);
+ }
else if (K == OO_Delete || K == OO_Array_Delete)
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
else
@@ -809,6 +842,55 @@
C.addTransition(State);
}
+// Performs a 0-sized allocations check.
+ProgramStateRef MallocChecker::ZeroAllocationCheck(CheckerContext &C,
+ const Expr *E,
+ const unsigned AllocationSizeArg,
+ ProgramStateRef State) const {
+ if (!State)
+ return nullptr;
+
+ const Expr *Arg = nullptr;
+
+ if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+ Arg = CE->getArg(AllocationSizeArg);
+ }
+ else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
+ if (NE->isArray())
+ Arg = NE->getArraySize();
+ else
+ return State;
+ }
+ else
+ llvm_unreachable("not a CallExpr or CXXNewExpr");
+
+ assert(Arg);
+
+ Optional<DefinedSVal> DefArgVal =
+ State->getSVal(Arg, C.getLocationContext()).getAs<DefinedSVal>();
+
+ if (!DefArgVal)
+ return State;
+
+ // Check if the allocation size is 0.
+ ProgramStateRef TrueState, FalseState;
+ SValBuilder &SvalBuilder = C.getSValBuilder();
+ DefinedSVal Zero =
+ SvalBuilder.makeZeroVal(Arg->getType()).castAs<DefinedSVal>();
+
+ std::tie(TrueState, FalseState) =
+ State->assume(SvalBuilder.evalEQ(State, *DefArgVal, Zero));
+
+ if (TrueState && !FalseState) {
+ ZeroAllocated.insert(E);
+ return TrueState;
+ }
+
+ // Assume the value is non-zero going forward.
+ assert(FalseState);
+ return FalseState;
+}
+
static QualType getDeepPointeeType(QualType T) {
QualType Result = T, PointeeType = T->getPointeeType();
while (!PointeeType.isNull()) {
@@ -868,6 +950,7 @@
// existing binding.
State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray
: AF_CXXNew);
+ State = ZeroAllocationCheck(C, NE, 0, State);
C.addTransition(State);
}
@@ -1741,6 +1824,47 @@
}
}
+// Note: nullptr Sym means that the report is for __builtin_alloca.
+void MallocChecker::ReportUseZeroAllocated(CheckerContext &C,
+ SourceRange Range,
+ SymbolRef Sym) const {
+
+ if (!ChecksEnabled[CK_MallocChecker] &&
+ !ChecksEnabled[CK_NewDeleteChecker])
+ return;
+
+ Optional<MallocChecker::CheckKind> CheckKind;
+ if (Sym) {
+ CheckKind = getCheckIfTracked(C, Sym);
+ }
+ else {
+ // nullptr Sym means that the report is for __builtin_alloca whose return
+ // value is bound to AllocaRegion and we do not break this binding in
+ // MallocChecker by binding return value to a new symbolic value.
+ if (ChecksEnabled[CK_MallocChecker])
+ CheckKind = CK_MallocChecker;
+ }
+
+ if (!CheckKind.hasValue())
+ return;
+
+ if (ExplodedNode *N = C.generateSink()) {
+ if (!BT_UseZerroAllocated[*CheckKind])
+ BT_UseZerroAllocated[*CheckKind].reset(new BugType(
+ CheckNames[*CheckKind], "Use zero allocated", "Memory Error"));
+
+ BugReport *R = new BugReport(*BT_UseZerroAllocated[*CheckKind],
+ "Use of zero-allocated memory", N);
+
+ R->addRange(Range);
+ if (Sym) {
+ R->markInteresting(Sym);
+ R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym));
+ }
+ C.emitReport(R);
+ }
+}
+
ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C,
const CallExpr *CE,
bool FreesOnFail,
@@ -2156,6 +2280,19 @@
return false;
}
+void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C,
+ const Stmt *S) const {
+ assert(Sym);
+ const RefState *RS = C.getState()->get<RegionState>(Sym);
+
+ if (RS && (RS->isAllocated() || RS->isRelinquished())) {
+ if (const Stmt *RSStmt = RS->getStmt()) {
+ if (ZeroAllocated.count(RSStmt))
+ ReportUseZeroAllocated(C, S->getSourceRange(), Sym);
+ }
+ }
+}
+
bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const {
if (isReleased(Sym, C)) {
@@ -2169,8 +2306,16 @@
void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S,
CheckerContext &C) const {
SymbolRef Sym = l.getLocSymbolInBase();
- if (Sym)
+ const MemRegion *MR = l.getAsRegion()->StripCasts();
+
+ if (Sym) {
checkUseAfterFree(Sym, C, S);
+ checkUseZeroAllocated(Sym, C, S);
+ }
+ else if (const AllocaRegion *AR = dyn_cast<AllocaRegion>(MR)) {
+ if (ZeroAllocated.count(AR->getExpr()))
+ ReportUseZeroAllocated(C, S->getSourceRange(), nullptr);
+ }
}
// If a symbolic region is assumed to NULL (or another constant), stop tracking
Index: test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
===================================================================
--- test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
+++ test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp
@@ -24,5 +24,16 @@
int *p4 = new int;
delete p4;
- int j = *p4; // no-warning
+ int j = *p4; // no-warning
+}
+
+void testUseZeroAllocNoWarn() {
+ int *p1 = (int *)operator new(0);
+ *p1 = 1; // no-warning
+
+ int *p2 = (int *)operator new[](0);
+ p2[0] = 1; // no-warning
+
+ int *p3 = new int[0];
+ p3[0] = 1; // no-warning
}
Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -87,6 +87,30 @@
new (w) PtrWrapper(new int); // no warn
}
+//-----------------------------------------
+// check for usage of zero-allocated memory
+//-----------------------------------------
+
+void testUseZeroAlloc1() {
+ int *p = (int *)operator new(0);
+ *p = 1; // expected-warning {{Use of zero-allocated memory}}
+ delete p;
+}
+
+int testUseZeroAlloc2() {
+ int *p = (int *)operator new[](0);
+ return p[0]; // expected-warning {{Use of zero-allocated memory}}
+ delete[] p;
+}
+
+void f(int);
+
+void testUseZeroAlloc3() {
+ int *p = new int[0];
+ f(*p); // expected-warning {{Use of zero-allocated memory}}
+ delete[] p;
+}
+
//---------------
// other checks
//---------------
Index: test/Analysis/NewDelete-intersections.mm
===================================================================
--- test/Analysis/NewDelete-intersections.mm
+++ test/Analysis/NewDelete-intersections.mm
@@ -43,6 +43,14 @@
delete p2; // no warn
}
+void testUseZeroAllocatedMalloced() {
+ int *p1 = (int *)malloc(0);
+ *p1 = 1; // no warn
+
+ int *p2 = (int *)__builtin_alloca(0);
+ *p2 = 1; // no warn
+}
+
//----- Test free standard new
void testFreeOpNew() {
void *p = operator new(0);
Index: test/Analysis/malloc.c
===================================================================
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -200,6 +200,54 @@
char *r = reallocf(0, 12);
} // expected-warning {{Potential leak of memory pointed to by}}
+//------------------- Check usage of zero-allocated memory ---------------------
+void CheckUseZeroAllocatedNoWarn1() {
+ int *p = malloc(0);
+ free(p); // no warning
+}
+
+void CheckUseZeroAllocatedNoWarn2() {
+ int *p = __builtin_alloca(0); // no warning
+}
+
+void CheckUseZeroAllocated1() {
+ int *p = malloc(0);
+ *p = 1; // expected-warning {{Use of zero-allocated memory}}
+ free(p);
+}
+
+char CheckUseZeroAllocated2() {
+ char *p = alloca(0);
+ return *p; // expected-warning {{Use of zero-allocated memory}}
+}
+
+void UseZeroAllocated(int *p) {
+ if (p)
+ *p = 7; // expected-warning {{Use of zero-allocated memory}}
+}
+void CheckUseZeroAllocated3() {
+ int *p = __builtin_alloca(0);
+ UseZeroAllocated(p);
+}
+
+void f(char);
+void CheckUseZeroAllocated4() {
+ char *p = valloc(0);
+ f(*p); // expected-warning {{Use of zero-allocated memory}}
+ free(p);
+}
+
+void CheckUseZeroAllocated5() {
+ int *p = calloc(0, 2);
+ *p = 1; // expected-warning {{Use of zero-allocated memory}}
+ free(p);
+}
+
+void CheckUseZeroAllocated6() {
+ int *p = calloc(2, 0);
+ *p = 1; // expected-warning {{Use of zero-allocated memory}}
+ free(p);
+}
// This case tests that storing malloc'ed memory to a static variable which is
// then returned is not leaked. In the absence of known contracts for functions
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
