Re: [PATCH][PR gdb/8527] Interrupt not functional in Eclipse/CDT on Solaris

[Joel Brobecker <brobecker@adacore.com>]

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  <phantall@gmail.com>
> 
> 	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