maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Move RegionStoreManager over to using new 

ValueManager::makeArrayIndex()/convertArrayIndex() methods.  This
handles yet another crash case when reasoning about array indices of
different bitwidth and signedness.

llvm-svn: 75884
This commit is contained in:
Ted Kremenek 2009-07-16 01:33:37 +00:00
parent 5a347d4bb7
commit c7b1dade86
5 changed files with 59 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
clang/include/clang/Analysis/PathSensitive/SValuator.h
View File

@ -31,9 +31,17 @@ public:
SValuator(ValueManager &valMgr) : ValMgr(valMgr) {}
virtual ~SValuator() {}
virtual SVal EvalCast(NonLoc val, QualType castTy) = 0;
virtual SVal EvalCastNL(NonLoc val, QualType castTy) = 0;
virtual SVal EvalCast(Loc val, QualType castTy) = 0;
virtual SVal EvalCastL(Loc val, QualType castTy) = 0;
SVal EvalCast(SVal val, QualType castTy) {
if (val.isUnknownOrUndef())
return val;
return isa<Loc>(val) ? EvalCastL(cast<Loc>(val), castTy)
: EvalCastNL(cast<NonLoc>(val), castTy);
}
virtual SVal EvalMinus(NonLoc val) = 0;

3
clang/lib/Analysis/MemRegion.cpp
View File

