From 566a6faa54235590ab8d7d177dfac08586f545b0 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Thu, 6 Aug 2009 22:33:36 +0000 Subject: [PATCH] Fix a few more false positives involving RegionStore and unions, but this time with array accesses. In the process, refactor some common logic in RetrieveElement() and RetrieveField() into RetrieveFieldOrElementCommon(). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@78349 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/RegionStore.cpp | 140 +++++++++++++--------------------- test/Analysis/unions-region.m | 17 ++++- 2 files changed, 68 insertions(+), 89 deletions(-) diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp index 8cb4d98021..48e6de7cd8 100644 --- a/lib/Analysis/RegionStore.cpp +++ b/lib/Analysis/RegionStore.cpp @@ -315,6 +315,9 @@ public: SVal RetrieveLazySymbol(const GRState *state, const TypedRegion *R); + SVal RetrieveFieldOrElementCommon(const GRState *state, const TypedRegion *R, + QualType Ty, const MemRegion *superR); + SValuator::CastResult CastRetrievedVal(SVal val, const GRState *state, const TypedRegion *R, QualType castTy); @@ -1053,109 +1056,66 @@ SVal RegionStoreManager::RetrieveElement(const GRState* state, } } - // Check if the super region has a default value. - if (const SVal *D = state->get(superR)) { - if (D->hasConjuredSymbol()) - return ValMgr.getRegionValueSymbolVal(R); - else - return *D; - } - - // Check if the super region has a binding. + // Check if the immediate super region has a direct binding. if (const SVal *V = B.lookup(superR)) { if (SymbolRef parentSym = V->getAsSymbol()) return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); if (V->isUnknownOrUndef()) return *V; - - // Handle LazyCompoundVals below. - if (const nonloc::LazyCompoundVal *LVC = - dyn_cast(V)) { - return RetrieveElement(LVC->getState(), - MRMgr.getElementRegionWithSuper(R, - LVC->getRegion())); - } + + // Handle LazyCompoundVals for the immediate super region. Other cases + // are handled in 'RetrieveFieldOrElementCommon'. + if (const nonloc::LazyCompoundVal *LCV = + dyn_cast(V)) { + R = MRMgr.getElementRegionWithSuper(R, LCV->getRegion()); + return RetrieveElement(LCV->getState(), R); + } + // Other cases: give up. return UnknownVal(); } - // Lazy binding? - const GRState *lazyBindingState = NULL; - const MemRegion *LazyBindingRegion = NULL; - llvm::tie(lazyBindingState, LazyBindingRegion) = GetLazyBinding(B, R); - - if (lazyBindingState) { - assert(LazyBindingRegion && "Lazy-binding region not set"); - return RetrieveElement(lazyBindingState, - cast(LazyBindingRegion)); - } - - // Default value cases. -#if 0 - if (R->hasHeapStorage()) { - // FIXME: If the region has heap storage and we know nothing special - // about its bindings, should we instead return UnknownVal? Seems like - // we should only return UndefinedVal in the cases where we know the value - // will be undefined. - return UndefinedVal(); - } -#endif - - if (R->hasStackStorage() && !R->hasParametersStorage()) { - // Currently we don't reason specially about Clang-style vectors. Check - // if superR is a vector and if so return Unknown. - if (const TypedRegion *typedSuperR = dyn_cast(superR)) { - if (typedSuperR->getValueType(getContext())->isVectorType()) - return UnknownVal(); - } - - return UndefinedVal(); - } - - QualType Ty = R->getValueType(getContext()); - - return ValMgr.getRegionValueSymbolValOrUnknown(R, Ty); + return RetrieveFieldOrElementCommon(state, R, R->getElementType(), superR); } SVal RegionStoreManager::RetrieveField(const GRState* state, const FieldRegion* R) { - QualType Ty = R->getValueType(getContext()); // Check if the region has a binding. RegionBindings B = GetRegionBindings(state->getStore()); if (const SVal* V = B.lookup(R)) return *V; - const MemRegion* superR = R->getSuperRegion(); + QualType Ty = R->getValueType(getContext()); + return RetrieveFieldOrElementCommon(state, R, Ty, R->getSuperRegion()); +} + +SVal RegionStoreManager::RetrieveFieldOrElementCommon(const GRState *state, + const TypedRegion *R, + QualType Ty, + const MemRegion *superR) { + + // At this point we have already checked in either RetrieveElement or + // RetrieveField if 'R' has a direct binding. + + RegionBindings B = GetRegionBindings(state->getStore()); + while (superR) { if (const Optional &D = getDefaultBinding(state, superR)) { if (SymbolRef parentSym = D->getAsSymbol()) return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); - + if (D->isZeroConstant()) return ValMgr.makeZeroVal(Ty); - - if (const nonloc::LazyCompoundVal *LCV = - dyn_cast(D)) { - const FieldRegion *FR = - MRMgr.getFieldRegionWithSuper(R, LCV->getRegion()); - return RetrieveField(LCV->getState(), FR); - } - + if (D->isUnknown()) return *D; - + assert(0 && "Unknown default value"); } - if (const SVal *V = B.lookup(superR)) { - // Handle LazyCompoundVals below. - if (isa(*V)) - break; - } - // If our super region is a field or element itself, walk up the region // hierarchy to see if there is a default value installed in an ancestor. if (isa(superR) || isa(superR)) { @@ -1168,28 +1128,38 @@ SVal RegionStoreManager::RetrieveField(const GRState* state, // Lazy binding? const GRState *lazyBindingState = NULL; - const MemRegion *LazyBindingRegion = NULL; - llvm::tie(lazyBindingState, LazyBindingRegion) = GetLazyBinding(B, R); + const MemRegion *lazyBindingRegion = NULL; + llvm::tie(lazyBindingState, lazyBindingRegion) = GetLazyBinding(B, R); if (lazyBindingState) { - assert(LazyBindingRegion && "Lazy-binding region not set"); + assert(lazyBindingRegion && "Lazy-binding region not set"); + + if (isa(R)) + return RetrieveElement(lazyBindingState, + cast(lazyBindingRegion)); + return RetrieveField(lazyBindingState, - cast(LazyBindingRegion)); - } - -#if HEAP_UNDEFINED - // FIXME: Is this correct? Should it be UnknownVal? - if (R->hasHeapStorage()) - return UndefinedVal(); -#endif + cast(lazyBindingRegion)); + } - if (R->hasStackStorage() && !R->hasParametersStorage()) + if (R->hasStackStorage() && !R->hasParametersStorage()) { + + if (isa(R)) { + // Currently we don't reason specially about Clang-style vectors. Check + // if superR is a vector and if so return Unknown. + if (const TypedRegion *typedSuperR = dyn_cast(superR)) { + if (typedSuperR->getValueType(getContext())->isVectorType()) + return UnknownVal(); + } + } + return UndefinedVal(); - + } + // All other values are symbolic. return ValMgr.getRegionValueSymbolValOrUnknown(R, Ty); } - + SVal RegionStoreManager::RetrieveObjCIvar(const GRState* state, const ObjCIvarRegion* R) { diff --git a/test/Analysis/unions-region.m b/test/Analysis/unions-region.m index d253009d8b..be4f1852bf 100644 --- a/test/Analysis/unions-region.m +++ b/test/Analysis/unions-region.m @@ -6,10 +6,9 @@ // //===----------------------------------------------------------------------===// -// When using RegionStore, this test case previously had a false positive -// of a 'pass-by-value argument is uninitialized' warning at the call to -// 'testA_aux'. - +// [testA] When using RegionStore, this test case previously had a +// false positive of a 'pass-by-value argument is uninitialized' +// warning at the call to 'testA_aux' and 'testA_aux_2'. union u_testA { unsigned i; float f; @@ -30,3 +29,13 @@ float testA(float f) { return swap.f; } +// [testB] When using RegionStore, this test case previously had a +// false positive of a 'pass-by-value argument is uninitialized' +// warning at the call to 'testB_aux'. +void testB(int i) { + void testB_aux(short z); + union { short x[2]; unsigned y; } val; + val.y = 10; + testB_aux(val.x[1]); // no-warning +} + -- 2.40.0