Message ID | 1467039989-12638-1-git-send-email-antoine.tremblay@ericsson.com |
---|---|
State | New |
Headers | show |
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 <http://www.gnu.org/licenses/>. */ > + > +#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 <http://www.gnu.org/licenses/>. > + > +# 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.
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 <http://www.gnu.org/licenses/>. */ + +#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 <http://www.gnu.org/licenses/>. + +# 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.