Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Chet Ramey <chet.ramey@case.edu>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: bug-readline@gnu.org, gdb-patches@sourceware.org,
	       Sterling Augustine <saugustine@google.com>,
	chet.ramey@case.edu
Subject: Re: [Bug-readline] [readline patch, gdb-7.3?] Avoid free from a signal handler [Re: [PATCH] Make interrupting tab-completion safe.]
Date: Thu, 07 Jul 2011 13:40:00 -0000	[thread overview]
Message-ID: <4E15A220.4030404@case.edu> (raw)
In-Reply-To: <20110706164400.GA31447@host1.jankratochvil.net>

On 7/6/11 12:44 PM, Jan Kratochvil wrote:
> On Wed, 06 Jul 2011 17:58:26 +0200, Chet Ramey wrote:
>> As I said, I'm willing to remove these references and see what happens.  Since
>> you have a way to readily reproduce the problem, I was hoping you'd do it
>> and let me know what you found.
> 
> I do not think any testing matters here.  This is a difficult to reproduce
> race + memory corruption.  While a crash proves it is wrong no crash does not
> prove anything.

The impression I got from your earlier message is that is is very easy
to reproduce using a GDB .exp file:

"Used this GDB .exp file, reproducible in several seconds"

All I am asking you do to is to check whether you can reproduce it using
the same .exp file after removing references to _rl_interrupt_immediately
in complete.c.

> 
> Even if no existing system ever crashes the code is still wrong because it
> violates POSIX:
> http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html
> 	The following table defines a set of functions that shall be either
> 	reentrant or non-interruptible by signals and shall be async-signal-safe.
> 
> Static code analysis is the only valid verification.  Currently the signal
> code calls free() which is not listed in the safe syscalls list above,
> therefore the code is not correct.
> 
> I do not know if it is possible to code _rl_handle_signal in a way which uses
> only the safe syscalls and only atomic operations on volatile data structures.
> Anyway even if it would be possible I find such code very fragile and
> I believe the signals should be always delayed through _rl_caught_signal.

Ironically, I changed it to respond immediately to signals when in callback
mode because of a bug you filed from gdb.  When readline was reading input
using rl_callback_read_char it did not respond quickly enough to SIGINT,
and gdb didn't catch it.  You will have to check and make sure the
conditions have changed enough to make it acceptable to delay signal
handling.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/


  reply	other threads:[~2011-07-07 12:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-11  0:19 [PATCH] Make interrupting tab-completion safe Sterling Augustine
2011-06-12 12:12 ` Jan Kratochvil
2011-06-13 17:45   ` Sterling Augustine
2011-06-26 22:22     ` [readline patch, gdb-7.3?] Avoid free from a signal handler [Re: [PATCH] Make interrupting tab-completion safe.] Jan Kratochvil
2011-06-27 16:03       ` Joel Brobecker
2011-06-29 21:49         ` Jan Kratochvil
2011-06-29 13:54       ` [Bug-readline] " Chet Ramey
2011-06-29 20:35         ` Jan Kratochvil
2011-06-30 14:38           ` Chet Ramey
2011-07-06 16:03             ` Jan Kratochvil
2011-07-06 16:07               ` Chet Ramey
2011-07-06 17:42                 ` Jan Kratochvil
2011-07-07 13:40                   ` Chet Ramey [this message]
2011-07-08 16:03                     ` Chet Ramey
2011-10-19 20:30                       ` Jan Kratochvil
2011-10-19 17:02                     ` Jan Kratochvil
2011-10-19 17:51                       ` Pedro Alves
2011-10-19 18:50                       ` Chet Ramey
2011-07-11 18:53     ` [PATCH] Make interrupting tab-completion safe Sterling Augustine
2011-07-11 18:54       ` Jan Kratochvil
     [not found]         ` <CAEG7qUxFvEoJ-E2YsoFPL-tKoK4kD3-pKn-h31uUeXQoDD2Gaw@mail.gmail.com>
2011-07-12 15:59           ` [dwarf2_mark_helper patch] " Jan Kratochvil
2011-07-12 17:48             ` Sterling Augustine
2011-07-12 18:56             ` Jan Kratochvil
2011-07-12 21:18             ` [commit] " Jan Kratochvil
2011-07-12 21:42               ` Tom Tromey
2011-07-12 22:51                 ` Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E15A220.4030404@case.edu \
    --to=chet.ramey@case.edu \
    --cc=bug-readline@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=saugustine@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox