Re: gawk: other double free(_wstr)

[Aharon Robbins]

Thanks for this. I am applying the fix below for field.c:rebuild_record. It
does the trick; at least valgrind is happy.

I have reverted the changes to free_wstr after your earlier mail.

Arnold

> Date: Fri, 12 Jan 2007 11:46:58 +0100
> From: Karel Zak <address@hidden>
> Subject: gawk: other double free(_wstr)
> To: address@hidden
>
>  Hi,
>
>  there is other double free() call in gawk. Try:
>
>  echo -e "AAA BBX\nAAA BBY" | ./gawk ' /^AAA BB/ { x = substr($2, 1); $1 = 
> "FOO"; print $0 }'
>
>  *** glibc detected *** gawk: double free or corruption (fasttop):
>  0x000000000065b8b0 ***
>  ======= Backtrace: =========
>  /lib64/libc.so.6[0x3d2c06ea60]
>  /lib64/libc.so.6(cfree+0x8c)[0x3d2c07217c]
>  gawk(free_wstr+0x18)[0x428578]
>  gawk(unref+0x4c)[0x4285fc]
>  gawk(reset_record+0x69)[0x41f699]
>  gawk(set_record+0x11)[0x41f7b1]
>  gawk[0x423115]
>  gawk(do_input+0x28)[0x4260d8]
>  gawk(main+0xe9c)[0x427d8c]
>
>
>  It's gawk-stable and Dmitry's patches doesn't help too much.
>
>  The problem is probably somewhere around rebuild_record(), because it
>  calls unref() (which correctly deallocates wstptr), but then the 
>  rebuild_record() reassigns deallocated wstptr back to fields_arr[i]:
>
>      n->stptr = cops;
>      unref(fields_arr[i]);   <--- free( fields_arr[i]->wstptr )
>      fields_arr[i] = n;      <--- deallocated wstptr is back 
>  
>  I'm not sure how correctly fix the problem, maybe the "n"
>  should be without WSTRCUR flag before reassigning to fields_arr[i]. 
>
>      n->flags &= ~(MALLOC|TEMP|PERM|STRING|CURSTR|WCURSTR);
>                                           ^^^^^^^^^^^^^^^
>                                          
>
>  The problem comes up, because gawk newly deallocates wstptr in
>  unref(). See node.c unref() diff between gawk-3.1.5 and gawk-stable
>  CVS:
>
>          if ((tmp->flags & FIELD) != 0) {
>  +               free_wstr(tmp);
>                  freenode(tmp);
>                  return;
>         }
>
>
>     Karel
>
> -- 
>  Karel Zak  <address@hidden>
>
>
> #####################################################################################
> This Mail Was Scanned by 012.net AntiVirus Service1- Powered by TrendMicro 
> Interscan
>

Index: ChangeLog
===================================================================
RCS file: /d/mongo/cvsrep/gawk-stable/ChangeLog,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- ChangeLog   12 Jan 2007 12:03:44 -0000      1.9
+++ ChangeLog   13 Jan 2007 19:21:34 -0000      1.10
@@ -1,3 +1,13 @@
+Sat Jan 13 20:56:56 2007  Arnold D. Robbins  <address@hidden>
+
+       * node.c (wcasestrstr): Revert use of continue, reinstate goto.
+       Thanks to Andrew Schorr.
+       (free_wstr): Move zeroing of wsptr and wslen and clearing of flag
+       back outside the if.
+       * field.c (rebuild_record): In loop that copies fields to new record, 
add
+       call to `free_wstr'. This ensures that flag values are correct and 
avoids
+       double free later. Thanks to Karel Zak for pointing out the problem.
+
 Fri Jan 12 14:01:51 2007  Dmitry V. Levin  <address@hidden>
 
        * builtin.c (do_match): In addition to "gawk_mb_cur_max > 1" check,
Index: field.c
===================================================================
RCS file: /d/mongo/cvsrep/gawk-stable/field.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- field.c     11 Aug 2006 12:49:40 -0000      1.2
+++ field.c     13 Jan 2007 19:21:35 -0000      1.3
@@ -166,6 +166,7 @@
        cops = ops;
        ops[0] = '\0';
        for (i = 1;  i <= NF; i++) {
+               free_wstr(fields_arr[i]);
                tmp = fields_arr[i];
                /* copy field */
                if (tmp->stlen == 1)
Index: node.c
===================================================================
RCS file: /d/mongo/cvsrep/gawk-stable/node.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- node.c      12 Jan 2007 10:14:35 -0000      1.4
+++ node.c      13 Jan 2007 19:21:35 -0000      1.5
@@ -768,10 +768,10 @@
        if ((n->flags & WSTRCUR) != 0) {
                assert(n->wstptr != NULL);
                free(n->wstptr);
-               n->wstptr = NULL;
-               n->wstlen = 0;
-               n->flags &= ~WSTRCUR;
        }
+       n->wstptr = NULL;
+       n->wstlen = 0;
+       n->flags &= ~WSTRCUR;
 }
 
 #if 0
@@ -836,10 +836,11 @@
                                h = towlower(*start);
                                n = towlower(needle[j]);
                                if (h != n)
-                                       continue;
+                                       goto out;
                        }
                        return haystack + i;
                }
+out:   ;
        }
 
        return NULL;