[PATCH] sim/m32c: fix memory leaks in opc2c

[Mike Frysinger]

On 05 Apr 2021 14:46, Simon Marchi via Gdb-patches wrote:
> On 2021-04-05 12:23 p.m., Mike Frysinger wrote:> On 05 Apr 2021 10:58, Simon Marchi via Gdb-patches wrote:
> >> Fix the leak in dump_lines by free-ing the elements of varnames.
> > 
> > i dislike stuffing a bunch of free's at the end of a program's lifetime just
> > to satisfy a tool that is not normally used.  which isn't to say LSAN isn't
> > useful, just that i think we should do better.
> 
> Why though?  Because it adds execution time where not necessary?

when the process exits, the kernel releases all the memory at once.  there's
no point to calling free() on all your allocated buffers before exiting.  it
only wastes time with the C library heap accounting & syscalls.

i get that we're talking about opc2c here which is used only twice to generate
two other files, so in the larger scheme of things, it's barely noise.  i'm
trying to define standard patterns we can apply in general for "the next one".

> > in other codebases, i've done things like:
> > #ifdef __SANITIZE_ADDRESS__
> > # define ENABLE_CLEANUP_ONEXIT 1
> > #else
> > # define ENABLE_CLEANUP_ONEXIT 0
> > #endif
> > 
> > then this could be written:
> > 
> > if (ENABLE_CLEANUP_ONEXIT) {
> >   for (i = 0; i < vn; i++)
> >     free (varnames[i]);
> > }
> > 
> > wdyt ?  feel free to bikeshed the name.  not sure if there's prior art in
> > the tree currently.
> 
> I find this complexity a bit unnecessary, versus just free-ing the
> stuff.  But I don't really mind, we can do as you like, I just want to
> my build to be fixed!
> 
> Note that the igen tool doesn't free anything, so it's next on the list
> of things that break the -fsanizite=address build.  I started to update
> it to free things, it's a bit tedious but it should be do-able.
> 
> Another option would be to change the Makefile to call igen with the
> ASAN_OPTIONS=detect_leaks=0 environment variable.

ah yes, this is exactly what i mean wrt "the tip of the iceberg" and it being
"a slippery slope" ;).  first it's the small build tools, then the larger
build tools, then the programs we actually install, ...

maybe an alternative would be to convert these to C++.  then it would handle
stack/heap resources for us.
-mike