Re: [PATCH 0/6] [mainline+7.6] PR gdb/15294: list with unlimited listsize broken

[Pedro Alves <palves at redhat dot com>]

On 03/22/2013 07:38 AM, Yao Qi wrote:
> On 03/22/2013 02:29 AM, Pedro Alves wrote:
>> Before the changes starting at
>> <http://sourceware.org/ml/gdb-patches/2012-08/msg00020.html>, the 'set
> 
> The patch of the link above was not committed.  You meant this patch, I think
> 
>  [PATCH 2/3] var_integer -> var_zuinteger_unlimited
>  http://sourceware.org/ml/gdb-patches/2012-08/msg00367.html

Yes, I had found it, but then got confused with the urls.

> and it was committed in revision ac6e6b6e4182a5fd3dfc50504fa4a5fe543faa41

Right.

> 
>> listsize' command only accepted "0" as special value, meaning
>> "unlimited".  The testsuite actually tried "set listsize -1" and
>> expected that to mean unlimited too.
> 
> The patch tried to keep the behaviour that "0" means suppressed and "-1" means unlimited.

-1 didn't mean unlimited at the time of the patch (though
that patch in the url above claims that too), or ever since:

 http://sourceware.org/ml/gdb-patches/2006-01/msg00176.html

despite the comment that it wouldn't change behavior:

 "This doesn't substantively change the result of typing "set listsize -1",
 which shouldn't really be allowed either before or after.  Someone else can
 get inspired and fix that one."

as that's when we started getting the "out of range" error,
cause var_integer handling looked like:

        case var_integer:
          {
            unsigned int val;
            if (arg == NULL)
              error_no_arg (_("integer to set it to."));
            val = parse_and_eval_long (arg);
            if (val == 0)
              *(int *) c->var = INT_MAX;
            else if (val >= INT_MAX)
              error (_("integer %u out of range"), val);
            else
              *(int *) c->var = val;
            break;
          }

and -1 would result in val==0xffffffff.

>>
>> If you tried testing list.exp at the time of that patch above,
> 
> FAOD, the list.exp is run _without_ the patch above.

Correct.  Sorry if the wording wasn't clear here.

> If we go to the revision that the patch was committed, we can see:
> 
> $ git checkout ac6e6b6e4182a5fd3dfc50504fa4a5fe543faa41
> 
> (gdb) PASS: gdb.base/list.exp: list line 10 with listsize 100
> set listsize 0^M
> (gdb) PASS: gdb.base/list.exp: setting listsize to 0 #6
> show listsize^M
> Number of source lines gdb will list by default is 0.^M
> (gdb) PASS: gdb.base/list.exp: show listsize 0 #6
> list 1^M
> (gdb) PASS: gdb.base/list.exp: listsize of 0 suppresses output
> set listsize -1^M
> (gdb) PASS: gdb.base/list.exp: setting listsize to -1 #7
> show listsize^M
> Number of source lines gdb will list by default is unlimited.^M
> (gdb) PASS: gdb.base/list.exp: show listsize unlimited #7
> 

And the following test was:

(gdb) PASS: gdb.base/list.exp: show listsize unlimited #7
list 1
(gdb) XFAIL: gdb.base/list.exp: list line 1 with unlimited listsize

Showing that "set listsize -1" didn't work.  The setup_xfail
and the buggy test masked it, though a bit of manual
"set listsize -1" testing would have caught it.  ;-)

Thanks,
-- 
Pedro Alves