From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25743 invoked by alias); 4 Jun 2018 20:11:10 -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 20027 invoked by uid 89); 4 Jun 2018 20:11:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Leading, exceeding, delegates X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Jun 2018 20:11:01 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A61E7560B0; Mon, 4 Jun 2018 16:10:59 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id jskwGcQmswf4; Mon, 4 Jun 2018 16:10:59 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 7BAF75612F; Mon, 4 Jun 2018 16:10:59 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 5E8CF808EA; Mon, 4 Jun 2018 13:10:57 -0700 (PDT) Date: Mon, 04 Jun 2018 20:11:00 -0000 From: Joel Brobecker To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [RFC] (windows) GDB/MI crash when using "-list-thread-groups --available" Message-ID: <20180604201057.klkmujt3zmzctthi@adacore.com> References: <1525978704-70543-1-git-send-email-brobecker@adacore.com> <20180602005941.f4l5nudlsl3xez4v@adacore.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="cqnalyhijbtsmyqf" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2018-06/txt/msg00077.txt.bz2 --cqnalyhijbtsmyqf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 1359 Hi Pedro, > It looks good to me. A few comments on minor details follow. > > How about naming the testcase list-thread-groups-no-inferior.exp > (no "mi-") so that it sits side-by-side with > list-thread-groups-available.exp? Good idea. Done. > We're missing an intro comment in the .exp file mentioning what the > testcase is about, otherwise it's not clear why we have two separate > testcases for seemingly the same thing. Also added. Let me know if you think it's not enough information, and I will add more. That's all I could think of from my perspective, though. > A couple microscopic nits more: > > > +gdb_test_multiple $test $test { > > + -re ".*\}" { > > Leading ".*" is not necessary, it's implied. Thanks - fixed! > > > +mi_gdb_test "-data-evaluate-expression 1" \ > > + ".*\\^done,value=\"1\"" \ > > + "check GDB is still alive" > > + > > "check" and "test" in test messages is one of my pet peeves. > OK, I'm really exaggerating. :-) The point is that all > tests are checking something, making it redundant. > "GDB is still alive" or even "GDB is alive" says the same. True, and I like it when it's shorter-with-no-loss. Fixed as well. Attached is the version I just pushed. I'll make further adjustments if you see something I missed or the description isn't complete for you. Thanks again! -- Joel --cqnalyhijbtsmyqf Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-windows-GDB-MI-crash-when-using-list-thread-groups-a.patch" Content-length: 5348 >From 178d6a638693d1a7a66b0553f16b1df95b4f27ee Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Mon, 4 Jun 2018 15:03:32 -0500 Subject: [PATCH] (windows) GDB/MI crash when using "-list-thread-groups --available" On Windows, using the "-list-thread-groups --available" GDB/MI command before an inferior is being debugged: % gdb -q -i=mi =thread-group-added,id="i1" =cmd-param-changed,param="auto-load safe-path",value="/" (gdb) -list-thread-groups --available Segmentation fault Ooops! The SEGV happens because the -list-thread-groups --available command triggers a windows_nat_target::xfer_partial call for a TARGET_OBJECT_OSDATA object. Until a program is being debugged, the target_ops layer that gets the call is the Windows "native" layer. Except for a couple of specific objects (TARGET_OBJECT_MEMORY and TARGET_OBJECT_LIBRARIES), this layer's xfer_partial method delegates the xfer of other objects to the target beneath: default: return beneath->xfer_partial (object, annex, readbuf, writebuf, offset, len, xfered_len); Unfortunately, there is no "beneath layer" in this case, so beneath is NULL and dereferencing it leads to the SEGV. This patch fixes the issue by checking beneath before trying to delegate the request. gdb/ChangeLog: * windows-nat.c (windows_nat_target::xfer_partial): Return TARGET_XFER_E_IO if we need to delegate to the target beneath but BENEATH is NULL. gdb/testsuite/ChangeLog: * gdb.mi/list-thread-groups-no-inferior.exp: New testcase. --- gdb/ChangeLog | 6 +++ gdb/testsuite/ChangeLog | 4 ++ .../gdb.mi/list-thread-groups-no-inferior.exp | 47 ++++++++++++++++++++++ gdb/windows-nat.c | 7 ++++ 4 files changed, 64 insertions(+) create mode 100644 gdb/testsuite/gdb.mi/list-thread-groups-no-inferior.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d0c43027d6..b5580c429a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2018-06-04 Joel Brobecker + + * windows-nat.c (windows_nat_target::xfer_partial): Return + TARGET_XFER_E_IO if we need to delegate to the target beneath + but BENEATH is NULL. + 2018-06-04 Simon Marchi * Makefile.in (config.status): Add configure.nat as a diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 8cef172a26..0e98d7e115 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2018-06-04 Joel Brobecker + + * gdb.mi/list-thread-groups-no-inferior.exp: New testcase. + 2018-06-01 Joel Brobecker * gdb.ada/bp_fun_addr: New testcase. diff --git a/gdb/testsuite/gdb.mi/list-thread-groups-no-inferior.exp b/gdb/testsuite/gdb.mi/list-thread-groups-no-inferior.exp new file mode 100644 index 0000000000..9ab38380c0 --- /dev/null +++ b/gdb/testsuite/gdb.mi/list-thread-groups-no-inferior.exp @@ -0,0 +1,47 @@ +# Copyright 2018 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 . + +# The purpose of this test is to verify that GDB is able to handle +# the "-list-thread-groups --available" command, even when there is +# no inferior. In particular, we want to verify that GDB does not +# crash. + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +gdb_exit +if [mi_gdb_start] { + continue +} + +# Try the "-list-thread-groups --available". This command can generate +# a very large amount of output, potentially exceeding expect's buffer +# size. So we consume the output in chunks. + +set test "-list-thread-groups --available" +gdb_test_multiple $test $test { + -re "\}" { + exp_continue + } + -re "\r\n$gdb_prompt " { + pass $test + } +} + +# Verify that GDB is still alive. + +mi_gdb_test "-data-evaluate-expression 1" \ + ".*\\^done,value=\"1\"" \ + "GDB is still alive" diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 0f24257935..e3e36cdc3e 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -2966,6 +2966,13 @@ windows_nat_target::xfer_partial (enum target_object object, writebuf, offset, len, xfered_len); default: + if (beneath == NULL) + { + /* This can happen when requesting the transfer of unsupported + objects before a program has been started (and therefore + with the current_target having no target beneath). */ + return TARGET_XFER_E_IO; + } return beneath->xfer_partial (object, annex, readbuf, writebuf, offset, len, xfered_len); -- 2.11.0 --cqnalyhijbtsmyqf--