From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38680 invoked by alias); 2 Jun 2015 15:37:43 -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 38670 invoked by uid 89); 2 Jun 2015 15:37:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-ob0-f180.google.com Received: from mail-ob0-f180.google.com (HELO mail-ob0-f180.google.com) (209.85.214.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 02 Jun 2015 15:37:41 +0000 Received: by obbea2 with SMTP id ea2so130683216obb.3 for ; Tue, 02 Jun 2015 08:37:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=t8BZ6SQess5/NF0IZjNx+867ZyxrEAOv32xRF+e/MpY=; b=SzYoe+BBipAYFpMSA1eO3qabfAgMoEmjHIk9w1etUaCPimJBvWahZnwOmsPTH5hx0s p2G3xhoca/sedARobSts6mQNFGDZatBGPkHVZCrqxUT0zQE7UhOcIoMwhaICj/K0BpAp nHWWkMaEA/mEQCUa3xBA4vYzOo1iQsiXrA4DKa/FFkA5eZuUT1+6XCIe9ZFFDNkf6j4H tlzbniVLJyhq32CpnCakaMJKH74lXSBtOAkXTwkjHOOIDKuwjArdS7iOlFH+pjxAGpXT bg2pxj04Ro/ecJ2PGXmBCP8WarfCz+xJt+4qDPKb2ptkU5un37QSNTUN+3V/JNcL6+jy RqfA== X-Gm-Message-State: ALoCoQmzn+GFI3c0T1qwH7OfBV6C2lAH6PNPAvNwtjUj1s7oHxba0RWN5PL1f3GsZ4ME1QdDVDdD X-Received: by 10.182.211.66 with SMTP id na2mr23291457obc.43.1433259459280; Tue, 02 Jun 2015 08:37:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.96.167 with HTTP; Tue, 2 Jun 2015 08:37:18 -0700 (PDT) In-Reply-To: <20150602141121.GP17330@embecosm.com> References: <1433210261-18328-1-git-send-email-patrick@parcs.ath.cx> <20150602141121.GP17330@embecosm.com> From: Patrick Palka Date: Tue, 02 Jun 2015 15:37:00 -0000 Message-ID: Subject: Re: [PATCH] Call target_terminal_ours() before refreshing TUI's frame info To: Andrew Burgess Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-06/txt/msg00027.txt.bz2 On Tue, Jun 2, 2015 at 10:11 AM, Andrew Burgess wrote: > * Patrick Palka [2015-06-01 21:57:41 -0400]: > >> Here is an example backtrace for when tui_show_frame_info() gets called >> while target_terminal_is_inferior() == 1: >> >> #1 0x00000000004988ee in tui_selected_frame_level_changed_hook (level=0) >> #2 0x0000000000617b99 in select_frame (fi=0x18c9820) >> #3 0x0000000000617c3f in get_selected_frame (message=message@entry=0x0) >> #4 0x00000000004ce534 in update_watchpoint (b=b@entry=0x2d9a760, >> reparse=reparse@entry=0) >> #5 0x00000000004d625e in insert_breakpoints () >> #6 0x0000000000531cfe in keep_going (ecs=ecs@entry=0x7ffea7884ac0) >> #7 0x00000000005326d7 in process_event_stop_test (ecs=ecs@entry=0x7ffea7884ac0) >> #8 0x000000000053596e in handle_inferior_event_1 (ecs=0x7ffea7884ac0) >> >> The fix is simple: call target_terminal_ours() before calling >> tui_show_frame_info() in TUI's frame-changed hook. > > If we assume that somewhere down the call stack the terminal is > supposed to belong to the inferior, then does your patch not leave gdb > in the wrong state, where the terminal now belongs to gdb? Good point -- I can confirm that this actually happens with the patch. do_target_resume() first calls target_terminal_inferior(), and the TUI hook gets called later in target_resume()... I should use make_cleanup_restore_target_terminal() instead. > > It feels like the terminal should probably have been claimed back by > gdb as soon as the inferior stopped, and should only be passed back to > the inferior just before it is resumed. That makes a lot of sense to me. I can see no other reason for the inferior's terminal settings to be in effect while GDB has control if not because it is going to get resumed shortly. But in the meantime I will change the patch to use make_cleanup_restore_target_terminal() in time for GDB 7.10. > > As an experiment, I replaced your call to target_terminal_ours with: > gdb_assert (!target_terminal_is_inferior ()); > in tui_selected_frame_level_changed_hook, this triggers frequently, > for example: > file ~/hello.exe > tui enable > start It seems that in a few code paths, target_terminal_ours() is called in anticipation of writing to stdout. Though the coverage is patchy to say the least. Thanks for reviewing. Will send v2 in a bit. > > Thanks, > Andrew > >> >> gdb/ChangeLog: >> >> * tui/tui-hooks.c (tui_selected_frame_level_changed_hook): Call >> target_terminal_ours() before calling tui_show_frame_info(). >> --- >> gdb/tui/tui-hooks.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c >> index e53f526..1eb5300 100644 >> --- a/gdb/tui/tui-hooks.c >> +++ b/gdb/tui/tui-hooks.c >> @@ -132,6 +132,8 @@ tui_selected_frame_level_changed_hook (int level) >> if (level < 0) >> return; >> >> + target_terminal_ours (); >> + >> fi = get_selected_frame (NULL); >> /* Ensure that symbols for this frame are read in. Also, determine >> the source language of this frame, and switch to it if >> -- >> 2.4.2.387.gf86f31a.dirty >>