This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH master+7.12 v2 3/3] Add test for user context selection sync
On 2016-09-14 15:31, Pedro Alves wrote:
Hi Simon,
I didn't try to understand everything in detail, but overall
it looks very nice now. Thank you very much. A few comments below.
Oops, while trying to rerun the full testsuite, I noticed that it leaves
"-ex \"set non-stop 1\"" in the GDBFLAGS... that's not good. You can
see it easily if you swap the order of the "all-stop" and "non-stop"
variations. Can you spot why the save_vars isn't working as intended?
When I use a temporary variable instead, it works as intended...
On 09/14/2016 06:45 PM, Simon Marchi wrote:
From: Antoine Tremblay <antoine.tremblay@ericsson.com>
This patch adds a test to verify that events are sent properly to all
UIs when the user selection context (inferior, thread, frame) changes.
The goal of the C test file is to provide two threads that are
interrupted with the same predictable backtrace (so that we can test
frame switching). This is achieved by having them loop on a single
line, such that when the main thread hits a breakpoint, they are both
stopped that line. It would not be practical to have the threads
sleep,
since their backtraces would not be predictable (the functions that
implement sleep may vary between systems).
There is a 1 second sleep in the main thread to make sure the threads
have time to spawn and reach the loop. If you can find a way that is
sleep-free and race-free to achieve the same result, it would be
really
nice, as most of the time taken by the test is spent sleeping.
Did you try using barriers and breakpoints? Several tests use that
to make sure threads are past a point.
I tried, but the issue is that depending on the scheduling, the threads
might still be in the pthread_barrier_wait function when you stop.
Consider this pseudo-code:
thread_function:
barrier_wait
infinite_loop # This is where I want the threads to be stopped.
main_thread ():
initialize_barrier with n = 3
spawn_thread (thread_function)
spawn_thread (thread_function)
barrier_wait
breakpoint
Once the main thread hits the breakpoint, we have the assurance that the
threads have started, but we don't know where they are. They might not
have exited the barrier_wait function, or they might be in the infinite
loop. Adding a sleep before the breakpoint in the main thread is the
only way to be reasonnably sure (assuming 1 second is enough...) that
both threads will have reached the infinite loop.
Actually, it might work by putting thread-specific breakpoints on the
single-line infinite loop, then doing two "continue". This way I think
we would be guaranteed that the two threads stop exactly at that line.
With a regular breakpoint it might not work, since a thread could hit
the breakpoint twice while the other still hasn't reached it.
+int
+main (void)
+{
+ int i = 0;
+ pthread_t threads[NUM_THREADS];
+
+ for (i = 0; i < NUM_THREADS; i++)
+ pthread_create (&threads[i], NULL, child_function, NULL);
+
+ /* Leave enough time for the threads to reach their infinite loop.
*/
+ sleep (1);
+
+ i = 0; /* main break line */
+
+ sleep (2);
+
+ /* Allow the test to exit cleanly. */
+ quit = 1;
+
+ for (i = 0; i < NUM_THREADS; i++)
+ pthread_join (threads[i], NULL);
Hmm, looks like this version of the test still runs forever.
I don't think so, the main thread sets the quit flag which unblocks the
threads. If you run the executable you'll see it exits in ~2 seconds.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
<http://www.gnu.org/licenses/>.
+
+# This test checks that thread, select-frame, frame or inferior
selection
+# events are properly sent to all uis.
This comment is talking about "select-frame", which I take is
the CLI command's name, not a selection event. It feels like the
comment
could be tweaked to be a bit clearer. Something like:
# This test checks that the "thread", "select-frame", "frame" and
"inferior"
# commands send the appropriate user-selection-change events to all
UIs.
Maybe say something about MI commands too, if the test exercises them.
Good idea. It became this:
# This test checks that the "thread", "select-frame", "frame" and
"inferior"
# CLI commands, as well as the "-thread-select" and
"-stack-select-frame" MI
# commands send the appropriate user-selection-change events to all UIs.
+#
+# This test considers the case where console and mi are two different
uis
+# and mi is created with the new-ui command.
+#
+# It also considers the case where the console commands are sent
directly in
+# the mi channel as described in PR 20487.
Could you do a quick skim over the test and uppercase "mi" and "ui",
as in "MI", and "UIs". IMO, that makes the comments easier to grok.
I think that too, I changed them when I saw them, but I must've missed
some. I think I got them all now. I changed those in the test messages
too.
+# Continue inferior INF until the breakpoint indicating the threads
are started.
+
+proc test_continue_to_start { mode inf } {
+ global gdb_prompt gdb_spawn_id gdb_main_spawn_id
+
+ if { $gdb_spawn_id != $gdb_main_spawn_id } {
+ error "This should not happen."
+ }
+
+ with_test_prefix "inferior $inf" {
+ with_spawn_id $gdb_main_spawn_id {
+ gdb_continue_to_breakpoint "main breakpoint"
+
+ if { $mode == "non-stop" } {
+ gdb_test "thread $inf.2" ".*" "switch to thread $inf.2"
+
+ send_gdb "interrupt\n"
+ gdb_expect {
gdb_test_multiple ?
Like this?
set test "interrupt thread $inf.2"
send_gdb "interrupt\n"
gdb_test_multiple "" $test {
-re "Thread.*2.*stopped" {
pass $test
}
}
+
+# Match a regular expression, or ensure that there was no output.
+#
+# If RE is non-empty, try to match the content of the program output
(using the
+# current spawn_id) and pass/fail TEST accordingly.
+# If RE is empty, ensure that the program did not output anything.
+
+proc match_re_or_ensure_not_output { re test } {
+ if { $re != "" } {
+ gdb_expect {
gdb_test_multiple? Or maybe this is used by MI too?
Indeed it it used by both.
+ with_test_prefix "thread 1.2 with --thread" {
+ # Test selecting a thread from MI with a --thread option. This test
+ # verifies that even if the thread GDB would switch to is the same
has
+ # the thread specified with --thread, an event is still sent to CLI.
+ # In this case this is thread 1.2
+
+ set mi_re [make_mi_re $mode 2 0 response]
+ set cli_re [make_cli_re $mode -1 1.2 0]
+
+ with_spawn_id $mi_spawn_id {
+ mi_gdb_test "-thread-select --thread 2 2" $mi_re
"-thread-select"
+ }
+
+ with_spawn_id $gdb_main_spawn_id {
+ # TODO: it doesn't work as of now.
+ # match_re_or_ensure_not_output "$cli_re\r\n" "-thread-select,
event on cli"
+ }
Is there a plan here?
I think that will go in the same basket as the fact that any MI command
with --thread currently changes the selected thread silently (without
any =thread-selected event). Currently, --thread changes the thread tot
he desired one, then when the mi_cmd_thread_select tries to change the
thread, it thinks that it was already the current thread, so that an
event isn't necessary. This should get fixed in the next iteration,
when we split the concepts of user-selected-ptid and
internally-selected-ptid. Specifying --thread won't mess with the
user-selected-ptid, but if you do "-thread-select --thread 2 2", then
mi_cmd_thread_select will change the user-selected-ptid, generating an
event.
It's not pretty to leave it like this in the test though. Should I
create a bug right now and kfail it? Leave it commented out but put a
better description?
Thanks,
Simon