Valgrind errors: valid, or false positives ?

[Simple Simon]

It's entirely possible that I'm completely in the wrong line of work,
but I don't see what Valgrind is complaining about in this code.

Invalid read/writes?

If I free the within main(), rather than within free_node_list(),
Valgrind reports 0 leaks?

OS: SuSE 9.3 Linux 2.6.11.4-21.11-default i686 i386
GCC: 3.3.5 20050117

CODE:

#include <stdio.h>
#include <stdlib.h>

struct node_tag {
int value ;
struct node_tag * next ;
} ;
typedef struct node_tag node ;

struct node_list_tag {
node * first ;
node * last ;
} ;
typedef struct node_list_tag node_list ;

int populate_node_list( node_list *, int ) ;
int print_node_list( node_list * ) ;
int free_node_list( node_list * ) ;

int
main(void) {
int i ;
node * n = NULL, * next = NULL ;
node_list nl = {0,0} ;
// populate
for ( i = 0 ; i < 100000 ; i++ )
{
populate_node_list( &nl, i ) ;
}
print_node_list( &nl ) ;
free_node_list( &nl ) ;
printf( "free()'ed node list: %p, %p\n", nl.first, nl.last ) ;
return 0 ;
}

int
populate_node_list( node_list * nlp, int val )
{
node * nn = NULL ;
nn = calloc( 1, sizeof(int) ) ;
nn->value = val;
if ( nlp->first == NULL )
nlp->first = nlp->last = nn ;
else
{
nlp->last->next = nn ;
nlp->last = nn ;
}
return 0 ;
}

int
print_node_list( node_list * nlp )
{
int i = 1 ;
node * nn = nlp->first ;

while ( nn )
{
//printf( "node: %.5d\tvalue: %.5d\n", i++, nn->value ) ;
if ( nn->next == NULL )
break ;

nn = nn->next ;
}
nlp->first = nlp->last = NULL ;

return 0 ;
}

int
free_node_list( node_list * nlp )
{
node * cur = nlp->first, * next ;
while ( cur )
{
next = cur->next ;
free( cur ) ;
cur = next ;
}
return 0 ;
}

Valgrind output (trimmed):

me@host:~/misc> gcc -O0 -ggdb -o valtest valtest.c
me@host:~/misc> valgrind -v --leak-check=yes ./valtest
==21068== Memcheck, a memory error detector.
==21068== Using valgrind-3.2.1, a dynamic binary instrumentation
framework.
--21068-- Contents of /proc/version:
--21068-- Linux version 2.6.11.4-21.11-default (geeko@buildhost)
(gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)) #1 Thu Feb
2 20:54:26 UTC 2006
--21068-- Arch and hwcaps: X86, x86-sse1-sse2
==21068== WARNING: new redirection conflicts with existing --
ignoring it
--21068-- new: 0x04012B60 (index ) R-> 0x0401DFC0 index

==21068== Invalid write of size 4
==21068== at 0x8048502: populate_node_list (valtest.c:58)
==21068== by 0x8048474: main (valtest.c:40)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
node: 00001 value: 00000
==21068==
==21068== Invalid read of size 4
==21068== at 0x8048553: print_node_list (valtest.c:73)
==21068== by 0x804848A: main (valtest.c:42)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
==21068==
==21068== Invalid read of size 4
==21068== at 0x804855E: print_node_list (valtest.c:76)
==21068== by 0x804848A: main (valtest.c:42)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)

