[cfe-commits] r75356 - in /cfe/trunk: lib/Analysis/GRExprEngine.cpp test/Analysis/misc-ps.m

[Ted Kremenek]

Thanks Zhongxing.  This is a tricky one.  I'll look at your patch this  
weekend.

On Jul 11, 2009, at 3:42 AM, Zhongxing Xu wrote:

> Hi Ted,
>
> Here is another fix for this bug. Instead of recovering from a wrong
> invalidation, this patch aims to invalidate the region correctly. It
> uses the cast-to type to invalidate the region when available. To
> avoid invalid cast-to type like 'void*' or 'id', region store now only
> records non-generic casts of regions.
>
> On Sat, Jul 11, 2009 at 12:38 PM, Ted Kremenek<kremenek at apple.com>  
> wrote:
>> Author: kremenek
>> Date: Fri Jul 10 23:38:49 2009
>> New Revision: 75356
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=75356&view=rev
>> Log:
>> Handle insidious corner case exposed by RegionStoreManager when  
>> handling void* values that are bound
>> to symbolic regions and then treated like integers.
>>
>> Modified:
>>    cfe/trunk/lib/Analysis/GRExprEngine.cpp
>>    cfe/trunk/test/Analysis/misc-ps.m
>>
>> Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=75356&r1=75355&r2=75356&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
>> +++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Fri Jul 10 23:38:49 2009
>> @@ -1110,6 +1110,19 @@
>>   }
>>   else {
>>     SVal V = state->getSVal(cast<Loc>(location), Ex->getType());
>> +
>> +    // Casts can create weird scenarios where a location must be  
>> implicitly
>> +    // converted to something else.  For example:
>> +    //
>> +    //  void *x;
>> +    //  int *y = (int*) &x; // void** -> int* cast.
>> +    //  invalidate(y);  // 'x' now binds to a symbolic region
>> +    //  int z = *y;
>> +    //
>> +    if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {
>> +      V = EvalCast(V, Ex->getType());
>> +    }
>> +
>>     MakeNode(Dst, Ex, Pred, state->bindExpr(Ex, V), K, tag);
>>   }
>>  }
>>
>> Modified: cfe/trunk/test/Analysis/misc-ps.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=75356&r1=75355&r2=75356&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/test/Analysis/misc-ps.m (original)
>> +++ cfe/trunk/test/Analysis/misc-ps.m Fri Jul 10 23:38:49 2009
>> @@ -350,3 +350,24 @@
>>     return;
>>  }
>>
>> +// RegionStoreManager previously crashed on this example.  The  
>> problem is that
>> +// the value bound to the field of b->grue after the call to  
>> testB_aux is
>> +// a symbolic region.  The second '*__gruep__' involves performing  
>> a load
>> +// from a 'int*' that really is a 'void**'.  The loaded location  
>> must be
>> +// implicitly converted to an integer that wraps a location.   
>> Previosly we would
>> +// get a crash here due to an assertion failure.
>> +typedef struct _BStruct { void *grue; } BStruct;
>> +void testB_aux(void *ptr);
>> +void testB(BStruct *b) {
>> +  {
>> +    int *__gruep__ = ((int *)&((b)->grue));
>> +    int __gruev__ = *__gruep__;
>> +    testB_aux(__gruep__);
>> +  }
>> +  {
>> +    int *__gruep__ = ((int *)&((b)->grue));
>> +    int __gruev__ = *__gruep__;
>> +    if (~0 != __gruev__) {}
>> +  }
>> +}
>> +
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
> <cast.diff>