Re: simple valgrind test found what appear to be serious memory problems

[Paul Eggert]

On 09/30/10 08:10, Thien-Thi Nguyen wrote:
> Running valgrind with the
> option --db-attach=yes, then typing y RET to have gdb attach to
> it, then examining values => things look as they should

By "things look as they should" I assume you mean that at the
affected line (b-grok.c:794, "if (ny->branches)"), things look
OK, because ny->branches is NULL.  But that doesn't mean there's
not a problem here.  Although ny->branches happens to be NULL,
which is what we want, it is indeed uninitialized, and
it might not be NULL on some other platform.

You can easily very that ny->branches is uninitialized by
temporarily trying the following change.

--- a/src/b-divvy.c                                                             
+++ b/src/b-divvy.c                                                             
@@ -99,7 +99,9 @@ alloc (struct divvy *divvy, char const *what USED_FOR_DEBUG, \
size_t len)
   complain ("%s: %6u  %s\n", divvy->name, len, what);                          
 #endif                                                                         
   divvy->count++;                                                              
-  return obstack_alloc (divvy->space, len);                                    
+  void *p = obstack_alloc (divvy->space, len);                                 
+  memset (p, 0x81, len);                                                       
+  return p;                                                                    
 }                                                                              
                                                                                
 void *

> Now i will grep the net for valgrind + obstacks, as i suspect
> this (relatively) unconventional dynamic memory allocation scheme
> is not as well supported as the standard malloc+munge+free usage

Yes, that's true.  Three thoughts, though.

First, valgrind was working just fine here.  Although valgrind
is not perfect, it's pretty good, and its diagnostics should not
be dismissed without understanding exactly why they are false
alarms.

Second, there are real advantages to the standard malloc+free forms:
among other things, programmers (and other tools) will understand
them better.  Is there a compelling reason for switching to the
unconventional form?

Third, I'm not happy with the idea of debugging this stuff via
valgrind.  Valgrind is not meant to be a first line of defense
against problems in software design or maintenance, and if it
is finding several problems like this, that's an indication that
the software development practices need to be changed, perhaps
by a careful code audit.