From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 34704 invoked by alias); 14 Sep 2016 19:31:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 34691 invoked by uid 89); 14 Sep 2016 19:31:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=forever, Several X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 14 Sep 2016 19:31:29 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C01D97D0E9; Wed, 14 Sep 2016 19:31:28 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8EJVReV013539; Wed, 14 Sep 2016 15:31:27 -0400 Subject: Re: [PATCH master+7.12 v2 3/3] Add test for user context selection sync To: Simon Marchi , gdb-patches@sourceware.org References: <20160914174548.5873-1-simon.marchi@ericsson.com> <20160914174548.5873-4-simon.marchi@ericsson.com> Cc: Antoine Tremblay From: Pedro Alves Message-ID: Date: Wed, 14 Sep 2016 19:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160914174548.5873-4-simon.marchi@ericsson.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-09/txt/msg00128.txt.bz2 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. On 09/14/2016 06:45 PM, Simon Marchi wrote: > From: Antoine Tremblay > > 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. > +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. e for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# 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. > +# > +# 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. > +# 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 ? > + -re "Thread.*2.*stopped" { > + pass "interrupt thread $inf.2" > + } > + } > + } > + } > + } > +} > + > +# 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? > + -re "$re" { > + pass $test > + } > + > + default { > + fail $test > + } > + } > + } else { > + ensure_no_output $test > + } > +} > + 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? -- Pedro Alves