Add test case for a known hang in infrun
Commit Message
The hang occurs when GDB tries to call inferior functions on two
different threads with scheduler-locking turned on. The first call
works fine, with the call to infrun_async(1) causing the
signal_handler to be marked and the event to be handled, but then
the event loop resets the "ready" member to zero, while leaving
infrun_is_async set to 1. As a result, GDB hangs if the user switches
to another thread and calls a second function because calling
infrun_async(1) a second time has no effect, meaning the inferior
call events are never handled.
The added test case provokes the above issue.
gdb/testsuite/ChangeLog:
* gdb.threads/multiple-successive-infcall.c: New test.
* gdb.threads/multiple-successive-infcall.exp: New file.
---
.../gdb.threads/multiple-successive-infcall.c | 104 +++++++++++++++++++++
.../gdb.threads/multiple-successive-infcall.exp | 55 +++++++++++
2 files changed, 159 insertions(+)
create mode 100644 gdb/testsuite/gdb.threads/multiple-successive-infcall.c
create mode 100644 gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
Comments
On 2018-03-27 12:23, Richard Bunt wrote:
> The hang occurs when GDB tries to call inferior functions on two
> different threads with scheduler-locking turned on. The first call
> works fine, with the call to infrun_async(1) causing the
> signal_handler to be marked and the event to be handled, but then
> the event loop resets the "ready" member to zero, while leaving
> infrun_is_async set to 1. As a result, GDB hangs if the user switches
> to another thread and calls a second function because calling
> infrun_async(1) a second time has no effect, meaning the inferior
> call events are never handled.
>
> The added test case provokes the above issue.
Hi Richard,
Thanks for taking the time to write and submit the test. We don't allow
adding new tests that FAIL in the testsuite, but I guess it's fine to
add one that KFAILs. The day someone fixes that bug (perhaps by
indirectly/by chance), it will start KPASSing (at which point we can
remove the KFAIL), and we'll already have a test for that. So I think
it's a valuable contribution.
The test looks good in general, I just have have a few comments:
> gdb/testsuite/ChangeLog:
>
> * gdb.threads/multiple-successive-infcall.c: New test.
> * gdb.threads/multiple-successive-infcall.exp: New file.
> ---
> .../gdb.threads/multiple-successive-infcall.c | 104
> +++++++++++++++++++++
> .../gdb.threads/multiple-successive-infcall.exp | 55 +++++++++++
> 2 files changed, 159 insertions(+)
> create mode 100644
> gdb/testsuite/gdb.threads/multiple-successive-infcall.c
> create mode 100644
> gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
>
> diff --git a/gdb/testsuite/gdb.threads/multiple-successive-infcall.c
> b/gdb/testsuite/gdb.threads/multiple-successive-infcall.c
> new file mode 100644
> index 0000000..3010c18
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/multiple-successive-infcall.c
> @@ -0,0 +1,104 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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 <stdio.h>
> +#include <stdlib.h>
> +#include <pthread.h>
> +
> +/* This defines the number of threads to spawn. */
> +#define THREADCOUNT 4
> +
> +/* Global barrier type to control synchronization between threads. */
> +pthread_barrier_t print_barrier;
> +/* Define global thread identifiers. */
> +pthread_t threads[THREADCOUNT];
> +/* Memory for threads to read from when get_value is called by the
> + debugger. */
> +int thread_ids[THREADCOUNT];
> +
> +/* Return the argument back, within range [0..THREADCOUNT).
> + This is the function called on the inferiors from the debugger. */
> +int
> +get_value (int index)
> +{
> + return thread_ids[index];
> +}
In the test, get_value is always called with index == 0, so I'm not sure
what the thread_ids array is for. I was under the impression that it
was going to be used to confirm that the infcall was made using the
correct thread (which would be a good thing to test for). Is this what
you intended? I guess this could be achieved by using thread local
storage in which the thread id is stored. If the wrong thread is used
to make the infcall, the wrong value will be returned.
> +/* Return the nth Fibonacci number. */
> +unsigned long
> +fast_fib (unsigned int n)
> +{
> + int a = 0;
> + int b = 1;
> + int t;
> + for (unsigned int i = 0; i < n; ++i)
> + {
> + t = b;
> + b = a + b;
> + a = t;
> + }
> + return a;
> +}
> +
> +/* Encapsulate the synchronization of the threads. Perform a barrier
> before
> + and after the computation. */
> +void *
> +thread_function (void *args)
> +{
> + int tid = get_value (*((int *) args));
> + int status = pthread_barrier_wait (&print_barrier);
> + if (status == PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + printf ("All threads entering compute region\n");
> + }
> + unsigned long result = fast_fib (100); // testmarker01
> + status = pthread_barrier_wait (&print_barrier);
> + if (status == PTHREAD_BARRIER_SERIAL_THREAD)
> + {
> + printf ("All threads outputting results\n");
> + }
> + pthread_barrier_wait (&print_barrier);
> + printf ("Thread %d Result: %lu\n", tid, result);
> + pthread_exit (NULL);
> +}
> +
> +int
> +main (void)
> +{
> + int err = pthread_barrier_init (&print_barrier, NULL, THREADCOUNT);
> + // Create the worker threads (main).
> + printf ("Spawning worker threads\n");
> + for (int tid = 0; tid < THREADCOUNT; ++tid)
> + {
> + thread_ids[tid] = tid;
> + err =
> + pthread_create (&threads[tid], NULL, thread_function,
> + (void *) &thread_ids[tid]);
> + if (err)
> + {
> + fprintf (stderr, "Thread creation failed\n");
> + return EXIT_FAILURE;
> + }
> + }
> + // Wait for the threads to complete then exit.
> + for (int tid = 0; tid < THREADCOUNT; ++tid)
> + {
> + pthread_join (threads[tid], NULL);
> + }
> + pthread_exit (NULL);
> + return EXIT_SUCCESS;
> +}
> diff --git a/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> b/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> new file mode 100644
> index 0000000..363f8e0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/multiple-successive-infcall.exp
> @@ -0,0 +1,55 @@
> +# Copyright (C) 2018 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/>. */
> +
> +# multiple-successive-infcall.exp -- Test if GDB can invoke functions
> on
> +# multiple inferiors, one after the other.
> +
> +standard_testfile
> +
> +if [get_compiler_info] {
> + return -1
> +}
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}"
> "${binfile}" \
> + executable {debug}] != "" } {
> + return -1
> +}
> +
> +clean_restart "${binfile}"
> +
> +if ![runto_main] then {
> + fail "Can't run to main"
Test messages throughout the fil should start with a lower case and not
have a period at the end:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Follow_the_test_name_convention
> + return 0
> +}
> +
> +gdb_breakpoint [gdb_get_line_number "testmarker01"]
> +gdb_continue_to_breakpoint "testmarker01"
> +gdb_test_no_output "set scheduler-locking on"
> +gdb_test "show scheduler-locking" \
> + "Mode for locking scheduler during execution is \"on\"."
> +
> +foreach thread {4 3 2 1} {
This will test with the main thread (thread 1), and the first 3 of the 4
spawned threads. Is this what you want? We might as well test with the
main thread and all 4 spawned threads.
Also, each test message should be unique. Executing the loop body
multiple times will give multiples tests with the same message. To make
each loop iteration unique, you can use foreach_with_prefix:
foreach_with_prefix thread {4 3 2 1} {
...
}
which will prefix each message with "thread=$thread: ". See:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
> + gdb_test "thread ${thread}" "Switching to .*"
> + set command "call get_value(0)"
> + gdb_test_multiple "${command}" "${command}" {
> + -re ".*\[\r\n]+${gdb_prompt} $" {
If the return value is predictable, it would be better to match it, at
least the " = 0" part.
> + pass "Call on inferior returned successfully."
When using gdb_test_multiple, all test outcomes should ideally use the
same message. So whatever you pass as the second argument to
gdb_test_multiple (which may be the same as the command, if it's clear
enough) you would also usually pass to "pass", "fail" or "kfail" inside
the body. Also, make sure the message follows the convention linked
above.
> + }
> + timeout {
> + kfail "gdb/22882" "Call on inferior hung."
> + return 0
> + }
> + }
Thanks,
Simon
new file mode 100644
@@ -0,0 +1,104 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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 <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+/* This defines the number of threads to spawn. */
+#define THREADCOUNT 4
+
+/* Global barrier type to control synchronization between threads. */
+pthread_barrier_t print_barrier;
+/* Define global thread identifiers. */
+pthread_t threads[THREADCOUNT];
+/* Memory for threads to read from when get_value is called by the
+ debugger. */
+int thread_ids[THREADCOUNT];
+
+/* Return the argument back, within range [0..THREADCOUNT).
+ This is the function called on the inferiors from the debugger. */
+int
+get_value (int index)
+{
+ return thread_ids[index];
+}
+
+/* Return the nth Fibonacci number. */
+unsigned long
+fast_fib (unsigned int n)
+{
+ int a = 0;
+ int b = 1;
+ int t;
+ for (unsigned int i = 0; i < n; ++i)
+ {
+ t = b;
+ b = a + b;
+ a = t;
+ }
+ return a;
+}
+
+/* Encapsulate the synchronization of the threads. Perform a barrier before
+ and after the computation. */
+void *
+thread_function (void *args)
+{
+ int tid = get_value (*((int *) args));
+ int status = pthread_barrier_wait (&print_barrier);
+ if (status == PTHREAD_BARRIER_SERIAL_THREAD)
+ {
+ printf ("All threads entering compute region\n");
+ }
+ unsigned long result = fast_fib (100); // testmarker01
+ status = pthread_barrier_wait (&print_barrier);
+ if (status == PTHREAD_BARRIER_SERIAL_THREAD)
+ {
+ printf ("All threads outputting results\n");
+ }
+ pthread_barrier_wait (&print_barrier);
+ printf ("Thread %d Result: %lu\n", tid, result);
+ pthread_exit (NULL);
+}
+
+int
+main (void)
+{
+ int err = pthread_barrier_init (&print_barrier, NULL, THREADCOUNT);
+ // Create the worker threads (main).
+ printf ("Spawning worker threads\n");
+ for (int tid = 0; tid < THREADCOUNT; ++tid)
+ {
+ thread_ids[tid] = tid;
+ err =
+ pthread_create (&threads[tid], NULL, thread_function,
+ (void *) &thread_ids[tid]);
+ if (err)
+ {
+ fprintf (stderr, "Thread creation failed\n");
+ return EXIT_FAILURE;
+ }
+ }
+ // Wait for the threads to complete then exit.
+ for (int tid = 0; tid < THREADCOUNT; ++tid)
+ {
+ pthread_join (threads[tid], NULL);
+ }
+ pthread_exit (NULL);
+ return EXIT_SUCCESS;
+}
new file mode 100644
@@ -0,0 +1,55 @@
+# Copyright (C) 2018 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/>. */
+
+# multiple-successive-infcall.exp -- Test if GDB can invoke functions on
+# multiple inferiors, one after the other.
+
+standard_testfile
+
+if [get_compiler_info] {
+ return -1
+}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable {debug}] != "" } {
+ return -1
+}
+
+clean_restart "${binfile}"
+
+if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+}
+
+gdb_breakpoint [gdb_get_line_number "testmarker01"]
+gdb_continue_to_breakpoint "testmarker01"
+gdb_test_no_output "set scheduler-locking on"
+gdb_test "show scheduler-locking" \
+ "Mode for locking scheduler during execution is \"on\"."
+
+foreach thread {4 3 2 1} {
+ gdb_test "thread ${thread}" "Switching to .*"
+ set command "call get_value(0)"
+ gdb_test_multiple "${command}" "${command}" {
+ -re ".*\[\r\n]+${gdb_prompt} $" {
+ pass "Call on inferior returned successfully."
+ }
+ timeout {
+ kfail "gdb/22882" "Call on inferior hung."
+ return 0
+ }
+ }
+}