[PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
Tom de Vries
tdevries@suse.de
Tue Apr 5 07:56:02 GMT 2022
On 4/4/22 18:08, Simon Marchi wrote:
> New in v2:
>
> - adjust test C code to avoid other potential racy failures pointed
> out by Tom de Vries.
>
> I got failures like this once on a CI:
>
> frame^M
> &"frame\n"^M
> ~"#0 child_sub_function () at /home/jenkins/workspace/binutils-gdb_master_build/arch/amd64/target_board/unix/src/binutils-gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:33\n"^M
> ~"33\t dummy = !dummy; /* thread loop line */\n"^M
> ^done^M
> (gdb) ^M
> FAIL: gdb.mi/mi-cmd-user-context.exp: frame 1 (unexpected output)
>
> The problem is that the test expects the following regexp:
>
> ".*#0 0x.*"
>
> And that typically works, when the output of the frame command looks
> like:
>
> #0 0x00005555555551bb in child_sub_function () at ...
>
> Note the lack of hexadecimal address in the failing case. Whether or
> not the hexadecimal address is printed (roughly) depends on whether the
> current PC is at the beginning of a line. So depending on where thread
> 2 was when GDB stopped it (after thread 1 hit its breakpoint), we can
> get either output. Adjust the regexps to not expect an hexadecimal
> prefix (0x) but a function name instead (either child_sub_function or
> child_function). That one is always printed, and is also a good check
> that we are in the frame we expect.
>
> Note that for test "frame 5", we are showing a pthread frame (on my
> system), so the function name is internal to pthread, not something we
> can rely on. In that case, it's almost certain that we are not at the
> beginning of a line, or that we don't have debug info, so I think it's
> fine to expect the hex prefix.
>
> And for test "frame 6", it's ok to _not_ expect a hex prefix (what the
> test currently does), since we are showing thread 1, which has hit a
> breakpoint placed at the beginning of a line.
>
> When testing this, Tom de Vries pointed out that the current test code
> doesn't ensure that the child threads are in child_sub_function when
> they are stopped. If the scheduler chooses so, it is possible for the
> child threads to be still in the pthread_barrier_wait or child_function
> functions when they get stopped. So that would be another racy failure
> waiting to happen.
>
> The only way I can think of to ensure the child threads are in the
> child_sub_function function when they get stopped is to synchronize the
> threads using some variables instead of pthread_barrier_wait. So,
> replace the barrier with an array of flags (one per child thread). Each
> child thread flips its flag in child_sub_function to allow the main
> thread to make progress and eventually hit the breakpoint.
>
> I copied user-selected-context-sync.c to a new mi-cmd-user-context.c and
> made modifications to that, to avoid interfering with
> user-selected-context-sync.exp.
>
Hi,
I've tested this, and can confirm that this fixes the FAILs I observed
in PR29025.
Also I've reviewed the patch, LGTM.
Please apply, also on the gdb-12-branch, which is where I observed the FAIL.
Thanks,
- Tom
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29025
> Change-Id: I919673bbf9927158beb0e8b7e9e980b8d65eca90
> ---
> gdb/testsuite/gdb.mi/mi-cmd-user-context.c | 73 ++++++++++++++++++++
> gdb/testsuite/gdb.mi/mi-cmd-user-context.exp | 10 +--
> 2 files changed, 78 insertions(+), 5 deletions(-)
> create mode 100644 gdb/testsuite/gdb.mi/mi-cmd-user-context.c
>
> diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.c b/gdb/testsuite/gdb.mi/mi-cmd-user-context.c
> new file mode 100644
> index 00000000000..bb74ab86b20
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-cmd-user-context.c
> @@ -0,0 +1,73 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2016-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/>.
> +
> +*/
> +
> +#include <pthread.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +
> +#define NUM_THREADS 2
> +
> +static volatile int unblock_main[NUM_THREADS];
> +
> +static void
> +child_sub_function (int child_idx)
> +{
> + volatile int dummy = 0;
> +
> + unblock_main[child_idx] = 1;
> +
> + while (1)
> + /* Dummy loop body to allow setting breakpoint. */
> + dummy = !dummy; /* thread loop line */
> +}
> +
> +static void *
> +child_function (void *args)
> +{
> + int child_idx = (int) (uintptr_t) args;
> +
> + child_sub_function (child_idx); /* thread caller line */
> +
> + return NULL;
> +}
> +
> +int
> +main (void)
> +{
> + int i = 0;
> + pthread_t threads[NUM_THREADS];
> +
> + /* Make the test exit eventually. */
> + alarm (20);
> +
> + for (i = 0; i < NUM_THREADS; i++)
> + pthread_create (&threads[i], NULL, child_function, (void *) (uintptr_t) i);
> +
> + /* Wait for child threads to reach child_sub_function. */
> + for (i = 0; i < NUM_THREADS; i++)
> + while (!unblock_main[i])
> + ;
> +
> + volatile int dummy = 0;
> + while (1)
> + /* Dummy loop body to allow setting breakpoint. */
> + dummy = !dummy; /* main break line */
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> index d7417d5ea7c..e5fccdd2308 100644
> --- a/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> +++ b/gdb/testsuite/gdb.mi/mi-cmd-user-context.exp
> @@ -18,7 +18,7 @@
>
> load_lib mi-support.exp
>
> -standard_testfile user-selected-context-sync.c
> +standard_testfile
>
> if {[build_executable $testfile.exp $testfile ${srcfile} "debug pthreads"] == -1} {
> untested "failed to compile"
> @@ -79,7 +79,7 @@ mi_gdb_test "thread" \
>
> # Check we're in frame 0.
> mi_gdb_test "frame" \
> - ".*#0 0x.*" \
> + ".*#0 .*child_sub_function .*" \
> "frame 1"
>
> # Ask about a different frame in the current thread, the current frame
> @@ -93,7 +93,7 @@ mi_gdb_test "thread" \
> "info thread 6"
>
> mi_gdb_test "frame" \
> - ".*#0 0x.*" \
> + ".*#0 .*child_sub_function.*" \
> "frame 2"
>
>
> @@ -108,7 +108,7 @@ mi_gdb_test "thread" \
> "info thread 7"
>
> mi_gdb_test "frame" \
> - ".*#0 0x.*" \
> + ".*#0 .*child_sub_function.*" \
> "frame 3"
>
> # Select a different frame in the current thread. Despite the use of
> @@ -123,7 +123,7 @@ mi_gdb_test "thread" \
> "info thread 8"
>
> mi_gdb_test "frame" \
> - ".*#1 0x.*" \
> + ".*#1 .*child_function.*" \
> "frame 4"
>
> # Similar to the previous test, but this time the --frame option is
More information about the Gdb-patches
mailing list