From patchwork Tue Oct 23 09:48:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 29847 Received: (qmail 45623 invoked by alias); 23 Oct 2018 09:49:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 24824 invoked by uid 89); 23 Oct 2018 09:49:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.3 required=5.0 tests=AWL, BAYES_50, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS autolearn=ham version=3.3.2 spammy=UD:p.m, p.m, owns, am X-HELO: sessmg23.ericsson.net Received: from sessmg23.ericsson.net (HELO sessmg23.ericsson.net) (193.180.251.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Oct 2018 09:49:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1540288131; x=1542880131; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=dh2BCiccmHiuB75prgKqwdErureY0l5fD8kf7zajL1s=; b=MjFDHmctYkjQpQ/lC+EjoqvPDRIcqeH/Nnjl1R2DJstc28ux3nRNzpoznlgXBpwb /RNRHDXgTaM7qg/2hBGikcp7TiI04CM+9wceW1q0J9O7pN9bK82tyufUPMlD+mVP ilPyAepI6YJFqizVVRjuzrSkF/ONuFtu3VKXZoV8ju0=; Received: from ESESSMB503.ericsson.se (Unknown_Domain [153.88.183.121]) by sessmg23.ericsson.net (Symantec Mail Security) with SMTP id C1.B4.14887.38EEECB5; Tue, 23 Oct 2018 11:48:51 +0200 (CEST) Received: from ESESSMB503.ericsson.se (153.88.183.164) by ESESSMB503.ericsson.se (153.88.183.164) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Tue, 23 Oct 2018 11:48:51 +0200 Received: from NAM05-CO1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESSMB503.ericsson.se (153.88.183.164) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Tue, 23 Oct 2018 11:48:51 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=dh2BCiccmHiuB75prgKqwdErureY0l5fD8kf7zajL1s=; b=V4RHRWAsByxI4Mvipb8PDoexCWpS4Xro0QhQYSh/T137YiTFerxTRB8z9tso3pC197JFw6DWn4gm+rcNYKbZfLv1sszJCj7yLsOf3XdcHeOMsXnZhGzLoWkxb43d4YXOwCokM0JItwdgQh8ngp/RUkr654A8a6JJQqcyutIm0R8= Received: from BYAPR15MB2390.namprd15.prod.outlook.com (52.135.198.30) by BYAPR15MB2326.namprd15.prod.outlook.com (52.135.197.151) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1250.30; Tue, 23 Oct 2018 09:48:47 +0000 Received: from BYAPR15MB2390.namprd15.prod.outlook.com ([fe80::619e:2dbd:180c:a2fb]) by BYAPR15MB2390.namprd15.prod.outlook.com ([fe80::619e:2dbd:180c:a2fb%5]) with mapi id 15.20.1228.035; Tue, 23 Oct 2018 09:48:47 +0000 From: Simon Marchi To: Pedro Alves , Simon Marchi , "gdb-patches@sourceware.org" Subject: Re: [PATCH 3/3] Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368) Date: Tue, 23 Oct 2018 09:48:47 +0000 Message-ID: References: <20181016033835.17594-1-simon.marchi@polymtl.ca> <20181016033835.17594-3-simon.marchi@polymtl.ca> <37bde004-853a-3ccc-3777-03cc43b36147@redhat.com> <5375d6a4-8b4d-af3a-2b37-f0b88363d5d3@polymtl.ca> <9e8dd186-86bd-5a6a-16a1-5aab96725a7b@redhat.com> In-Reply-To: <9e8dd186-86bd-5a6a-16a1-5aab96725a7b@redhat.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; received-spf: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) Content-ID: <82B1F56BC9BE964C840497B310AA3CEC@namprd15.prod.outlook.com> MIME-Version: 1.0 Return-Path: simon.marchi@ericsson.com X-IsSubscribed: yes On 2018-10-22 4:56 p.m., Pedro Alves wrote: > On 10/20/2018 04:38 PM, Simon Marchi wrote: >> On 2018-10-20 11:13 a.m., Simon Marchi wrote: >>> On 2018-10-17 1:04 p.m., Pedro Alves wrote: >>>> I think the "for both inferiors" part is what's dubious here. >>>> >>>> The process lives on in the new inferior, but we lost its >>>> terminal settings! Seems to me that they should be migrated >>>> from the old inferior to the new one. And then the problem >>>> sorts itself out, because then the new inferior will have >>>> target_terminal_state::is_inferior state. >>> >>> Yeah that makes sense. This crossed my mind when I look at the issue, >>> but for some reason I didn't went that route. But now that you say it, >>> it appears obvious that this should be the fix. >>> >>> I just tried copying the inferior::terminal_state value from the old >>> inferior to the new inferior, and it fixes the problem. But >>> actually, we should also be transferring all of the struct terminal_info >>> associated to the inferior, is that right? >> >> This would be the updated patch (testing on the buildbot at the moment). > > Yes. Looks good. I think you could avoid the > copying-with-garanteed-deleting by adding a "swap_terminal_info" function > that swaps the terminal_info and terminal_state between the inferiors. Like this? (commit message and ChangeLog to be updated) From a43edf2aada8d5312370429a4d7fad7cfb1ec85c Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sat, 20 Oct 2018 10:57:33 -0400 Subject: [PATCH] Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368) Here's a summary of PR 23368: #include int main (void) { char *exec_args[] = { "/bin/ls", NULL }; execve (exec_args[0], exec_args, NULL); } $ gdb -nx t -ex "catch exec" -ex "set follow-exec-mode new" -ex run ... [1] + 13146 suspended (tty output) gdb -q -nx t -ex "catch exec" -ex "set follow-exec-mode new" -ex run $ Here's what happens: when the inferior execs with "follow-exec-mode new", we first "mourn" it before creating the new one. This ends up calling inflow_inferior_exit, which sets the per-inferior terminal state to "is_ours": inf->terminal_state = target_terminal_state::is_ours; At this point, the inferior's terminal_state is is_ours, while the "reality", tracked by gdb_tty_state, is is_inferior (GDB doesn't own the terminal). Later, we continue processing the exec inferior event and decide we want to stop (because of the "catch exec") and call target_terminal::ours to make sure we own the terminal. However, we don't actually go to the target backend to change the settings, because the core thinks that no inferior owns the terminal (inf->terminal_state is target_terminal_state::is_ours, as checked in target_terminal_is_ours_kind, for both inferiors). When something in readline tries to mess with the terminal settings, it generates a SIGTTOU. This patch fixes this by tranferring the state of the terminal from the old inferior to the new inferior. gdb/ChangeLog: * infrun.c (follow_exec): In the follow_exec_mode_new case, transfer terminal state from old new new inferior. --- gdb/inflow.c | 16 ++++++++++++++++ gdb/infrun.c | 9 +++++---- gdb/terminal.h | 3 +++ 3 files changed, 24 insertions(+), 4 deletions(-) -- 2.19.1 diff --git a/gdb/inflow.c b/gdb/inflow.c index e355f4aa9fc..d673bef8b43 100644 --- a/gdb/inflow.c +++ b/gdb/inflow.c @@ -700,6 +700,22 @@ copy_terminal_info (struct inferior *to, struct inferior *from) to->terminal_state = from->terminal_state; } +/* See terminal.h. */ + +void +swap_terminal_info (inferior *a, inferior *b) +{ + terminal_info *info_a + = (struct terminal_info *) inferior_data (a, inflow_inferior_data); + terminal_info *info_b + = (struct terminal_info *) inferior_data (a, inflow_inferior_data); + + set_inferior_data (a, inflow_inferior_data, info_b); + set_inferior_data (b, inflow_inferior_data, info_a); + + std::swap (a->terminal_state, b->terminal_state); +} + void info_terminal_command (const char *arg, int from_tty) { diff --git a/gdb/infrun.c b/gdb/infrun.c index 72e24961767..cedaaecd91a 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1189,12 +1189,13 @@ follow_exec (ptid_t ptid, char *exec_file_target) /* The user wants to keep the old inferior and program spaces around. Create a new fresh one, and switch to it. */ - /* Do exit processing for the original inferior before adding - the new inferior so we don't have two active inferiors with - the same ptid, which can confuse find_inferior_ptid. */ + /* Do exit processing for the original inferior before setting the new + inferior's pid. Having two inferiors with the same pid would confuse + find_inferior_p(t)id. */ + inf = add_inferior_with_spaces (); + swap_terminal_info (inf, current_inferior ()); exit_inferior_silent (current_inferior ()); - inf = add_inferior_with_spaces (); inf->pid = pid; target_follow_exec (inf, exec_file_target); diff --git a/gdb/terminal.h b/gdb/terminal.h index 7774eefcd46..da27e46f6ec 100644 --- a/gdb/terminal.h +++ b/gdb/terminal.h @@ -29,6 +29,9 @@ extern void new_tty_postfork (void); extern void copy_terminal_info (struct inferior *to, struct inferior *from); +/* Exchange the terminal info and state between inferiors A and B. */ +extern void swap_terminal_info (inferior *a, inferior *b); + extern pid_t create_tty_session (void); /* Set up a serial structure describing standard input. In inflow.c. */