* [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
@ 2017-01-30 10:05 ` Markus Metzger
2017-01-31 13:00 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests Markus Metzger
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:05 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
With --target_board=native-extended-gdbserver non-stop tests are failing with
UNTESTED: gdb.btrace/non-stop.exp: failed to run to main
Fix that by adding '-ex "set non-stop on"' to GDBFLAGS before restarting.
2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
testsuite/
* gdb.btrace/non-stop.exp: Add '-ex "set non-stop on"' to GDBFLAGS.
---
gdb/testsuite/gdb.btrace/non-stop.exp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp b/gdb/testsuite/gdb.btrace/non-stop.exp
index cda15e2..a671b9c 100644
--- a/gdb/testsuite/gdb.btrace/non-stop.exp
+++ b/gdb/testsuite/gdb.btrace/non-stop.exp
@@ -25,9 +25,11 @@ if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debu
untested "failed to prepare"
return -1
}
-clean_restart $testfile
-gdb_test_no_output "set non-stop on"
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ clean_restart $testfile
+}
if ![runto_main] {
untested "failed to run to main"
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads
@ 2017-01-30 10:05 Markus Metzger
2017-01-30 10:05 ` [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test Markus Metzger
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:05 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
This refers to: https://sourceware.org/ml/gdb-patches/2016-11/msg00994.html.
The first version tried to detect the error in gdbserver, propagate it back, and
handle it in btrace.
Versions 2 and 3 check whether registers can be accessed before making the request
to gdbserver based on feedback from Pedro.
Version 3 addresses Pedro's feedback and adds more patches to address test suite
issues that were identified during reviews.
Markus Metzger (5):
thread: add can_access_registers_ptid
btrace: allow recording to be started (and stopped) for running
threads
btrace: add unsupported/untested messages when skipping tests
btrace, testsuite: fix extended-remote non-stop test
btrace, testsuite: fix extended-remote fail
gdb/btrace.c | 46 ++++++++++-
gdb/gdbthread.h | 4 +
gdb/testsuite/gdb.btrace/buffer-size.exp | 8 +-
gdb/testsuite/gdb.btrace/data.exp | 9 ++-
gdb/testsuite/gdb.btrace/delta.exp | 9 ++-
gdb/testsuite/gdb.btrace/dlopen.exp | 16 +++-
gdb/testsuite/gdb.btrace/enable-running.c | 47 +++++++++++
gdb/testsuite/gdb.btrace/enable-running.exp | 90 ++++++++++++++++++++++
gdb/testsuite/gdb.btrace/enable.exp | 17 +++-
gdb/testsuite/gdb.btrace/exception.exp | 9 ++-
gdb/testsuite/gdb.btrace/function_call_history.exp | 10 ++-
gdb/testsuite/gdb.btrace/gcore.exp | 8 +-
gdb/testsuite/gdb.btrace/instruction_history.exp | 9 ++-
gdb/testsuite/gdb.btrace/multi-thread-step.exp | 9 ++-
gdb/testsuite/gdb.btrace/nohist.exp | 8 +-
gdb/testsuite/gdb.btrace/non-stop.exp | 16 ++--
gdb/testsuite/gdb.btrace/reconnect.exp | 10 ++-
gdb/testsuite/gdb.btrace/record_goto-step.exp | 9 ++-
gdb/testsuite/gdb.btrace/record_goto.exp | 10 ++-
gdb/testsuite/gdb.btrace/rn-dl-bind.exp | 9 ++-
gdb/testsuite/gdb.btrace/segv.exp | 8 +-
gdb/testsuite/gdb.btrace/step.exp | 9 ++-
gdb/testsuite/gdb.btrace/stepi.exp | 14 ++--
gdb/testsuite/gdb.btrace/tailcall-only.exp | 10 ++-
gdb/testsuite/gdb.btrace/tailcall.exp | 9 ++-
gdb/testsuite/gdb.btrace/tsx.exp | 14 +++-
gdb/testsuite/gdb.btrace/unknown_functions.exp | 9 ++-
gdb/testsuite/gdb.btrace/vdso.exp | 9 ++-
gdb/thread.c | 20 +++++
29 files changed, 371 insertions(+), 84 deletions(-)
create mode 100644 gdb/testsuite/gdb.btrace/enable-running.c
create mode 100644 gdb/testsuite/gdb.btrace/enable-running.exp
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 1/5] thread: add can_access_registers_ptid
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
2017-01-30 10:05 ` [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test Markus Metzger
2017-01-30 10:06 ` [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests Markus Metzger
@ 2017-01-30 10:06 ` Markus Metzger
2017-01-30 10:06 ` [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads Markus Metzger
2017-01-30 10:06 ` [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail Markus Metzger
4 siblings, 0 replies; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:06 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
Add a function can_access_registers_ptid that behaves like
validate_registers_access but returns a boolean value instead of throwing an
exception.
2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
* gdbthread.h (can_access_registers_ptid): New.
* thread.c (can_access_registers_ptid): New.
---
gdb/gdbthread.h | 4 ++++
gdb/thread.c | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 455cfd8..06ed78f 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -625,6 +625,10 @@ extern void thread_cancel_execution_command (struct thread_info *thr);
executing). */
extern void validate_registers_access (void);
+/* Check whether it makes sense to access a register of PTID at this point.
+ Returns true if registers may be accessed; false otherwise. */
+extern bool can_access_registers_ptid (ptid_t ptid);
+
/* Returns whether to show which thread hit the breakpoint, received a
signal, etc. and ended up causing a user-visible stop. This is
true iff we ever detected multiple threads. */
diff --git a/gdb/thread.c b/gdb/thread.c
index e45b257..99fe424 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1141,6 +1141,26 @@ validate_registers_access (void)
error (_("Selected thread is running."));
}
+/* See gdbthread.h. */
+
+bool
+can_access_registers_ptid (ptid_t ptid)
+{
+ /* No thread, no registers. */
+ if (ptid_equal (ptid, null_ptid))
+ return false;
+
+ /* Don't try to read from a dead thread. */
+ if (is_exited (ptid))
+ return false;
+
+ /* ... or from a spinning thread. FIXME: see validate_registers_access. */
+ if (is_executing (ptid))
+ return false;
+
+ return true;
+}
+
int
pc_in_thread_step_range (CORE_ADDR pc, struct thread_info *thread)
{
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
2017-01-30 10:05 ` [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test Markus Metzger
@ 2017-01-30 10:06 ` Markus Metzger
2017-01-31 13:00 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 1/5] thread: add can_access_registers_ptid Markus Metzger
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:06 UTC (permalink / raw)
To: gdb-patches; +Cc: palves, Luis Machado
We may silently skip gdb.btrace tests if
- the target does not support record-btrace
- the target does not support TSX
- the target does not support gdbserver
- we fail to compile the test
- we fail to run to main
Add unsupported/untested messages for each of those.
CC: Luis Machado <lgustavo@codesourcery.com>
2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
testsuite/
* gdb.btrace/buffer-size.exp: Add unsupported/untested message if
the test is skipped.
* gdb.btrace/data.exp: Likewise.
* gdb.btrace/delta.exp: Likewise.
* gdb.btrace/dlopen.exp: Likewise.
* gdb.btrace/enable-running.exp: Likewise.
* gdb.btrace/enable.exp: Likewise.
* gdb.btrace/exception.exp: Likewise.
* gdb.btrace/function_call_history.exp: Likewise.
* gdb.btrace/gcore.exp: Likewise.
* gdb.btrace/instruction_history.exp: Likewise.
* gdb.btrace/multi-thread-step.exp: Likewise.
* gdb.btrace/nohist.exp: Likewise.
* gdb.btrace/non-stop.exp: Likewise.
* gdb.btrace/reconnect.exp: Likewise.
* gdb.btrace/record_goto-step.exp: Likewise.
* gdb.btrace/record_goto.exp: Likewise.
* gdb.btrace/rn-dl-bind.exp: Likewise.
* gdb.btrace/segv.exp: Likewise.
* gdb.btrace/step.exp: Likewise.
* gdb.btrace/stepi.exp: Likewise.
* gdb.btrace/tailcall-only.exp: Likewise.
* gdb.btrace/tailcall.exp: Likewise.
* gdb.btrace/tsx.exp: Likewise.
* gdb.btrace/unknown_functions.exp: Likewise.
* gdb.btrace/vdso.exp: Likewise.
---
gdb/testsuite/gdb.btrace/buffer-size.exp | 8 +++++---
gdb/testsuite/gdb.btrace/data.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/delta.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/dlopen.exp | 16 ++++++++++++----
gdb/testsuite/gdb.btrace/enable-running.exp | 7 ++++++-
gdb/testsuite/gdb.btrace/enable.exp | 14 +++++++++++---
gdb/testsuite/gdb.btrace/exception.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/function_call_history.exp | 10 +++++++---
gdb/testsuite/gdb.btrace/gcore.exp | 8 +++++---
gdb/testsuite/gdb.btrace/instruction_history.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/multi-thread-step.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/nohist.exp | 8 +++++---
gdb/testsuite/gdb.btrace/non-stop.exp | 10 ++++++----
gdb/testsuite/gdb.btrace/reconnect.exp | 10 ++++++++--
gdb/testsuite/gdb.btrace/record_goto-step.exp | 9 +++++----
gdb/testsuite/gdb.btrace/record_goto.exp | 10 +++++++---
gdb/testsuite/gdb.btrace/rn-dl-bind.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/segv.exp | 8 +++++---
gdb/testsuite/gdb.btrace/step.exp | 9 +++++----
gdb/testsuite/gdb.btrace/stepi.exp | 14 ++++++++------
gdb/testsuite/gdb.btrace/tailcall-only.exp | 10 +++++++---
gdb/testsuite/gdb.btrace/tailcall.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/tsx.exp | 14 +++++++++++---
gdb/testsuite/gdb.btrace/unknown_functions.exp | 9 ++++++---
gdb/testsuite/gdb.btrace/vdso.exp | 9 ++++++---
25 files changed, 167 insertions(+), 79 deletions(-)
diff --git a/gdb/testsuite/gdb.btrace/buffer-size.exp b/gdb/testsuite/gdb.btrace/buffer-size.exp
index 6d02016..c547835 100644
--- a/gdb/testsuite/gdb.btrace/buffer-size.exp
+++ b/gdb/testsuite/gdb.btrace/buffer-size.exp
@@ -17,16 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile record_goto.c
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/data.exp b/gdb/testsuite/gdb.btrace/data.exp
index 5f2d6d3..5c6fce4 100644
--- a/gdb/testsuite/gdb.btrace/data.exp
+++ b/gdb/testsuite/gdb.btrace/data.exp
@@ -17,15 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/delta.exp b/gdb/testsuite/gdb.btrace/delta.exp
index 5648d08..926edff 100644
--- a/gdb/testsuite/gdb.btrace/delta.exp
+++ b/gdb/testsuite/gdb.btrace/delta.exp
@@ -17,15 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile record_goto.c
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/dlopen.exp b/gdb/testsuite/gdb.btrace/dlopen.exp
index 209c83f..f0cb80e 100644
--- a/gdb/testsuite/gdb.btrace/dlopen.exp
+++ b/gdb/testsuite/gdb.btrace/dlopen.exp
@@ -15,8 +15,15 @@
# 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 }
-if { [skip_shlib_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
+
+if { [skip_shlib_tests] } {
+ unsupported "target does not support shared library tests"
+ return -1
+}
standard_testfile
@@ -26,7 +33,7 @@ set binfile_lib [standard_output_file $basename_lib.so]
if { [gdb_compile_shlib $srcfile_lib $binfile_lib \
[list additional_flags=-fPIC]] != "" } {
- untested "failed to compile shared library"
+ untested "failed to prepare shlib"
return -1
}
@@ -36,7 +43,8 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
}
if ![runto_main] {
- return 0
+ untested "failed to run to main"
+ return -1
}
# Trace the test function
diff --git a/gdb/testsuite/gdb.btrace/enable-running.exp b/gdb/testsuite/gdb.btrace/enable-running.exp
index fe6aa0b..461c5a5 100644
--- a/gdb/testsuite/gdb.btrace/enable-running.exp
+++ b/gdb/testsuite/gdb.btrace/enable-running.exp
@@ -15,10 +15,14 @@
# 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 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
standard_testfile
if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ untested "failed to prepare"
return -1
}
@@ -29,6 +33,7 @@ save_vars { GDBFLAGS } {
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/enable.exp b/gdb/testsuite/gdb.btrace/enable.exp
index b35218e..be20c08 100644
--- a/gdb/testsuite/gdb.btrace/enable.exp
+++ b/gdb/testsuite/gdb.btrace/enable.exp
@@ -17,8 +17,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
# start fresh - without an executable
gdb_exit
@@ -35,12 +37,13 @@ gdb_test "record function-call-history" "No record target is currently active\\.
gdb_test "record instruction-history" "No record target is currently active\\..*" "record instruction-history without target"
gdb_test "info record" "No record target is currently active\\." "info record without target"
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile {} {debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
@@ -75,23 +78,28 @@ gdb_test "continue" ".*Inferior.*exited.*" "continue to end"
# otherwise rerun twice, target should be automatically disabled
load_lib gdbserver-support.exp
if [skip_gdbserver_tests] {
+ unsupported "target does not support gdbserver"
return 0
}
clean_restart $testfile
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
# make sure record-btrace can be enabled after re-run
clean_restart $testfile
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
gdb_test_no_output "record btrace"
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
gdb_test_no_output "record btrace" "enable after re-run"
diff --git a/gdb/testsuite/gdb.btrace/exception.exp b/gdb/testsuite/gdb.btrace/exception.exp
index 026054f..2bac063 100755
--- a/gdb/testsuite/gdb.btrace/exception.exp
+++ b/gdb/testsuite/gdb.btrace/exception.exp
@@ -17,15 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile exception.cc
if [prepare_for_testing "failed to prepare" $testfile $srcfile {c++ debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/function_call_history.exp b/gdb/testsuite/gdb.btrace/function_call_history.exp
index f9e3ac1..6de1615 100644
--- a/gdb/testsuite/gdb.btrace/function_call_history.exp
+++ b/gdb/testsuite/gdb.btrace/function_call_history.exp
@@ -17,15 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile {} {debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
@@ -226,6 +229,7 @@ gdb_test "record function-call-history /c 21, +11" [multi_line \
# make sure we can handle incomplete trace with respect to indentation
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
# navigate to the fib in line 24 above
diff --git a/gdb/testsuite/gdb.btrace/gcore.exp b/gdb/testsuite/gdb.btrace/gcore.exp
index 3792a9d..d6b311b 100644
--- a/gdb/testsuite/gdb.btrace/gcore.exp
+++ b/gdb/testsuite/gdb.btrace/gcore.exp
@@ -17,16 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile record_goto.c
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/instruction_history.exp b/gdb/testsuite/gdb.btrace/instruction_history.exp
index 905f76c..b5d6d3b 100644
--- a/gdb/testsuite/gdb.btrace/instruction_history.exp
+++ b/gdb/testsuite/gdb.btrace/instruction_history.exp
@@ -17,15 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# compile and run to main
standard_testfile .c .S
if [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" {debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/multi-thread-step.exp b/gdb/testsuite/gdb.btrace/multi-thread-step.exp
index d86fb0a..e926a98 100644
--- a/gdb/testsuite/gdb.btrace/multi-thread-step.exp
+++ b/gdb/testsuite/gdb.btrace/multi-thread-step.exp
@@ -17,17 +17,20 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ untested "failed to prepare"
return -1
}
clean_restart $testfile
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/nohist.exp b/gdb/testsuite/gdb.btrace/nohist.exp
index 51f7e5a..f05fbbb 100644
--- a/gdb/testsuite/gdb.btrace/nohist.exp
+++ b/gdb/testsuite/gdb.btrace/nohist.exp
@@ -17,16 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile record_goto.c
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/non-stop.exp b/gdb/testsuite/gdb.btrace/non-stop.exp
index cf27160..cda15e2 100644
--- a/gdb/testsuite/gdb.btrace/non-stop.exp
+++ b/gdb/testsuite/gdb.btrace/non-stop.exp
@@ -15,13 +15,14 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
-
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ untested "failed to prepare"
return -1
}
clean_restart $testfile
@@ -29,6 +30,7 @@ clean_restart $testfile
gdb_test_no_output "set non-stop on"
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/reconnect.exp b/gdb/testsuite/gdb.btrace/reconnect.exp
index 4538934..a9269af 100644
--- a/gdb/testsuite/gdb.btrace/reconnect.exp
+++ b/gdb/testsuite/gdb.btrace/reconnect.exp
@@ -19,8 +19,14 @@
load_lib gdbserver-support.exp
-if { [skip_btrace_tests] } { return -1 }
-if { [skip_gdbserver_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
+if { [skip_gdbserver_tests] } {
+ unsupported "target does not support gdbserver"
+ return -1
+}
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
diff --git a/gdb/testsuite/gdb.btrace/record_goto-step.exp b/gdb/testsuite/gdb.btrace/record_goto-step.exp
index fb6a712..3819ed8 100644
--- a/gdb/testsuite/gdb.btrace/record_goto-step.exp
+++ b/gdb/testsuite/gdb.btrace/record_goto-step.exp
@@ -17,17 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
standard_testfile record_goto.c
-
-# start inferior
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/record_goto.exp b/gdb/testsuite/gdb.btrace/record_goto.exp
index 34763e5..341da7b 100644
--- a/gdb/testsuite/gdb.btrace/record_goto.exp
+++ b/gdb/testsuite/gdb.btrace/record_goto.exp
@@ -17,8 +17,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
# The "record goto" command jumps to a specific instruction in the execution
# trace. To guarantee that we always get the same execution trace, we use
@@ -40,14 +42,16 @@ if [info exists COMPILE] {
standard_testfile i686-record_goto.S
}
} else {
- verbose "Skipping ${testfile}."
+ unsupported "target architecture not supported"
return -1
}
if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/rn-dl-bind.exp b/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
index ee29af3..1add6af 100644
--- a/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
+++ b/gdb/testsuite/gdb.btrace/rn-dl-bind.exp
@@ -21,15 +21,18 @@
# Test that we can reverse-next over the dynamic linker's symbol
# lookup code.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile {c++ debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/segv.exp b/gdb/testsuite/gdb.btrace/segv.exp
index 562037a..f146cdc 100644
--- a/gdb/testsuite/gdb.btrace/segv.exp
+++ b/gdb/testsuite/gdb.btrace/segv.exp
@@ -17,15 +17,17 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/step.exp b/gdb/testsuite/gdb.btrace/step.exp
index 4e1b036..3dc1d39 100644
--- a/gdb/testsuite/gdb.btrace/step.exp
+++ b/gdb/testsuite/gdb.btrace/step.exp
@@ -17,17 +17,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
standard_testfile record_goto.c
-
-# start inferior
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/stepi.exp b/gdb/testsuite/gdb.btrace/stepi.exp
index fb32821..6087d86 100644
--- a/gdb/testsuite/gdb.btrace/stepi.exp
+++ b/gdb/testsuite/gdb.btrace/stepi.exp
@@ -17,8 +17,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
# This test is stepping on instruction level. To guarantee that we always
# get the same execution trace, we use an assembly source file.
@@ -38,21 +40,21 @@ if [info exists COMPILE] {
standard_testfile i686-record_goto.S
}
} else {
- verbose "Skipping ${testfile}."
+ unsupported "target architecture not supported"
return -1
}
-# start inferior
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
-global gdb_prompt
-
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
+global gdb_prompt
+
proc check_replay_at { insn } {
gdb_test "info record" [multi_line \
"Active record target: record-btrace" \
diff --git a/gdb/testsuite/gdb.btrace/tailcall-only.exp b/gdb/testsuite/gdb.btrace/tailcall-only.exp
index 75116c9..c5b29a9 100644
--- a/gdb/testsuite/gdb.btrace/tailcall-only.exp
+++ b/gdb/testsuite/gdb.btrace/tailcall-only.exp
@@ -20,8 +20,10 @@
# calls. This used to cause a crash in get_frame_type.
#
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
# This test requires the compiler to generate a tail call. To guarantee that
# we always get one, we use an assembly source file.
@@ -42,14 +44,16 @@ if [info exists COMPILE] {
standard_testfile i686-tailcall-only.S
}
} else {
- verbose "Skipping ${testfile}."
+ unsupported "target architecture not supported"
return -1
}
if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/tailcall.exp b/gdb/testsuite/gdb.btrace/tailcall.exp
index 623ca15..4f70c7c 100644
--- a/gdb/testsuite/gdb.btrace/tailcall.exp
+++ b/gdb/testsuite/gdb.btrace/tailcall.exp
@@ -17,8 +17,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
# This test requires the compiler to generate a tail call. To guarantee that
# we always get one, we use an assembly source file.
@@ -39,7 +41,7 @@ if [info exists COMPILE] {
standard_testfile i686-tailcall.S
}
} else {
- verbose "Skipping ${testfile}."
+ unsupported "target architecture not supported"
return -1
}
@@ -47,6 +49,7 @@ if [prepare_for_testing "failed to prepare" $testfile $srcfile $opts] {
return -1
}
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/tsx.exp b/gdb/testsuite/gdb.btrace/tsx.exp
index da3a939..014acf6 100644
--- a/gdb/testsuite/gdb.btrace/tsx.exp
+++ b/gdb/testsuite/gdb.btrace/tsx.exp
@@ -15,15 +15,23 @@
# 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_pt_tests] } { return -1 }
-if { [skip_tsx_tests] } { return -1 }
+if { [skip_btrace_pt_tests] } {
+ unsupported "target does not support PT"
+ return -1
+}
+
+if { [skip_tsx_tests] } {
+ unsupported "target does not support TSX"
+ return -1
+}
-# compile and run to main
standard_testfile .c x86-tsx.S
if [prepare_for_testing "failed to prepare" $testfile "$srcfile $srcfile2" {debug}] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/unknown_functions.exp b/gdb/testsuite/gdb.btrace/unknown_functions.exp
index 22ab7ba..7b5ea27 100644
--- a/gdb/testsuite/gdb.btrace/unknown_functions.exp
+++ b/gdb/testsuite/gdb.btrace/unknown_functions.exp
@@ -17,10 +17,11 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
# discard local symbols
@@ -28,7 +29,9 @@ set ldflags "additional_flags=-Wl,-x"
if [prepare_for_testing "failed to prepare" $testfile $srcfile $ldflags] {
return -1
}
+
if ![runto test] {
+ untested "failed to run to main"
return -1
}
diff --git a/gdb/testsuite/gdb.btrace/vdso.exp b/gdb/testsuite/gdb.btrace/vdso.exp
index d5f6d99..283d920 100644
--- a/gdb/testsuite/gdb.btrace/vdso.exp
+++ b/gdb/testsuite/gdb.btrace/vdso.exp
@@ -20,15 +20,18 @@
#
# Test that we can access the vdso memory during replay for stepping.
-# check for btrace support
-if { [skip_btrace_tests] } { return -1 }
+if { [skip_btrace_tests] } {
+ unsupported "target does not support record-btrace"
+ return -1
+}
-# start inferior
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
return -1
}
+
if ![runto_main] {
+ untested "failed to run to main"
return -1
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
` (2 preceding siblings ...)
2017-01-30 10:06 ` [PATCH v3 1/5] thread: add can_access_registers_ptid Markus Metzger
@ 2017-01-30 10:06 ` Markus Metzger
2017-01-31 12:59 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail Markus Metzger
4 siblings, 1 reply; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:06 UTC (permalink / raw)
To: gdb-patches; +Cc: palves, Simon Marchi
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. Skip that step to allow recording to be started while
threads are running.
If we do run into errors, we need to undo the tracing enable to not leak this
thread. The operation did not complete so our caller won't clean up this
thread.
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.
CC: Simon Marchi <simon.marchi@ericsson.com>
2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
gdb/
* btrace.c (btrace_enable): Do not call btrace_add_pc for
BTRACE_FORMAT_PT or if can_access_registers_ptid returns false.
(validate_registers_access_ptid): New.
(btrace_fetch): Call validate_registers_access_ptid.
testsuite/
* gdb.btrace/enable-running.c: New.
* gdb.btrace/enable-running.exp: New.
---
gdb/btrace.c | 46 ++++++++++++++--
gdb/testsuite/gdb.btrace/enable-running.c | 47 ++++++++++++++++
gdb/testsuite/gdb.btrace/enable-running.exp | 85 +++++++++++++++++++++++++++++
3 files changed, 174 insertions(+), 4 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 d266af7..fec2cce 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -141,6 +141,18 @@ ftrace_debug (const struct btrace_function *bfun, const char *prefix)
prefix, fun, file, level, ibegin, iend);
}
+/* Validate that we can read PTID's registers. */
+
+static void
+validate_registers_access_ptid (ptid_t ptid)
+{
+ struct cleanup *cleanup = save_inferior_ptid ();
+
+ inferior_ptid = ptid;
+ validate_registers_access ();
+ do_cleanups (cleanup);
+}
+
/* Return non-zero if BFUN does not match MFUN and FUN,
return zero otherwise. */
@@ -1472,10 +1484,33 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
tp->btrace.target = target_enable_btrace (tp->ptid, 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);
+ /* We're done if we failed to enable tracing. */
+ if (tp->btrace.target == NULL)
+ return;
+
+ /* We need to undo the enable in case of errors. */
+ TRY
+ {
+ /* Add an entry for the current PC so we start tracing from where we
+ enabled it.
+
+ If we can't access TP's registers, TP is most likely running. In this
+ case, we can't really say where tracing was enabled so it should be
+ safe to simply skip this step.
+
+ This is not relevant for BTRACE_FORMAT_PT since the trace will already
+ start at the PC at which tracing was enabled. */
+ if (conf->format != BTRACE_FORMAT_PT
+ && can_access_registers_ptid (tp->ptid))
+ btrace_add_pc (tp);
+ }
+ CATCH (exception, RETURN_MASK_ALL)
+ {
+ btrace_disable (tp);
+
+ throw_exception (exception);
+ }
+ END_CATCH
}
/* See btrace.h. */
@@ -1709,6 +1744,9 @@ btrace_fetch (struct thread_info *tp)
if (btinfo->replay != NULL)
return;
+ /* Make sure the thread is not running or has already exited. */
+ validate_registers_access_ptid (tp->ptid);
+
btrace_data_init (&btrace);
cleanup = make_cleanup_btrace_data (&btrace);
diff --git a/gdb/testsuite/gdb.btrace/enable-running.c b/gdb/testsuite/gdb.btrace/enable-running.c
new file mode 100644
index 0000000..a2e9c14
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 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>
+
+#define NTHREADS 3
+
+static void *
+test (void *arg)
+{
+ /* Let's hope this is long enough for GDB to enable tracing and check that
+ everything is working as expected. */
+ sleep (10);
+
+ return arg;
+}
+
+int
+main (void)
+{
+ pthread_t th[NTHREADS];
+ int i;
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_create (&th[i], NULL, test, NULL);
+
+ test (NULL); /* bp.1 */
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_join (th[i], NULL);
+
+ 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..fe6aa0b
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.exp
@@ -0,0 +1,85 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2017 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 }
+
+standard_testfile
+if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ return -1
+}
+
+# We need to enable non-stop mode for the remote case.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ clean_restart $testfile
+}
+
+if ![runto_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
+
+ with_test_prefix "thread $thread" {
+ gdb_test "thread $thread" "(running).*" "is running"
+
+ # We can't read the trace while the thread is running.
+ gdb_test "info record" "Selected thread is running\." \
+ "info record while running"
+
+ # Stop the thread before reading the trace.
+ gdb_test_multiple "interrupt" "interrupt" {
+ -re "interrupt\r\n$gdb_prompt " {
+ pass "interrupt"
+ }
+ }
+ # Wait until the thread actually stopped.
+ gdb_test_multiple "" "stopped" {
+ -re "Thread $thread.*stopped\." {
+ pass "stopped"
+ }
+ }
+ # 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\]*" \
+ ]
+
+ # Continue the thread again.
+ gdb_test "cont&" "Continuing\."
+ }
+}
+
+# Check that recording was started on each thread.
+foreach thread {1 2 3 4} {
+ check_tracing_enabled $thread
+}
+
+# Stop recording while all threads are running.
+gdb_test "record stop" "Process record is stopped \[^\\\r\\\n\]*"
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
` (3 preceding siblings ...)
2017-01-30 10:06 ` [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads Markus Metzger
@ 2017-01-30 10:06 ` Markus Metzger
2017-01-31 13:00 ` Pedro Alves
4 siblings, 1 reply; 17+ messages in thread
From: Markus Metzger @ 2017-01-30 10:06 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
Parts of gdb.btrace/enable.exp are only valid for native debug. The check for
skip_gdbserver_tests is done while GDB is running, though, which causes it to
fail with --target_board=native-extended-gdbserver. Exit GDB before that check.
2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
testsuite/
* gdb.btrace/enable.exp: Call gdb_exit before skip_gdbserver_tests.
---
gdb/testsuite/gdb.btrace/enable.exp | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gdb/testsuite/gdb.btrace/enable.exp b/gdb/testsuite/gdb.btrace/enable.exp
index be20c08..61abe0b 100644
--- a/gdb/testsuite/gdb.btrace/enable.exp
+++ b/gdb/testsuite/gdb.btrace/enable.exp
@@ -74,6 +74,9 @@ gdb_test "record btrace" "The process is already being recorded\\. Use \"record
# continue to the end and make sure we don't die
gdb_test "continue" ".*Inferior.*exited.*" "continue to end"
+# skip_gdbserver_tests requires GDB not running.
+gdb_exit
+
# skip the rerun test when using gdbserver
# otherwise rerun twice, target should be automatically disabled
load_lib gdbserver-support.exp
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-01-30 10:06 ` [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads Markus Metzger
@ 2017-01-31 12:59 ` Pedro Alves
2017-01-31 16:06 ` Metzger, Markus T
0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 12:59 UTC (permalink / raw)
To: Markus Metzger, gdb-patches; +Cc: Simon Marchi
On 01/30/2017 10:05 AM, Markus Metzger wrote:
> ---
> gdb/btrace.c | 46 ++++++++++++++--
> gdb/testsuite/gdb.btrace/enable-running.c | 47 ++++++++++++++++
> gdb/testsuite/gdb.btrace/enable-running.exp | 85 +++++++++++++++++++++++++++++
> 3 files changed, 174 insertions(+), 4 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 d266af7..fec2cce 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -141,6 +141,18 @@ ftrace_debug (const struct btrace_function *bfun, const char *prefix)
> prefix, fun, file, level, ibegin, iend);
> }
>
> +/* Validate that we can read PTID's registers. */
> +
> +static void
> +validate_registers_access_ptid (ptid_t ptid)
> +{
> + struct cleanup *cleanup = save_inferior_ptid ();
> +
> + inferior_ptid = ptid;
> + validate_registers_access ();
> + do_cleanups (cleanup);
> +}
> +
Please let's refactor things a bit to avoid the need to frob
inferior_ptid and restore with a cleanup. Change the existing
validate_registers_access like:
static void
-validate_registers_access ()
+validate_registers_access_ptid (ptid_t ptid)
{
/* No selected thread, no registers. */
-if (ptid_equal (inferior_ptid, null_ptid))
+if (ptid_equal (ptid, null_ptid))
error (_("No thread selected."));
[etc.]
and then reimplement it back on top of validate_registers_access_ptid:
validate_registers_access ()
{
return validate_registers_access_ptid (inferior_ptid);
}
> +
> +#include <pthread.h>
> +
> +#define NTHREADS 3
> +
> +static void *
> +test (void *arg)
> +{
> + /* Let's hope this is long enough for GDB to enable tracing and check that
> + everything is working as expected. */
> + sleep (10);
Need to include <unistd.h> for sleep. I got:
..../src/gdb/testsuite/gdb.btrace/enable-running.c: In function 'test':
..../src/gdb/testsuite/gdb.btrace/enable-running.c:27:3: warning: implicit declaration of function 'sleep' [-Wimplicit-function-declaration]
sleep (10);
^
Otherwise LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail
2017-01-30 10:06 ` [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail Markus Metzger
@ 2017-01-31 13:00 ` Pedro Alves
2017-01-31 16:06 ` Metzger, Markus T
0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 13:00 UTC (permalink / raw)
To: Markus Metzger, gdb-patches
On 01/30/2017 10:05 AM, Markus Metzger wrote:
> Parts of gdb.btrace/enable.exp are only valid for native debug. The check for
> skip_gdbserver_tests is done while GDB is running, though, which causes it to
> fail with --target_board=native-extended-gdbserver. Exit GDB before that check.
>
Can you clarify how it fails? It's not obvious to me from looking at
skip_gdbserver_tests.
Should we add something like:
global gdb_spawn_id
if {[info exists gdb_spawn_id]} {
error "....."
return
}
... to skip_gdbserver_tests ?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests
2017-01-30 10:06 ` [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests Markus Metzger
@ 2017-01-31 13:00 ` Pedro Alves
0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 13:00 UTC (permalink / raw)
To: Markus Metzger, gdb-patches; +Cc: Luis Machado
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test
2017-01-30 10:05 ` [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test Markus Metzger
@ 2017-01-31 13:00 ` Pedro Alves
0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 13:00 UTC (permalink / raw)
To: Markus Metzger, gdb-patches
On 01/30/2017 10:05 AM, Markus Metzger wrote:
> With --target_board=native-extended-gdbserver non-stop tests are failing with
>
> UNTESTED: gdb.btrace/non-stop.exp: failed to run to main
>
> Fix that by adding '-ex "set non-stop on"' to GDBFLAGS before restarting.
>
> 2017-01-30 Markus Metzger <markus.t.metzger@intel.com>
>
> testsuite/
> * gdb.btrace/non-stop.exp: Add '-ex "set non-stop on"' to GDBFLAGS.
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-01-31 12:59 ` Pedro Alves
@ 2017-01-31 16:06 ` Metzger, Markus T
2017-01-31 16:36 ` Pedro Alves
0 siblings, 1 reply; 17+ messages in thread
From: Metzger, Markus T @ 2017-01-31 16:06 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Simon Marchi
Hi Pedro,
Thanks for your review.
> > +static void
> > +validate_registers_access_ptid (ptid_t ptid)
> > +{
> > + struct cleanup *cleanup = save_inferior_ptid ();
> > +
> > + inferior_ptid = ptid;
> > + validate_registers_access ();
> > + do_cleanups (cleanup);
> > +}
> > +
>
> Please let's refactor things a bit to avoid the need to frob
> inferior_ptid and restore with a cleanup. Change the existing
> validate_registers_access like:
>
> static void
> -validate_registers_access ()
> +validate_registers_access_ptid (ptid_t ptid)
> {
> /* No selected thread, no registers. */
> -if (ptid_equal (inferior_ptid, null_ptid))
> +if (ptid_equal (ptid, null_ptid))
> error (_("No thread selected."));
> [etc.]
>
> and then reimplement it back on top of validate_registers_access_ptid:
>
> validate_registers_access ()
> {
> return validate_registers_access_ptid (inferior_ptid);
> }
I had that and then chose to rewrite it again because the errors that
validate_registers_access throws refer to "selected thread".
In the btrace case, the argument thread is the selected thread (I think
this is true for all flows that call btrace_fetch) but this is not necessarily
the case in general. Do you think that might be confusing to the user?
I was also thinking of changing the error messages to refer to the argument
thread but that would make them less clear when PTID really refers to the
selected thread - which should be the case most of the time if not always.
Maybe this is not an issue in practice and the "selected thread" errors are
understandable enough. What do you think?
Thanks,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail
2017-01-31 13:00 ` Pedro Alves
@ 2017-01-31 16:06 ` Metzger, Markus T
2017-01-31 16:42 ` Pedro Alves
0 siblings, 1 reply; 17+ messages in thread
From: Metzger, Markus T @ 2017-01-31 16:06 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, January 31, 2017 2:00 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail
>
> On 01/30/2017 10:05 AM, Markus Metzger wrote:
> > Parts of gdb.btrace/enable.exp are only valid for native debug. The check for
> > skip_gdbserver_tests is done while GDB is running, though, which causes it to
> > fail with --target_board=native-extended-gdbserver. Exit GDB before that
> check.
> >
>
> Can you clarify how it fails? It's not obvious to me from looking at
> skip_gdbserver_tests.
spawn /nfs/site/disks/idb_team/mmetzger/gdb/build/ulll001/gdb/testsuite/../gdbserver/gdbserver --once --multi :2349
Listening on port 2349
target extended-remote localhost:2349
Already connected to a remote target. Disconnect? (y or n) ^CQuit
(gdb) set tdesc filename /nfs/site/disks/idb_team/mmetzger/gdb/build/ulll001/gdb/testsuite/outputs/gdb.btrace/enable/tri
vial.xml
(gdb) n
The program is not being run.
(gdb) FAIL: gdb.btrace/enable.exp: set tdesc filename /nfs/site/disks/idb_team/mmetzger/gdb/build/ulll001/gdb/testsuite/outputs/gdb.btrace/enable/trivial.xml (got interactive prompt)
monitor exit
There's a comment in lib/gdb.exp before gdb_skip_xml_test, which is called
by skip_gdbserver_tests:
# Return true if a test should be skipped due to lack of XML support
# in the host GDB.
# NOTE: This must be called while gdb is *not* running.
gdb_caching_proc gdb_skip_xml_test {
I thought this is supposed to be general knowledge so I didn't bother to go into more
detail in the commit message.
> Should we add something like:
>
> global gdb_spawn_id
> if {[info exists gdb_spawn_id]} {
> error "....."
> return
> }
>
> ... to skip_gdbserver_tests ?
That would certainly make it more obvious:
Running .../gdb.btrace/enable.exp ...
ERROR: tcl error sourcing .../gdb.btrace/enable.exp.
ERROR: GDB must not be running in skip_gdbserver_tests.
while executing
"error "GDB must not be running in skip_gdbserver_tests.""
(procedure "skip_gdbserver_tests" line 9)
invoked from within
"skip_gdbserver_tests"
invoked from within
"if [skip_gdbserver_tests] {
unsupported "target does not support gdbserver"
return 0
}"
(file ".../gdb.btrace/enable.exp" line 83)
invoked from within
"source .../gdb.btrace/enable.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source /.../gdb.btrace/enable.exp"
invoked from within
"catch "uplevel #0 source $test_file_name""
Looks like we don't need the return, though. And the check should
probably go into skip_xml_tests. Let me add a separate patch for this.
Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-01-31 16:06 ` Metzger, Markus T
@ 2017-01-31 16:36 ` Pedro Alves
2017-02-01 8:53 ` Metzger, Markus T
0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 16:36 UTC (permalink / raw)
To: Metzger, Markus T, gdb-patches; +Cc: Simon Marchi
On 01/31/2017 04:06 PM, Metzger, Markus T wrote:
> I was also thinking of changing the error messages to refer to the argument
> thread but that would make them less clear when PTID really refers to the
> selected thread - which should be the case most of the time if not always.
>
> Maybe this is not an issue in practice and the "selected thread" errors are
> understandable enough. What do you think?
I think we may be able to sidestep deciding this. :-)
The only path that leads to btrace_fetch that isn't clearly about
the selected thread is record_btrace_resume_thread, AFAICS.
But in that case, we have:
if (!target_is_non_stop_p ())
{
gdb_assert (ptid_match (inferior_ptid, ptid));
ALL_NON_EXITED_THREADS (tp)
if (ptid_match (tp->ptid, ptid))
{
if (ptid_match (tp->ptid, inferior_ptid))
record_btrace_resume_thread (tp, flag);
else
record_btrace_resume_thread (tp, cflag);
}
}
else
{
ALL_NON_EXITED_THREADS (tp)
if (ptid_match (tp->ptid, ptid))
record_btrace_resume_thread (tp, flag);
}
ALL_NON_EXITED_THREADS guarantees btrace_fetch won't see
exited threads through that path. And then in all-stop,
by definition, all threads are stopped. And in the non-stop
branch, we can be sure that the core won't ever try to resume
a running or exited thread. If it does, and we reach as far
as btrace_fetch, it's an internal error, IMO.
That leaves us with this version of the patch (CL already updated),
which calls validate_registers_access very close to the inferior_ptid
references and the "no thread" checks, making it more obvious these are
about user/selected context.
WDYT?
From 3927351221c1b49827e82a9e94dda2e2f268f90a Mon Sep 17 00:00:00 2001
From: Markus Metzger <markus.t.metzger@intel.com>
Date: Tue, 31 Jan 2017 16:29:59 +0000
Subject: [PATCH] btrace: allow recording to be started (and stopped) for
running threads
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. Skip that step to allow recording to be started while
threads are running.
If we do run into errors, we need to undo the tracing enable to not leak this
thread. The operation did not complete so our caller won't clean up this
thread.
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.
CC: Simon Marchi <simon.marchi@ericsson.com>
gdb/ChangeLog:
2017-01-31 Markus Metzger <markus.t.metzger@intel.com>
* btrace.c (btrace_enable): Do not call btrace_add_pc for
BTRACE_FORMAT_PT or if can_access_registers_ptid returns false.
(btrace_fetch): Assert we can access thread registers.
* record-btrace.c (require_btrace_thread, record_btrace_info):
Call validate_registers_access.
gdb/testsuite/ChangeLog:
2017-01-31 Markus Metzger <markus.t.metzger@intel.com>
* gdb.btrace/enable-running.c: New.
* gdb.btrace/enable-running.exp: New.
---
gdb/btrace.c | 34 ++++++++++--
gdb/record-btrace.c | 2 +
gdb/testsuite/gdb.btrace/enable-running.c | 48 ++++++++++++++++
gdb/testsuite/gdb.btrace/enable-running.exp | 85 +++++++++++++++++++++++++++++
4 files changed, 165 insertions(+), 4 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 d266af7..07f747c 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1472,10 +1472,33 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
tp->btrace.target = target_enable_btrace (tp->ptid, 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);
+ /* We're done if we failed to enable tracing. */
+ if (tp->btrace.target == NULL)
+ return;
+
+ /* We need to undo the enable in case of errors. */
+ TRY
+ {
+ /* Add an entry for the current PC so we start tracing from where we
+ enabled it.
+
+ If we can't access TP's registers, TP is most likely running. In this
+ case, we can't really say where tracing was enabled so it should be
+ safe to simply skip this step.
+
+ This is not relevant for BTRACE_FORMAT_PT since the trace will already
+ start at the PC at which tracing was enabled. */
+ if (conf->format != BTRACE_FORMAT_PT
+ && can_access_registers_ptid (tp->ptid))
+ btrace_add_pc (tp);
+ }
+ CATCH (exception, RETURN_MASK_ALL)
+ {
+ btrace_disable (tp);
+
+ throw_exception (exception);
+ }
+ END_CATCH
}
/* See btrace.h. */
@@ -1709,6 +1732,9 @@ btrace_fetch (struct thread_info *tp)
if (btinfo->replay != NULL)
return;
+ /* We shouldn't be called on running or exited threads. */
+ gdb_assert (can_access_registers_ptid (tp->ptid));
+
btrace_data_init (&btrace);
cleanup = make_cleanup_btrace_data (&btrace);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 8896241..49f61e6 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -116,6 +116,7 @@ require_btrace_thread (void)
tp = find_thread_ptid (inferior_ptid);
if (tp == NULL)
error (_("No thread."));
+ validate_registers_access ();
btrace_fetch (tp);
@@ -416,6 +417,7 @@ record_btrace_info (struct target_ops *self)
tp = find_thread_ptid (inferior_ptid);
if (tp == NULL)
error (_("No thread."));
+ validate_registers_access ();
btinfo = &tp->btrace;
diff --git a/gdb/testsuite/gdb.btrace/enable-running.c b/gdb/testsuite/gdb.btrace/enable-running.c
new file mode 100644
index 0000000..c2edf1e
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 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>
+#include <unistd.h>
+
+#define NTHREADS 3
+
+static void *
+test (void *arg)
+{
+ /* Let's hope this is long enough for GDB to enable tracing and check that
+ everything is working as expected. */
+ sleep (10);
+
+ return arg;
+}
+
+int
+main (void)
+{
+ pthread_t th[NTHREADS];
+ int i;
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_create (&th[i], NULL, test, NULL);
+
+ test (NULL); /* bp.1 */
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_join (th[i], NULL);
+
+ 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..fe6aa0b
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.exp
@@ -0,0 +1,85 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2017 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 }
+
+standard_testfile
+if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ return -1
+}
+
+# We need to enable non-stop mode for the remote case.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ clean_restart $testfile
+}
+
+if ![runto_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
+
+ with_test_prefix "thread $thread" {
+ gdb_test "thread $thread" "(running).*" "is running"
+
+ # We can't read the trace while the thread is running.
+ gdb_test "info record" "Selected thread is running\." \
+ "info record while running"
+
+ # Stop the thread before reading the trace.
+ gdb_test_multiple "interrupt" "interrupt" {
+ -re "interrupt\r\n$gdb_prompt " {
+ pass "interrupt"
+ }
+ }
+ # Wait until the thread actually stopped.
+ gdb_test_multiple "" "stopped" {
+ -re "Thread $thread.*stopped\." {
+ pass "stopped"
+ }
+ }
+ # 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\]*" \
+ ]
+
+ # Continue the thread again.
+ gdb_test "cont&" "Continuing\."
+ }
+}
+
+# Check that recording was started on each thread.
+foreach thread {1 2 3 4} {
+ check_tracing_enabled $thread
+}
+
+# Stop recording while all threads are running.
+gdb_test "record stop" "Process record is stopped \[^\\\r\\\n\]*"
--
2.5.5
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail
2017-01-31 16:06 ` Metzger, Markus T
@ 2017-01-31 16:42 ` Pedro Alves
0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2017-01-31 16:42 UTC (permalink / raw)
To: Metzger, Markus T, gdb-patches
On 01/31/2017 04:06 PM, Metzger, Markus T wrote:
> There's a comment in lib/gdb.exp before gdb_skip_xml_test, which is called
> by skip_gdbserver_tests:
>
> # Return true if a test should be skipped due to lack of XML support
> # in the host GDB.
> # NOTE: This must be called while gdb is *not* running.
>
> gdb_caching_proc gdb_skip_xml_test {
Ah, that starts gdb itself, runs "set tdesc filename" and then
exits gdb. Indeed, that can't work in the middle of some
other test... A command without side effects ("maint check-supports-xml"
or something), would avoid this... But let's not bother.
> I thought this is supposed to be general knowledge so I didn't bother to go into more
> detail in the commit message.
I don't recall ever noticing this before.
> Looks like we don't need the return, though. And the check should
> probably go into skip_xml_tests. Let me add a separate patch for this.
Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-01-31 16:36 ` Pedro Alves
@ 2017-02-01 8:53 ` Metzger, Markus T
2017-02-01 9:08 ` Metzger, Markus T
0 siblings, 1 reply; 17+ messages in thread
From: Metzger, Markus T @ 2017-02-01 8:53 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Simon Marchi
> > I was also thinking of changing the error messages to refer to the argument
> > thread but that would make them less clear when PTID really refers to the
> > selected thread - which should be the case most of the time if not always.
> >
> > Maybe this is not an issue in practice and the "selected thread" errors are
> > understandable enough. What do you think?
>
> I think we may be able to sidestep deciding this. :-)
That seems to work. I added a few tests to cover other record commands
while threads are running.
Thanks,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-02-01 8:53 ` Metzger, Markus T
@ 2017-02-01 9:08 ` Metzger, Markus T
2017-02-01 10:40 ` Pedro Alves
0 siblings, 1 reply; 17+ messages in thread
From: Metzger, Markus T @ 2017-02-01 9:08 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Simon Marchi
Hi Pedro,
> > > I was also thinking of changing the error messages to refer to the argument
> > > thread but that would make them less clear when PTID really refers to the
> > > selected thread - which should be the case most of the time if not always.
> > >
> > > Maybe this is not an issue in practice and the "selected thread" errors are
> > > understandable enough. What do you think?
> >
> > I think we may be able to sidestep deciding this. :-)
>
> That seems to work. I added a few tests to cover other record commands
> while threads are running.
Here's the resulting patch.
You commented on "[PATCH v3 5/5] btrace, testsuite: fix extended-remote fail"
but as far as I can tell this has been resolved and we don't need an update.
commit eccb3848eadfc5d7a89e53d21d975e80f050a98a
Author: Markus Metzger <markus.t.metzger@intel.com>
Date: Wed Nov 30 11:05:38 2016 +0100
btrace: allow recording to be started (and stopped) for running threads
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. Skip that step to allow recording to be started while
threads are running.
If we do run into errors, we need to undo the tracing enable to not leak this
thread. The operation did not complete so our caller won't clean up this
thread.
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.
CC: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: Markus Metzger <markus.t.metzger@intel.com>
gdb/
* btrace.c (btrace_enable): Do not call btrace_add_pc for
BTRACE_FORMAT_PT or if can_access_registers_ptid returns false.
(btrace_fetch): Assert can_access_registers_ptid.
* record-btrace.c (require_btrace_thread, record_btrace_info): Call
validate_registers_access.
testsuite/
* gdb.btrace/enable-running.c: New.
* gdb.btrace/enable-running.exp: New.
diff --git a/gdb/btrace.c b/gdb/btrace.c
index d266af7..6d621e4 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1472,10 +1472,33 @@ btrace_enable (struct thread_info *tp, const struct btrace_config *conf)
tp->btrace.target = target_enable_btrace (tp->ptid, 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);
+ /* We're done if we failed to enable tracing. */
+ if (tp->btrace.target == NULL)
+ return;
+
+ /* We need to undo the enable in case of errors. */
+ TRY
+ {
+ /* Add an entry for the current PC so we start tracing from where we
+ enabled it.
+
+ If we can't access TP's registers, TP is most likely running. In this
+ case, we can't really say where tracing was enabled so it should be
+ safe to simply skip this step.
+
+ This is not relevant for BTRACE_FORMAT_PT since the trace will already
+ start at the PC at which tracing was enabled. */
+ if (conf->format != BTRACE_FORMAT_PT
+ && can_access_registers_ptid (tp->ptid))
+ btrace_add_pc (tp);
+ }
+ CATCH (exception, RETURN_MASK_ALL)
+ {
+ btrace_disable (tp);
+
+ throw_exception (exception);
+ }
+ END_CATCH
}
/* See btrace.h. */
@@ -1709,6 +1732,9 @@ btrace_fetch (struct thread_info *tp)
if (btinfo->replay != NULL)
return;
+ /* We should not be called on running or exited threads. */
+ gdb_assert (can_access_registers_ptid (tp->ptid));
+
btrace_data_init (&btrace);
cleanup = make_cleanup_btrace_data (&btrace);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 8896241..ee0d22c 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -117,6 +117,8 @@ require_btrace_thread (void)
if (tp == NULL)
error (_("No thread."));
+ validate_registers_access ();
+
btrace_fetch (tp);
if (btrace_is_empty (tp))
@@ -417,6 +419,8 @@ record_btrace_info (struct target_ops *self)
if (tp == NULL)
error (_("No thread."));
+ validate_registers_access ();
+
btinfo = &tp->btrace;
conf = btrace_conf (btinfo);
diff --git a/gdb/testsuite/gdb.btrace/enable-running.c b/gdb/testsuite/gdb.btrace/enable-running.c
new file mode 100644
index 0000000..c2edf1e
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.c
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 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>
+#include <unistd.h>
+
+#define NTHREADS 3
+
+static void *
+test (void *arg)
+{
+ /* Let's hope this is long enough for GDB to enable tracing and check that
+ everything is working as expected. */
+ sleep (10);
+
+ return arg;
+}
+
+int
+main (void)
+{
+ pthread_t th[NTHREADS];
+ int i;
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_create (&th[i], NULL, test, NULL);
+
+ test (NULL); /* bp.1 */
+
+ for (i = 0; i < NTHREADS; ++i)
+ pthread_join (th[i], NULL);
+
+ 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..d549a40
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/enable-running.exp
@@ -0,0 +1,95 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2017 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 }
+
+standard_testfile
+if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" "$binfile" executable {debug}] != "" } {
+ return -1
+}
+
+# We need to enable non-stop mode for the remote case.
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ clean_restart $testfile
+}
+
+if ![runto_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
+
+ with_test_prefix "thread $thread" {
+ gdb_test "thread $thread" "(running).*" "is running"
+
+ # We can't read the trace while the thread is running.
+ gdb_test "info record" "Selected thread is running\." \
+ "info record while running"
+
+ # Try various commands that try to read trace.
+ gdb_test "record instruction-history" "Selected thread is running\."
+ gdb_test "record function-call-history" "Selected thread is running\."
+
+ # Including reverse-stepping commands.
+ gdb_test "reverse-continue" "\[Ss\]elected thread is running\."
+ gdb_test "reverse-step" "\[Ss\]elected thread is running\."
+ gdb_test "reverse-next" "\[Ss\]elected thread is running\."
+ gdb_test "reverse-finish" "\[Ss\]elected thread is running\."
+
+ # Stop the thread before reading the trace.
+ gdb_test_multiple "interrupt" "interrupt" {
+ -re "interrupt\r\n$gdb_prompt " {
+ pass "interrupt"
+ }
+ }
+ # Wait until the thread actually stopped.
+ gdb_test_multiple "" "stopped" {
+ -re "Thread $thread.*stopped\." {
+ pass "stopped"
+ }
+ }
+ # 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\]*" \
+ ]
+
+ # Continue the thread again.
+ gdb_test "cont&" "Continuing\."
+ }
+}
+
+# Check that recording was started on each thread.
+foreach thread {1 2 3 4} {
+ check_tracing_enabled $thread
+}
+
+# Stop recording while all threads are running.
+gdb_test "record stop" "Process record is stopped \[^\\\r\\\n\]*"
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads
2017-02-01 9:08 ` Metzger, Markus T
@ 2017-02-01 10:40 ` Pedro Alves
0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2017-02-01 10:40 UTC (permalink / raw)
To: Metzger, Markus T, gdb-patches; +Cc: Simon Marchi
On 02/01/2017 09:08 AM, Metzger, Markus T wrote:
> Here's the resulting patch.
>
LGTM.
> You commented on "[PATCH v3 5/5] btrace, testsuite: fix extended-remote fail"
> but as far as I can tell this has been resolved and we don't need an update.
Yes, that's right.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-02-01 10:40 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-30 10:05 [PATCH v3 0/5] thread, btrace: allow "record btrace" for running threads Markus Metzger
2017-01-30 10:05 ` [PATCH v3 4/5] btrace, testsuite: fix extended-remote non-stop test Markus Metzger
2017-01-31 13:00 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 3/5] btrace: add unsupported/untested messages when skipping tests Markus Metzger
2017-01-31 13:00 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 1/5] thread: add can_access_registers_ptid Markus Metzger
2017-01-30 10:06 ` [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads Markus Metzger
2017-01-31 12:59 ` Pedro Alves
2017-01-31 16:06 ` Metzger, Markus T
2017-01-31 16:36 ` Pedro Alves
2017-02-01 8:53 ` Metzger, Markus T
2017-02-01 9:08 ` Metzger, Markus T
2017-02-01 10:40 ` Pedro Alves
2017-01-30 10:06 ` [PATCH v3 5/5] btrace, testsuite: fix extended-remote fail Markus Metzger
2017-01-31 13:00 ` Pedro Alves
2017-01-31 16:06 ` Metzger, Markus T
2017-01-31 16:42 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).