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

Jan Kratochvil
2011-06-26T22:22:20Z

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 *));