From patchwork Fri Jun 24 18:11:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 13364 Received: (qmail 6038 invoked by alias); 24 Jun 2016 18:12:07 -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 6025 invoked by uid 89); 24 Jun 2016 18:12:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=decimal, traces, terminate 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 (AES256-SHA encrypted) ESMTPS; Fri, 24 Jun 2016 18:11:56 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id B491E116B45; Fri, 24 Jun 2016 14:11:54 -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 wUVNWRb21lq2; Fri, 24 Jun 2016 14:11:54 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 80492116B3E; Fri, 24 Jun 2016 14:11:54 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id D7FA1428F4; Fri, 24 Jun 2016 11:11:52 -0700 (PDT) Date: Fri, 24 Jun 2016 11:11:52 -0700 From: Joel Brobecker To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: RFA/gdbserver: GDB internal-error debugging threaded program with breakpoint and forks (was: "Re: RFH: failed assert debugging threaded+fork program over gdbserver") Message-ID: <20160624181152.GD3295@adacore.com> References: <20160512171650.GC26324@adacore.com> <5734C06C.8040008@codesourcery.com> <20160623225935.GC3295@adacore.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160623225935.GC3295@adacore.com> User-Agent: Mutt/1.5.23 (2014-03-12) > I'm thinking the right thing to do, here is to enhance select_event_lwp > to look for threads that received a fork/vfork event first, and report > that event to gdb ahead of all the other kinds of events. That seems to work well. Attached is the patch doing this. The commit's revision log gives I think a reasonable overview of what the issue is and how the patch fixes it. But more information that I found while I was investigating this which is only tangential to the issue can be found in the following message, exchanged on this list: https://www.sourceware.org/ml/gdb-patches/2016-05/msg00207.html https://www.sourceware.org/ml/gdb-patches/2016-05/msg00209.html https://www.sourceware.org/ml/gdb-patches/2016-06/msg00391.html gdb/gdbserver/ChangeLog: * linux-low.c (select_fork_vfork_lwp_callback): New function. (select_event_lwp): Give priority fork/vfork events over all other types of events. gdb/testsuite/ChangeLog: * gdb.ada/bp_and_fork: New test. Tested on x86_64-linux. OK to apply? Thanks! From cccc5072db8eeefbdfe11840c4e6e2330bac46a8 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Fri, 24 Jun 2016 11:56:32 -0400 Subject: [PATCH] GDB internal-error debugging threaded program with breakpoint and forks Debugging through gdbserver a program uses threads hitting breakpoints as well as doing a fork/exec, can lead to an internal error. This happens randomly, since it depends on timing, but here is what we can see from the user's perpective: % gdb a_test (gdb) break a_test.adb:30 (gdb) break a_test.adb:39 (gdb) target remote my_board:4444 (gdb) continue Continuing. [...] [New Thread 866.868] [New Thread 866.869] [New Thread 870.870] /[...]/gdb/thread.c:89: internal-error: thread_info* inferior_thread(): Assertion `tp' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) What happens is that the inferior forks around the same time its threads hit one of the breakpoints we inserted. As a result, one single round of linux_wait_for_event_filtered, is able to fetch breakpoints events from our inferior's two threads, as well as the fork event reported by the inferior's main thread, all in one go. What GDBserver then has to do next, is decide which of these events to report to GDB first. Currently, it gives priority to threads that we are single-stepping, and then just grabs one of the threads that have a pending event, selected at random (see select_event_lwp). At the point where see the problem, the single-step-out-of-breakpoint has already been completed, so we do not have any thread doing a single-step, and so GDBserver selects one of the threads that received a SIGTRAP. This can been seen by looking at the debug traces, which contain the following hint: SEL: Found 3 SIGTRAP events, selecting #1 This leads GDBserver to report a SIGTRAP event on that thread. Processing that event, GDB requests the updated list of threads, which now contains the forked process' thread. Seeing that thread with a different PID which confuses GDB, eventually leading to the internal error. This patch fixes the issue by make sure GDBserver always reports fork/vfork events first. That way, GDB is aware that the new thread comes from the fork/vfork event. In particular, in the mode where we are in (follow the parent only), it knows to ignore the new thread, thus avoiding the issue entirely. gdb/gdbserver/ChangeLog: * linux-low.c (select_fork_vfork_lwp_callback): New function. (select_event_lwp): Give priority fork/vfork events over all other types of events. gdb/testsuite/ChangeLog: * gdb.ada/bp_and_fork: New test. Tested on x86_64-linux. --- gdb/gdbserver/linux-low.c | 44 +++++++++++++++++- gdb/testsuite/gdb.ada/bp_and_fork.exp | 41 +++++++++++++++++ gdb/testsuite/gdb.ada/bp_and_fork/a_test.adb | 68 ++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.ada/bp_and_fork.exp create mode 100644 gdb/testsuite/gdb.ada/bp_and_fork/a_test.adb diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index dd92e78..d71595b 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -2826,6 +2826,23 @@ count_events_callback (struct inferior_list_entry *entry, void *data) return 0; } +/* Select the first LWP (if any) for which a fork or vfork event + was reported. */ + +static int +select_fork_vfork_lwp_callback (struct inferior_list_entry *entry, void *data) +{ + struct thread_info *thread = (struct thread_info *) entry; + struct lwp_info *lp = get_thread_lwp (thread); + + if ((lp->waitstatus.kind == TARGET_WAITKIND_FORKED + || thread->last_status.kind == TARGET_WAITKIND_VFORKED) + && lp->status_pending_p) + return 1; + else + return 0; +} + /* Select the LWP (if any) that is currently being single-stepped. */ static int @@ -2871,6 +2888,31 @@ select_event_lwp (struct lwp_info **orig_lp) int random_selector; struct thread_info *event_thread = NULL; + /* If an LWP forked/vforked, select that LWP over all the others, + so as to tell GDB right away about the the new child process + being the result of a fork/vfork. + + It is important to do so first, because GDB needs to be told + about fork/vfork threads to allow GDB to separate the new + process' thread from the threads of the current (parent) + inferior. In particular, we ran into an issue where GDB + requested the list of threads which included both parent + and the thread of the fork/vfork, and because GDB didn't know + about the fork/vfork, did not ignore the fork/vfork. This + eventually caused an internal error because that thread had + a PID for which there was not corresponding thread (see + inferior_thread). */ + event_thread + = (struct thread_info *) find_inferior (&all_threads, + select_fork_vfork_lwp_callback, + NULL); + if (event_thread != NULL) + { + if (debug_threads) + debug_printf ("SEL: Select fork/vfork %s\n", + target_pid_to_str (ptid_of (event_thread))); + } + /* In all-stop, give preference to the LWP that is being single-stepped. There will be at most one, and it's the LWP that the core is most interested in. If we didn't do this, then we'd @@ -2879,7 +2921,7 @@ select_event_lwp (struct lwp_info **orig_lp) report the pending SIGTRAP, and the core, not having stepped the thread, wouldn't understand what the trap was for, and therefore would report it to the user as a random signal. */ - if (!non_stop) + if (event_thread == NULL && !non_stop) { event_thread = (struct thread_info *) find_inferior (&all_threads, diff --git a/gdb/testsuite/gdb.ada/bp_and_fork.exp b/gdb/testsuite/gdb.ada/bp_and_fork.exp new file mode 100644 index 0000000..e9ec08c --- /dev/null +++ b/gdb/testsuite/gdb.ada/bp_and_fork.exp @@ -0,0 +1,41 @@ +# Copyright 2016 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 "ada.exp" + +standard_ada_testfile a_test + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } { + return -1 +} + +clean_restart ${testfile} + +set bp_loc_1 [gdb_get_line_number "Task 1" ${testdir}/a_test.adb] +gdb_test "break a_test.adb:$bp_loc_1" \ + "Breakpoint $decimal at $hex: file .*a_test.adb, line $decimal\\." + +set bp_loc_2 [gdb_get_line_number "Task 2" ${testdir}/a_test.adb] +gdb_test "break a_test.adb:$bp_loc_2" \ + "Breakpoint $decimal at $hex: file .*a_test.adb, line $decimal\\." + +gdb_run_cmd +gdb_test "" \ + "Breakpoint $decimal, a_test.task$decimal \\(.*\\).*" \ + "run to first breakpoint" + +gdb_test "continue" \ + "Breakpoint $decimal, a_test.task$decimal \\(.*\\).*" \ + "continue to second breakpoint" diff --git a/gdb/testsuite/gdb.ada/bp_and_fork/a_test.adb b/gdb/testsuite/gdb.ada/bp_and_fork/a_test.adb new file mode 100644 index 0000000..095c2eb --- /dev/null +++ b/gdb/testsuite/gdb.ada/bp_and_fork/a_test.adb @@ -0,0 +1,68 @@ +-- Copyright 2003-2016 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 . + +with System; +with Text_Io; + +procedure A_Test is + + Command : constant String := "echo Hello World &" & Ascii.Nul; + Status : Integer; + + --------------------------------- + -- System shell command interface + --------------------------------- + function Shell_Cmd (Command : in System.Address) return Integer; + pragma Import (C, Shell_Cmd, "system"); + + ------------------ + -- Start 2 threads + ------------------ + + task Task1 is + entry Start_Task; + end Task1; + + task Task2 is + entry Start_Task; + end Task2; + + task body Task1 is + begin + select -- Task 1 + accept Start_Task; + or + terminate; + end select; + end Task1; + + task body Task2 is + begin + select -- Task 2 + accept Start_Task; + or + terminate; + end select; + end Task2; + + procedure My_Routine is + begin + Text_Io.Put_Line ("Program successful"); + end My_Routine; + +begin + Status := Shell_Cmd (Command'Address); + My_Routine; +end A_Test; -- 2.1.4