Skip to content

Commit

Permalink
[analyzer] Improve the modeling of memset().
Browse files Browse the repository at this point in the history 
Since there is no perfect way bind the non-zero value with the default binding, this patch only considers the case where buffer's offset is zero and the char value is 0. And according to the value for overwriting, decide how to update the string length.

Reviewers: dcoughlin, NoQ, xazax.hun, a.sidorin, george.karpenkov

Reviewed By: NoQ

Differential Revision: https://reviews.llvm.org/D44934

llvm-svn: 332463
  • Loading branch information
@movie-travel-code
movie-travel-code committed May 16, 2018
1 parent 6c35f8f commit afe62cd
Show file tree
Hide file tree
Showing 4 changed files with 509 additions and 12 deletions.
102 changes: 99 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
View file
Expand Up @@ -158,6 +158,10 @@ class CStringChecker : public Checker< eval::Call,
static bool SummarizeRegion(raw_ostream &os, ASTContext &Ctx,
const MemRegion *MR);

static bool memsetAux(const Expr *DstBuffer, const Expr *CharE,
const Expr *Size, CheckerContext &C,
ProgramStateRef &State);

// Re-usable checks
ProgramStateRef checkNonNull(CheckerContext &C,
ProgramStateRef state,
Expand Down Expand Up @@ -999,6 +1003,95 @@ bool CStringChecker::SummarizeRegion(raw_ostream &os, ASTContext &Ctx,
}
}

bool CStringChecker::memsetAux(const Expr *DstBuffer, const Expr *CharE,
const Expr *Size, CheckerContext &C,
ProgramStateRef &State) {
SVal MemVal = C.getSVal(DstBuffer);
SVal CharVal = C.getSVal(CharE);
SVal SizeVal = C.getSVal(Size);
const MemRegion *MR = MemVal.getAsRegion();
if (!MR)
return false;

// We're about to model memset by producing a "default binding" in the Store.
// Our current implementation - RegionStore - doesn't support default bindings
// that don't cover the whole base region. So we should first get the offset
// and the base region to figure out whether the offset of buffer is 0.
RegionOffset Offset = MR->getAsOffset();
const MemRegion *BR = Offset.getRegion();

Optional<NonLoc> SizeNL = SizeVal.getAs<NonLoc>();
if (!SizeNL)
return false;

SValBuilder &svalBuilder = C.getSValBuilder();
ASTContext &Ctx = C.getASTContext();

// void *memset(void *dest, int ch, size_t count);
// For now we can only handle the case of offset is 0 and concrete char value.
if (Offset.isValid() && !Offset.hasSymbolicOffset() &&
Offset.getOffset() == 0) {
// Get the base region's extent.
auto *SubReg = cast<SubRegion>(BR);
DefinedOrUnknownSVal Extent = SubReg->getExtent(svalBuilder);

ProgramStateRef StateWholeReg, StateNotWholeReg;
std::tie(StateWholeReg, StateNotWholeReg) =
State->assume(svalBuilder.evalEQ(State, Extent, *SizeNL));

// With the semantic of 'memset()', we should convert the CharVal to
// unsigned char.
CharVal = svalBuilder.evalCast(CharVal, Ctx.UnsignedCharTy, Ctx.IntTy);

ProgramStateRef StateNullChar, StateNonNullChar;
std::tie(StateNullChar, StateNonNullChar) =
assumeZero(C, State, CharVal, Ctx.UnsignedCharTy);

if (StateWholeReg && !StateNotWholeReg && StateNullChar &&
!StateNonNullChar) {
// If the 'memset()' acts on the whole region of destination buffer and
// the value of the second argument of 'memset()' is zero, bind the second
// argument's value to the destination buffer with 'default binding'.
// FIXME: Since there is no perfect way to bind the non-zero character, we
// can only deal with zero value here. In the future, we need to deal with
// the binding of non-zero value in the case of whole region.
State = State->bindDefaultZero(svalBuilder.makeLoc(BR),
C.getLocationContext());
} else {
// If the destination buffer's extent is not equal to the value of
// third argument, just invalidate buffer.
State = InvalidateBuffer(C, State, DstBuffer, MemVal,
/*IsSourceBuffer*/ false, Size);
}

if (StateNullChar && !StateNonNullChar) {
// If the value of the second argument of 'memset()' is zero, set the
// string length of destination buffer to 0 directly.
State = setCStringLength(State, MR,
svalBuilder.makeZeroVal(Ctx.getSizeType()));
} else if (!StateNullChar && StateNonNullChar) {
SVal NewStrLen = svalBuilder.getMetadataSymbolVal(
CStringChecker::getTag(), MR, DstBuffer, Ctx.getSizeType(),
C.getLocationContext(), C.blockCount());

// If the value of second argument is not zero, then the string length
// is at least the size argument.
SVal NewStrLenGESize = svalBuilder.evalBinOp(
State, BO_GE, NewStrLen, SizeVal, svalBuilder.getConditionType());

State = setCStringLength(
State->assume(NewStrLenGESize.castAs<DefinedOrUnknownSVal>(), true),
MR, NewStrLen);
}
} else {
// If the offset is not zero and char value is not concrete, we can do
// nothing but invalidate the buffer.
State = InvalidateBuffer(C, State, DstBuffer, MemVal,
/*IsSourceBuffer*/ false, Size);
}
return true;
}

//===----------------------------------------------------------------------===//
// evaluation of individual function calls.
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -2048,6 +2141,7 @@ void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
CurrentFunctionDescription = "memory set function";

