[readline patch, gdb-7.3?] Avoid free from a signal handler [Re: [PATCH] Make interrupting tab-completion safe.]

[Jan Kratochvil]

GDB reproducer for the readline bug:
------------------------------------

On Mon, 13 Jun 2011 19:44:57 +0200, Sterling Augustine wrote:
> gdb itself isn't big enough to make it easy to reproduce (~24k
> symbols).

The goal is to do very many memory allocation functions in GDB, no matter
which ones.  I found for example libwebkitgtk.so.debug as good sample data,
optionally added .gdb_index is OK for faster startup, it should contain C++
functions for more memory operations (therefore GDB itself is not usable as
the sample data).  Used this GDB .exp file, reproducible in several seconds:

set binfile "/home/jkratoch/t/rh575292-gdbindex.debug"
gdb_exit
gdb_start
gdb_load ${binfile}
while 1 {
    send_gdb "b \t\t"
    sleep 0.1
    send_gdb "\003"
    gdb_test "" "^b \\^CQuit" "Quit seen"
}

b ^CQuit
(gdb) PASS: gdb.base/quit.exp: Quit seen
b *** glibc detected *** /home/jkratoch/redhat/gdb-clean/gdb/testsuite/../../gdb/gdb: munmap_chunk(): invalid pointer: 0x0000000005b950b0 ***

One can also add stub expensive mallinfo calls into GDB xmalloc/xfree/etc. for
better reproducibility.


The readline bug:
-----------------

readline now calls memory allocation/free functions from the readline signal
handler.  This is not permitted by POSIX and it does corrupt memory such as
during SiGINT:

#0  __lll_lock_wait_private () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:100
#1  in _L_lock_10461 () from /lib64/libc.so.6
#2  in __libc_malloc (bytes=139919578014176) at malloc.c:3657
#3  in __libc_message (do_abort=2, fmt=0x7f41909aebe8 "*** glibc detected *** %s: %s: 0x%s ***\n") at ../sysdeps/unix/sysv/linux/libc_fatal.c:137
#4  in malloc_printerr (action=3, str=0x7f41909aec18 "munmap_chunk(): invalid pointer", ptr=<optimized out>) at malloc.c:6283
#5  in xfree (ptr=0x5b950b0) at utils.c:1303
#6  in rl_free_undo_list () at undo.c:119
#7  in rl_free_line_state () at signals.c:503
#8  in _rl_handle_signal (sig=2) at signals.c:188
#9  in rl_signal_handler (sig=2) at signals.c:149
#10 <signal handler called>
#11 _int_malloc (av=0x7f4190bea1e0, bytes=177) at malloc.c:4727
#12 in __libc_malloc (bytes=177) at malloc.c:3660
#13 in reallochook (ptr=<optimized out>, size=128, caller=0xd6f531) at mcheck.c:335
#14 in d_growable_string_resize (dgs=0x7fff34b0d150, need=122) at ./cp-demangle.c:3247
#15 in d_growable_string_init (dgs=0x7fff34b0d150, estimate=122) at ./cp-demangle.c:3226
#16 in cplus_demangle_print (options=3, dc=0x4114fe8, estimate=122, palc=0x7fff34b0d198) at ./cp-demangle.c:3416
#17 in cp_comp_to_string (result=0x4114fe8, estimated_len=122) at cp-name-parser.y:1965
#18 in cp_canonicalize_string (string=0x93f4e20 "WebCore::FrameLoader::loadProvisionalItemFromCachedPage(void)") at cp-support.c:139
[...]

Unfortunately it leaks a bit now and also for example during SIGWINCH readline
calls xmalloc which cannot be ignored/delayed so easily as xfree.

Please postpone any memory management function only after the signal handler.


Thanks,
Jan


readline/
2011-06-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Avoid free from a signal handler.
	* Makefile.in (xfree.o): Add readline.h.
	* xfree.c: Include stdio.h and readline.h.
	(xfree): Return on RL_STATE_SIGHANDLER.
	* xmalloc.h (xfree): New definition.

--- a/readline/Makefile.in
+++ b/readline/Makefile.in
@@ -422,7 +422,7 @@ vi_mode.o: rldefs.h ${BUILD_DIR}/config.h rlconf.h
 vi_mode.o: readline.h keymaps.h rltypedefs.h chardefs.h tilde.h
 vi_mode.o: history.h ansi_stdlib.h rlstdc.h
 xfree.o: ${BUILD_DIR}/config.h
-xfree.o: ansi_stdlib.h
+xfree.o: ansi_stdlib.h readline.h
 xmalloc.o: ${BUILD_DIR}/config.h
 xmalloc.o: ansi_stdlib.h
 
--- a/readline/xfree.c
+++ b/readline/xfree.c
@@ -31,7 +31,10 @@
 #  include "ansi_stdlib.h"
 #endif /* HAVE_STDLIB_H */
 
+#include <stdio.h>
+
 #include "xmalloc.h"
+#include "readline.h"
 
 /* **************************************************************** */
 /*								    */
@@ -45,6 +48,10 @@ void
 xfree (string)
      PTR_T string;
 {
+  /* Leak a bit.  */
+  if (RL_ISSTATE(RL_STATE_SIGHANDLER))
+    return;
+
   if (string)
     free (string);
 }
--- a/readline/xmalloc.h
+++ b/readline/xmalloc.h
@@ -38,6 +38,9 @@
 
 #endif /* !PTR_T */
 
+/* xmalloc and xrealloc should be also protected from RL_STATE_SIGHANDLER.  */
+#define xfree xfree_readline
+
 extern PTR_T xmalloc PARAMS((size_t));
 extern PTR_T xrealloc PARAMS((void *, size_t));
 extern void xfree PARAMS((void *));