--21068-- REDIR: 0x40A1980 (strnlen) redirected to 0x401E200
(strnlen)
free()'ed node list: (nil), (nil)
--21068-- REDIR: 0x409C640 (free) redirected to 0x401D047 (free)
==21068==
==21068== ERROR SUMMARY: 299998 errors from 3 contexts (suppressed:
11 from 1)
==21068==
==21068== 99999 errors in context 1 of 3:
==21068== Invalid read of size 4
==21068== at 0x804855E: print_node_list (valtest.c:76)
==21068== by 0x804848A: main (valtest.c:42)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
==21068==
==21068== 99999 errors in context 2 of 3:
==21068== Invalid write of size 4
==21068== at 0x8048502: populate_node_list (valtest.c:58)
==21068== by 0x8048474: main (valtest.c:40)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
==21068==
==21068== 100000 errors in context 3 of 3:
==21068== Invalid read of size 4
==21068== at 0x8048553: print_node_list (valtest.c:73)
==21068== by 0x804848A: main (valtest.c:42)
==21068== Address 0x415402C is 0 bytes after a block of size 4
alloc'd
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
--21068--
--21068-- supp: 11 Ubuntu-stripped-ld.so
==21068==
==21068== IN SUMMARY: 299998 errors from 3 contexts (suppressed: 11
from 1)
==21068==
==21068== malloc/free: in use at exit: 400,000 bytes in 100,000
blocks.
==21068== malloc/free: 100,000 allocs, 0 frees, 400,000 bytes
allocated.
==21068==
==21068== searching for pointers to 100,000 not-freed blocks.
==21068== checked 56,584 bytes.
==21068==
==21068==
==21068== 400,000 bytes in 100,000 blocks are definitely lost in loss
record 1 of 1
==21068== at 0x401D940: calloc (vg_replace_malloc.c:279)
==21068== by 0x80484D2: populate_node_list (valtest.c:52)
==21068== by 0x8048474: main (valtest.c:40)
==21068==
==21068== LEAK SUMMARY:
==21068== definitely lost: 400,000 bytes in 100,000 blocks.
==21068== possibly lost: 0 bytes in 0 blocks.
==21068== still reachable: 0 bytes in 0 blocks.
==21068== suppressed: 0 bytes in 0 blocks.

 

[Eric Sosman]

It's entirely possible that I'm completely in the wrong line of work,
but I don't see what Valgrind is complaining about in this code.
[...]
struct node_tag {
int value ;
struct node_tag * next ;
} ;
typedef struct node_tag node ;
[...]
node * nn = NULL ;
nn = calloc( 1, sizeof(int) ) ;

^^^^^^^^^^^

This would not have happened if you had used the Guaranteed
100% Fat-Free All-Singing All-Dancing Preferred Idiom:

nn = calloc(1, sizeof *nn);

 

[Paul Pluzhnikov]

It's entirely possible that I'm completely in the wrong line of work,
but I don't see what Valgrind is complaining about in this code.

Perhaps this Insure++ report will make it clearer for you?
It contains the exact same info, except line numbers have changed.

VG output from your test case on my machine:

==19256== Invalid write of size 4
==19256== at 0x80484B2: populate_node_list (junk.c:46)
==19256== by 0x8048424: main (junk.c:28)
==19256== Address 0x1B91802C is 0 bytes after a block of size 4 alloc'd
==19256== at 0x1B902BD5: calloc (vg_replace_malloc.c:175)
==19256== by 0x8048482: populate_node_list (junk.c:40)
==19256== by 0x8048424: main (junk.c:28)

Insure output from the same:

[junk.c:46] **WRITE_OVERFLOW**
Structure reference out of range: (nlp->last)

bbbbb
| 4 | 4 |
wwwww

