From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53564 invoked by alias); 20 Oct 2018 15:14:16 -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 47797 invoked by uid 89); 20 Oct 2018 15:14:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=transferring, ours, resume, reality X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 20 Oct 2018 15:14:05 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w9KFDs3Z023094 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 20 Oct 2018 11:13:59 -0400 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id CD7C81E514; Sat, 20 Oct 2018 11:13:53 -0400 (EDT) Subject: Re: [PATCH 3/3] Avoid GDB SIGTTOU on catch exec + set follow-exec-mode new (PR 23368) To: Pedro Alves , gdb-patches@sourceware.org References: <20181016033835.17594-1-simon.marchi@polymtl.ca> <20181016033835.17594-3-simon.marchi@polymtl.ca> <37bde004-853a-3ccc-3777-03cc43b36147@redhat.com> From: Simon Marchi Message-ID: Date: Sat, 20 Oct 2018 15:14:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <37bde004-853a-3ccc-3777-03cc43b36147@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg00448.txt.bz2 On 2018-10-17 1:04 p.m., Pedro Alves wrote: > On 10/16/2018 04:38 AM, Simon Marchi wrote: >> 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). > ^^^^^^^^^^^^^^^^^^ > > 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? > >> When something in >> readline tries to mess with the terminal settings, it generates a >> SIGTTOU. >> >> This patch manages to fix this particular case by calling >> target_terminal::ours() in inflow_inferior_exit. This makes so that >> inflow actually changes the terminal settings so that GDB owns it, which >> avoids the SIGTTOU later. >> >> The buildbot doesn't complain, but I am not sure this is the most >> bestest way to fix this, maybe it's just papering over the actual >> problem or introduces some latent bug. In particular, I am not sure if >> this is correct in case we have multiple inferiors sharing the same >> terminal. But I thought I would still submit this patch to get the ball >> rolling. > > Yeah, I don't think this is right in the multi-inferior (or multi-target) > case. It may happen to just work because we always stop everything > when an inferior exits. In the exec case, if you don't catch the exec, > then we won't stop; likely it ends up working because we end up calling > target_terminal::inferior() on resume. Still, I think this is > a hack that would likely cause trouble down the road. Ok. Thanks, Simon