On Jul 13, 2009, at 6:28 PM, Zhongxing Xu wrote:
Makes sense. I think we should add a comment about why the check for
'isGenericPtr' is there. Also, we should document that if the region
already has a cast type, casting it to void* doesn't remove that
cast type.
What should the semantics be if there already is a cast type
associated with
a region?
I think it's seldom that programmer would cast it back to generic
pointer intentionally. Often it is cast back to generic
pointer passively. E.g.:
void invalidate(void* p);
int *p = (int*)alloca();
void *q = (void*) p; // people don't do this.
invalidate(p); // people often do this.
Agreed. Should we also consider 'char *' as a case in 'isGenericPtr'?
This code might actually be valid if 'sizeof(struct s) <= sizeof
(int)' and
the alignments of both 'int' and 'struct s' are compatible.
Otherwise,
we'll get a potential buffer overflow that we won't catch.
Alternatively, we
might not actually be doing the right thing. I think this is
something
worth discussing.
The check is done (or should be done) in NewCastRegion(). If the cast
is legal, then type 'struct s' is used to
invalidate the region. I actually am not very clear about what you are
worrying about here.
I think you're right. I think I may have thought too hard about it
and confused myself.
Index: lib/Analysis/GRExprEngine.cpp
===================================================================
--- lib/Analysis/GRExprEngine.cpp (revision 75492)
+++ lib/Analysis/GRExprEngine.cpp (working copy)
@@ -1119,9 +1119,9 @@
// 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());
- }
+ //if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {
+ // V = EvalCast(V, Ex->getType());
+ // }
This is the main change, but I cannot actually evaluate it yet.
Using the
'-analyzer-viz-egraph-graphviz' option, I saw that for the
following code:
int *__gruep__ = ((int *)&((b)->grue));
int __gruev__ = *__gruep__;
that all we end up doing is conjuring a new symbol for the second
declaration of '__gruev__' (even when using RegionStoreManager).
This is expected. We invalidate *__gruep__ with a conjured symbol.
Right! Once again I confused myself; there is a difference between
the conjured symbols used by RegionStore and the ones used to recover
path-sensitivity in GRExprEngine. It would be nice if if we had a way
to distinguish them.
Just as a test, I augmented my copy of 'testB' to contain two
infeasible null dereferences. With RegionStore we get no null
dereference errors (as it should be). BasicStore flags two:
typedef struct _BStruct { void *grue; } BStruct;
void testB_aux(void *ptr);
void testB(BStruct *b) {
{
int *__gruep__ = ((int *)&((b)->grue));
int __gruev__ = *__gruep__;
int __gruev2__ = *__gruep__;
if (__gruev__ != __gruev2__) {
int *p = 0;
*p = 0xDEADBEEF;
}
testB_aux(__gruep__);
}
{
int *__gruep__ = ((int *)&((b)->grue));
int __gruev__ = *__gruep__;
int __gruev2__ = *__gruep__;
if (__gruev__ != __gruev2__) {
int *p = 0;
*p = 0xDEADBEEF;
}
if (~0 != __gruev__) {}
}
}_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
