From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55022 invoked by alias); 3 May 2016 21:57:37 -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 55004 invoked by uid 89); 3 May 2016 21:57:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=states 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 03 May 2016 21:57:35 +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 7B24BC05E174; Tue, 3 May 2016 21:57:34 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u43LvXiW011720; Tue, 3 May 2016 17:57:33 -0400 Subject: Re: [PATCH] Add mi-threads-interrupt.exp test (PR 20039) To: Simon Marchi , gdb-patches@sourceware.org References: <1462305612-16493-1-git-send-email-simon.marchi@ericsson.com> From: Pedro Alves Message-ID: <5983d4d2-016a-8020-c109-cb7ea2cfd179@redhat.com> Date: Tue, 03 May 2016 21:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1462305612-16493-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-05/txt/msg00047.txt.bz2 On 05/03/2016 09:00 PM, Simon Marchi wrote: > Add a new test for PR 20039. The test spawns new threads, then tries to > interrupt, continue, and interrupt again. This use case was fixed by > commit 5fe966540d6b748f825774868463003700f0c878 in master, but gdb 7.11 > is affected (so if you try it on the gdb-7.11-branch right now, the test > will fail). Thanks for the test! I debugged this a little, and I see that the bug is that -exec-continue ends up with thread 3 selected, which generates a =thread-selected event. -exec-continue ^running *running,thread-id="all" (gdb) =thread-selected,id="3" And then the code that emits that =thread-selected calls target_terminal_ours, and doesn't restore the terminal with target_terminal_inferior: if (report_change) { struct thread_info *ti = inferior_thread (); target_terminal_ours (); fprintf_unfiltered (mi->event_channel, "thread-selected,id=\"%d\"", ti->global_num); gdb_flush (mi->event_channel); } We'll end up calling target_terminal_inferior when we next re-resume the inferior after some internal event, but if no such event ever happens, the target will remain running free with target_terminal_ours in effect... So two bugs: calling target_terminal_ours instead of target_terminal_ours_for_output, and not restoring the target terminal, both addressed in 5fe966540d6b (Use target_terminal_ours_for_output in MI): --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -2171,12 +2171,17 @@ mi_execute_command (const char *cmd, int from_tty) if (report_change) { struct thread_info *ti = inferior_thread (); + struct cleanup *old_chain; + + old_chain = make_cleanup_restore_target_terminal (); + target_terminal_ours_for_output (); - target_terminal_ours (); fprintf_unfiltered (mi->event_channel, "thread-selected,id=\"%d\"", ti->global_num); gdb_flush (mi->event_channel); + + do_cleanups (old_chain); } > + # Load the binary in gdb and run it. > + mi_gdb_load $binfile > + mi_runto "all_threads_created" I think we could add a comment saying: # Note this test relies on mi_runto deleting the breakpoint. # A step-over here would mask the bug. > + > + # Consistency check. > + mi_check_thread_states {"stopped" "stopped" "stopped"} "check thread states" > + > + # Continue. > + mi_gdb_test "565-exec-continue" "565\\^running\r\n\\*running,thread-id=\"all\"" "continue #1" > + > + # Send ctrl-C > + send_gdb "\003" I guess you didn't add a match for =thread-selected because that detail may change in future. I agree with that. However, I think it'll good to wait a second or some such before sending the ctrl-C, to make sure all such events were indeed output. Otherwise, if the machine is fast enough, we may end up sending the ctrl-C before the =thread-selected event is reached. > + mi_expect_interrupt "interrupt #1" > + > + # Continue. > + mi_gdb_test "565-exec-continue" "565\\^running\r\n\\*running,thread-id=\"all\"" "continue #2" > + > + # Send ctrl-C again. > + send_gdb "\003" Ditto. > + mi_expect_interrupt "interrupt #2" > +} AFAICS, the test relies on "set mi-async off". Could you make sure that if you run it against a board file that forces that on, the test either passes (probably with -exec-interrupt in async mode) or is skipped? See mi_detect_async and the async global. Thanks, Pedro Alves