From 7304027c6d8505598f5bbb58022109d33d1c926c Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 19 Sep 2016 20:39:52 +0000 Subject: [PATCH] [analyzer] Calculate extent size for memory regions allocated by new expression. ArrayBoundChecker did not detect out of bounds memory access errors in case an array was allocated by the new expression. This patch resolves this issue. Patch by Daniel Krupp! Differential Revision: https://reviews.llvm.org/D24307 llvm-svn: 281934 --- .../Checkers/ArrayBoundCheckerV2.cpp | 17 +-- .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 52 +++++++ clang/test/Analysis/out-of-bounds-new.cpp | 150 +++++++++++++++++++++ 3 files changed, 207 insertions(+), 12 deletions(-) create mode 100644 clang/test/Analysis/out-of-bounds-new.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 77c75bf..848c266 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -68,18 +68,11 @@ public: static SVal computeExtentBegin(SValBuilder &svalBuilder, const MemRegion *region) { - while (true) - switch (region->getKind()) { - default: - return svalBuilder.makeZeroArrayIndex(); - case MemRegion::SymbolicRegionKind: - // FIXME: improve this later by tracking symbolic lower bounds - // for symbolic regions. - return UnknownVal(); - case MemRegion::ElementRegionKind: - region = cast(region)->getSuperRegion(); - continue; - } + const MemSpaceRegion *SR = region->getMemorySpace(); + if (SR->getKind() == MemRegion::UnknownSpaceRegionKind) + return UnknownVal(); + else + return svalBuilder.makeZeroArrayIndex(); } // TODO: once the constraint manager is smart enough to handle non simplified diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 611d9b4..e3c940f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -287,6 +287,9 @@ private: ProgramStateRef State, AllocationFamily Family = AF_Malloc); + static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE, + ProgramStateRef State); + // Check if this malloc() for special flags. At present that means M_ZERO or // __GFP_ZERO (in which case, treat it like calloc). llvm::Optional @@ -981,10 +984,59 @@ void MallocChecker::checkPostStmt(const CXXNewExpr *NE, // existing binding. State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray : AF_CXXNew); + State = addExtentSize(C, NE, State); State = ProcessZeroAllocation(C, NE, 0, State); C.addTransition(State); } +// Sets the extent value of the MemRegion allocated by +// new expression NE to its size in Bytes. +// +ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C, + const CXXNewExpr *NE, + ProgramStateRef State) { + if (!State) + return nullptr; + SValBuilder &svalBuilder = C.getSValBuilder(); + SVal ElementCount; + const LocationContext *LCtx = C.getLocationContext(); + const SubRegion *Region; + if (NE->isArray()) { + const Expr *SizeExpr = NE->getArraySize(); + ElementCount = State->getSVal(SizeExpr, C.getLocationContext()); + // Store the extent size for the (symbolic)region + // containing the elements. + Region = (State->getSVal(NE, LCtx)) + .getAsRegion() + ->getAs() + ->getSuperRegion() + ->getAs(); + } else { + ElementCount = svalBuilder.makeIntVal(1, true); + Region = (State->getSVal(NE, LCtx)).getAsRegion()->getAs(); + } + assert(Region); + + // Set the region's extent equal to the Size in Bytes. + QualType ElementType = NE->getAllocatedType(); + ASTContext &AstContext = C.getASTContext(); + CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType); + + if (Optional DefinedSize = + ElementCount.getAs()) { + DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder); + // size in Bytes = ElementCount*TypeSize + SVal SizeInBytes = svalBuilder.evalBinOpNN( + State, BO_Mul, ElementCount.castAs(), + svalBuilder.makeArrayIndex(TypeSize.getQuantity()), + svalBuilder.getArrayIndexType()); + DefinedOrUnknownSVal extentMatchesSize = svalBuilder.evalEQ( + State, Extent, SizeInBytes.castAs()); + State = State->assume(extentMatchesSize, true); + } + return State; +} + void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const { diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp new file mode 100644 index 0000000..41ecbee --- /dev/null +++ b/clang/test/Analysis/out-of-bounds-new.cpp @@ -0,0 +1,150 @@ +// RUN: %clang_cc1 -std=c++11 -Wno-array-bounds -analyze -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s + +// Tests doing an out-of-bounds access after the end of an array using: +// - constant integer index +// - constant integer size for buffer +void test1(int x) { + int *buf = new int[100]; + buf[100] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ok(int x) { + int *buf = new int[100]; + buf[99] = 1; // no-warning +} + +// Tests doing an out-of-bounds access after the end of an array using: +// - indirect pointer to buffer +// - constant integer index +// - constant integer size for buffer +void test1_ptr(int x) { + int *buf = new int[100]; + int *p = buf; + p[101] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_ok(int x) { + int *buf = new int[100]; + int *p = buf; + p[99] = 1; // no-warning +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer, manipulated using simple pointer arithmetic +// - constant integer index +// - constant integer size for buffer +void test1_ptr_arith(int x) { + int *buf = new int[100]; + int *p = buf; + p = p + 100; + p[0] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_arith_ok(int x) { + int *buf = new int[100]; + int *p = buf; + p = p + 99; + p[0] = 1; // no-warning +} + +void test1_ptr_arith_bad(int x) { + int *buf = new int[100]; + int *p = buf; + p = p + 99; + p[1] = 1; // expected-warning{{Out of bound memory access}} +} + +void test1_ptr_arith_ok2(int x) { + int *buf = new int[100]; + int *p = buf; + p = p + 99; + p[-1] = 1; // no-warning +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - constant integer index +// - constant integer size for buffer +void test2(int x) { + int *buf = new int[100]; + buf[-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer +// - constant integer index +// - constant integer size for buffer +void test2_ptr(int x) { + int *buf = new int[100]; + int *p = buf; + p[-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests doing an out-of-bounds access before the start of an array using: +// - indirect pointer to buffer, manipulated using simple pointer arithmetic +// - constant integer index +// - constant integer size for buffer +void test2_ptr_arith(int x) { + int *buf = new int[100]; + int *p = buf; + --p; + p[0] = 1; // expected-warning {{Out of bound memory access (accessed memory precedes memory block)}} +} + +// Tests under-indexing +// of a multi-dimensional array +void test2_multi(int x) { + auto buf = new int[100][100]; + buf[0][-1] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests under-indexing +// of a multi-dimensional array +void test2_multi_b(int x) { + auto buf = new int[100][100]; + buf[-1][0] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests over-indexing +// of a multi-dimensional array +void test2_multi_c(int x) { + auto buf = new int[100][100]; + buf[100][0] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests over-indexing +// of a multi-dimensional array +void test2_multi_2(int x) { + auto buf = new int[100][100]; + buf[99][100] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests normal access of +// a multi-dimensional array +void test2_multi_ok(int x) { + auto buf = new int[100][100]; + buf[0][0] = 1; // no-warning +} + +// Tests over-indexing using different types +// array +void test_diff_types(int x) { + int *buf = new int[10]; //10*sizeof(int) Bytes allocated + char *cptr = (char *)buf; + cptr[sizeof(int) * 9] = 1; // no-warning + cptr[sizeof(int) * 10] = 1; // expected-warning{{Out of bound memory access}} +} + +// Tests over-indexing +//if the allocated area is non-array +void test_non_array(int x) { + int *ip = new int; + ip[0] = 1; // no-warning + ip[1] = 2; // expected-warning{{Out of bound memory access}} +} + +//Tests over-indexing +//if the allocated area size is a runtime parameter +void test_dynamic_size(int s) { + int *buf = new int[s]; + buf[0] = 1; // no-warning +} -- 2.7.4