<div dir="ltr"><br><br><div class="gmail_quote">On Thu, Oct 9, 2008 at 4:01 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d"><br>
On Oct 7, 2008, at 7:43 PM, Zhongxing Xu wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
New patch:<br>
<br>
Index: lib/Analysis/GRExprEngine.cpp<br>
===================================================================<br>
--- lib/Analysis/GRExprEngine.cpp       (revision 57240)<br>
+++ lib/Analysis/GRExprEngine.cpp       (working copy)<br>
@@ -1132,10 +1132,12 @@<br>
           case 5:<br>
             if (!memcmp(s, "panic", 5)) Builder->BuildSinks = true;<br>
             else if (!memcmp(s, "error", 5)) {<br>
-              Expr* Arg = *CE->arg_begin();<br>
-              if (IntegerLiteral* IL = dyn_cast<IntegerLiteral>(Arg))<br>
-                if (IL->getValue() != 0)<br>
+              if (CE->getNumArgs() > 0) {<br>
+                RVal X = GetRVal(St, *CE->arg_begin());<br>
+                nonlval::ConcreteInt* CI = dyn_cast<nonlval::ConcreteInt>(&X);<br>
+                if (CI && CI->getValue() != 0)<br>
                   Builder->BuildSinks = true;<br>
+              }<br>
             }<br>
             break;<br>
</blockquote>
<br></div>
Looks great!  Please apply!<br>
<br>
Two possible (optional) amendments.<br>
<br>
If you expect that "error" takes a exact number of arguments and has a particular signature (e.g., the first parameter must be a typedef of a specific name), you may wish to match against those specific properties, since "error" may be a function that is implemented differently in different code bases.  Incidentally, what code base are you analyzing?</blockquote>
<div><br>I was scanning GNU tar 1.20: increment.c. And omit error() caused a false alarm of null dereference.<br><br>SYNOPSIS<br>       #include <error.h><br><br>       void error(int status, int errnum, const char *format, ...);<br>
<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<br>
A second suggestion is that you could possibly use the "Assume" logic to check if the argument could be '1', and when it could be then make the node a sink.  This allows the path pruning to also work in the case of symbolic values, e.g:<br>

<br>
  int foo(int code) {<br>
   error(code);<br>
   // more stuff that executes if code != 1<br>
  }<br>
<br>
The idea would be to split the state, just as we do with dereferences (i.e., where we assume that the pointer could be null and then also assume that the pointer is null).  It makes the code a little more complicated, but it does make it handle many more cases, and causes the analysis engine to reason about all the constraints that it has and not just prune the path when the RVal evaluates to the constant '1'.<br>

</blockquote></div><br>I'll leave a FIXME for this. I shall fix some Visit* logic in GRExprEngine.cpp before extending the RegionStoreManager.<br></div>