Writing (w) : 0x0894c01c thru 0x0894c01f (4 bytes)
To block (b) : 0x0894c018 thru 0x0894c01b (4 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
populate_node_list() junk.c, 46
main() junk.c, 28

**Memory corrupted. Program may crash!!**

Invalid read/writes?

Yes, writing beyond allocated memory is invalid.

If I free the within main(), rather than within free_node_list(),
Valgrind reports 0 leaks?

Where you free (or whether you free at all) has nothing to do with
invalid writes.

int
populate_node_list( node_list * nlp, int val )
{
node * nn = NULL ;

This is line 40:

nn = calloc( 1, sizeof(int) ) ;

Should be:

nn = calloc( 1, sizeof(*nn) ) ;

nn->value = val;
if ( nlp->first == NULL )
nlp->first = nlp->last = nn ;
else
{
nlp->last->next = nn ;
nlp->last = nn ;
}
return 0 ;
}

After the fix above, all invalid read/writes are gone, and you are just left with

==19303== LEAK SUMMARY:
==19303== definitely lost: 515528 bytes in 64441 blocks.

Here is what Insure has to tell you about that leak:

[junk.c:66] **LEAK_ASSIGN**
Memory leaked due to pointer reassignment: nn

Lost block : 0x09632018 thru 0x0963201f (8 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
print_node_list() junk.c, 66
main() junk.c, 30

[junk.c:68] **LEAK_SCOPE**
Memory leaked leaving scope: nn

Lost block : 0x096321c8 thru 0x096321cf (8 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
print_node_list() junk.c, 68
main() junk.c, 30


It is unusual for "print-data-structure" routine to modify that
structure. In your case, line 66 should be deleted entirely.

nlp->first = nlp->last = NULL ; // line 66


Cheers,

 

[Simple Simon]

(e-mail address removed) wrote...

It's entirely possible that I'm completely in the wrong line of work,
but I don't see what Valgrind is complaining about in this code.
[...]
struct node_tag {
int value ;
struct node_tag * next ;
} ;
typedef struct node_tag node ;
[...]
node * nn = NULL ;
nn = calloc( 1, sizeof(int) ) ;

^^^^^^^^^^^

This would not have happened if you had used the Guaranteed
100% Fat-Free All-Singing All-Dancing Preferred Idiom:

nn = calloc(1, sizeof *nn);

Oh, ****.

"Career-Changes-R-Us? Send me a pamphlet right away."

 

[Simple Simon]

(e-mail address removed) wrote...

Perhaps this Insure++ report will make it clearer for you?
It contains the exact same info, except line numbers have changed.

Insure++, and your post, are stunningly illuminating. Thank you for
the time you took on this.

That bug in print_node_list() was just stoopid programmer stuff --
that assignment was supposed to be in free_node_list() instead.

VG output from your test case on my machine:

==19256== Invalid write of size 4
==19256== at 0x80484B2: populate_node_list (junk.c:46)
==19256== by 0x8048424: main (junk.c:28)
==19256== Address 0x1B91802C is 0 bytes after a block of size 4 alloc'd
==19256== at 0x1B902BD5: calloc (vg_replace_malloc.c:175)
==19256== by 0x8048482: populate_node_list (junk.c:40)
==19256== by 0x8048424: main (junk.c:28)

Insure output from the same:

[junk.c:46] **WRITE_OVERFLOW**
Structure reference out of range: (nlp->last)

bbbbb
| 4 | 4 |
wwwww

Writing (w) : 0x0894c01c thru 0x0894c01f (4 bytes)
To block (b) : 0x0894c018 thru 0x0894c01b (4 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
populate_node_list() junk.c, 46
main() junk.c, 28

**Memory corrupted. Program may crash!!**

Invalid read/writes?

Yes, writing beyond allocated memory is invalid.

If I free the within main(), rather than within free_node_list(),
Valgrind reports 0 leaks?

Where you free (or whether you free at all) has nothing to do with
invalid writes.

int
populate_node_list( node_list * nlp, int val )
{
node * nn = NULL ;

This is line 40:

nn = calloc( 1, sizeof(int) ) ;

Should be:

nn = calloc( 1, sizeof(*nn) ) ;

nn->value = val;
if ( nlp->first == NULL )
nlp->first = nlp->last = nn ;
else
{
nlp->last->next = nn ;
nlp->last = nn ;
}
return 0 ;
}

After the fix above, all invalid read/writes are gone, and you are just left with

==19303== LEAK SUMMARY:
==19303== definitely lost: 515528 bytes in 64441 blocks.

Here is what Insure has to tell you about that leak:

[junk.c:66] **LEAK_ASSIGN**
Memory leaked due to pointer reassignment: nn

Lost block : 0x09632018 thru 0x0963201f (8 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
print_node_list() junk.c, 66
main() junk.c, 30

[junk.c:68] **LEAK_SCOPE**
Memory leaked leaving scope: nn

Lost block : 0x096321c8 thru 0x096321cf (8 bytes)
nn, allocated at junk.c, 40
calloc() (interface)
populate_node_list() junk.c, 40
main() junk.c, 28

Stack trace where the error occurred:
print_node_list() junk.c, 68
main() junk.c, 30


It is unusual for "print-data-structure" routine to modify that
structure. In your case, line 66 should be deleted entirely.

nlp->first = nlp->last = NULL ; // line 66


Cheers,