[cfe-commits] r59957 - in /cfe/trunk: include/clang/Analysis/PathSensitive/Store.h lib/Analysis/GRExprEngine.cpp lib/Analysis/RegionStore.cpp

Ted Kremenek kremenek at apple.com
Mon Nov 24 11:03:29 PST 2008


Hi Zhongxing,

Why use the GDM to represent the byte extent of an AllocaRegion  
instead of storing it directly in the region object itself?  I don't  
think it's necessarily a bad idea; I'm just curious about the overall  
design.

As for your comments about the signedness/unsignedness, this is  
definitely something we should discuss in a separate thread.  I'll try  
and write up some thoughts I have on the matter over the next couple  
days.

Ted

On Nov 24, 2008, at 1:44 AM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Mon Nov 24 03:44:56 2008
> New Revision: 59957
>
> URL: http://llvm.org/viewvc/llvm-project?rev=59957&view=rev
> Log:
> Add support for AllocaRegion extent with GDM.
>
> One design problem that is emerging is the signed-ness problem  
> during static
> analysis. Many unsigned value have to be converted into signed value  
> because
> it partipates in operations with signed values.
>
> On the other hand, we cannot blindly make all values occuring in  
> static analysis
> signed, because we do have cases where unsignedness is required, for  
> example,
> integer overflow detection.
>
> Modified:
>    cfe/trunk/include/clang/Analysis/PathSensitive/Store.h
>    cfe/trunk/lib/Analysis/GRExprEngine.cpp
>    cfe/trunk/lib/Analysis/RegionStore.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Store.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Store.h?rev=59957&r1=59956&r2=59957&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/include/clang/Analysis/PathSensitive/Store.h (original)
> +++ cfe/trunk/include/clang/Analysis/PathSensitive/Store.h Mon Nov  
> 24 03:44:56 2008
> @@ -97,6 +97,11 @@
>   virtual Store BindDecl(Store store, const VarDecl* VD, SVal*  
> InitVal,
>                          unsigned Count) = 0;
>
> +  virtual const GRState* setExtent(const GRState* St,
> +                                   const MemRegion* R, SVal Extent) {
> +    return St;
> +  }
> +
>   virtual void print(Store store, std::ostream& Out,
>                      const char* nl, const char *sep) = 0;
>
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=59957&r1=59956&r2=59957&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Mon Nov 24 03:44:56 2008
> @@ -1291,6 +1291,13 @@
>             MemRegionManager& RM =  
> getStateManager().getRegionManager();
>             const MemRegion* R =
>               RM.getAllocaRegion(CE, Builder->getCurrentBlockCount());
> +
> +            // Set the extent of the region in bytes. This enables  
> us to use the
> +            // SVal of the argument directly. If we save the extent  
> in bits, we
> +            // cannot represent values like symbol*8.
> +            SVal Extent = GetSVal(St, *(CE->arg_begin()));
> +            St = getStoreManager().setExtent(St, R, Extent);
> +
>             MakeNode(Dst, CE, *DI, BindExpr(St, CE,  
> loc::MemRegionVal(R)));
>             continue;
>           }
>
> Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=59957&r1=59956&r2=59957&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
> +++ cfe/trunk/lib/Analysis/RegionStore.cpp Mon Nov 24 03:44:56 2008
> @@ -26,12 +26,13 @@
>
> using namespace clang;
>
> +// Actual Store type.
> typedef llvm::ImmutableMap<const MemRegion*, SVal> RegionBindingsTy;
> +
> +// RegionView GDM stuff.
> typedef llvm::ImmutableList<const MemRegion*> RegionViewTy;
> typedef llvm::ImmutableMap<const MemRegion*, RegionViewTy>  
> RegionViewMapTy;
> -
> static int RegionViewMapTyIndex = 0;
> -
> namespace clang {
> template<> struct GRStateTrait<RegionViewMapTy>
>   : public GRStatePartialTrait<RegionViewMapTy> {
> @@ -39,6 +40,18 @@
> };
> }
>
> +// RegionExtents GDM stuff.
> +// Currently RegionExtents are in bytes. We can change this  
> representation when
> +// there are real requirements.
> +typedef llvm::ImmutableMap<const MemRegion*, SVal> RegionExtentsTy;
> +static int RegionExtentsTyIndex = 0;
> +namespace clang {
> +template<> struct GRStateTrait<RegionExtentsTy>
> +  : public GRStatePartialTrait<RegionExtentsTy> {
> +  static void* GDMIndex() { return &RegionExtentsTyIndex; }
> +};
> +}
> +
> namespace {
>
> class VISIBILITY_HIDDEN RegionStoreManager : public StoreManager {
> @@ -112,6 +125,8 @@
>
>   Store BindDecl(Store store, const VarDecl* VD, SVal* InitVal,  
> unsigned Count);
>
> +  const GRState* setExtent(const GRState* St, const MemRegion* R,  
> SVal Extent);
> +
>   static inline RegionBindingsTy GetRegionBindings(Store store) {
>    return RegionBindingsTy(static_cast<const  
> RegionBindingsTy::TreeTy*>(store));
>   }
> @@ -279,9 +294,38 @@
>   }
>
>   if (const AnonTypedRegion* ATR = dyn_cast<AnonTypedRegion>(R)) {
> -    // FIXME: Unsupported yet.
> -    ATR = 0;
> -    return UnknownVal();
> +    GRStateRef state(St, StateMgr);
> +
> +    // Get the size of the super region in bytes.
> +    RegionExtentsTy::data_type* T
> +      = state.get<RegionExtentsTy>(ATR->getSuperRegion());
> +
> +    assert(T && "region extent not exist");
> +
> +    // Assume it's ConcreteInt for now.
> +    llvm::APSInt SSize = cast<nonloc::ConcreteInt>(*T).getValue();
> +
> +    // Get the size of the element in bits.
> +    QualType ElemTy = cast<PointerType>(ATR- 
> >getType(getContext()).getTypePtr())
> +                      ->getPointeeType();
> +
> +    uint64_t X = getContext().getTypeSize(ElemTy);
> +
> +    const llvm::APSInt& ESize = getBasicVals().getValue(X,  
> SSize.getBitWidth(),
> +                                                        false);
> +
> +    // Calculate the number of elements.
> +
> +    // FIXME: What do we do with signed-ness problem? Shall we make  
> all APSInts
> +    // signed?
> +    if (SSize.isUnsigned())
> +      SSize.setIsSigned(true);
> +
> +    // FIXME: move this operation into BasicVals.
> +    const llvm::APSInt S =
> +      (SSize * getBasicVals().getValue(8, SSize.getBitWidth(),  
> false)) / ESize;
> +
> +    return NonLoc::MakeVal(getBasicVals(), S);
>   }
>
>   if (const FieldRegion* FR = dyn_cast<FieldRegion>(R)) {
> @@ -547,6 +591,13 @@
>   return store;
> }
>
> +const GRState* RegionStoreManager::setExtent(const GRState* St,
> +                                             const MemRegion* R,  
> SVal Extent) {
> +  GRStateRef state(St, StateMgr);
> +  return state.set<RegionExtentsTy>(R, Extent);
> +}
> +
> +
> Store RegionStoreManager::RemoveDeadBindings(Store store, Stmt* Loc,
>                                              const LiveVariables&  
> Live,
>                            llvm::SmallVectorImpl<const MemRegion*>&  
> RegionRoots,
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list