From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8865 invoked by alias); 27 Mar 2013 20:44:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 8798 invoked by uid 89); 27 Mar 2013 20:44:03 -0000 X-Spam-SWARE-Status: No, score=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 27 Mar 2013 20:44:00 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2RKhxmb019215 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 27 Mar 2013 16:43:59 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2RKhwao010461 for ; Wed, 27 Mar 2013 16:43:58 -0400 Subject: [PATCH 0/4 v2] [mainline+7.6] PR gdb/15294: list with unlimited listsize broken To: gdb-patches@sourceware.org From: Pedro Alves Date: Thu, 28 Mar 2013 06:28:00 -0000 Message-ID: <20130327204357.24420.94722.stgit@brno.lan> User-Agent: StGit/0.16 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-SW-Source: 2013-03/txt/msg01044.txt.bz2 "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 , 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