[PATCH 2/2] btrace: allow recording to be started for running threads
Luis Machado
lgustavo@codesourcery.com
Wed Dec 7 20:20:00 GMT 2016
On 12/06/2016 09:54 AM, Markus Metzger wrote:
> When recording is started for a running thread, GDB was able to start tracing
> but then failed to read registers to insert the initial entry for the current
> PC. We don't really need that initial entry if we don't know where exactly we
> started recording. Silently ignore such errors to allow recording to be started
> while threads are running.
>
> For the BTRACE_FORMAT_PT btrace format, we don't need that initial entry since
> it will be recorded in the trace. We can omit the call to btrace_add_pc.
>
> 2016-12-06 Markus Metzger <markus.t.metzger@intel.com>
>
> gdb/
> * btrace.c (btrace_enable): Do not call btrace_add_pc for
> BTRACE_FORMAT_PT. Silently ignore errors from btrace_add_pc.
>
> testsuite/
> * gdb.btrace/enable-running.c: New.
> * gdb.btrace/enable-running.exp: New.
> ---
> gdb/btrace.c | 24 +++++++++-
> gdb/testsuite/gdb.btrace/enable-running.c | 47 +++++++++++++++++++
> gdb/testsuite/gdb.btrace/enable-running.exp | 72 +++++++++++++++++++++++++++++
> 3 files changed, 141 insertions(+), 2 deletions(-)
> create mode 100644 gdb/testsuite/gdb.btrace/enable-running.c
> create mode 100644 gdb/testsuite/gdb.btrace/enable-running.exp
>
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 39d537c..920b9ab 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -1474,8 +1474,28 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
>
> /* Add an entry for the current PC so we start tracing from where we
> enabled it. */
> - if (tp->btrace.target != NULL)
> - btrace_add_pc (tp);
> + TRY
> + {
> + /* This is not relevant for BTRACE_FORMAT_PT since the trace will already
> + start at the enable location. */
enabled location?
> + if ((tp->btrace.target != NULL) && (conf->format != BTRACE_FORMAT_PT))
> + btrace_add_pc (tp);
> + }
> + CATCH (exception, RETURN_MASK_ALL)
> + {
> + /* We may fail to add the initial entry, for example if TP is currently
> + running.
> +
> + We won't be able to stitch the initial trace chunk to this initial
> + entry so tracing will start at the next branch target instead of at the
> + current PC. Since TP is currently running, this shouldn't make a
> + difference.
> +
> + If TP were waiting most of the time and made only a little bit of
> + progress before it was stopped, we'd lose the instructions until the
> + first branch. */
> + }
> + END_CATCH
> }
>
> /* See btrace.h. */
> diff --git a/gdb/testsuite/gdb.btrace/enable-running.c b/gdb/testsuite/gdb.btrace/enable-running.c
> new file mode 100644
> index 0000000..6380c29
> --- /dev/null
> +++ b/gdb/testsuite/gdb.btrace/enable-running.c
> @@ -0,0 +1,47 @@
> +/* 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 <pthread.h>
> +
> +static int global;
> +
> +static void *
> +test (void *arg)
> +{
> + for (;;)
> + global += 1;
> +
> + return arg;
> +}
> +
> +int
> +main (void)
> +{
> + int i;
> +
> + for (i = 0; i < 3; ++i)
> + {
> + pthread_t th;
> +
> + pthread_create (&th, NULL, test, NULL);
> + pthread_detach (th);
> + }
> +
> + test (NULL); /* bp.1 */
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.btrace/enable-running.exp b/gdb/testsuite/gdb.btrace/enable-running.exp
> new file mode 100644
> index 0000000..ead86af
> --- /dev/null
> +++ b/gdb/testsuite/gdb.btrace/enable-running.exp
> @@ -0,0 +1,72 @@
> +# 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/>.
> +
> +if { [skip_btrace_tests] } { return -1 }
unsupported "btrace not supported on this target"?
> +
> +standard_testfile
> +if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
untested "failed to compile"
> + return -1
> +}
> +clean_restart $testfile
> +
> +# We need to enable non-stop mode for the remote case.
> +gdb_test_no_output "set non-stop on"
> +
> +if ![runto_main] {
untested "couldn't run to main"
> + return -1
> +}
> +
> +set bp_1 [gdb_get_line_number "bp.1" $srcfile]
> +
> +gdb_breakpoint $bp_1
> +gdb_continue_to_breakpoint "cont to $bp_1" ".*$bp_1\r\n.*"
> +gdb_test "cont&" "Continuing\."
> +
> +# All threads are running. Let's start recording.
> +gdb_test_no_output "record btrace"
> +
> +proc check_tracing_enabled { thread } {
> + global gdb_prompt
> +
> + gdb_test "thread $thread" "(running).*"
Maybe add a more meaningful test name here? "Check if thread $thread is
running"?
> + with_test_prefix "thread $thread" {
> + # Stop the thread before reading the trace.
> + gdb_test_multiple "interrupt" "interrupt" {
> + -re "interrupt\r\n$gdb_prompt " {
> + pass "interrupt"
> + }
Any reason why we couldn't use gdb_test here? Also, we a more meaningful
test name and make the name unique (by mentioning the thread number,
possibly)
> + }
> + # Wait until the thread actually stopped.
> + gdb_test_multiple "" "stopped" {
> + -re "Thread $thread.*stopped\." {
> + pass "stopped"
> + }
> + }
Same here. Couldn't we use gdb_test? Also a more meaningful and unique
test name.
> + # We will consume the thread's current location as part of the
> + # "info record" output.
> + gdb_test "info record" [multi_line \
> + "Active record target: record-btrace" \
> + "Recording format: .*" \
> + "Recorded \[0-9\]+ instructions \[^\\\r\\\n\]*" \
> + ]
> + }
> +}
> +
> +# Check that recording was started on each thread.
> +foreach thread {1 2 3 4} {
> + check_tracing_enabled $thread
> +}
>
More information about the Gdb-patches
mailing list