[cfe-commits] r57240 - /cfe/trunk/lib/Analysis/GRExprEngine.cpp

[Zhongxing Xu]

New patch:

Index: lib/Analysis/GRExprEngine.cpp
===================================================================
--- lib/Analysis/GRExprEngine.cpp       (revision 57240)
+++ lib/Analysis/GRExprEngine.cpp       (working copy)
@@ -1132,10 +1132,12 @@
           case 5:
             if (!memcmp(s, "panic", 5)) Builder->BuildSinks = true;
             else if (!memcmp(s, "error", 5)) {
-              Expr* Arg = *CE->arg_begin();
-              if (IntegerLiteral* IL = dyn_cast<IntegerLiteral>(Arg))
-                if (IL->getValue() != 0)
+              if (CE->getNumArgs() > 0) {
+                RVal X = GetRVal(St, *CE->arg_begin());
+                nonlval::ConcreteInt* CI =
dyn_cast<nonlval::ConcreteInt>(&X);
+                if (CI && CI->getValue() != 0)
                   Builder->BuildSinks = true;
+              }
             }
             break;

On Wed, Oct 8, 2008 at 10:28 AM, Zhongxing Xu <xuzhongxing at gmail.com> wrote:

> Is this OK?
>
> Index: lib/Analysis/GRExprEngine.cpp
> ===================================================================
> --- lib/Analysis/GRExprEngine.cpp       (revision 57240)
> +++ lib/Analysis/GRExprEngine.cpp       (working copy)
> @@ -1132,9 +1132,9 @@
>            case 5:
>              if (!memcmp(s, "panic", 5)) Builder->BuildSinks = true;
>              else if (!memcmp(s, "error", 5)) {
> -              Expr* Arg = *CE->arg_begin();
> -              if (IntegerLiteral* IL = dyn_cast<IntegerLiteral>(Arg))
> -                if (IL->getValue() != 0)
> +              RVal X = GetRVal(St, *CE->arg_begin());
> +              if (nonlval::ConcreteInt* CI =
> dyn_cast<nonlval::ConcreteInt>(&X))
> +                if (CI->getValue() != 0)
>                    Builder->BuildSinks = true;
>              }
>              break;
>
>
>
> On Wed, Oct 8, 2008 at 12:49 AM, Ted Kremenek <kremenek at apple.com> wrote:
>
>>
>> On Oct 7, 2008, at 3:06 AM, Zhongxing Xu wrote:
>>
>>             if (!memcmp(s, "panic", 5)) Builder->BuildSinks = true;
>>> +            else if (!memcmp(s, "error", 5)) {
>>> +              Expr* Arg = *CE->arg_begin();
>>> +              if (IntegerLiteral* IL = dyn_cast<IntegerLiteral>(Arg))
>>> +                if (IL->getValue() != 0)
>>> +                  Builder->BuildSinks = true;
>>> +            }
>>>            break;
>>>
>>
>>
>> Hi Zhongxing,
>>
>> I think this code will crash if error() isn't passed any arguments.  It
>> should check to see if error() has the number of arguments that you expect.
>>
>> Also, is there a reason you check the AST node for an IntegerLiteral
>> instead of just consulting the value of 'CE' that is in the state?  You'll
>> get far more accurate information by looking at the semantics of the
>> argument instead of its syntax.
>>
>> For example, checking for an integer literal will miss the following
>> cases:
>>
>>  error(1 - 1);
>>
>>  error((1));  // note the '()'
>>
>>  x = 1;
>>  error(x);
>>
>> I think just doing GetRVal(St, CE) should do the trick.
>>
>> Ted
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081008/1af727b3/attachment.html>