[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 1:55 PM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Dec 15, 2008, at 5:39 PM, Zhongxing Xu wrote:
>
>  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.
>>
>
> Yes, but a "char*" can be either a pointer to an array or a single
> character.  In this case (with alloca) we know p is a pointer to an array of
> bytes, so it simplifies things, but if (for example) the region is symbolic
> we don't know if the void* is a pointer to a single character or to an array
> of bytes.


The semantics of a pointer has two folds: first it indicates an address,
second it has a type, which basically delimits its range. The concept of
region captures this semantics perfectly: a region indicates a location and
has an extent.

Now that we have an explicit type 'char' for the pointer, we'd better stick
to the semantics of the pointer: give a right extent to the pointer. If we
leave the pointer an AnonTypedRegion, we actually contradicts the 1 byte
range specified by the pointer type. On the other hand, an ElementRegion
matches the type of the pointer perfectly.

>
>
>  And later in ArraySubscriptExpr we don't have a chance to do the
>> conversion.
>>
>
>
> Not true.  Just because there isn't a pointer-to-array cast in the AST
> doesn't mean we can't do the conversion.  We can do it when handling the
> ArraySubscriptExpr:
>
> /// VisitArraySubscriptExpr - Transfer function for array accesses
> void GRExprEngine::VisitArraySubscriptExpr(ArraySubscriptExpr* A, NodeTy*
> Pred,
>                                           NodeSet& Dst, bool asLValue) {
>
>  Expr* Base = A->getBase()->IgnoreParens();
>  Expr* Idx  = A->getIdx()->IgnoreParens();
>  NodeSet Tmp;
>  Visit(Base, Pred, Tmp);   // Get Base's rvalue, which should be an LocVal.
>
>  for (NodeSet::iterator I1=Tmp.begin(), E1=Tmp.end(); I1!=E1; ++I1) {
>    NodeSet Tmp2;
>    Visit(Idx, *I1, Tmp2);     // Evaluate the index.
>
>    for (NodeSet::iterator I2=Tmp2.begin(), E2=Tmp2.end(); I2!=E2; ++I2) {
>      const GRState* St = GetState(*I2);
>      // **** Possible insert a call to StoreManager::PointerToArray here
> for the
>      // ****  results of GetSVal(St, Base).
>      SVal V = StateMgr.GetLValue(St, GetSVal(St, Base), GetSVal(St, Idx));
>     ...
>
> I'm not saying this is the best solution; it just seems that using an
> ElementRegion early seems speculative and premature, especially if it really
> isn't an array.  Why not just do it lazily when a region is *used* as an
> array?


This is possible. But I don't think it's necessary for this case. We can
save the trouble by putting a little more effort in CastRegion() where the
semantics also stipulates.


>
>
> Of course, this leads to the question of how we handle general pointer
> arithmetic (one of those things we're basically punting on right now).
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081216/6f97e15e/attachment.html>