const Expr *Mem = CE->getArg(0);
const Expr *CharE = CE->getArg(1);
const Expr *Size = CE->getArg(2);
ProgramStateRef State = C.getState();

Expand Down Expand Up @@ -2080,9 +2174,11 @@ void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
State = CheckBufferAccess(C, State, Size, Mem);
if (!State)
return;
State = InvalidateBuffer(C, State, Mem, C.getSVal(Mem),
/*IsSourceBuffer*/false, Size);
if (!State)

// According to the values of the arguments, bind the value of the second
// argument to the destination buffer and set string length, or just
// invalidate the destination buffer.
if (!memsetAux(Mem, CharE, Size, C, State))
return;

State = State->BindExpr(CE, LCtx, MemVal);
Expand Down
124 changes: 120 additions & 4 deletions clang/test/Analysis/bstring.cpp
View file
@@ -1,7 +1,8 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -analyzer-store=region -verify %s

#include "Inputs/system-header-simulator-cxx.h"
#include "Inputs/system-header-simulator-for-malloc.h"
Expand Down Expand Up @@ -77,3 +78,118 @@ class b {
unsigned *f;
};
}

void *memset(void *dest, int ch, std::size_t count);
namespace memset_non_pod {
class Base {
public:
int b_mem;
Base() : b_mem(1) {}
};

class Derived : public Base {
public:
int d_mem;
Derived() : d_mem(2) {}
};

void memset1_inheritance() {
Derived d;
memset(&d, 0, sizeof(Derived));
clang_analyzer_eval(d.b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(d.d_mem == 0); // expected-warning{{TRUE}}
}

#ifdef SUPPRESS_OUT_OF_BOUND
void memset2_inheritance_field() {
Derived d;
memset(&d.d_mem, 0, sizeof(Derived));
clang_analyzer_eval(d.b_mem == 0); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(d.d_mem == 0); // expected-warning{{UNKNOWN}}
}

void memset3_inheritance_field() {
Derived d;
memset(&d.b_mem, 0, sizeof(Derived));
clang_analyzer_eval(d.b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(d.d_mem == 0); // expected-warning{{TRUE}}
}
#endif

void memset4_array_nonpod_object() {
Derived array[10];
clang_analyzer_eval(array[1].b_mem == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(array[1].d_mem == 2); // expected-warning{{UNKNOWN}}
memset(&array[1], 0, sizeof(Derived));
clang_analyzer_eval(array[1].b_mem == 0); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(array[1].d_mem == 0); // expected-warning{{UNKNOWN}}
}

void memset5_array_nonpod_object() {
Derived array[10];
clang_analyzer_eval(array[1].b_mem == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(array[1].d_mem == 2); // expected-warning{{UNKNOWN}}
memset(array, 0, sizeof(array));
clang_analyzer_eval(array[1].b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(array[1].d_mem == 0); // expected-warning{{TRUE}}
}

void memset6_new_array_nonpod_object() {
Derived *array = new Derived[10];
clang_analyzer_eval(array[2].b_mem == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(array[2].d_mem == 2); // expected-warning{{UNKNOWN}}
memset(array, 0, 10 * sizeof(Derived));
clang_analyzer_eval(array[2].b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(array[2].d_mem == 0); // expected-warning{{TRUE}}
delete[] array;
}

void memset7_placement_new() {
Derived *d = new Derived();
clang_analyzer_eval(d->b_mem == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(d->d_mem == 2); // expected-warning{{TRUE}}

memset(d, 0, sizeof(Derived));
clang_analyzer_eval(d->b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(d->d_mem == 0); // expected-warning{{TRUE}}

Derived *d1 = new (d) Derived();
clang_analyzer_eval(d1->b_mem == 1); // expected-warning{{TRUE}}
clang_analyzer_eval(d1->d_mem == 2); // expected-warning{{TRUE}}

memset(d1, 0, sizeof(Derived));
clang_analyzer_eval(d->b_mem == 0); // expected-warning{{TRUE}}
clang_analyzer_eval(d->d_mem == 0); // expected-warning{{TRUE}}
}

class BaseVirtual {
public:
int b_mem;
virtual int get() { return 1; }
};

class DerivedVirtual : public BaseVirtual {
public:
int d_mem;
};

#ifdef SUPPRESS_OUT_OF_BOUND
void memset8_virtual_inheritance_field() {
DerivedVirtual d;
memset(&d.b_mem, 0, sizeof(Derived));
clang_analyzer_eval(d.b_mem == 0); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(d.d_mem == 0); // expected-warning{{UNKNOWN}}
}
#endif
} // namespace memset_non_pod

#ifdef SUPPRESS_OUT_OF_BOUND
void memset1_new_array() {
int *array = new int[10];
memset(array, 0, 10 * sizeof(int));
clang_analyzer_eval(array[2] == 0); // expected-warning{{TRUE}}
memset(array + 1, 'a', 10 * sizeof(9));
clang_analyzer_eval(array[2] == 0); // expected-warning{{UNKNOWN}}
delete[] array;
}
#endif
2 changes: 1 addition & 1 deletion clang/test/Analysis/null-deref-ps-region.c
View file
Expand Up @@ -22,7 +22,7 @@ void f14(int *a) {
void foo() {
int *x = malloc(sizeof(int));
memset(x, 0, sizeof(int));
int n = 1 / *x; // FIXME: no-warning
int n = 1 / *x; // expected-warning {{Division by zero}}
free(x);
}

Expand Down

0 comments on commit afe62cd

Please sign in to comment.