@ -259,7 +259,8 @@ MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr* CL) {
ElementRegion*
MemRegionManager::getElementRegion(QualType elementType, SVal Idx,
const MemRegion* superRegion, ASTContext& Ctx){
const MemRegion* superRegion,
ASTContext& Ctx){
QualType T = Ctx.getCanonicalType(elementType);

79
clang/lib/Analysis/RegionStore.cpp
View File

@ -500,6 +500,9 @@ SVal RegionStoreManager::getLValueElement(const GRState *St,
// Pointer of any type can be cast and used as array base.
const ElementRegion *ElemR = dyn_cast<ElementRegion>(BaseRegion);
// Convert the offset to the appropriate size and signedness.
Offset = ValMgr.convertToArrayIndex(Offset);
if (!ElemR) {
//
// If the base region is not an ElementRegion, create one.
@ -510,16 +513,6 @@ SVal RegionStoreManager::getLValueElement(const GRState *St,
//
// Observe that 'p' binds to an AllocaRegion.
//
// Offset might be unsigned. We have to convert it to signed ConcreteInt.
if (nonloc::ConcreteInt* CI = dyn_cast<nonloc::ConcreteInt>(&Offset)) {
const llvm::APSInt& OffI = CI->getValue();
if (OffI.isUnsigned()) {
llvm::APSInt Tmp = OffI;
Tmp.setIsSigned(true);
Offset = ValMgr.makeIntVal(Tmp);
}
}
return loc::MemRegionVal(MRMgr.getElementRegion(elementType, Offset,
BaseRegion, getContext()));
}
@ -533,29 +526,11 @@ SVal RegionStoreManager::getLValueElement(const GRState *St,
const llvm::APSInt& OffI = cast<nonloc::ConcreteInt>(Offset).getValue();
assert(BaseIdxI.isSigned());
// FIXME: This appears to be the assumption of this code. We should review
// whether or not BaseIdxI.getBitWidth() < OffI.getBitWidth(). If it
// can't we need to put a comment here. If it can, we should handle it.
assert(BaseIdxI.getBitWidth() >= OffI.getBitWidth());
// Compute the new index.
SVal NewIdx = nonloc::ConcreteInt(getBasicVals().getValue(BaseIdxI + OffI));
// Construct the new ElementRegion.
const MemRegion *ArrayR = ElemR->getSuperRegion();
SVal NewIdx;
if (OffI.isUnsigned() || OffI.getBitWidth() < BaseIdxI.getBitWidth()) {
// 'Offset' might be unsigned. We have to convert it to signed and
// possibly extend it.
llvm::APSInt Tmp = OffI;
if (OffI.getBitWidth() < BaseIdxI.getBitWidth())
Tmp.extend(BaseIdxI.getBitWidth());
Tmp.setIsSigned(true);
Tmp += BaseIdxI; // Compute the new offset.
NewIdx = ValMgr.makeIntVal(Tmp);
}
else
NewIdx = nonloc::ConcreteInt(getBasicVals().getValue(BaseIdxI + OffI));
return loc::MemRegionVal(MRMgr.getElementRegion(elementType, NewIdx, ArrayR,
getContext()));
}
@ -760,20 +735,13 @@ SVal RegionStoreManager::EvalBinOp(const GRState *state,
// Only support concrete integer indexes for now.
if (Base && Offset) {
// FIXME: For now, convert the signedness and bitwidth of offset in case
// they don't match. This can result from pointer arithmetic. In reality,
// we should figure out what are the proper semantics and implement them.
//
// This addresses the test case test/Analysis/ptr-arith.c
//
nonloc::ConcreteInt OffConverted(getBasicVals().Convert(Base->getValue(),
Offset->getValue()));
SVal NewIdx = Base->evalBinOp(ValMgr, Op, OffConverted);
// FIXME: Should use SValuator here.
SVal NewIdx = Base->evalBinOp(ValMgr, Op,
cast<nonloc::ConcreteInt>(ValMgr.convertToArrayIndex(*Offset)));
const MemRegion* NewER =
MRMgr.getElementRegion(ER->getElementType(), NewIdx, ER->getSuperRegion(),
getContext());
return ValMgr.makeLoc(NewER);
}
return UnknownVal();
@ -837,8 +805,8 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) {
ASTContext &Ctx = getContext();
if (!T.isNull() && IsReinterpreted(RTy, T, Ctx)) {
SVal idx = ValMgr.makeIntVal(0, Ctx.IntTy);
R = MRMgr.getElementRegion(T, idx, R, Ctx);
SVal ZeroIdx = ValMgr.makeZeroArrayIndex();
R = MRMgr.getElementRegion(T, ZeroIdx, R, Ctx);
RTy = T;
assert(Ctx.getCanonicalType(RTy) ==
Ctx.getCanonicalType(R->getValueType(Ctx)));
@ -1083,11 +1051,9 @@ SVal RegionStoreManager::RetrieveArray(const GRState *state,
ConstantArrayType* CAT = cast<ConstantArrayType>(T.getTypePtr());
llvm::ImmutableList<SVal> ArrayVal = getBasicVals().getEmptySValList();
llvm::APSInt Size(CAT->getSize(), false);
llvm::APSInt i = getBasicVals().getZeroWithPtrWidth(false);
for (; i < Size; ++i) {
SVal Idx = ValMgr.makeIntVal(i);
uint64_t size = CAT->getSize().getZExtValue();
for (uint64_t i = 0; i < size; ++i) {
SVal Idx = ValMgr.makeArrayIndex(i);
ElementRegion* ER = MRMgr.getElementRegion(CAT->getElementType(), Idx, R,
getContext());
QualType ETy = ER->getElementType();
@ -1167,8 +1133,7 @@ const GRState *RegionStoreManager::BindArray(const GRState *state,
ConstantArrayType* CAT = cast<ConstantArrayType>(T.getTypePtr());
QualType ElementTy = CAT->getElementType();
llvm::APSInt Size(CAT->getSize(), false);
llvm::APSInt i(llvm::APInt::getNullValue(Size.getBitWidth()), false);
uint64_t size = CAT->getSize().getZExtValue();
// Check if the init expr is a StringLiteral.
if (isa<loc::MemRegionVal>(Init)) {
@ -1181,12 +1146,13 @@ const GRState *RegionStoreManager::BindArray(const GRState *state,
// Copy bytes from the string literal into the target array. Trailing bytes
// in the array that are not covered by the string literal are initialized
// to zero.
for (; i < Size; ++i, ++j) {
for (uint64_t i = 0; i < size; ++i, ++j) {
if (j >= len)
break;
SVal Idx = ValMgr.makeIntVal(i);
ElementRegion* ER = MRMgr.getElementRegion(ElementTy, Idx,R,getContext());
SVal Idx = ValMgr.makeArrayIndex(i);
ElementRegion* ER = MRMgr.getElementRegion(ElementTy, Idx, R,
getContext());
SVal V = ValMgr.makeIntVal(str[j], sizeof(char)*8, true);
state = Bind(state, loc::MemRegionVal(ER), V);
@ -1197,13 +1163,14 @@ const GRState *RegionStoreManager::BindArray(const GRState *state,
nonloc::CompoundVal& CV = cast<nonloc::CompoundVal>(Init);
nonloc::CompoundVal::iterator VI = CV.begin(), VE = CV.end();
uint64_t i = 0;
for (; i < Size; ++i, ++VI) {
for (; i < size; ++i, ++VI) {
// The init list might be shorter than the array length.
if (VI == VE)
break;
SVal Idx = ValMgr.makeIntVal(i);
SVal Idx = ValMgr.makeArrayIndex(i);
ElementRegion* ER = MRMgr.getElementRegion(ElementTy, Idx, R, getContext());
if (CAT->getElementType()->isStructureType())
@ -1214,7 +1181,7 @@ const GRState *RegionStoreManager::BindArray(const GRState *state,
// If the init list is shorter than the array length, set the array default
// value.
if (i < Size) {
if (i < size) {
if (ElementTy->isIntegerType()) {
SVal V = ValMgr.makeZeroVal(ElementTy);
state = setDefaultValue(state, R, V);

8
clang/lib/Analysis/SimpleSValuator.cpp
View File

@ -23,8 +23,8 @@ public:
SimpleSValuator(ValueManager &valMgr) : SValuator(valMgr) {}
virtual ~SimpleSValuator() {}
virtual SVal EvalCast(NonLoc val, QualType castTy);
virtual SVal EvalCast(Loc val, QualType castTy);
virtual SVal EvalCastNL(NonLoc val, QualType castTy);
virtual SVal EvalCastL(Loc val, QualType castTy);
virtual SVal EvalMinus(NonLoc val);
virtual SVal EvalComplement(NonLoc val);
virtual SVal EvalBinOpNN(BinaryOperator::Opcode op, NonLoc lhs, NonLoc rhs,
@ -44,7 +44,7 @@ SValuator *clang::CreateSimpleSValuator(ValueManager &valMgr) {
// Transfer function for Casts.
//===----------------------------------------------------------------------===//
SVal SimpleSValuator::EvalCast(NonLoc val, QualType castTy) {
SVal SimpleSValuator::EvalCastNL(NonLoc val, QualType castTy) {
if (!isa<nonloc::ConcreteInt>(val))
return UnknownVal();
@ -64,7 +64,7 @@ SVal SimpleSValuator::EvalCast(NonLoc val, QualType castTy) {
return ValMgr.makeIntVal(i);
}
SVal SimpleSValuator::EvalCast(Loc val, QualType castTy) {
SVal SimpleSValuator::EvalCastL(Loc val, QualType castTy) {
// Casts from pointers -> pointers, just return the lval.
//

15
clang/test/Analysis/misc-ps.m
View File

@ -415,3 +415,18 @@ void rdar_7062158_2() {
*p = 0xDEADBEEF; // no-warning
}
// This test reproduces a case for a crash when analyzing ClamAV using
// RegionStoreManager (the crash doesn't exhibit in BasicStoreManager because
// it isn't doing anything smart about arrays). The problem is that on the
// second line, 'p = &p[i]', p is assigned an ElementRegion whose index
// is a 16-bit integer. On the third line, a new ElementRegion is created
// based on the previous region, but there the region uses a 32-bit integer,
// resulting in a clash of values (an assertion failure at best). We resolve
// this problem by implicitly converting index values to 'int' when the
// ElementRegion is created.
unsigned char test_array_index_bitwidth(const unsigned char *p) {
unsigned short i = 0;
for (i = 0; i < 2; i++) p = &p[i];
return p[i+1];
}