[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

[Zhongxing Xu]

On Tue, Dec 16, 2008 at 5:02 AM, Ted Kremenek <kremenek at apple.com> wrote:

>
> 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.
>

I agree in your example we should not convert p to point to the zero-index
element. But in my example:

char * p = alloca(10);
p[1] = 3;

p *is* a pointer to 'char' when we do CastRegion. And later in
ArraySubscriptExpr we don't have a chance to do the conversion.

(BinaryOperator 0xa7c4088 <line:4:3, col:10> 'char' '='
    (ArraySubscriptExpr 0xa7c4028 <col:3, col:6> 'char'
      (DeclRefExpr 0xa7c3fe0 <col:3> 'char *' Var='p' 0xa7bd608)
      (IntegerLiteral 0xa7c4000 <col:5> 'int' 1))
    (ImplicitCastExpr 0xa7c4068 <col:10> 'char'
      (CharacterLiteral 0xa7c4048 <col:10> 'int' 97))))
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081216/5e3d6a12/attachment.html>