From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119088 invoked by alias); 18 Jan 2018 15:57:31 -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 119039 invoked by uid 89); 18 Jan 2018 15:57:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=heard 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; Thu, 18 Jan 2018 15:57:29 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2F5C949026; Thu, 18 Jan 2018 15:57:28 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 911A978E7D; Thu, 18 Jan 2018 15:57:27 +0000 (UTC) From: Pedro Alves Subject: Re: [PATCH 1/3] Remove args from target detach To: Simon Marchi , gdb-patches@sourceware.org References: <1514699454-18587-1-git-send-email-simon.marchi@ericsson.com> Message-ID: <14e51528-6323-a8b1-43ab-534095ea19d1@redhat.com> Date: Thu, 18 Jan 2018 15:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1514699454-18587-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-01/txt/msg00373.txt.bz2 On 12/31/2017 05:50 AM, Simon Marchi wrote: > I was looking into adding a parameter to target_detach, and was > wondering what the args parameter was. It seems like in the distant > past, it was possible to specify a signal number when detaching. That > signal was injected in the process before it was detached. There is an > example of code handling this in linux_nat_detach. With today's GDB, I > can't get this to work. Doing "detach 15" (15 == SIGTERM) doesn't work, > because detach is a prefix command and doesn't recognize the sub-command > 15. Doing "detach inferiors 15" doesn't work because it expects a list > of inferior id to detach. Therefore, I don't think there's a way of > invoking detach_command with a non-NULL args. I also didn't find any > documentation related to this feature. > > I assume that this feature stopped working when detach was made a prefix > command, which is in f73adfeb8bae36885e6ea248d12223ab0d5eb9cb (sorry, > there's no commit title) from 2006. Given that this feature was broken > for such a long time and we haven't heard anything (AFAIK, I did not > find any related bug), I think it's safe to remove it, as well as the > args parameter to target_detach. If someone wants to re-introduce it, I > would suggest rethinking the user interface, and in particular would > suggest using signal name instead of numbers. > > I tried to fix all the impacted code, but I might have forgotten some > spots. It shouldn't be hard to fix if that's the case. I also couldn't > build-test everything I changed, especially the nto and solaris stuff. > Eh. I knew about "detach SIG", from touching the detach-related code many times before. Had no idea that it was broken. I think the main use cases are/were: - To be able to detach without passing the just-intercepted signal to the program. I.e., "detach 0" was to "detach", like "signal 0" is to "continue". - To be able to attach to a program (maybe job-stopped), debug it a bit, and then detach it, leaving it job-stopped. I.e., queue a SIGSTOP when you detach, with "detach SIGTOP". I think you can do the former today with either: "handle SIGFOO nopass" + "detach" "queue-signal 0" + "detach" and the latter with either: "queue-signal SIGSTOP" + "detach" "shell kill -SIGSTOP $PID" + "detach" The "shell kill" variant is a bit more cumbersome, of course, and only works with native debugging. We're probably missing testcases for the above scenarios. IWBN to add them, of course. The patch looks fine to me. One nit: > static void > -procfs_detach (struct target_ops *ops, const char *args, int from_tty) > +procfs_detach (struct target_ops *ops, int from_tty) > { > - int sig = 0; > int pid = ptid_get_pid (inferior_ptid); > > - if (args) > - sig = atoi (args); > - > if (from_tty) > { > const char *exec_file; > @@ -1945,7 +1941,7 @@ procfs_detach (struct target_ops *ops, const char *args, int from_tty) > gdb_flush (gdb_stdout); > } > > - do_detach (sig); > + do_detach (0); I think the 'signo' parameter of do_detach is now always 0, so it could be removed. I understand that you might want to avoid breaking the port (too much), but it looks very easy to remove. > > inferior_ptid = null_ptid; > detach_inferior (pid); Thanks, Pedro Alves