[cfe-commits] r60995 - in /cfe/trunk: include/clang/Analysis/PathSensitive/MemRegion.h include/clang/Analysis/PathSensitive/Store.h lib/Analysis/BasicObjCFoundationChecks.cpp lib/Analysis/BasicStore.cpp lib/Analysis/CFRefCount.cpp lib/Analysis/GR

[Ted Kremenek]

On Dec 14, 2008, at 11:30 AM, Zhongxing Xu wrote:

>
> -    // Add a RegionView to base region.
> -    return std::make_pair(AddRegionView(St, TR, AR),  
> loc::MemRegionVal(ER));
> +StoreManager::CastResult
> +RegionStoreManager::CastRegion(const GRState* state, const  
> MemRegion* R,
> +                               QualType CastToTy) {
> +
> +  // Return the same region if the region types are compatible.
> +  if (const TypedRegion* TR = dyn_cast<TypedRegion>(R)) {
> +    ASTContext& Ctx = StateMgr.getContext();
> +    QualType Ta = Ctx.getCanonicalType(TR->getLValueType(Ctx));
> +    QualType Tb = Ctx.getCanonicalType(CastToTy);
> +
> +    if (Ta == Tb)
> +      return CastResult(state, R);
>   }
> -
> -  // Default case.
> -  return std::make_pair(St, UnknownVal());
> +
> +  const MemRegion* ViewR = MRMgr.getAnonTypedRegion(CastToTy, R);
> +  return CastResult(AddRegionView(state, ViewR, R), ViewR);
>  }
>
> Hi Ted,
>
> Here we still wish to return an ElementRegion instead of a raw  
> AnonTypedRegion. Consider this code:
>
> char* p = alloca(10); // after this expression, p is expected to be  
> a pointer to the first element of the array.
> p[0] = 'a';
>
> p is expected to have ElementRegion location value in  
> RegionStoreManager::getLValueElement().
>

I'm not convinced.  Fundamentally why should an untyped region be  
assumed to a be a pointer to an array before the moment it is actually  
*used* as an array?  An untyped pointer could easily be a pointer to a  
single object.

Consider:

   void* p = alloca(4);
   ((int*) p)[0] = 4;

The ast dump for these two statements is:

(CompoundStmt 0x1027263a0 <stack-addr-ps.c:44:21, line:47:1>
   (DeclStmt 0x1027260c0 <line:45:3>
     0x102725fa0 "void *p =
       (CallExpr 0x1027261a0 </usr/include/alloca.h:44:24, col:45>  
'void *'
         (ImplicitCastExpr 0x102726090 <col:24> 'void *(*)(unsigned  
long)'
           (DeclRefExpr 0x102725ed0 <col:24> 'void *(unsigned long)'  
FunctionDecl='__builtin_alloca' 0x102725bc0))
         (ImplicitCastExpr 0x1027261d0 <stack-addr-ps.c:45:20>  
'unsigned long'
           (IntegerLiteral 0x102726050 <col:20> 'int' 4)))"
   (BinaryOperator 0x102726360 <line:46:3, col:19> 'int' '='
     (ArraySubscriptExpr 0x1027262e0 <col:3, col:15> 'int'
       (ParenExpr 0x102726270 <col:3, col:12> 'int *'
         (CStyleCastExpr 0x102726230 <col:4, col:11> 'int *'
           (DeclRefExpr 0x102726200 <col:11> 'void *' Var='p'  
0x102725fa0)))
       (IntegerLiteral 0x1027262a0 <col:14> 'int' 0))
     (IntegerLiteral 0x102726320 <col:19> 'int' 4)))

It seems to me that when handling 'ArraySubscriptExpr' we can layer an  
'ElementRegion' at the zero-index for the base.  The logic in  
GRExprEngine that handles ArraySubscriptExpr can call a  
"ConvertPointerToArray" in StoreManager to "convert" the base to  
whatever the StoreManager desires for handling arrays.

Having a callout function for this also allows the StoreManager to  
know what is going on (i.e., a pointer is now being used as an array)  
and do some extra checking for finding bugs.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081215/153a3c54/attachment.html>