RE: [PATCH 2/5] [func_call] New tests for a btrace crash.

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Hello Ari,

>During remote debugging when stoppin in the start-routine of a pthread,

Typo: stoppin

>GDB crashes when attempting to display function-call-history.
>
>gdb/testsuite/ChangeLog:
>2015-04-27  ravitheja.addepally  <ravitheja.addepally@intel.com>
>
>	* gdb.btrace/server-multithreaded-function-call-history.c: New file.
>	* gdb.btrace/server-multithreaded-function-call-history.exp: New file.

GDB is no longer using change logs.

This patch is quite old.  Is GDB still crashing?  Is there a fix for the crash in
your patch series?

We shouldn't introduce failing tests before the fix.  This hurts bisecting.
It would be better to fix an issue and add a regression test in the same patch.


>diff --git a/gdb/testsuite/gdb.btrace/server-multithreaded-function-call-
>history.exp b/gdb/testsuite/gdb.btrace/server-multithreaded-function-call-
>history.exp
>new file mode 100644
>index 0000000000..3af51c4228
>--- /dev/null
>+++ b/gdb/testsuite/gdb.btrace/server-multithreaded-function-call-history.exp
>@@ -0,0 +1,61 @@
>+# This testcase is part of GDB, the GNU debugger.
>+
>+# Copyright 2005-2022 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/>.

Could you briefly describe what this test is doing in an initial comment?

>+
>+load_lib gdbserver-support.exp
>+
>+standard_testfile
>+
>+if { [skip_gdbserver_tests] } {

There should be an untested message here...

>+    return 0
>+}
>+
>+if { [skip_btrace_tests] } {

...and here.

>+    return -1

Not sure it matters but why are there different return values for the
different skip checks?

>+}
>+
>+set opts { debug }
>+if [info exists DEBUG] {
>+    lappend opts "additional_flags=-DDEBUG"
>+}
>+
>+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
>$opts] != "" } {
>+    return -1
>+}

I think you should be able to use prepare_for_testing and pass pthreads as
build option similar to debug.

>+
>+gdb_exit
>+gdb_start
>+gdb_load $binfile
>+
>+# Make sure we're disconnected, in case we're testing with an
>+# extended-remote board, therefore already connected.
>+gdb_test "disconnect" ".*"
>+
>+gdbserver_run ""
>+gdb_reinitialize_dir $srcdir/$subdir
>+
>+gdb_breakpoint main
>+gdb_test "continue" "Breakpoint.* main .*" "continue to main"
>+
>+gdb_test_no_output "record btrace"
>+
>+gdb_breakpoint thread_function
>+gdb_test "continue" "Breakpoint.* thread_function .*" "continue to
>thread_function one"

How about gdb_continue_to_breakpoint?

>+
>+# test passes if this does not crash
>+gdb_test "record function-call-history" ".*"
>+
>+gdb_test "continue" "Breakpoint.* thread_function .*" "continue to
>thread_function two"

Is this last continue still necessary?

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928