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

[Pedro Alves <palves at redhat dot com>]

"set listsize -1" is currently broken:

(gdb) set listsize -1
(gdb) show listsize 
Number of source lines gdb will list by default is unlimited.
(gdb) list 1
(gdb) list 1
(gdb) list 1
(gdb) set listsize 10
(gdb) list 1
1       /* Main function for CLI gdb.  
2          Copyright (C) 2002-2013 Free Software Foundation, Inc.
3
4          This file is part of GDB.
5
6          This program is free software; you can redistribute it and/or modify
7          it under the terms of the GNU General Public License as published by
8          the Free Software Foundation; either version 3 of the License, or
9          (at your option) any later version.
10

Looking up the history behind the behaviour changes of
"set listsize -1", I wrote the below.  Patches will follow as reply to
this message.

Before the changes starting at
<http://sourceware.org/ml/gdb-patches/2012-08/msg00020.html>, the 'set
listsize' command only accepted "0" as special value, meaning
"unlimited".  The testsuite actually tried "set listsize -1" and
expected that to mean unlimited too.

If you tried testing list.exp at the time of that patch above,
you'd get:

  (gdb) PASS: gdb.base/list.exp: list line 10 with listsize 100
  set listsize 0
  (gdb) PASS: gdb.base/list.exp: setting listsize to 0 #6
  show listsize
  Number of source lines gdb will list by default is unlimited.
  (gdb) PASS: gdb.base/list.exp: show listsize unlimited #6
  list 1
  1       #include "list0.h"
  2
  ...
  42          /* Not used for anything */
  43      }
  (gdb) PASS: gdb.base/list.exp: listsize of 0 suppresses output
  set listsize -1
  integer 4294967295 out of range
  (gdb) PASS: gdb.base/list.exp: setting listsize to -1 #7
  show listsize
  Number of source lines gdb will list by default is unlimited.
  (gdb) PASS: gdb.base/list.exp: show listsize unlimited #7
  list 1
  1       #include "list0.h"

Notice that "set listsize -1" actually failed with "integer 4294967295
out of range", but we issued a PASS anyway.

Looking through ancient history, I see e.g., in gdb 4.1 (back in 1991,
yes), "set listsize" was a var_uinteger command (therefore, supposedly
treated as unsigned):

https://github.com/palves/gdb-old-releases/blob/7e43f573878c3c1b8458ddb3240b635f284b59c2/gdb/source.c

  add_show_from_set
    (add_set_cmd ("listsize", class_support, var_uinteger,
		  (char *)&lines_to_list,
	"Set number of source lines gdb will list by default.",
		  &setlist),

But the set command handling actually treated it as signed ...

https://github.com/palves/gdb-old-releases/blob/7e43f573878c3c1b8458ddb3240b635f284b59c2/gdb/command.c

	case var_uinteger:
	  if (arg == NULL)
	    error_no_arg ("integer to set it to.");
	  *(int *) c->var = parse_and_eval_address (arg);
	  if (*(int *) c->var == 0)
	    *(int *) c->var = UINT_MAX;
	  break;


But then, by 4.9 (1993), var_integer was added, and var_uinteger was
fixed like so:

	case var_uinteger:
	  if (arg == NULL)
	    error_no_arg ("integer to set it to.");
	  *(unsigned int *) c->var = parse_and_eval_address (arg);
	  if (*(unsigned int *) c->var == 0)
	    *(unsigned int *) c->var = UINT_MAX;
	  break;

However, "listsize" was still registered as an unsigned command, with
a signed control variable.  This meant that from 4.9 on, "set listsize -1"
actually was equivalent to "set listsize UINT_MAX".

I didn't find any old gdb where "set listsize 0" actually ever meant
"suppress printing".  The first testsuite we have access to appears on
4.10 (thus after set listsize -1 meaning unlimited", and it has:

    # Try listsize of 0 which suppresses printing.

    send "set listsize 0\n"
    expect {
	-re "set listsize 0\r\n$prompt $" {
	    setup_xfail "*-*-*"
	    send "show listsize\n"
	    expect {
		-re "Number of source lines .* is 0.\r\n.*$prompt $" {
		    pass "listsize of 0 displays as 0"
		}
		-re "Number of source lines .* is unlimited.\r\n.*$prompt $" {
		    fail "listsize of 0 displays as unlimited"
		}
	    }
    }

Notice the "setup_xfail".  So this was failing at the time, and the
test was written to accept the failure as indication that GDB's
behavior was not ideal.

Later on, the "set listsize" command was changed to a var_integer
(made sense, since the controlling variable is actually signed), which
made "set listsize -1" not work anymore.

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

Dan wrote:

 "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."

The one change that caused was that the var_integer set command
handling did have range validation, so -1 has since been flagged as
invalid, but the testsuite was buggy and never caught the "integer
4294967295 out of range" regression.

So from that point on, _only_ "set listsize 0" meant unlimited.

The testsuite disagreed with GDB's behavior all along, because whoever
wrote the list.exp test clearly thought -1 should mean unlimited, and
0 no output.

It was only years later, recently (after 7.5) that after all this
confusion, we changed GDB to actually do what the original list.exp
test expected.  Well, set listsize -1 got broken in the progress, and
that went unnoticed because the test is actually broken, and, has a
setup_xfail "*-*-*" anyway...

In this version of the series, I'm actually proposing to revert back
the behavior of "set listsize 0" to mean unlimited.
    
In v1, I suggested fixing the code and keeping the new behavior, but I
found that "set listsize 0" is currently used in the wild, and we do
have a bunch of other commands where "0" means unlimited, so I'm
thinking that changing this command alone in isolation is not a good
idea.
    
So I now strongly prefer reverting back the behavior in 7.6 to the
same behavior the command has had since 2006 (0==unlimited, -1=error).
(Before that, set listsize -1 would be accepted as unlimited as well.)
    
After 7.6 is out, in mainline, we can get back to reconsidering
changing this command's behavior, if there's a real need for being
able to suppress output.  For now, let's play it safe.
    
All tested on x86_64 Fedora 17.

---

Pedro Alves (4):
      list.exp: Catch "set listsize" failures (and "set listsize -1/0"'s history).
      list.exp: Adjust "set listsize -1" to current test source's real line count.
      list.exp: Avoid hardcoding line numbers.
      Fix PR gdb/15294: list with unlimited listsize broken


 gdb/source.c                    |    8 ++++---
 gdb/testsuite/gdb.base/list.exp |   43 ++++++++++++++++++++++++---------------
 gdb/testsuite/gdb.base/list0.c  |    2 +-
 3 files changed, 31 insertions(+), 22 deletions(-)

-- 
Pedro Alves