Re: [PATCH] [RFC] Sync readline to version 6.3 patchlevel 8

Patrick Palka
2015-05-18T11:41:45Z

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(-)

diff --git a/gdb/testsuite/gdb.gdb/selftest.exp b/gdb/testsuite/gdb.gdb/selftest.exp
index 9f25a48..6170ac8 100644
--- a/gdb/testsuite/gdb.gdb/selftest.exp
+++ b/gdb/testsuite/gdb.gdb/selftest.exp
@@ -444,9 +444,26 @@ proc test_with_self { executable } {
}

set description "send SIGINT signal to child process"
- gdb_test "signal SIGINT" \
- "Continuing with signal SIGINT.*" \
- "$description"
+ gdb_test_multiple "signal SIGINT" "$description" {
+ -re "^signal SIGINT\r\nContinuing with signal SIGINT.\r\nQuit" {
+ pass "$description"
+ }
+ }
+
+ set description "send ^C to child process again"
+ send_gdb "\003"
+ gdb_expect {
+ -re "Program received signal SIGINT.*$gdb_prompt $" {
+ pass "$description"
+ }
+ -re ".*$gdb_prompt $" {
+ fail "$description"
+ }
+ timeout {
+ fail "$description (timeout)"
+ }
+ }
+

# get a stack trace
#
--
2.4.0.194.gc518059