* [PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
@ 2022-04-04 16:08 Simon Marchi
2022-04-05 7:56 ` Tom de Vries
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2022-04-04 16:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Vrany, Tom de Vries, Simon Marchi
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.
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
--
2.35.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
2022-04-04 16:08 [PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp Simon Marchi
@ 2022-04-05 7:56 ` Tom de Vries
2022-04-05 12:13 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2022-04-05 7:56 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Jan Vrany
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp
2022-04-05 7:56 ` Tom de Vries
@ 2022-04-05 12:13 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2022-04-05 12:13 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Jan Vrany
> 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, pushed to both branches.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-05 12:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 16:08 [PATCH v2] gdb/testsuite: fix intermittent failures in gdb.mi/mi-cmd-user-context.exp Simon Marchi
2022-04-05 7:56 ` Tom de Vries
2022-04-05 12:13 ` Simon Marchi
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).