[RFC] Sync readline to version 6.3 patchlevel 8

  On Mon, 18 May 2015, Patrick Palka wrote:

> On Wed, May 13, 2015 at 9:40 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> On Wed, May 13, 2015 at 8:12 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>>> This patch syncs our upstream copy of readline to version 6.3
>>> patchlevel 8.
>>>
>>> I basically copied what was done when Jan updated to readline 6.2 in
>>> 2011: http://sourceware.org/ml/gdb-patches/2011-05/msg00003.html
>>>
>>> Specifically, I:
>>>
>>> 1. Extracted the readline 6.3 tarball on top of readline/
>>> 2. Applied patches 1-8 from ftp://ftp.cwru.edu/pub/bash/readline-6.3-patches/
>>> 3. Omitted all the files in doc/ that were intentionally omitted before.
>>> 4. Regenerated readline/configure and readline/examples/rlfe/configure
>>>    using autoconf 2.64.  No other configure files needed regenerating.
>>> 5. Reapplied the only local patch since the update to readline 6.2 that
>>>    has not already been applied to readline 6.3, 05942d8a1 ("Fix
>>>    executable indicator in file name completeion on Windows.").  This
>>>    particular patch has been applied upstream but readline 6.3 does not
>>>    have it.  Whether or not a local patch has already been applied to
>>>    readline 6.3 was determined via manual inspection.  (Wasn't too bad
>>>    really.)
>>>
>>> The new files to make it into the tree are:
>>>
>>>     colors.{c,h}
>>>     configure.ac
>>>     parse-colors.{c,h}
>>>     examples/hist_erasedups.c
>>>     examples/hist_purgecmd.c
>>>
>>> Deleted files:
>>>
>>>     configure.in
>>>
>>> I've been using this patch locally for a few months now and I've
>>> experienced only a single regression which has already been preemptively
>>> fixed by 8900d71e3 ("Explicitly call rl_resize_terminal() in TUI's
>>> SIGWINCH handler").  Other than that, no issues in either the CLI or the
>>> TUI, or changes in the testsuite.  Though I have only been able to test
>>> this patch on Linux.
>>
>> On second inspection it seems I am getting a few anomalous testsuite
>> failures.  I will take a deeper look.
>
> So the only regression appears to be
>
> FAIL: gdb.gdb/selftest.exp: send SIGINT signal to child process (timeout)
>
> What happens here is that a stopped inferior GDB is resumed using
> "signal SIGINT" with the expectation that the parent GDB process will
> catch this SIGINT and thus pause the inferior again.  But with
> readline 6.3 the parent GDB process no longer catches the signal
> raised by "signal SIGINT" and so the inferior GDB continues to run
> until the test times out.  This seems to be caused by the fact that
> readline 6.3 makes sure to not have its own signal handlers installed
> when readline is not in control, whereas readline 6.2 always has its
> signal handlers installed.  readline's signal handler basically
> installs the applications original signal handler and then calls via
> raise (signal).  This call to "raise (signal);" in readline's signal
> handler is what's responsible for re-stopping the GDB inferior after
> it was resumed with "signal SIGINT". The parent GDB process does not
> seem to catch the first SIGINT raised by "signal SIGINT" itself
> (regardless of what the inferior is).  So without readline's signal
> handlers installed at the point when the inferior is resumed via
> "signal SIGINT", nothing calls "raise (signal)" later on.  The parent
> GDB process does not catch the initial SIGINT it itself raised and
> there is no longer subsequent SIGINT to catch because readline's
> signal handler, which calls raise(), is no longer permanently
> installed.
>
> It seems it is expected behavior that resuming an inferior via "raise
> SIGINT" does not immediately stop the inferior giving control back to
> GDB since interrupt.exp tests for it.  Therefore when importing
> readline 6.3 I think we should just fix the selftest.exp test to no
> longer expect that the inferior GDB will get stopped following "signal
> SIGINT".  Something like:
>

Oops, here it is:


Subject: [PATCH] Update "signal SIGINT" test

---
  gdb/testsuite/gdb.gdb/selftest.exp | 23 ++++++++++++++++++++---
  1 file changed, 20 insertions(+), 3 deletions(-)
  

Hi Patrick,

On 05/18/2015 12:41 PM, Patrick Palka wrote:

>> So the only regression appears to be
>>
>> FAIL: gdb.gdb/selftest.exp: send SIGINT signal to child process (timeout)
>>

Thanks for investigating this.  I'm looking forward to get the readline
update in.

>> What happens here is that a stopped inferior GDB is resumed using
>> "signal SIGINT" with the expectation that the parent GDB process will
>> catch this SIGINT and thus pause the inferior again.  But with
>> readline 6.3 the parent GDB process no longer catches the signal
>> raised by "signal SIGINT" and so the inferior GDB continues to run
>> until the test times out.  This seems to be caused by the fact that
>> readline 6.3 makes sure to not have its own signal handlers installed
>> when readline is not in control, whereas readline 6.2 always has its
>> signal handlers installed.  readline's signal handler basically
>> installs the applications original signal handler and then calls via
>> raise (signal).  This call to "raise (signal);" in readline's signal
>> handler is what's responsible for re-stopping the GDB inferior after
>> it was resumed with "signal SIGINT". The parent GDB process does not
>> seem to catch the first SIGINT raised by "signal SIGINT" itself
>> (regardless of what the inferior is).  

Yes, the "signal" command delivers the signal to the inferior.
The passed signal is not intercepted.

>> So without readline's signal
>> handlers installed at the point when the inferior is resumed via
>> "signal SIGINT", nothing calls "raise (signal)" later on.  The parent
>> GDB process does not catch the initial SIGINT it itself raised and
>> there is no longer subsequent SIGINT to catch because readline's
>> signal handler, which calls raise(), is no longer permanently
>> installed.
>>
>> It seems it is expected behavior that resuming an inferior via "raise
>> SIGINT" does not immediately stop the inferior giving control back to
>> GDB since interrupt.exp tests for it. 

Correct.

>> Therefore when importing
>> readline 6.3 I think we should just fix the selftest.exp test to no
>> longer expect that the inferior GDB will get stopped following "signal
>> SIGINT".  Something like:
>>

Seems reasonable to me.

Thanks,
Pedro Alves