gnupic: Thread: Re: [gnupic] gpasm memory leaks

David Barnett writes ("Re: [gnupic] gpasm memory leaks"):
> A status update on the memory leaks for those interested: [...]

Sorry to rain on your parade, but I have to ask: _why_ are we fixing
memory leaks in a PIC assembler ?  Even if the assembler is hugely
profligate with memory allocation the amount of RAM used during an
assembly is bounded to a few times the size of the flash in the
device or a few times the size of the input file (larger of those,
obviously).

The obvious conclusion is that the gpasm code shouldn't ever call
`free'.  That way there can be no double-free bugs or use-after-free
bugs and the like.

Ian.

----- Original Message ----- 
From: "Ian Jackson" ####@####.####
Newsgroups: chiark.mail.linuxhacker.gnupic
To: ####@####.####
Sent: Monday, June 04, 2007 11:02 AM
Subject: Re: [gnupic] gpasm memory leaks


> David Barnett writes ("Re: [gnupic] gpasm memory leaks"):
>> A status update on the memory leaks for those interested: [...]
>
> Sorry to rain on your parade, but I have to ask: _why_ are we fixing
> memory leaks in a PIC assembler ?
 <snip>
> The obvious conclusion is that the gpasm code shouldn't ever call
> `free'.  That way there can be no double-free bugs or use-after-free
> bugs and the like.
*Is* that the obvious conclusion?  I really don't know about the coding 
standards we adhere to, but I got the impression all well-written software 
should be free of memory leaks period.  I realize most of the leaks won't 
affect much, but I had no idea anyone would recommend intentionally leaving 
memory leaks to prevent other bugs.  If that's the consensus, I certainly 
won't object.

Does everyone else agree?

David Barnett 

David,

Ian questions the value of your gpasm memory leak investigation and goes 
so far to suggest that free never be called. I was going to question the 
value of your investigation too when I realized that the real value is 
in the knowledge you'll gain in understanding gpasm. However, in general 
for small programs that have constrained memory footprint I don't even 
bother with deallocating memory. gpasm sort of fits into that category. 
On the other hand, gpasm is really just a part of gputils which consists 
of several utilities and a library. If we anticipate the library being 
used in some other application where memory consumption is critical, 
then free'ing memory is a good idea. The pCode optimizer may be such a 
case.

Scott

On Mon, 04 Jun 2007 13:28:05 -0700
Scott Dattalo ####@####.#### wrote:

> David,
> 
> However, in general 
> for small programs that have constrained memory footprint I don't even 
> bother with deallocating memory. gpasm sort of fits into that category. 
> On the other hand, gpasm is really just a part of gputils which consists 
> of several utilities and a library. If we anticipate the library being 
> used in some other application where memory consumption is critical, 
> then free'ing memory is a good idea. The pCode optimizer may be such a 
> case.

In general, I _never_ leave allocated memory leaking, even for a very
small program. However, my reasoning is much more akin to Scott's: if I
ever wanted to reuse that code then having unhandled memory leaks may
prove to be a problem. Far better to design the thing right in the
first place, even if it's (a little bit) more work.

That said, for tools "like" gpasm (in that they conceptually read in
and process lots of stuff, then exit) the cleaning up often happens
just before the program terminates. After all, trying to figure out
whether an entry in the symbol table will become redundant before the
end of pass 2 is too much effort for too little gain.

And as to the "don't free it because that way avoids trying to use
already free'd heap" argument... I shall restrain from my instinctive
one word answer and merely point out that the last program "of this
sort" (i.e. part of a build environment and behaving something like a
compiler) that I inherited from a third party (actually a student)
whose approach to heap memory was less than rigourous, adopted that
approach and contained so many tough to find memory allocation errors
you wouldn't believe it. It leaked like a sieve _and_ regularly used
memory it hadn't really allocated.

I know this is an open source hacker community, but that doesn't change
the truth - discipline in software development is a Good Thing.

----- Original Message ----- 
From: "Scott Dattalo" ####@####.####
To: ####@####.####
Sent: Monday, June 04, 2007 3:28 PM
Subject: Re: [gnupic] gpasm memory leaks


> David,
>
> I was going to question the value of your investigation too when I 
> realized
> that the real value is in the knowledge you'll gain in understanding 
> gpasm.
Now I think about it, some of the changes required for the fixes were 
drastic enough they could cause other bugs, and my whole aim was to work on 
something minor for one final release before we/I make some of the drastic 
changes we've already discussed.  I did learn some things in the process, so 
it's not too big a step backwards to forget it and change directions, 
although I would still recommend applying the first patch since it's 
straightforward.  I think at this point, if the memory leak fixes aren't 
really a step forward, I could change directions and learn just as much 
tracking down other bugs.

BTW, several of the bugs in the tracker seem to have already been fixed.  I 
can't reproduce "[1343989] gpasm RES directive segfault"...do you know if 
that's been fixed?  If you want to give me access I can do some 
housecleaning in the bug tracker, but I'll still get your okay on any 
CVS/svn commits until I'm well in the swing of things.

> However, in general for small programs that have constrained memory
> footprint I don't even bother with deallocating memory.
Is that a design choice or just "laziness"?  I can definitely understand the 
position that every extra bit of code is another opportunity for bugs, 
especially in this case, I've just never seen any indication that it's 
acceptable to release software with memory leaks.  Actually, until recently 
I didn't know the OS would clean up the memory leaks when you close the 
program (I've heard AmigaOS actually wouldn't).  Would there tend to be a 
performance improvement one way or the other?

Regardless, if that's a common position and you know of any discussion or 
articles about it, please point me to them.

David Barnett 

Hi David,

> Actually, until recently I didn't know the OS would clean up the
> memory leaks when you close the program (I've heard AmigaOS actually
> wouldn't).

It's not really cleaning up the memory leaks.  The OS has a concept of a
process, and the resources it's using, and frees those resources for use
by other processes when a process is finished.  This includes normal
memory.  The OS has no concept of malloc() and free() inside the
process;  you probably realise this.

Yes, I've heard a rumour AmigaOS didn't do this.  Also, in most OS there
are types of memory a process can request, e.g. shared memory, that
remain allocated after a process exits by design.

> Would there tend to be a performance improvement one way or the other?

There can be.  Imagine every memory area returned by malloc() has a
couple of words of information before the address it returns.  One might
be the length of the area.  If your program calls free() for this area
then the length has to be read from memory and perhaps, when it's added
back into a "free memory area list" a "next" pointer will be written
into the now freed memory.  This read and write by the processor takes
time, fills process cache lines, perhaps pages in memory from swap
space, etc.  So running through all your data structures freeing them
just before exit() can be a lot more taxing than just exiting allowing
the OS to know all memory pages used by your process are now available.

(Some malloc() implementations may not work like this for the downsides
given, but they used to and some still do.)

> Regardless, if that's a common position and you know of any discussion
> or articles about it, please point me to them.

If you don't mind me saying so, you seem a bit short of experience based
on comments like keeping allocation and deallocation at the same stack
level.  That would possibly suggest using the stack instead of the heap.
A few decent books include

    The C Programming Language, 2 Ed., by Kernighan and Ritchie.
    The Practice of Programming, by Kernighan and Pike.
    The Unix Programming Environment, by Kernighan and Pike.

Cheers,


Ralph.