Re: [bug-gawk] memory issues

[Andrew J. Schorr]

On Thu, Aug 29, 2019 at 07:29:12AM -0600, address@hidden wrote:
> > > Below is a patch that reverts this change. For me it fixes the issue
> > > on master.
> >
> > At the cost of hurting performance, which is why I got rid of that
> > needless copy in the 1st place.
> 
> Tradeoffs, tradeoffs, tradeoffs.

I don't think it's really a tradeoff. I think we have sloppy reference
counting, and the copy somehow masks that issue. I believe the fix is
to get the reference counting right, not to make copies of everything.

> > > Andy - how does this jive with the analysis you did? It looks to me
> > > like the original code, which this goes back to, made a new copy of
> > > the field, that was used for the regex match, and that is what was
> > > freed by the code as it stands now.
> > > 
> > > It'd be nice to be sure we've fixed this in a fundamental way.
> >
> > The old code was copying everything like crazy, which was safe,
> > but was hurting performance. To truly understand why one leaks memory
> > and the other doesn't will require me to go back into the debugger and/or
> > add print statements to chase it down. I will try to find time to do
> > that later.
> 
> Your efforts are and will be appreciated, but if you need to do
> other things, that's OK.

This is really tricky stuff. With my patch, the Node_dynregex re_exp
node pointer is released immediately after the regexp search is done.
Without my patch, the re_exp pointer is retained, and it's released
the next time that a search is done. So in the interim, the code has
to retain a copy. I'm not certain why in Finn's case, the nodes were
never getting released. But it seems clear to me that releasing them
proactively when we are done with them is a win, as it saves
field.c:purge_record from having to copy the string and save the node.

To understand memory allocation issues better, I think the block allocation
scheme gets in the way. So I came up with the attached patch for memory
debugging. With this patch in place, "make check" passes, but "make
valgrind-noleak" spews "invalid read" errors on several test cases:

Scanning valgrind log files for problems:
log.27953:   ../gawk -f synerr3.awk
         Invalid read of size 4
         ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
log.29078:   ../gawk -f nsprof2.awk --pretty-print=_nsprof2
         Invalid read of size 4
         ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
log.29172:   ../gawk --profile=ap-profile2.out -v sortcmd=sort -f ./xref.awk 
./dtdgport.awk
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         ERROR SUMMARY: 58 errors from 24 contexts (suppressed: 0 from 0)
log.29177:   ../gawk --profile=ap-profile3.out -f ./profile3.awk
         Invalid read of size 4
         ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
log.29186:   ../gawk -f profile4.awk --pretty-print=_profile4
         Invalid read of size 4
         ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)
log.29193:   ../gawk -f profile5.awk --pretty-print=_profile5
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         ERROR SUMMARY: 2220 errors from 101 contexts (suppressed: 0 from 0)
log.29197:   ../gawk --profile=ap-profile6.out -f ./profile6.awk
         Invalid read of size 4
         ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)
log.29233:   ../gawk -f profile11.awk --pretty-print=_profile11
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         ERROR SUMMARY: 7 errors from 3 contexts (suppressed: 0 from 0)
log.29667:   ../gawk --pretty-print=ap-profile1.out -f ./xref.awk
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 4
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         Invalid read of size 8
         ERROR SUMMARY: 58 errors from 24 contexts (suppressed: 0 from 0)

So unless there's a bug in my MEMDEBUG code, I believe there are some
memory corruption issues in the current code. This is in the master codebase
without any other pathces except your GIVE_BACK_SIZE patch in format_tree.

If I now enable my Node_dynregex patch in interpret.h, it still
passes "make check", and "make valgrind-noleak" gives the same errors.

I guess the good news is that this memory corruption seems to be limited
to pretty-printing and profiling, so not likely to be terribly important.

Regards,
Andy

memdebug.patch
Description: Text document