Re: multiple frees in awk, sometimes leading to crash

[Aharon Robbins]

Hi. This is all fixed in my current sources, available from the CVS archive
on savannah.gnu.org.  Check out the module gawk-stable.

Thanks,

Arnold

In article <address@hidden> you write:
>This bug is described to some detail at
>https://bugs.launchpad.net/ubuntu/+source/gawk/+bug/58256
>
>The problem can be seen in an example like the following (doesn't crash
>on my system, but valgrind will complain):
>
>  printf '\n\na\nb\n' | valgrind gawk '{length($1)}'
>
>(A "print" may be inserted before the length() call, if desired.) See
>the link above for a version that crashes, at least on some systems.
>
>The problem is that do_length() invokes str2wstr() on its argument. In
>the case of the initial blank line, above, this results in
>Null_field->wstptr being given a pointer, and Null_field->flags |= WSTRCUR.
>
>On the next line gawk encounters, the $1 reference, since it exists this
>time, will cause gawk to grow fields_arr[], copying the value of
>*Null_field to the new element. After this copy, set_field() copies the
>new text ("a") into the element, and resets the flags (clearing
>WSTRCUR). str2wstr(), seeing a pointer value in n->wstptr, but no
>WSTRCUR flag, will free(n->wstptr).
>
>On the final line, reset_fields() will ensure that the value of
>*Null_field is copied over the previous value of fields_arr[1] (still
>with the same value for n->wstptr as before). When set_field() is called
>again to set it with the "b" text, the flags will again be cleared, and
>str2wstr() will end up freeing the very same wstptr value (double free).
>
>I can think of a couple of ways to solve this. One would be to have
>r_get_lhs() return a new node with a copy of the value from Null_field,
>rather than &Null_field itself, when get_field() returns &Null_field
>(I'm guessing that this shouldn't be done in get_field() itself, as
>sometimes it may be used only for reading?). This will prevent the
>actual value of Null_field from being modified by str2wstr().
>
>That has the disadvantage that every time length() is called on a
>nonexistent field, we are invoking a decode operation (and allocating
>associated space) to create a zero-length wide-string, just to get its
>length, and then discard it. A solution for this is that we can have
>do_length() detect zero-length strings, and automatically return zero
>for them, short-circuiting the useless wide-string conversion (and,
>indeed, we should do this anyway). Since this would prevent the
>conversion in all cases for Null_field, it would never get a value
>written to its wstptr, so problem solved...
>
>Except that (1) this doesn't self-document enough to help people
>understand the problem should they decide to alter code in that area
>again, and (2) I haven't verified this, but I believe do_match() may
>have the same issue (when it sets rlength), in which case we'll have
>logic duplication: two fragile places where the code could be broken again.
>
>I suppose another, somewhat kludgy fix, would be for str2wstr() itself
>to detect zero-length strings, in which case it sets wstlen to 0, and
>wstptr to NULL. In this case, it may not really even matter whether
>WSTRCUR gets set; and free(NULL) is guaranteed well-defined. This also
>has the advantage of automatically resolving the
>useless-allocation-for-zero-length-strings inefficiency.
>
>I'm not sure I completely like any of these fixes. What do you think?
>
>-- 
>Micah J. Cowan
>Programmer, musician, typesetting enthusiast, gamer...
>http://micah.cowan.name/
>
>


-- 
Aharon (Arnold) Robbins --- Pioneer Consulting Ltd.     arnold AT skeeve DOT com
P.O. Box 354            Home Phone: +972  8 979-0381    Fax: +1 206 350 8765
Nof Ayalon              Cell Phone: +972 50  729-7545
D.N. Shimshon 99785     ISRAEL