[comp.lang.perl] make test passes, h2ph reloc.h SEGV's real Perl bug

eichin@athena.mit.edu (Mark W. Eichin) (06/17/91)

I've gone further on this bug... with one fix to gcc2, it actually
built as well as it had with gcc-1.40, and crashed in exactly the same
place (but *with* valid gdb information...) So I hauled out an old
version of GNU malloc that I had hacked to do bounds checking at
malloc and free time, linked it in[*] and discovered that a string was
overrunning the end of the block it was in. I then discovered that the
VAX/BSD version does the same thing, but it only shows up under a
scribble-checking malloc -- but it makes this a perl bug, not a
compiler bug :-)
	Further experimentation (on the VAX) indicated that the
following was enough to trigger the bug:

struct foo {
    long		bar;	/* the value the item to be relocated is
				   refering to (without any offset added) */
};

	If you can't tell which of those are tabs and which are
spaces, the following has the tabs converted to ^I. There are no
trailing spaces, and there are exactly four lines in the file:

struct foo {
    long^I^Ibar;^I/* the value the item to be relocated is
^I^I^I^I   refering to (without any offset added) */
};

	The stack trace I now get on the VAX for this particular
script is:

(gdb) where
#0  0x30354 in kill (29693, 4)
#1  0x30325 in abort ()
#2  0x16d6d in botch (s=(char *) 0x339fa "*ap++ == MAGIC1") (malloc.c line 18)
#3  0x17194 in free (mem=(char *) 0x6518c "    long\t\tbar;\t\200\t\t\t\t   refering to (without any offset added) \201") (malloc.c line 623)
#4  0x26c51 in safefree (where=(char *) 0x6518c "    long\t\tbar;\t\200\t\t\t\t   refering to (without any offset added) \201") (util.c line 176)
#5  0x1e576 in str_replace (str=(struct string *) 0x3fc4c, nstr=(struct string *) 0x4bd8c) (str.c line 594)
#6  0x673a in do_subst (str=(struct string *) 0x3fc4c, arg=(struct arg *) 0x4be4c, sp=-1) (doarg.c line 290)
#7  0x1099e in eval (arg=(struct arg *) 0x4be4c, gimme=0, sp=-1) (eval.c line 608)
#8  0x1467 in cmd_exec (cmdparm=(struct cmd *) 0x4a20c, gimme=0, sp=-1) (cmd.c line 624)
#9  0x1804 in cmd_exec (cmdparm=(struct cmd *) 0x49d8c, gimme=0, sp=-1) (cmd.c line 737)
#10 0x1a86 in cmd_exec (cmdparm=(struct cmd *) 0x59b8c, gimme=0, sp=-1) (cmd.c line 853)
#11 0x1a86 in cmd_exec (cmdparm=(struct cmd *) 0x5c50c, gimme=0, sp=-1) (cmd.c line 853)
#12 0x18b7f in main (argc=4, argv=(char **) 0x7fffe4cc, env=(char **) 0x7fffe4e0) (perl.c line 814)
(gdb) frame 5
#5  0x1e576 in str_replace (str=(struct string *) 0x3fc4c, nstr=(struct string *) 0x4bd8c) (str.c line 594)
594             Safefree(str->str_ptr);
(gdb) p *str
$2 = {str_ptr = 0x6518c "    long\t\tbar;\t\200\t\t\t\t   refering to (without any offset added) \201", str_len = 55, str_u = {str_nval = -5.165090029488535e+21, str_stab = 0x3e48c, str_useful = 255116, str_args = 0x3e48c, str_hash = 0x3e48c, str_array = 0x3e48c, str_cmd = 0x3e48c}, str_cur = 63, str_magic = 0x0, str_pok = 1 '\001', str_nok = 0 '\000', str_rare = 0 '\000', str_state = 0 '\000'}
(gdb) 

	Note in particular that str->str_cur == 63 but str->str_len ==
55. The botch "*ap++ == MAGIC1" indicates that free did not find the
expected "magic cookie" value at the end of the allocated block, which
according to the header was malloc'd at 55 bytes.
	Let me know if you have any trouble duplicating this (and if
you find a fix, so I can stop looking :-) 

				_Mark_ <eichin@athena.mit.edu>
				MIT Student Information Processing Board
				Watchmaker Computing <eichin@watch.com>

[*] ps. Some systems (in particular, the NeXT) can't stand *any* name
conflicts, so that linking in malloc.o isn't possible because it
conflicts with the malloc in libsys_s.a. It might be worth adding a
bit of code to Configure to notice this, and add a
 "-Dmalloc=MYmalloc -Dfree=MYfree -Drealloc=MYrealloc" to $ccflags...

vinoski@apollo.hp.com (Stephen Vinoski) (06/19/91)

In article <1991Jun17.102103.17445@uvaarpa.Virginia.EDU> eichin@athena.mit.edu writes:
>I've gone further on this bug... with one fix to gcc2, it actually
>built as well as it had with gcc-1.40, and crashed in exactly the same
>place (but *with* valid gdb information...) So I hauled out an old
>version of GNU malloc that I had hacked to do bounds checking at
>malloc and free time, linked it in[*] and discovered that a string was
>overrunning the end of the block it was in. I then discovered that the
>VAX/BSD version does the same thing, but it only shows up under a
>scribble-checking malloc -- but it makes this a perl bug, not a
>compiler bug :-)
>
>   [...lengthy traceback deleted...]
>  
>	Note in particular that str->str_cur == 63 but str->str_len ==
>55. The botch "*ap++ == MAGIC1" indicates that free did not find the
>expected "magic cookie" value at the end of the allocated block, which
>according to the header was malloc'd at 55 bytes.

I have found a fix for this bug, but I'm not sure of it's completeness.  Perl
builds and passes all of its tests with the fix, but maybe everyone should
wait for Larry to bless the included patch before applying it.

The problem lies in str_gets() in str.c.  The allocated space is checked to see
if it can hold the string to be fetched, but the check doesn't always work when
the string is being appended to.

Could someone a little more familiar with the (hairy) code in str_gets()
validate this patch and let the rest of us know if it's OK?  Thanks.

*** str.c.orig	Mon Jun 10 13:26:13 1991
--- str.c	Tue Jun 18 16:56:14 1991
***************
*** 742,748 ****
      cnt = fp->_cnt;			/* get count into register */
      str->str_nok = 0;			/* invalidate number */
      str->str_pok = 1;			/* validate pointer */
!     if (str->str_len <= cnt + 1) {	/* make sure we have the room */
  	if (cnt > 80 && str->str_len > append) {
  	    shortbuffered = cnt - str->str_len + append + 1;
  	    cnt -= shortbuffered;
--- 742,748 ----
      cnt = fp->_cnt;			/* get count into register */
      str->str_nok = 0;			/* invalidate number */
      str->str_pok = 1;			/* validate pointer */
!     if (str->str_len - str->str_cur <= cnt + 1) {	/* make sure we have the room */
  	if (cnt > 80 && str->str_len > append) {
  	    shortbuffered = cnt - str->str_len + append + 1;
  	    cnt -= shortbuffered;



-steve