<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 13, 2009, at 6:28 PM, Zhongxing Xu wrote:</div><blockquote type="cite"><div><blockquote type="cite"><font class="Apple-style-span" color="#000000"><br></font></blockquote><blockquote type="cite">Makes sense. I think we should add a comment about why the check for<br></blockquote><blockquote type="cite">'isGenericPtr' is there. Also, we should document that if the region<br></blockquote><blockquote type="cite">already has a cast type, casting it to void* doesn't remove that cast type.<br></blockquote><blockquote type="cite">What should the semantics be if there already is a cast type associated with<br></blockquote><blockquote type="cite">a region?<br></blockquote><br>I think it's seldom that programmer would cast it back to generic<br>pointer intentionally. Often it is cast back to generic<br>pointer passively. E.g.:<br><br>void invalidate(void* p);<br>int *p = (int*)alloca();<br>void *q = (void*) p; // people don't do this.<br>invalidate(p); // people often do this.<br></div></blockquote><div><br></div><div>Agreed. Should we also consider 'char *' as a case in 'isGenericPtr'?</div><div><br></div><blockquote type="cite"><div><blockquote type="cite">This code might actually be valid if 'sizeof(struct s) <= sizeof(int)' and</blockquote><blockquote type="cite">the alignments of both 'int' and 'struct s' are compatible. Otherwise,<br></blockquote><blockquote type="cite">we'll get a potential buffer overflow that we won't catch. Alternatively, we<br></blockquote><blockquote type="cite">might not actually be doing the right thing. I think this is something<br></blockquote><blockquote type="cite">worth discussing.<br></blockquote><br>The check is done (or should be done) in NewCastRegion(). If the cast<br>is legal, then type 'struct s' is used to<br>invalidate the region. I actually am not very clear about what you are<br>worrying about here.<br></div></blockquote><div><br></div><div>I think you're right. I think I may have thought too hard about it and confused myself.</div><div><br></div><div><br></div><br><blockquote type="cite"><div><br><blockquote type="cite">Index: lib/Analysis/GRExprEngine.cpp<br></blockquote><blockquote type="cite">===================================================================<br></blockquote><blockquote type="cite">--- lib/Analysis/GRExprEngine.cpp (revision 75492)<br></blockquote><blockquote type="cite">+++ lib/Analysis/GRExprEngine.cpp (working copy)<br></blockquote><blockquote type="cite">@@ -1119,9 +1119,9 @@<br></blockquote><blockquote type="cite"> // invalidate(y); // 'x' now binds to a symbolic region<br></blockquote><blockquote type="cite"> // int z = *y;<br></blockquote><blockquote type="cite"> //<br></blockquote><blockquote type="cite">- if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {<br></blockquote><blockquote type="cite">- V = EvalCast(V, Ex->getType());<br></blockquote><blockquote type="cite">- }<br></blockquote><blockquote type="cite">+ //if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {<br></blockquote><blockquote type="cite">+ // V = EvalCast(V, Ex->getType());<br></blockquote><blockquote type="cite">+ // }<br></blockquote><blockquote type="cite">This is the main change, but I cannot actually evaluate it yet. Using the<br></blockquote><blockquote type="cite">'-analyzer-viz-egraph-graphviz' option, I saw that for the following code:<br></blockquote><blockquote type="cite"> int *__gruep__ = ((int *)&((b)->grue));<br></blockquote><blockquote type="cite"> int __gruev__ = *__gruep__;<br></blockquote><blockquote type="cite">that all we end up doing is conjuring a new symbol for the second<br></blockquote><blockquote type="cite">declaration of '__gruev__' (even when using RegionStoreManager).<br></blockquote><br>This is expected. We invalidate *__gruep__ with a conjured symbol.<br></div></blockquote><div><br></div><div>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.</div><div><br></div><div>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:</div><div><br></div><div><div>typedef struct _BStruct { void *grue; } BStruct;</div><div>void testB_aux(void *ptr);</div><div>void testB(BStruct *b) {</div><div> {</div><div> int *__gruep__ = ((int *)&((b)->grue));</div><div> int __gruev__ = *__gruep__;</div><div> int __gruev2__ = *__gruep__;</div><div> if (__gruev__ != __gruev2__) {</div><div> int *p = 0;</div><div> *p = 0xDEADBEEF;</div><div> }</div><div> </div><div> testB_aux(__gruep__);</div><div> }</div><div> {</div><div> int *__gruep__ = ((int *)&((b)->grue));</div><div> int __gruev__ = *__gruep__;</div><div> int __gruev2__ = *__gruep__;</div><div> if (__gruev__ != __gruev2__) {</div><div> int *p = 0;</div><div> *p = 0xDEADBEEF;</div><div> }</div><div> </div><div> if (~0 != __gruev__) {}</div><div> }</div><div>}</div></div></div></body></html>