From patchwork Wed Aug 8 12:23:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 28786 Received: (qmail 54358 invoked by alias); 8 Aug 2018 12:23:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 53855 invoked by uid 89); 8 Aug 2018 12:23:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=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=burgess, D*embecosm.com, sk:andrew, andrewburgessembecosmcom X-HELO: mail-wr1-f54.google.com Received: from mail-wr1-f54.google.com (HELO mail-wr1-f54.google.com) (209.85.221.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 08 Aug 2018 12:23:21 +0000 Received: by mail-wr1-f54.google.com with SMTP id h15-v6so1837614wrs.7 for ; Wed, 08 Aug 2018 05:23:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=xKmB40Ah9D1zLjPNMTIsjrpK6IUtVyxp+baKN5AVNao=; b=ImX6sliZhYbpcfjdA7emvvXJv9IkfeDoDxBnzFF+mbjBwarSFePs9Kf7HnK2qo/BBP 0u7UarJkpQVqM8WCegjKdTLKJVAertQl13pQjmqbgckPHlN7vSHg77cLCCxyxfipA2hx WaGQa3YI1d357PczXp7Nnsc/e4slrWJ1UoGYi7+Z4YVdUOFP6neK1b3T8pISNsxTBwx/ oMrSEJiWs0tWy6mO/2HdHgotx84dFateUFVPoUgdhGR0/apuVBrIIScHG6KtY1tGNy77 7FoDhx/t+xhokYx6emP/gzyLIMZDXm+aWcDUBATYWrBJ36U7g4yMJOTTYBHGEF0QBi3O gCvw== Return-Path: Received: from localhost (host81-140-215-41.range81-140.btcentralplus.com. [81.140.215.41]) by smtp.gmail.com with ESMTPSA id o14-v6sm7725608wmd.35.2018.08.08.05.23.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 08 Aug 2018 05:23:18 -0700 (PDT) Date: Wed, 8 Aug 2018 13:23:17 +0100 From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCHv2 1/2] gdb: Fix assert for extended-remote target Message-ID: <20180808122317.GI3155@embecosm.com> References: <488a345e49b3d156bf48dd16854debef5140636a.1533158307.git.andrew.burgess@embecosm.com> <87effg377j.fsf@tromey.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87effg377j.fsf@tromey.com> X-Fortune: Life is the urge to ecstasy. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Tom Tromey [2018-08-02 13:21:36 -0600]: > >>>>> "Andrew" == Andrew Burgess writes: > > Andrew> gdb/ChangeLog: > > Andrew> * target.c (dispose_inferior): Don't dispose of inferiors that are > Andrew> already killed. > > I think this is PR gdb/18050, so please mention that in the ChangeLog. > > Thanks for the patch. This is ok with that little change. Thanks for the review. The version I pushed is below, I made the following changes: * Updated description to mention PR gdb/18050 * Moved the test into gdb.server/ as this seemed a better location. Thanks, Andrew --- gdb: Fix assert for extended-remote target (PR gdb/18050) Consider the following GDB session: (gdb) target extended-remote :2347 (gdb) file /path/to/exe (gdb) set remote exec-file /path/to/exe (gdb) set detach-on-fork off (gdb) break breakpt (gdb) run # ... hits breakpoint (gdb) info inferiors Num Description Executable * 1 process 17001 /path/to/exe 2 process 17002 /path/to/exe (gdb) kill (gdb) info inferiors Num Description Executable * 1 /path/to/exe 2 process 17002 /path/to/exe (gdb) target extended-remote :2348 ../../src/gdb/thread.c:660: internal-error: thread_info* any_thread_of_process(int): Assertion `pid != 0' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Or, from bug PR gdb/18050: (gdb) start (gdb) add-inferior -exec /path/to/exe (gdb) target extended-remote :2347 ../../src/gdb/thread.c:660: internal-error: thread_info* any_thread_of_process(int): Assertion `pid != 0' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. The issue is calling target.c:dispose_inferior with a killed inferior in the inferior list. This assertion is fixed in this commit. The new test for this issue only runs on platforms that support 'detach-on-fork', and when using '--target_board=native-extended-gdbserver'. gdb/ChangeLog: PR gdb/18050: * target.c (dispose_inferior): Don't dispose of inferiors that are already killed. gdb/testsuite/ChangeLog: PR gdb/18050: * gdb.server/extended-remote-restart.c: New file. * gdb.server/extended-remote-restart.exp: New file. --- gdb/ChangeLog | 6 + gdb/target.c | 6 + gdb/testsuite/ChangeLog | 6 + gdb/testsuite/gdb.server/extended-remote-restart.c | 60 ++++++++++ .../gdb.server/extended-remote-restart.exp | 132 +++++++++++++++++++++ 5 files changed, 210 insertions(+) create mode 100644 gdb/testsuite/gdb.server/extended-remote-restart.c create mode 100644 gdb/testsuite/gdb.server/extended-remote-restart.exp diff --git a/gdb/target.c b/gdb/target.c index a5245abb281..115e9ae4948 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2022,6 +2022,12 @@ target_pre_inferior (int from_tty) static int dispose_inferior (struct inferior *inf, void *args) { + /* Not all killed inferiors can, or will ever be, removed from the + inferior list. Killed inferiors clearly don't need to be killed + again, so, we're done. */ + if (inf->pid == 0) + return 0; + thread_info *thread = any_thread_of_inferior (inf); if (thread != NULL) { diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.c b/gdb/testsuite/gdb.server/extended-remote-restart.c new file mode 100644 index 00000000000..e195f48e080 --- /dev/null +++ b/gdb/testsuite/gdb.server/extended-remote-restart.c @@ -0,0 +1,60 @@ +/* This testcase is part of GDB, the GNU debugger. + + 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 . */ + +#include +#include + +static void +breakpt () +{ + asm ("" ::: "memory"); +} + +static void +go_child () +{ + breakpt (); + + while (1) + sleep (1); +} + +static void +go_parent () +{ + breakpt (); + + while (1) + sleep (1); +} + +int +main () +{ + pid_t pid; + + pid = fork (); + if (pid == -1) + abort (); + + if (pid == 0) + go_child (); + else + go_parent (); + + exit (EXIT_SUCCESS); +} diff --git a/gdb/testsuite/gdb.server/extended-remote-restart.exp b/gdb/testsuite/gdb.server/extended-remote-restart.exp new file mode 100644 index 00000000000..39fcb9e2e58 --- /dev/null +++ b/gdb/testsuite/gdb.server/extended-remote-restart.exp @@ -0,0 +1,132 @@ +# 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 . + +# This test is about restarting execution of a forked application when +# using gdb extended remote target. +# +# There are two issues that the test tries to expose in GDB: +# +# 1. GDB would throw an assertion upon reconnecting to a remote target +# if there was more than one inferior already active in GDB, and +# +# 2. GDB would not prune transient inferiors from the inferior list +# when reconnecting to a remote target. So, for example, an inferior +# created by GDB to track the child of a fork would usually be removed +# from the inferior list once the child exited. However, reconnecting +# to a remote target would result in the child inferior remaining in +# the inferior list. + +# This test is only for extended remote targets. +if {[target_info gdb_protocol] != "extended-remote"} { + continue +} + +# This test also makes use of 'detach-on-fork' which is not supported +# on all platforms. +if { ![istarget "*-*-linux*"] && ![istarget "*-*-openbsd*"] } then { + continue +} + +# And we need to be able to reconnect to gdbserver. +set gdbserver_reconnect_p 1 +if { [info proc gdb_reconnect] == "" } { + return 0 +} + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +# Core of the test. DO_KILL_P controls whether we kill one of the +# inferiors before reconnecting. And FOLLOW_CHILD_P controls whether +# we follow the child or the parent at the fork. +proc test_reload { do_kill_p follow_child_p } { + global decimal + global binfile + + clean_restart ${binfile} + + if ![runto_main] then { + fail "can't run to main" + return 0 + } + + # Set detach-on-fork off + gdb_test_no_output "set detach-on-fork off" + + set live_inf_ptn "process $decimal" + set dead_inf_ptn "" + + if ${follow_child_p} { + gdb_test_no_output "set follow-fork child" + set parent_prefix " " + set child_prefix "\\*" + set parent_inf_after_kill_ptn ${live_inf_ptn} + set child_inf_after_kill_ptn ${dead_inf_ptn} + } else { + gdb_test_no_output "set follow-fork parent" + set parent_prefix "\\*" + set child_prefix " " + set parent_inf_after_kill_ptn ${dead_inf_ptn} + set child_inf_after_kill_ptn ${live_inf_ptn} + } + + gdb_breakpoint "breakpt" + gdb_continue_to_breakpoint "breakpt" + + # Check we have the expected inferiors. + gdb_test "info inferiors" \ + [multi_line \ + " Num Description Executable.*" \ + "${parent_prefix} 1 +${live_inf_ptn} \[^\r\n\]+" \ + "${child_prefix} 2 +${live_inf_ptn} \[^\r\n\]+" ] \ + "Check inferiors at breakpoint" + + if { $do_kill_p } { + # (Optional) Kill one of the inferiors. + gdb_test "kill" \ + "" \ + "Kill inferior" \ + "Kill the program being debugged.*y or n. $" \ + "y" + + # Check the first inferior really did die. + gdb_test "info inferiors" \ + [multi_line \ + " Num Description Executable.*" \ + "${parent_prefix} 1 +${parent_inf_after_kill_ptn} \[^\r\n\]+" \ + "${child_prefix} 2 +${child_inf_after_kill_ptn} \[^\r\n\]+" ] \ + "Check inferior was killed" + } + + # Reconnect to the target. + if { [gdb_reconnect] == 0 } { + pass "reconnect after fork" + } else { + fail "reconnect after fork" + } +} + +# Run all combinations of the test. +foreach do_kill_p [list 1 0] { + foreach follow_child_p [list 1 0] { + with_test_prefix \ + "kill: ${do_kill_p}, follow-child ${follow_child_p}" { + test_reload ${do_kill_p} ${follow_child_p} + } + } +}