From 49d5f8825d2e450f55c3d483487014ab464acff9 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Fri, 1 Feb 2013 19:49:57 +0000 Subject: [PATCH] [analyzer] Reuse a LazyCompoundVal if its type matches the new region. This allows us to keep from chaining LazyCompoundVals in cases like this: CGRect r = CGRectMake(0, 0, 640, 480); CGRect r2 = r; CGRect r3 = r2; Previously we only made this optimization if the struct did not begin with an aggregate member, to make sure that we weren't picking up an LCV for the first field of the struct. But since LazyCompoundVals are typed, we can make that inference directly by comparing types. This is a pure optimization; the test changes are to guard against possible future regressions. llvm-svn: 174211 --- .../StaticAnalyzer/Core/PathSensitive/SVals.h | 4 +- clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 59 ++++++++----------- clang/lib/StaticAnalyzer/Core/SVals.cpp | 2 +- clang/test/Analysis/array-struct-region.c | 44 ++++++++++++++ 4 files changed, 72 insertions(+), 37 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h index 48f0ab962f96..0fb5af6d07e1 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h @@ -33,7 +33,7 @@ class LazyCompoundValData; class ProgramState; class BasicValueFactory; class MemRegion; -class TypedRegion; +class TypedValueRegion; class MemRegionManager; class ProgramStateManager; class SValBuilder; @@ -379,7 +379,7 @@ public: return static_cast(Data); } const void *getStore() const; - const TypedRegion *getRegion() const; + const TypedValueRegion *getRegion() const; static bool classof(const SVal *V) { return V->getBaseKind() == NonLocKind && diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 06218664f7a9..61518e61162f 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -475,9 +475,9 @@ public: // Part of public interface to class. /// struct s x, y; /// x = y; /// y's value is retrieved by this method. - SVal getBindingForStruct(RegionBindingsConstRef B, const TypedValueRegion* R); - - SVal getBindingForArray(RegionBindingsConstRef B, const TypedValueRegion* R); + SVal getBindingForStruct(RegionBindingsConstRef B, const TypedValueRegion *R); + SVal getBindingForArray(RegionBindingsConstRef B, const TypedValueRegion *R); + NonLoc createLazyBinding(RegionBindingsConstRef B, const TypedValueRegion *R); /// Used to lazily generate derived symbols for bindings that are defined /// implicitly by default bindings in a super region. @@ -1551,48 +1551,39 @@ SVal RegionStoreManager::getBindingForLazySymbol(const TypedValueRegion *R) { return svalBuilder.getRegionValueSymbolVal(R); } -static bool mayHaveLazyBinding(QualType Ty) { - return Ty->isArrayType() || Ty->isStructureOrClassType(); +NonLoc RegionStoreManager::createLazyBinding(RegionBindingsConstRef B, + const TypedValueRegion *R) { + // If we already have a lazy binding, and it's for the whole structure, + // don't create a new lazy binding. + if (Optional V = B.getDefaultBinding(R)) { + const nonloc::LazyCompoundVal *LCV = + dyn_cast(V.getPointer()); + if (LCV) { + QualType RegionTy = R->getValueType(); + QualType SourceRegionTy = LCV->getRegion()->getValueType(); + if (RegionTy.getCanonicalType() == SourceRegionTy.getCanonicalType()) + return *LCV; + } + } + + return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R); } SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B, - const TypedValueRegion* R) { + const TypedValueRegion *R) { const RecordDecl *RD = R->getValueType()->castAs()->getDecl(); if (RD->field_empty()) return UnknownVal(); - // If we already have a lazy binding, don't create a new one, - // unless the first field might have a lazy binding of its own. - // (Right now we can't tell the difference.) - QualType FirstFieldType = RD->field_begin()->getType(); - if (!mayHaveLazyBinding(FirstFieldType)) { - BindingKey K = BindingKey::Make(R, BindingKey::Default); - if (const nonloc::LazyCompoundVal *V = - dyn_cast_or_null(B.lookup(K))) { - return *V; - } - } - - return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R); + return createLazyBinding(B, R); } SVal RegionStoreManager::getBindingForArray(RegionBindingsConstRef B, - const TypedValueRegion * R) { - const ConstantArrayType *Ty = Ctx.getAsConstantArrayType(R->getValueType()); - assert(Ty && "Only constant array types can have compound bindings."); + const TypedValueRegion *R) { + assert(Ctx.getAsConstantArrayType(R->getValueType()) && + "Only constant array types can have compound bindings."); - // If we already have a lazy binding, don't create a new one, - // unless the first element might have a lazy binding of its own. - // (Right now we can't tell the difference.) - if (!mayHaveLazyBinding(Ty->getElementType())) { - BindingKey K = BindingKey::Make(R, BindingKey::Default); - if (const nonloc::LazyCompoundVal *V = - dyn_cast_or_null(B.lookup(K))) { - return *V; - } - } - - return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R); + return createLazyBinding(B, R); } bool RegionStoreManager::includedInBindings(Store store, diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp index 04f24fa1d3e3..5906068c8961 100644 --- a/clang/lib/StaticAnalyzer/Core/SVals.cpp +++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp @@ -144,7 +144,7 @@ const void *nonloc::LazyCompoundVal::getStore() const { return static_cast(Data)->getStore(); } -const TypedRegion *nonloc::LazyCompoundVal::getRegion() const { +const TypedValueRegion *nonloc::LazyCompoundVal::getRegion() const { return static_cast(Data)->getRegion(); } diff --git a/clang/test/Analysis/array-struct-region.c b/clang/test/Analysis/array-struct-region.c index c4d9aff95fbc..dfb4cf13a9b9 100644 --- a/clang/test/Analysis/array-struct-region.c +++ b/clang/test/Analysis/array-struct-region.c @@ -267,6 +267,50 @@ static void radar13116945(struct point centerCoordinate) { test13116945(r.center); // no-warning } + +typedef struct { + char data[4]; +} ShortString; + +typedef struct { + ShortString str; + int length; +} ShortStringWrapper; + +void testArrayStructCopy() { + ShortString s = { "abc" }; + ShortString s2 = s; + ShortString s3 = s2; + + clang_analyzer_eval(s3.data[0] == 'a'); // expected-warning{{TRUE}} + clang_analyzer_eval(s3.data[1] == 'b'); // expected-warning{{TRUE}} + clang_analyzer_eval(s3.data[2] == 'c'); // expected-warning{{TRUE}} +} + +void testArrayStructCopyNested() { + ShortString s = { "abc" }; + ShortString s2 = s; + + ShortStringWrapper w = { s2, 0 }; + + clang_analyzer_eval(w.str.data[0] == 'a'); // expected-warning{{TRUE}} + clang_analyzer_eval(w.str.data[1] == 'b'); // expected-warning{{TRUE}} + clang_analyzer_eval(w.str.data[2] == 'c'); // expected-warning{{TRUE}} + clang_analyzer_eval(w.length == 0); // expected-warning{{TRUE}} + + ShortStringWrapper w2 = w; + clang_analyzer_eval(w2.str.data[0] == 'a'); // expected-warning{{TRUE}} + clang_analyzer_eval(w2.str.data[1] == 'b'); // expected-warning{{TRUE}} + clang_analyzer_eval(w2.str.data[2] == 'c'); // expected-warning{{TRUE}} + clang_analyzer_eval(w2.length == 0); // expected-warning{{TRUE}} + + ShortStringWrapper w3 = w2; + clang_analyzer_eval(w3.str.data[0] == 'a'); // expected-warning{{TRUE}} + clang_analyzer_eval(w3.str.data[1] == 'b'); // expected-warning{{TRUE}} + clang_analyzer_eval(w3.str.data[2] == 'c'); // expected-warning{{TRUE}} + clang_analyzer_eval(w3.length == 0); // expected-warning{{TRUE}} +} + // -------------------- // False positives // --------------------