From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46694 invoked by alias); 1 Nov 2018 21:46:12 -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 46584 invoked by uid 89); 1 Nov 2018 21:46:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=HX-Received:sk:u9-v6mr, flexibility, H*c:alternative X-HELO: mail-yb1-f171.google.com Received: from mail-yb1-f171.google.com (HELO mail-yb1-f171.google.com) (209.85.219.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Nov 2018 21:46:01 +0000 Received: by mail-yb1-f171.google.com with SMTP id n140-v6so8769443yba.1 for ; Thu, 01 Nov 2018 14:46:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Mr6stBZjQuIyUHDgoRGmLhMI7BcidSZpMihaH+zh8Fs=; b=BRCVwe4UFbyNWB+NcWpOkyrDhEHohwcRM9kGiEQwjlayytdBfXf83Xzn3QilN4StT8 8lNpxLWRfxyzhVYOh9j/hF/oNHLMaMsjHRBfMqeP5j0sQXhH4om0zSeoQdSrzq01aGjc p+Q2na3U/1GHDi2nxY4ZOUC7RPl2OzmdrTi25YUJAMuXdsfiW5wkCsZF5lFu2rJuGJDS RtyzLGuqRKt6exRVc0XfiNIoqAT6DXTpQNHlTh046N3tNtqZLRsVpU0Ck14YvVzKAIS9 k+NuknlJ09mBX3MAV1ejhrp0TnFdTx5JHVOfWWkxj8ayBRMKxPmJQIlwHq+F8r9XglfK QWeQ== MIME-Version: 1.0 References: <20181101211949.GB2705@adacore.com> In-Reply-To: <20181101211949.GB2705@adacore.com> From: Brian Vandenberg Date: Thu, 01 Nov 2018 21:46:00 -0000 Message-ID: Subject: Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris To: brobecker@adacore.com Cc: gdb-patches@sourceware.org, ro@cebitec.uni-bielefeld.de Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-11/txt/msg00009.txt.bz2 Greetings, Did you run the testsuite before and after the patch, by any chance? Nope. In my work environment I don't have much flexibility on getting/installing software. If I run the test suite I would probably have to setup an IllumOS VM at home to run it, but that'd be x86 not SPARC. For multiline comments like the above, we do not repeat the '*' > at the beginning of each line. > /* PR gdb/8527: Was not correctly interrupting the inferior process > when ^C was pressed in the debug terminal. */ > And if I may, reading this sentence, it's a bit hard to understand > what the comment is trying to explain. The following might be > a little more precise: > /* PR gdb/8527: Call set_sigint_trap to make sure that a ctrl-c > pressed in the debugger terminal gets passed down to the > inferior, thus causing it to be interrupted. */ I've no qualms with those changes. Thanks for your feedback. -brian On Thu, Nov 1, 2018 at 3:19 PM Joel Brobecker wrote: > Hi Brian, > > > In Solaris: > > > > If gdb attaches to a process that either has no controlling terminal, > > or the controlling terminal differs from the one gdb is running under, > > break/^C doesn't interrupt the debugged process. > > > > This is a fix that was proposed for this problem quite awhile ago but > > never implemented; it's been in the Adacore GDB branch for quite > > awhile. > > > > Without going into unnecessary details I cannot easily run the test > > suite against this change right now. If this patch gets rejected > > based on that, when I have time I'll see about getting IllumOS > > installed in a VM and test it there, but the problem was originally > > found in sparc Solaris. > > > > ---- > > > > note: this patch was tested against 8.1.1. It looks like it probably > > still applies on the 8.2 branch, but I won't be able to test it until > > 8.2 is released. > > > > -brian > > > > ps, my assignment/release forms were completed/received 10/30/2017 > > > gdb/Changelog: > > 2018-08-07 Brian Vandenberg > > > > PR gdb/8527 > > * procfs.c (proc_wait_for_stop): calls to set_sigint_trap and > > clear_sigint_trap. > > I'm not sure anyone took the time to answer this message; if not, > I apologize. > > I can testify that, for as long as we got the patch in the AdaCore > sources, we never noticed any ill effect. We never got to validate > it against the official testsuite, unfortunately, because for some > reason, when I did so, our Solaris machines would badly crash. Did > you run the testsuite before and after the patch, by any chance? > > Rainer (in Cc:) is our maintainer for Solaris, so he may have an opinion. > > In the meantime, I have a trivial coding style comment: > > > > > diff --git a/gdb/procfs.c b/gdb/procfs.c > > index 7b7ff45..7cd870c 100644 > > --- a/gdb/procfs.c > > +++ b/gdb/procfs.c > > @@ -913,7 +913,12 @@ proc_wait_for_stop (procinfo *pi) > > > > procfs_ctl_t cmd = PCWSTOP; > > > > + /* PR gdb/8527 > > + * Was not correctly interrupting the inferior process > > + * when ^C was pressed in the debug terminal. > > + */ > > For multiline comments like the above, we do not repeat the '*' > at the beginning of each line. > > /* PR gdb/8527: Was not correctly interrupting the inferior process > when ^C was pressed in the debug terminal. */ > > And if I may, reading this sentence, it's a bit hard to understand > what the comment is trying to explain. The following might be > a little more precise: > > /* PR gdb/8527: Call set_sigint_trap to make sure that a ctrl-c > pressed in the debugger terminal gets passed down to the > inferior, thus causing it to be interrupted. */ > > > + set_sigint_trap(); > > + > > win = (write (pi->ctl_fd, (char *) &cmd, sizeof (cmd)) == sizeof > (cmd)); > > + > > + /* PR gdb/8527 */ > > + clear_sigint_trap(); > > + > > /* We been runnin' and we stopped -- need to update status. */ > > pi->status_valid = 0; > > > -- > Joel >