callback api, isearch, signals issue

[Andrew Burgess]

Hi,

While working on GDB, which uses the callback API, I've run into some
issues relating to reverse isearch and signal handling.

At the end of this email is a small example program that uses the
callback API.  This is a modified version of the example program that
is included in the readline manual.

The issue I have relies on a SIGINT arriving at just the wrong time.
To reproduce this the test program uses rl_getc_function to setup a
wrapper function (my_getc) that calls rl_getc.  After a couple of
times calling my_getc, a SIGINT is sent to the process using kill().

To see the issue you should do exactly this:

  1. Compile and run the test program,
  2. Press ctrl-r to enter reverse-i-search mode,
  3. Press any "normal" key, e.g. 'a', this triggers the SIGINT.
  4. Hopefully, observe the test program segfault.  If you don't
     though, then that's fine.  The bug is undefined behaviour, so the
     exact failure mode is *anything*.

What happens is that, when the SIGINT arrives the stack looks like
this:

  #0  0x00007f932354392b in kill () from /lib64/libc.so.6
  #1  0x0000000000401335 in my_getc (stream=0x7f93236c97e0 <_IO_2_1_stdin_>) at 
rl-callback.c:80
  #2  0x00007f9323773f41 in rl_read_key () at ../../src/input.c:793
  #3  0x00007f9323766524 in _rl_search_getchar (cxt=0x20b3da0) at 
../../src/isearch.c:320
  #4  0x00007f9323767c07 in _rl_isearch_callback (cxt=0x20b3da0) at 
../../src/isearch.c:887
  #5  0x00007f9323774853 in rl_callback_read_char () at ../../src/callback.c:174
  #6  0x0000000000401540 in main (c=1, v=0x7ffc2f324038) at rl-callback.c:146

After registering that the signal arrived we return and call rl_getc,
this does a RL_CHECK_SIGNALS call, which causes readline to cleanup
the isearch state, the stack looks like this:

  #0  _rl_isearch_cleanup (cxt=0x20b3da0, r=0) at ../../src/isearch.c:830
  #1  0x00007f9323774e7e in rl_callback_sigcleanup () at 
../../src/callback.c:361
  #2  0x00007f932376f864 in _rl_handle_signal (sig=2) at ../../src/signals.c:219
  #3  0x00007f932376f763 in _rl_signal_handler (sig=2) at 
../../src/signals.c:149
  #4  0x00007f9323773faa in rl_getc (stream=0x7f93236c97e0 <_IO_2_1_stdin_>) at 
../../src/input.c:816
  #5  0x0000000000401341 in my_getc (stream=0x7f93236c97e0 <_IO_2_1_stdin_>) at 
rl-callback.c:84
  #6  0x00007f9323773f41 in rl_read_key () at ../../src/input.c:793
  #7  0x00007f9323766524 in _rl_search_getchar (cxt=0x20b3da0) at 
../../src/isearch.c:320
  #8  0x00007f9323767c07 in _rl_isearch_callback (cxt=0x20b3da0) at 
../../src/isearch.c:887
  #9  0x00007f9323774853 in rl_callback_read_char () at ../../src/callback.c:174
  #10 0x0000000000401540 in main (c=1, v=0x7ffc2f324038) at rl-callback.c:146

As we have sent some input, the 'a' in our example, the rl_getc call
does return, which eventually lands readline back in
_rl_isearch_callback, which then calls _rl_isearch_dispatch, which
makes use of the CXT local variable, the stack looks like this:

  #0  _rl_isearch_dispatch (cxt=0x20b3da0, c=49) at ../../src/isearch.c:349
  #1  0x00007f9323767c1f in _rl_isearch_callback (cxt=0x20b3da0) at 
../../src/isearch.c:889
  #2  0x00007f9323774853 in rl_callback_read_char () at ../../src/callback.c:174
  #3  0x0000000000401540 in main (c=1, v=0x7ffc2f324038) at rl-callback.c:146

Notice the cxt passed to _rl_isearch_dispatch is the same pointer as
was cleared in _rl_isearch_cleanup.  There's no path through
_rl_isearch_dispatch that doesn't make use of cxt, so at this point
readline experiences undefined behaviour.  For me this manifests as a
segfault in a strlen call, but obviously you might see other results.

I'm not really sure how to go about fixing this issue right now.

Would appreciate any thoughts that you have on this.

Thanks,
Andrew

---

/* Standard include files. stdio.h is required. */
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <locale.h>

/* Used for select(2) */
#include <sys/types.h>
#include <sys/select.h>

#include <signal.h>

#include <stdio.h>

/* Standard readline include files. */
#include <readline/readline.h>
#include <readline/history.h>

static void cb_linehandler (char *);
static void sigwinch_handler (int);
static void sigint_handler (int);

int running;
int sigwinch_received;
int sigint_received;
const char *prompt = "rltest$ ";

/* Handle SIGWINCH and window size changes when readline is not active and
   reading a character. */
static void
sigwinch_handler (int sig)
{
  sigwinch_received = 1;
}

/* Handle SIGWINCH and window size changes when readline is not active and
   reading a character. */
static void
sigint_handler (int sig)
{
  sigint_received = 1;
}

/* Callback function called for each line when accept-line executed, EOF
   seen, or EOF character read.  This sets a flag and returns; it could
   also call exit(3). */
static void
cb_linehandler (char *line)
{
  /* Can use ^D (stty eof) or `exit' to exit. */
  if (line == NULL || strcmp (line, "exit") == 0)
    {
      if (line == 0)
        printf ("\n");
      printf ("exit\n");
      /* This function needs to be called to reset the terminal settings,
         and calling it from the line handler keeps one extra prompt from
         being displayed. */
      rl_callback_handler_remove ();

      running = 0;
    }
  else
    {
      if (*line)
        add_history (line);
      printf ("input line: %s\n", line);
      free (line);
    }
}

int count = 2;

static int
my_getc (FILE *stream)
{
  if (--count == 0)
    {
      kill (getpid (), SIGINT);
    }

  int ch = rl_getc (stream);

  return ch;
}


int
main (int c, char **v)
{
  fd_set fds;
  int r;

  /* Set the default locale values according to environment variables. */
  setlocale (LC_ALL, "");

  /* Handle window size changes when readline is not active and reading
     characters. */
  signal (SIGWINCH, sigwinch_handler);

  signal (SIGINT, sigint_handler);

  rl_getc_function = my_getc;

  /* Install the line handler. */
  rl_callback_handler_install (prompt, cb_linehandler);

  /* Enter a simple event loop.  This waits until something is available
     to read on readline's input stream (defaults to standard input) and
     calls the builtin character read callback to read it.  It does not
     have to modify the user's terminal settings. */
  running = 1;
  while (running)
    {
      FD_ZERO (&fds);
      FD_SET (fileno (rl_instream), &fds);

      r = select (FD_SETSIZE, &fds, NULL, NULL, NULL);
      if (r < 0 && errno != EINTR)
        {
          perror ("rltest: select");
          rl_callback_handler_remove ();
          break;
        }
      if (sigwinch_received)
        {
          rl_resize_terminal ();
          sigwinch_received = 0;
        }
      if (sigint_received)
        {
          printf ("Quit\n");

          rl_callback_handler_remove ();
          rl_callback_handler_install (prompt, cb_linehandler);

          sigint_received = 0;
          continue;
        }
      if (r < 0)
        continue;

      if (FD_ISSET (fileno (rl_instream), &fds))
        rl_callback_read_char ();
    }

  printf ("rltest: Event loop has exited\n");
  return 0;
}