Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -79,6 +79,12 @@ // FIXME: Make these protected again once RegionStoreManager correctly // handles loads from different bound value types. virtual SVal dispatchCast(SVal val, QualType castTy) = 0; + const ElementRegion *MakeElementRegion(const SubRegion *Base, QualType EleTy, + uint64_t index = 0); + /// castRegion - Used by ExprEngine::VisitCast to handle casts from + /// a MemRegion* to a specific location type. 'R' is the region being + /// casted and 'CastToTy' the result type of the cast. + const MemRegion *castRegion(const MemRegion *R, QualType CastToTy); public: SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context, Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h @@ -178,11 +178,6 @@ const ElementRegion *GetElementZeroRegion(const SubRegion *R, QualType T); - /// castRegion - Used by ExprEngine::VisitCast to handle casts from - /// a MemRegion* to a specific location type. 'R' is the region being - /// casted and 'CastToTy' the result type of the cast. - const MemRegion *castRegion(const MemRegion *region, QualType CastToTy); - virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx, SymbolReaper &SymReaper) = 0; @@ -276,9 +271,6 @@ virtual void iterBindings(Store store, BindingsHandler& f) = 0; protected: - const ElementRegion *MakeElementRegion(const SubRegion *baseRegion, - QualType pointeeTy, - uint64_t index = 0); /// CastRetrievedVal - Used by subclasses of StoreManager to implement /// implicit casts that arise from loads from regions that are reinterpreted Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -530,6 +530,169 @@ return evalCast(val, castTy, originalTy); } +const ElementRegion *SValBuilder::MakeElementRegion(const SubRegion *Base, + QualType EleTy, + uint64_t index /*= 0*/) { + NonLoc idx = makeArrayIndex(index); + return MemMgr.getElementRegion(EleTy, idx, Base, getContext()); +} + +const MemRegion *SValBuilder::castRegion(const MemRegion *R, + QualType CastToTy) { + ASTContext &Ctx = getContext(); + + // Handle casts to Objective-C objects. + if (CastToTy->isObjCObjectPointerType()) + return R->StripCasts(); + + if (CastToTy->isBlockPointerType()) { + // FIXME: We may need different solutions, depending on the symbol + // involved. Blocks can be casted to/from 'id', as they can be treated + // as Objective-C objects. This could possibly be handled by enhancing + // our reasoning of downcasts of symbolic objects. + if (isa(R) || isa(R)) + return R; + + // We don't know what to make of it. Return a NULL region, which + // will be interpreted as UnknownVal. + return nullptr; + } + + // Now assume we are casting from pointer to pointer. Other cases should + // already be handled. + QualType PointeeTy = CastToTy->getPointeeType(); + QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); + + // Handle casts to void*. We just pass the region through. + if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy) + return R; + + // Handle casts from compatible types. + if (R->isBoundable()) + if (const auto *TR = dyn_cast(R)) { + QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); + if (CanonPointeeTy == ObjTy) + return R; + } + + // Process region cast according to the kind of the region being cast. + switch (R->getKind()) { + case MemRegion::CXXThisRegionKind: + case MemRegion::CodeSpaceRegionKind: + case MemRegion::StackLocalsSpaceRegionKind: + case MemRegion::StackArgumentsSpaceRegionKind: + case MemRegion::HeapSpaceRegionKind: + case MemRegion::UnknownSpaceRegionKind: + case MemRegion::StaticGlobalSpaceRegionKind: + case MemRegion::GlobalInternalSpaceRegionKind: + case MemRegion::GlobalSystemSpaceRegionKind: + case MemRegion::GlobalImmutableSpaceRegionKind: { + llvm_unreachable("Invalid region cast"); + } + + case MemRegion::FunctionCodeRegionKind: + case MemRegion::BlockCodeRegionKind: + case MemRegion::BlockDataRegionKind: + case MemRegion::StringRegionKind: + // FIXME: Need to handle arbitrary downcasts. + case MemRegion::SymbolicRegionKind: + case MemRegion::AllocaRegionKind: + case MemRegion::CompoundLiteralRegionKind: + case MemRegion::FieldRegionKind: + case MemRegion::ObjCIvarRegionKind: + case MemRegion::ObjCStringRegionKind: + case MemRegion::NonParamVarRegionKind: + case MemRegion::ParamVarRegionKind: + case MemRegion::CXXTempObjectRegionKind: + case MemRegion::CXXBaseObjectRegionKind: + case MemRegion::CXXDerivedObjectRegionKind: + return MakeElementRegion(cast(R), PointeeTy); + + case MemRegion::ElementRegionKind: { + // If we are casting from an ElementRegion to another type, the + // algorithm is as follows: + // + // (1) Compute the "raw offset" of the ElementRegion from the + // base region. This is done by calling 'getAsRawOffset()'. + // + // (2a) If we get a 'RegionRawOffset' after calling + // 'getAsRawOffset()', determine if the absolute offset + // can be exactly divided into chunks of the size of the + // casted-pointee type. If so, create a new ElementRegion with + // the pointee-cast type as the new ElementType and the index + // being the offset divded by the chunk size. If not, create + // a new ElementRegion at offset 0 off the raw offset region. + // + // (2b) If we don't a get a 'RegionRawOffset' after calling + // 'getAsRawOffset()', it means that we are at offset 0. + // + // FIXME: Handle symbolic raw offsets. + + const ElementRegion *elementR = cast(R); + const RegionRawOffset &rawOff = elementR->getAsArrayOffset(); + const MemRegion *baseR = rawOff.getRegion(); + + // If we cannot compute a raw offset, throw up our hands and return + // a NULL MemRegion*. + if (!baseR) + return nullptr; + + CharUnits off = rawOff.getOffset(); + + if (off.isZero()) { + // Edge case: we are at 0 bytes off the beginning of baseR. We + // check to see if type we are casting to is the same as the base + // region. If so, just return the base region. + if (const auto *TR = dyn_cast(baseR)) { + QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); + QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); + if (CanonPointeeTy == ObjTy) + return baseR; + } + + // Otherwise, create a new ElementRegion at offset 0. + return MakeElementRegion(cast(baseR), PointeeTy); + } + + // We have a non-zero offset from the base region. We want to determine + // if the offset can be evenly divided by sizeof(PointeeTy). If so, + // we create an ElementRegion whose index is that value. Otherwise, we + // create two ElementRegions, one that reflects a raw offset and the other + // that reflects the cast. + + // Compute the index for the new ElementRegion. + int64_t newIndex = 0; + const MemRegion *newSuperR = nullptr; + + // We can only compute sizeof(PointeeTy) if it is a complete type. + if (!PointeeTy->isIncompleteType()) { + // Compute the size in **bytes**. + CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy); + if (!pointeeTySize.isZero()) { + // Is the offset a multiple of the size? If so, we can layer the + // ElementRegion (with elementType == PointeeTy) directly on top of + // the base region. + if (off % pointeeTySize == 0) { + newIndex = off / pointeeTySize; + newSuperR = baseR; + } + } + } + + if (!newSuperR) { + // Create an intermediate ElementRegion to represent the raw byte. + // This will be the super region of the final ElementRegion. + newSuperR = MakeElementRegion(cast(baseR), Ctx.CharTy, + off.getQuantity()); + } + + return MakeElementRegion(cast(newSuperR), PointeeTy, newIndex); + } + } + + llvm_unreachable("unreachable"); +} + // FIXME: should rewrite according to the cast kind. SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { castTy = Context.getCanonicalType(castTy); @@ -574,8 +737,7 @@ if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) { if (Optional LV = val.getAs()) { if (const MemRegion *R = LV->getLoc().getAsRegion()) { - StoreManager &storeMgr = StateMgr.getStoreManager(); - R = storeMgr.castRegion(R, castTy); + R = castRegion(R, castTy); return R ? SVal(loc::MemRegionVal(R)) : UnknownVal(); } return LV->getLoc(); @@ -650,12 +812,10 @@ assert(Loc::isLocType(originalTy) || originalTy->isFunctionType() || originalTy->isBlockPointerType() || castTy->isReferenceType()); - StoreManager &storeMgr = StateMgr.getStoreManager(); - - // Delegate to store manager to get the result of casting a region to a - // different type. If the MemRegion* returned is NULL, this expression - // Evaluates to UnknownVal. - R = storeMgr.castRegion(R, castTy); + // Get the result of casting a region to a different type. + // If the MemRegion* returned is NULL, this expression evaluates to + // UnknownVal. + R = castRegion(R, castTy); return R ? SVal(loc::MemRegionVal(R)) : UnknownVal(); } Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -65,8 +65,34 @@ // Transfer function for Casts. //===----------------------------------------------------------------------===// +static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) { + return ty1->getPointeeType().getCanonicalType().getTypePtr() == + ty2->getPointeeType().getCanonicalType().getTypePtr(); +} + SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) { assert(Val.getAs() || Val.getAs()); + + // When retrieving symbolic pointer and expecting a non-void pointer, + // wrap them into element regions of the expected type if necessary. + // it is necessary to make sure that the retrieved value makes sense, + // because there's no other cast in the AST that would tell us to cast + // it to the correct pointer type. We might need to do that for non-void + // pointers as well. + // FIXME: We really need a single good function to perform casts for us + // correctly every time we need it. + if (CastTy->isPointerType() && !CastTy->isVoidPointerType()) { + if (const auto *SR = dyn_cast_or_null(Val.getAsRegion())) { + QualType sr = SR->getSymbol()->getType(); + if (!hasSameUnqualifiedPointeeType(sr, CastTy)) + return loc::MemRegionVal(castRegion(SR, CastTy)); + } + // Next fixes pointer dereference using type different from its initial + // one. See PR37503 for details. + if (const auto *ER = dyn_cast_or_null(Val.getAsRegion())) + return loc::MemRegionVal(castRegion(ER, CastTy)); + } + return Val.getAs() ? evalCastFromLoc(Val.castAs(), CastTy) : evalCastFromNonLoc(Val.castAs(), CastTy); } @@ -135,7 +161,7 @@ // can be introduced by the frontend for corner cases, e.g // casting from va_list* to __builtin_va_list&. // - if (Loc::isLocType(castTy) || castTy->isReferenceType()) + if (Loc::isLocType(castTy)) return val; // FIXME: Handle transparent unions where a value can be "transparently" Index: clang/lib/StaticAnalyzer/Core/Store.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/Store.cpp +++ clang/lib/StaticAnalyzer/Core/Store.cpp @@ -57,13 +57,6 @@ return Store; } -const ElementRegion *StoreManager::MakeElementRegion(const SubRegion *Base, - QualType EleTy, - uint64_t index) { - NonLoc idx = svalBuilder.makeArrayIndex(index); - return MRMgr.getElementRegion(EleTy, idx, Base, svalBuilder.getContext()); -} - const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R, QualType T) { NonLoc idx = svalBuilder.makeZeroArrayIndex(); @@ -71,161 +64,6 @@ return MRMgr.getElementRegion(T, idx, R, Ctx); } -const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) { - ASTContext &Ctx = StateMgr.getContext(); - - // Handle casts to Objective-C objects. - if (CastToTy->isObjCObjectPointerType()) - return R->StripCasts(); - - if (CastToTy->isBlockPointerType()) { - // FIXME: We may need different solutions, depending on the symbol - // involved. Blocks can be casted to/from 'id', as they can be treated - // as Objective-C objects. This could possibly be handled by enhancing - // our reasoning of downcasts of symbolic objects. - if (isa(R) || isa(R)) - return R; - - // We don't know what to make of it. Return a NULL region, which - // will be interpreted as UnknownVal. - return nullptr; - } - - // Now assume we are casting from pointer to pointer. Other cases should - // already be handled. - QualType PointeeTy = CastToTy->getPointeeType(); - QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); - - // Handle casts to void*. We just pass the region through. - if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy) - return R; - - // Handle casts from compatible types. - if (R->isBoundable()) - if (const auto *TR = dyn_cast(R)) { - QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); - if (CanonPointeeTy == ObjTy) - return R; - } - - // Process region cast according to the kind of the region being cast. - switch (R->getKind()) { - case MemRegion::CXXThisRegionKind: - case MemRegion::CodeSpaceRegionKind: - case MemRegion::StackLocalsSpaceRegionKind: - case MemRegion::StackArgumentsSpaceRegionKind: - case MemRegion::HeapSpaceRegionKind: - case MemRegion::UnknownSpaceRegionKind: - case MemRegion::StaticGlobalSpaceRegionKind: - case MemRegion::GlobalInternalSpaceRegionKind: - case MemRegion::GlobalSystemSpaceRegionKind: - case MemRegion::GlobalImmutableSpaceRegionKind: { - llvm_unreachable("Invalid region cast"); - } - - case MemRegion::FunctionCodeRegionKind: - case MemRegion::BlockCodeRegionKind: - case MemRegion::BlockDataRegionKind: - case MemRegion::StringRegionKind: - // FIXME: Need to handle arbitrary downcasts. - case MemRegion::SymbolicRegionKind: - case MemRegion::AllocaRegionKind: - case MemRegion::CompoundLiteralRegionKind: - case MemRegion::FieldRegionKind: - case MemRegion::ObjCIvarRegionKind: - case MemRegion::ObjCStringRegionKind: - case MemRegion::NonParamVarRegionKind: - case MemRegion::ParamVarRegionKind: - case MemRegion::CXXTempObjectRegionKind: - case MemRegion::CXXBaseObjectRegionKind: - case MemRegion::CXXDerivedObjectRegionKind: - return MakeElementRegion(cast(R), PointeeTy); - - case MemRegion::ElementRegionKind: { - // If we are casting from an ElementRegion to another type, the - // algorithm is as follows: - // - // (1) Compute the "raw offset" of the ElementRegion from the - // base region. This is done by calling 'getAsRawOffset()'. - // - // (2a) If we get a 'RegionRawOffset' after calling - // 'getAsRawOffset()', determine if the absolute offset - // can be exactly divided into chunks of the size of the - // casted-pointee type. If so, create a new ElementRegion with - // the pointee-cast type as the new ElementType and the index - // being the offset divded by the chunk size. If not, create - // a new ElementRegion at offset 0 off the raw offset region. - // - // (2b) If we don't a get a 'RegionRawOffset' after calling - // 'getAsRawOffset()', it means that we are at offset 0. - // - // FIXME: Handle symbolic raw offsets. - - const ElementRegion *elementR = cast(R); - const RegionRawOffset &rawOff = elementR->getAsArrayOffset(); - const MemRegion *baseR = rawOff.getRegion(); - - // If we cannot compute a raw offset, throw up our hands and return - // a NULL MemRegion*. - if (!baseR) - return nullptr; - - CharUnits off = rawOff.getOffset(); - - if (off.isZero()) { - // Edge case: we are at 0 bytes off the beginning of baseR. We - // check to see if type we are casting to is the same as the base - // region. If so, just return the base region. - if (const auto *TR = dyn_cast(baseR)) { - QualType ObjTy = Ctx.getCanonicalType(TR->getValueType()); - QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); - if (CanonPointeeTy == ObjTy) - return baseR; - } - - // Otherwise, create a new ElementRegion at offset 0. - return MakeElementRegion(cast(baseR), PointeeTy); - } - - // We have a non-zero offset from the base region. We want to determine - // if the offset can be evenly divided by sizeof(PointeeTy). If so, - // we create an ElementRegion whose index is that value. Otherwise, we - // create two ElementRegions, one that reflects a raw offset and the other - // that reflects the cast. - - // Compute the index for the new ElementRegion. - int64_t newIndex = 0; - const MemRegion *newSuperR = nullptr; - - // We can only compute sizeof(PointeeTy) if it is a complete type. - if (!PointeeTy->isIncompleteType()) { - // Compute the size in **bytes**. - CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy); - if (!pointeeTySize.isZero()) { - // Is the offset a multiple of the size? If so, we can layer the - // ElementRegion (with elementType == PointeeTy) directly on top of - // the base region. - if (off % pointeeTySize == 0) { - newIndex = off / pointeeTySize; - newSuperR = baseR; - } - } - } - - if (!newSuperR) { - // Create an intermediate ElementRegion to represent the raw byte. - // This will be the super region of the final ElementRegion. - newSuperR = MakeElementRegion(cast(baseR), Ctx.CharTy, - off.getQuantity()); - } - - return MakeElementRegion(cast(newSuperR), PointeeTy, newIndex); - } - } - - llvm_unreachable("unreachable"); -} - static bool regionMatchesCXXRecordType(SVal V, QualType Ty) { const MemRegion *MR = V.getAsRegion(); if (!MR) @@ -418,21 +256,6 @@ return UnknownVal(); } - // When retrieving symbolic pointer and expecting a non-void pointer, - // wrap them into element regions of the expected type if necessary. - // SValBuilder::dispatchCast() doesn't do that, but it is necessary to - // make sure that the retrieved value makes sense, because there's no other - // cast in the AST that would tell us to cast it to the correct pointer type. - // We might need to do that for non-void pointers as well. - // FIXME: We really need a single good function to perform casts for us - // correctly every time we need it. - if (castTy->isPointerType() && !castTy->isVoidPointerType()) - if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) { - QualType sr = SR->getSymbol()->getType(); - if (!hasSameUnqualifiedPointeeType(sr, castTy)) - return loc::MemRegionVal(castRegion(SR, castTy)); - } - return svalBuilder.dispatchCast(V, castTy); } Index: clang/test/Analysis/casts.c =================================================================== --- clang/test/Analysis/casts.c +++ clang/test/Analysis/casts.c @@ -245,3 +245,8 @@ return a * a; } +void no_crash_reinterpret_char_as_uchar(char ***a, int *b) { + *(unsigned char **)a = (unsigned char *)b; + if (**a == 0) // no-crash + ; +} Index: clang/test/Analysis/string.c =================================================================== --- clang/test/Analysis/string.c +++ clang/test/Analysis/string.c @@ -363,6 +363,14 @@ strcpy(x, y); // no-warning } +// PR37503 +void *get_void_ptr(); +char ***type_punned_ptr; +void strcpy_no_assertion(char c) { + *(unsigned char **)type_punned_ptr = (unsigned char *)(get_void_ptr()); + strcpy(**type_punned_ptr, &c); // no-crash +} + //===----------------------------------------------------------------------=== // stpcpy() //===----------------------------------------------------------------------===