From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62015 invoked by alias); 28 Jul 2016 12:19:16 -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 61992 invoked by uid 89); 28 Jul 2016 12:19:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy=H*r:0400, *tp, 6288, Stopping X-HELO: usplmg20.ericsson.net Received: from usplmg20.ericsson.net (HELO usplmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 28 Jul 2016 12:19:03 +0000 Received: from EUSAAHC005.ericsson.se (Unknown_Domain [147.117.188.87]) by (Symantec Mail Security) with SMTP id 86.E4.03967.958F9975; Thu, 28 Jul 2016 14:19:38 +0200 (CEST) Received: from elxa4wqvvz1 (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.87) with Microsoft SMTP Server (TLS) id 14.3.301.0; Thu, 28 Jul 2016 08:18:58 -0400 References: <1464888683-31530-1-git-send-email-antoine.tremblay@ericsson.com> <1467039989-12638-1-git-send-email-antoine.tremblay@ericsson.com> User-agent: mu4e 0.9.17; emacs 24.4.1 From: Antoine Tremblay To: Antoine Tremblay CC: Subject: Re: [PATCH v3] Fix gdbserver crash when doing ftrace commands after detach or process exit In-Reply-To: <1467039989-12638-1-git-send-email-antoine.tremblay@ericsson.com> Date: Thu, 28 Jul 2016 12:19:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00369.txt.bz2 Antoine Tremblay writes: > In this v3: > - Reworked the test to be more comprehensive. > - Removed uneeded .* before Connection timeout test > > -- > This patch fixes a gdbserver crash that is triggered by the following > sequence of events: > > - A process with the in-process agent loaded is debugged under gdbserver. > - The process is detached or exits with or without stopping the trace before. > - Commands tstatus, enable/disable, ftrace, tstop, disconnect with fast > tracepoints are used. > > Using either of tstatus or enable/disable ends up sending the qtstatus > packet to gdbserver. During the handling of qtstatus, agent_loaded_p () > returns true, even though the process that once had the agent loaded is > not present anymore. We end up trying to read memory with > current_thread == NULL, causing a segfault here: > > gdb/gdbserver/linux-low.c:5560(linux_read_memory)[0x43583c] > gdb/gdbserver/target.c:153(read_inferior_memory)[0x415d78] > gdb/gdbserver/tracepoint.c:424(read_inferior_uinteger)[0x41c7fb] > gdb/gdbserver/tracepoint.c:6288(upload_fast_traceframes)[0x425558] > gdb/gdbserver/tracepoint.c:3645(cmd_qtstatus)[0x420dee] > gdb/gdbserver/tracepoint.c:4239(handle_tracepoint_query)[0x4222ab] > gdb/gdbserver/server.c:2543(handle_query)[0x411639] > gdb/gdbserver/server.c:3910(process_serial_event)[0x413f39] > gdb/gdbserver/server.c:4347(handle_serial_event)[0x415010] > gdb/gdbserver/event-loop.c:428(handle_file_event)[0x41bed7] > gdb/gdbserver/event-loop.c:184(process_event)[0x41b69e] > gdb/gdbserver/event-loop.c:547(start_event_loop)[0x41c41d] > gdb/gdbserver/server.c:3723(captured_main)[0x413a53] > gdb/gdbserver/server.c:3802(main)[0x413c2f] > > ftrace, tstop and quit need to be protected from current_thread == NULL in a > similar manner. > > This patch adds a test called > gdb.trace/ftrace-commands-after-detach-or-exit.exp. > > No regression on x86-linux { native-gdbserver , native-extended-gdbserver } > > gdb/gdbserver/ChangeLog: > > * tracepoint.c (cmd_qtdp): Check for current_thread == NULL. > (cmd_qtenable_disable): Likewise. > (cmd_qtstart): Likewise. > (stop_tracing): Likewise. > (cmd_qtstop): Likewise. > (cmd_qtstatus): Likewise. > > gdb/testsuite/ChangeLog: > > * gdb.trace/ftrace-commands-after-detach-or-exit.c: New file. > * gdb.trace/ftrace-commands-after-detach-or-exit.exp: New test. > * lib/gdbserver-support.exp (gdb_target_cmd): Add support for connection > timed out error. > --- > gdb/gdbserver/tracepoint.c | 36 ++++- > .../ftrace-commands-after-detach-or-exit.c | 25 ++++ > .../ftrace-commands-after-detach-or-exit.exp | 160 +++++++++++++++++++++ > gdb/testsuite/lib/gdbserver-support.exp | 4 + > 4 files changed, 224 insertions(+), 1 deletion(-) > create mode 100644 gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.c > create mode 100644 gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.exp > > diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c > index c07e525..d80e0c98 100644 > --- a/gdb/gdbserver/tracepoint.c > +++ b/gdb/gdbserver/tracepoint.c > @@ -2488,6 +2488,13 @@ cmd_qtdp (char *own_buf) > char *actparm; > char *packet = own_buf; > > + /* Can't do this command without a pid attached. */ > + if (current_thread == NULL) > + { > + write_enn (packet); > + return; > + } > + > packet += strlen ("QTDP:"); > > /* A hyphen at the beginning marks a packet specifying actions for a > @@ -2752,6 +2759,13 @@ cmd_qtenable_disable (char *own_buf, int enable) > ULONGEST num, addr; > struct tracepoint *tp; > > + /* Can't do this command without a pid attached. */ > + if (current_thread == NULL) > + { > + write_enn (packet); > + return; > + } > + > packet += strlen (enable ? "QTEnable:" : "QTDisable:"); > packet = unpack_varlen_hex (packet, &num); > ++packet; /* skip a colon */ > @@ -3202,6 +3216,13 @@ cmd_qtstart (char *packet) > struct tracepoint *tpoint, *prev_ftpoint, *prev_stpoint; > CORE_ADDR tpptr = 0, prev_tpptr = 0; > > + /* Can't do this command without a pid attached. */ > + if (current_thread == NULL) > + { > + write_enn (packet); > + return; > + } > + > trace_debug ("Starting the trace"); > > /* Pause all threads temporarily while we patch tracepoints. */ > @@ -3420,6 +3441,12 @@ stop_tracing (void) > return; > } > > + if (current_thread == NULL) > + { > + trace_debug ("Current thread null, can't stop threads"); > + return; > + } > + > trace_debug ("Stopping the trace"); > > /* Pause all threads before removing fast jumps from memory, > @@ -3532,6 +3559,13 @@ flush_trace_buffer_handler (CORE_ADDR addr) > static void > cmd_qtstop (char *packet) > { > + /* Can't do this command without a pid attached. */ > + if (current_thread == NULL) > + { > + write_enn (packet); > + return; > + } > + > stop_tracing (); > write_ok (packet); > } > @@ -3650,7 +3684,7 @@ cmd_qtstatus (char *packet) > trace_debug ("Returning trace status as %d, stop reason %s", > tracing, tracing_stop_reason); > > - if (agent_loaded_p ()) > + if (current_thread != NULL && agent_loaded_p ()) > { > pause_all (1); > > diff --git a/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.c b/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.c > new file mode 100644 > index 0000000..9f93b9b > --- /dev/null > +++ b/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.c > @@ -0,0 +1,25 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + 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 . */ > + > +#include "trace-common.h" > + > +int > +main (void) > +{ > + FAST_TRACEPOINT_LABEL(set_point); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.exp b/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.exp > new file mode 100644 > index 0000000..2034cf5 > --- /dev/null > +++ b/gdb/testsuite/gdb.trace/ftrace-commands-after-detach-or-exit.exp > @@ -0,0 +1,160 @@ > +# 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 . > + > +# This test verifies that GDBServer does not crash with the following > +# commands: tstatus, enable, disable, ftrace, tstop after a detach or run > +# to end whether we stopped the trace before doing detach or run to end > +# or not. > +# This test also verifies that GDBServer does not crash on disconnect. > + > +load_lib "trace-support.exp" > + > +standard_testfile > +set executable $testfile > +set expfile $testfile.exp > + > +# Some targets have leading underscores on assembly symbols. > +set options [list debug [gdb_target_symbol_prefix_flags]] > + > +# Check that the target supports trace. > +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } { > + untested "Couldn't compile test program" > + return -1 > +} > + > +clean_restart ${testfile} > + > +if ![runto_main] { > + fail "Can't run to main to check for trace support" > + return -1 > +} > + > +if $use_gdb_stub { > + # This test is about testing commands after detaching from a process or > + # after letting a process exit, so it doesn't make sense to run it if the > + # target is stub-like. > + unsupported "This test is not supported for GDB stub targets." > + return -1 > +} > + > +if ![gdb_target_supports_trace] { > + unsupported "target does not support trace" > + return -1 > +} > + > +# Compile the test case with the in-process agent library. > +set libipa [get_in_proc_agent] > +set remote_libipa [gdb_load_shlib $libipa] > + > +lappend options shlib=$libipa > + > +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } { > + untested "Couldn't compile test program with in-process agent library" > + return -1 > +} > + > +clean_restart ${testfile} > + > +if ![runto_main] { > + fail "Can't run to main to check for trace support" > + return -1 > +} > + > +if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 } { > + untested "Could not find IPA lib loaded" > + return -1 > +} > + > +proc do_test {command detach_method tstop} { > + global executable binfile decimal gdbserver_reconnect_p > + set gdbserver_reconnect_p 1 > + clean_restart ${executable} > + > + if ![runto_main] { > + fail "Can't run to main." > + return -1 > + } > + > + gdb_test_no_output "set confirm off" > + gdb_test "ftrace set_point" "Fast tracepoint .*" > + gdb_test_no_output "tstart" > + > + if {$tstop} { > + gdb_test_no_output "tstop" > + } > + > + if {$detach_method == "detach"} { > + gdb_test "detach" "Detaching from program: $binfile, process $decimal" > + } elseif {$detach_method == "exit"} { > + gdb_continue_to_end > + } > + > + switch $command { > + "tstatus" { > + if {$detach_method == "exit" && $tstop == 0} { > + gdb_test "tstatus" "Trace is running on the target\..*" > + } else { > + gdb_test "tstatus" "Trace stopped by a tstop command ()\..*" > + } > + } > + "disable" { > + if {!$tstop} { > + gdb_test "$command" "Target returns error code \'01\'\." > + } else { > + gdb_test_no_output "$command" > + } > + } > + "enable" { > + if {!$tstop && $detach_method == "exit"} { > + gdb_test "$command" "Target returns error code \'01\'\." > + } else { > + gdb_test_no_output "$command" > + } > + } > + "ftrace" { > + gdb_test "ftrace set_point" ".*Fast tracepoint \[0-9]+ at.*" > + } > + "tstop" { > + gdb_test "tstop" "Target returns error code \'01\'\." > + } > + } > + > + test_gdbserver_still_alive > +} > + > +# Test if gdbserver is still alive by reconnecting to it. > +proc test_gdbserver_still_alive { } { > + gdb_test "disconnect" "Ending remote debugging\\." > + set test "reconnect to GDBserver" > + if { [gdb_reconnect] == 0 } { > + pass $test > + } else { > + fail $test > + return 0 > + } > +} > + > + > +foreach command {"tstatus" "disable" "enable" "ftrace" "tstop" } { > + foreach detach_method {"detach" "exit"} { > + foreach tstop {0 1} { > + #Don't use tstop context if tstop is to be tested. > + if {$command == "tstop" && $tstop} { } else { > + with_test_prefix "$command after $detach_method , tracing stopped: $tstop" { > + do_test $command $detach_method $tstop > + } > + } > + } > + } > +} > diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp > index 951afe5..9f65f60 100644 > --- a/gdb/testsuite/lib/gdbserver-support.exp > +++ b/gdb/testsuite/lib/gdbserver-support.exp > @@ -90,6 +90,10 @@ proc gdb_target_cmd { targetname serialport } { > -re "Timeout reading from remote system.*$gdb_prompt $" { > verbose "Got timeout error from gdb." > } > + -re "Connection timed out.*$gdb_prompt $" { > + verbose "Got timeout error from gdb." > + } > + > -notransfer -re "Remote debugging using .*\r\n> $" { > # We got an unexpected prompt while creating the target. > # Leave it there for the test to diagnose. Ping.