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<[email protected]> > 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > <cast.diff> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
