From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25763 invoked by alias); 18 Jun 2013 12:42:57 -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 25738 invoked by uid 89); 18 Jun 2013 12:42:54 -0000 X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_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; Tue, 18 Jun 2013 12:42:52 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5ICgnRc014766 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 18 Jun 2013 08:42:49 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r5ICglF7003627; Tue, 18 Jun 2013 08:42:48 -0400 Message-ID: <51C055C7.90506@redhat.com> Date: Tue, 18 Jun 2013 13:19:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: Simon Marchi CC: GDB Patches Subject: Re: [PATCH v3] Exit code of exited inferiors in -list-thread-groups References: <51BF4D9B.8060105@ericsson.com> In-Reply-To: <51BF4D9B.8060105@ericsson.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00406.txt.bz2 On 06/17/2013 06:55 PM, Simon Marchi wrote: > Here is a new version of my patch to show the exit code of exited inferiors in -list-thread-groups. > I made changes following Pedro's comments at: > http://sourceware.org/ml/gdb-patches/2013-06/msg00126.html Thanks. > gdb/doc/ChangeLog: > 2013-06-17 Simon Marchi > > * gdb.texinfo: Document new exit-code field. This should mention the node name in the bit that goes in ()'s. See existing examples. It should also mention -list-thread-groups. > --- > gdb/NEWS | 3 ++ > gdb/doc/gdb.texinfo | 5 +++ > gdb/inferior.c | 4 +- > gdb/mi/mi-main.c | 3 ++ > gdb/testsuite/gdb.mi/mi-exit-code.c | 29 ++++++++++++++ > gdb/testsuite/gdb.mi/mi-exit-code.exp | 71 +++++++++++++++++++++++++++++++++++ > 6 files changed, 113 insertions(+), 2 deletions(-) > create mode 100644 gdb/testsuite/gdb.mi/mi-exit-code.c > create mode 100644 gdb/testsuite/gdb.mi/mi-exit-code.exp > > +@item exit-code > +The exit code of this thread group when it last exited. This field is > +only present for thread groups of type @samp{process} and only if the > +process is not running. > + > ui_out_field_fmt (uiout, "id", "i%d", inferior->num); > ui_out_field_string (uiout, "type", "process"); > + if (inferior->has_exit_code) > + ui_out_field_string (uiout, "exit-code", > + int_string (inferior->exit_code, 8, 0, 0, 1)); Indentation is still wrong here though. Should be: if (inferior->has_exit_code) ui_out_field_string (uiout, "exit-code", int_string (inferior->exit_code, 8, 0, 0, 1)); (note: the tabs are not optional) > if (inferior->pid != 0) > ui_out_field_int (uiout, "pid", inferior->pid); > > diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.c b/gdb/testsuite/gdb.mi/mi-exit-code.c > new file mode 100644 > index 0000000..a523385 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-exit-code.c > @@ -0,0 +1,29 @@ > +/* Copyright 1999-2013 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +int exit_code = 0; > + > +int > +main(int argc, char **argv) Space before parens. > +{ > + if (argc > 1) > + { > + exit_code = atoi (argv[1]); > + } Single statement gets no braces. > + > + return exit_code; > +} > diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.exp b/gdb/testsuite/gdb.mi/mi-exit-code.exp > new file mode 100644 > index 0000000..50d8530 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-exit-code.exp > @@ -0,0 +1,71 @@ > +# Copyright 1999-2013 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +load_lib mi-support.exp > +set MIFLAGS "-i=mi" > + > +gdb_exit > +if [mi_gdb_start] { > + continue > +} > + > +standard_testfile > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > + > + untested mi-exit-code.exp untested "failed to compile $testfile" http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#A.22untested.22_calls > + return -1 > +} > + > +proc test_list_thread_groups { } { > + global hex Note this one is unused. (though my proposal below adds one). > + global decimal > + > + # Check before any run, exit-code should not be present. > + mi_gdb_test "122-list-thread-groups" "122\\^done,groups=\\\[\{id=\"i1\",type=\"process\"\}\]" "-list-thread-groups before any run" IMO, it's better to split these long lines between arguments. > + > + mi_run_to_main > + > + # Check during the run, exit-code should not be present. > + mi_gdb_test "123-list-thread-groups" "123\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" "-list-thread-groups during first run" > + > + # Exit the inferior. > + mi_send_resuming_command "exec-continue" "continuing to exit inferior (first run)" > + mi_expect_stop "exited-normally" "" "" "" "" "" "exiting inferior (first run)" > + > + # Check after the run, exit-code should be present. > + mi_gdb_test "124-list-thread-groups" "124\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"0\",executable=\".*\"\}\]" "-list-thread-groups after first run" > + > + mi_run_to_main http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique $ cat testsuite/gdb.sum| grep PASS | sort | uniq -c | sort -n 1 PASS: gdb.mi/mi-exit-code.exp: exiting inferior (first run) 1 PASS: gdb.mi/mi-exit-code.exp: exiting inferior (second run) 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups after first run 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups after second run 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups before any run 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups during first run 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups during second run 2 PASS: gdb.mi/mi-exit-code.exp: breakpoint at main 2 PASS: gdb.mi/mi-exit-code.exp: mi runto main Note the 2's. Instead of manually caring to do "first run", "second run" etc. in each test, we can use with_test_prefix, and that neatly takes care of those 2's as well. > + > + # Write the exit code we want in the global var > + mi_gdb_test "-data-write-memory-bytes &exit_code 08" This -data-write-memory-bytes is writing one byte to the address of exit_code (and int), and expecting exit_code to end up with value 8. That assumes a little endian target; it'll do the wrong thing on big endian targets, like e.g., PowerPC. We can just do "set var exit_code = 8" instead, and let GDB worry about such details. I went ahead and did all these changes on top of your patch, so you could see what I mean. I tweaked the messages and comments a little. We now get: $ cat testsuite/gdb.sum | grep PASS | sort | uniq -c | sort -n 1 PASS: gdb.mi/mi-exit-code.exp: first run: breakpoint at main 1 PASS: gdb.mi/mi-exit-code.exp: first run: exit normally 1 PASS: gdb.mi/mi-exit-code.exp: first run: -list-thread-groups after exit shows exit-code 1 PASS: gdb.mi/mi-exit-code.exp: first run: -list-thread-groups during run shows no exit-code 1 PASS: gdb.mi/mi-exit-code.exp: first run: mi runto main 1 PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups before run shows no exit-code 1 PASS: gdb.mi/mi-exit-code.exp: second run: breakpoint at main 1 PASS: gdb.mi/mi-exit-code.exp: second run: exit with code 1 PASS: gdb.mi/mi-exit-code.exp: second run: -list-thread-groups after exit shows exit-code 1 PASS: gdb.mi/mi-exit-code.exp: second run: -list-thread-groups during run shows no exit-code 1 PASS: gdb.mi/mi-exit-code.exp: second run: mi runto main 1 PASS: gdb.mi/mi-exit-code.exp: second run: write exit code $ cat testsuite/gdb.sum | grep PASS PASS: gdb.mi/mi-exit-code.exp: -list-thread-groups before run shows no exit-code PASS: gdb.mi/mi-exit-code.exp: first run: breakpoint at main PASS: gdb.mi/mi-exit-code.exp: first run: mi runto main PASS: gdb.mi/mi-exit-code.exp: first run: -list-thread-groups during run shows no exit-code PASS: gdb.mi/mi-exit-code.exp: first run: exit normally PASS: gdb.mi/mi-exit-code.exp: first run: -list-thread-groups after exit shows exit-code PASS: gdb.mi/mi-exit-code.exp: second run: breakpoint at main PASS: gdb.mi/mi-exit-code.exp: second run: mi runto main PASS: gdb.mi/mi-exit-code.exp: second run: write exit code PASS: gdb.mi/mi-exit-code.exp: second run: -list-thread-groups during run shows no exit-code PASS: gdb.mi/mi-exit-code.exp: second run: exit with code PASS: gdb.mi/mi-exit-code.exp: second run: -list-thread-groups after exit shows exit-code Could you merge this into your patch, please? --- gdb/testsuite/gdb.mi/mi-exit-code.c | 6 +- gdb/testsuite/gdb.mi/mi-exit-code.exp | 81 ++++++++++++++++++++------------- 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 4fa0c93..0a10930 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -572,8 +572,8 @@ print_one_inferior (struct inferior *inferior, void *xdata) ui_out_field_fmt (uiout, "id", "i%d", inferior->num); ui_out_field_string (uiout, "type", "process"); if (inferior->has_exit_code) - ui_out_field_string (uiout, "exit-code", - int_string (inferior->exit_code, 8, 0, 0, 1)); + ui_out_field_string (uiout, "exit-code", + int_string (inferior->exit_code, 8, 0, 0, 1)); if (inferior->pid != 0) ui_out_field_int (uiout, "pid", inferior->pid); diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.c b/gdb/testsuite/gdb.mi/mi-exit-code.c index a523385..564efc7 100644 --- a/gdb/testsuite/gdb.mi/mi-exit-code.c +++ b/gdb/testsuite/gdb.mi/mi-exit-code.c @@ -18,12 +18,10 @@ int exit_code = 0; int -main(int argc, char **argv) +main (int argc, char **argv) { if (argc > 1) - { - exit_code = atoi (argv[1]); - } + exit_code = atoi (argv[1]); return exit_code; } diff --git a/gdb/testsuite/gdb.mi/mi-exit-code.exp b/gdb/testsuite/gdb.mi/mi-exit-code.exp index 50d8530..c68c7cc 100644 --- a/gdb/testsuite/gdb.mi/mi-exit-code.exp +++ b/gdb/testsuite/gdb.mi/mi-exit-code.exp @@ -24,8 +24,7 @@ if [mi_gdb_start] { standard_testfile if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { - - untested mi-exit-code.exp + untested "failed to compile $testfile" return -1 } @@ -33,36 +32,54 @@ proc test_list_thread_groups { } { global hex global decimal - # Check before any run, exit-code should not be present. - mi_gdb_test "122-list-thread-groups" "122\\^done,groups=\\\[\{id=\"i1\",type=\"process\"\}\]" "-list-thread-groups before any run" - - mi_run_to_main - - # Check during the run, exit-code should not be present. - mi_gdb_test "123-list-thread-groups" "123\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" "-list-thread-groups during first run" - - # Exit the inferior. - mi_send_resuming_command "exec-continue" "continuing to exit inferior (first run)" - mi_expect_stop "exited-normally" "" "" "" "" "" "exiting inferior (first run)" - - # Check after the run, exit-code should be present. - mi_gdb_test "124-list-thread-groups" "124\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"0\",executable=\".*\"\}\]" "-list-thread-groups after first run" - - mi_run_to_main - - # Write the exit code we want in the global var - mi_gdb_test "-data-write-memory-bytes &exit_code 08" - - # Check during the second run, exit-code should not be present. - mi_gdb_test "125-list-thread-groups" "125\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" "-list-thread-groups during second run" - - # Exit the inferior. - mi_send_resuming_command "exec-continue" "continuing to exit inferior (second run)" - mi_expect_stop "exited" "" "" "" "" "" "exiting inferior (second run)" - - # Check after the second run, exit-code should be present. - mi_gdb_test "126-list-thread-groups" "126\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"010\",executable=\".*\"\}\]" "-list-thread-groups after second run" - + # Before any run, exit-code should not be present. + mi_gdb_test \ + "122-list-thread-groups" \ + "122\\^done,groups=\\\[\{id=\"i1\",type=\"process\"\}\]" \ + "-list-thread-groups before run shows no exit-code" + + with_test_prefix "first run" { + mi_run_to_main + + # During the run, exit-code should not be present. + mi_gdb_test \ + "123-list-thread-groups" \ + "123\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" \ + "-list-thread-groups during run shows no exit-code" + + # Exit the inferior. + mi_send_resuming_command "exec-continue" "continuing to inferior exit" + mi_expect_stop "exited-normally" "" "" "" "" "" "exit normally" + + # After the run, exit-code should be present. + mi_gdb_test \ + "124-list-thread-groups" \ + "124\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"0\",executable=\".*\"\}\]" \ + "-list-thread-groups after exit shows exit-code" + } + + with_test_prefix "second run" { + mi_run_to_main + + # Write the exit code we want in the global var + mi_gdb_test "set var exit_code = 8" ".*\\^done" "write exit code" + + # During the second run, exit-code should not be present. + mi_gdb_test \ + "125-list-thread-groups" \ + "125\\^done,groups=\\\[\{id=\"i1\",type=\"process\",pid=\"$decimal\",executable=\".*\".*" \ + "-list-thread-groups during run shows no exit-code" + + # Exit the inferior. + mi_send_resuming_command "exec-continue" "continuing to inferior exit" + mi_expect_stop "exited" "" "" "" "" "" "exit with code" + + # After the second run, exit-code should be present. + mi_gdb_test \ + "126-list-thread-groups" \ + "126\\^done,groups=\\\[\{id=\"i1\",type=\"process\",exit-code=\"010\",executable=\".*\"\}\]" \ + "-list-thread-groups after exit shows exit-code" + } } test_list_thread_groups