This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

"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.
4          This file is part of GDB.
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.

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
<>, 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"
  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):

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

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

	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;

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;

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.

Dan wrote:

 "As Mark also noticed, changing lines_to_list to an unsigned integer is the
 wrong fix for this problem.  This is the right one.  It has to remain
 signed, because we use max (sal.line - lines_to_list, 1); if the left
 promotes to unsigned, then the result will be a large number on overflow
 instead of 1."

The var_integer set command handling did have range validation, but
the testsuite never caught the "integer 4294967295 out of range"

So from that point on, only "set listsize 0" meant unlimited, while
the testsuite disagreed (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...

I'll post patches to fix all these issues.

All tested on x86_64 Fedora 17.



Pedro Alves (6):
      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
      list.exp: Remove "list line 1 with unlimited listsize" xfail.
      NEWS: mention "set listsize 0"/"set listsize -1" behavior change.

 gdb/NEWS                        |   12 ++++++++++++
 gdb/source.c                    |   36 ++++++++++++++++++++++--------------
 gdb/testsuite/gdb.base/list.exp |   34 ++++++++++++++++++++++++----------
 gdb/testsuite/gdb.base/list0.c  |    2 +-
 4 files changed, 59 insertions(+), 25 deletions(-)

Pedro Alves

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]