firstfire |
10-02-2012 11:44 PM |
Hi.
Let's debug your program using valgrind.
Code:
$ gcc -g crash.c && valgrind ./a.out
crash.c: In function ‘main’:
crash.c:78:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
crash.c:79:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
==5524== Memcheck, a memory error detector
==5524== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==5524== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==5524== Command: ./a.out
==5524==
==5524== Invalid write of size 1
==5524== at 0x40091E: main (crash.c:31)
==5524== Address 0x51f1041 is 0 bytes after a block of size 1 alloc'd
==5524== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5524== by 0x40090B: main (crash.c:30)
==5524==
==5524== Invalid read of size 4
==5524== at 0x4EA10EE: fgets (iofgets.c:52)
==5524== by 0x400968: main (crash.c:36)
==5524== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==5524==
==5524==
==5524== Process terminating with default action of signal 11 (SIGSEGV)
==5524== Access not within mapped region at address 0x0
==5524== at 0x4EA10EE: fgets (iofgets.c:52)
==5524== by 0x400968: main (crash.c:36)
==5524== If you believe this happened as a result of a stack
==5524== overflow in your program's main thread (unlikely but
==5524== possible), you can try to increase the size of the
==5524== main thread stack using the --main-stacksize= flag.
==5524== The main thread stack size used in this run was 8388608.
Error opening hometown.map
==5524==
==5524== HEAP SUMMARY:
==5524== in use at exit: 1 bytes in 1 blocks
==5524== total heap usage: 2 allocs, 1 frees, 569 bytes allocated
==5524==
==5524== LEAK SUMMARY:
==5524== definitely lost: 0 bytes in 0 blocks
==5524== indirectly lost: 0 bytes in 0 blocks
==5524== possibly lost: 0 bytes in 0 blocks
==5524== still reachable: 1 bytes in 1 blocks
==5524== suppressed: 0 bytes in 0 blocks
==5524== Rerun with --leak-check=full to see details of leaked memory
==5524==
==5524== For counts of detected and suppressed errors, rerun with: -v
==5524== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 2 from 2)
Okay, we have invalid write on line 31:
Code:
ch = malloc(1);
ch[1] = '\0'; // line 31
This bug has already been discovered by ntubski. And we will see that the same error occurs two more times in the program. You allocated only one byte for ch, but on the next line (N31) you try to assign zero value to the second element of string ch. C uses zero-based indexing: first element has index 0, second element -- 1 etc. So to fix this bug you should write
Code:
ch = malloc(1);
ch[0] = '\0'; // line 31
(BTW why you use one-character dynamic string here instead of just char ch;?)
Fixed.
Repeat the procedure:
Code:
$ gcc -g crash.c && valgrind ./a.out
crash.c: In function ‘main’:
crash.c:78:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
crash.c:79:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
==5568== Memcheck, a memory error detector
==5568== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==5568== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==5568== Command: ./a.out
==5568==
==5568== Invalid read of size 4
==5568== at 0x4EA10EE: fgets (iofgets.c:52)
==5568== by 0x400964: main (crash.c:36)
==5568== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==5568==
==5568==
==5568== Process terminating with default action of signal 11 (SIGSEGV)
==5568== Access not within mapped region at address 0x0
==5568== at 0x4EA10EE: fgets (iofgets.c:52)
==5568== by 0x400964: main (crash.c:36)
==5568== If you believe this happened as a result of a stack
==5568== overflow in your program's main thread (unlikely but
==5568== possible), you can try to increase the size of the
==5568== main thread stack using the --main-stacksize= flag.
==5568== The main thread stack size used in this run was 8388608.
Error opening hometown.map
==5568==
==5568== HEAP SUMMARY:
==5568== in use at exit: 1 bytes in 1 blocks
==5568== total heap usage: 2 allocs, 1 frees, 569 bytes allocated
==5568==
==5568== LEAK SUMMARY:
==5568== definitely lost: 0 bytes in 0 blocks
==5568== indirectly lost: 0 bytes in 0 blocks
==5568== possibly lost: 0 bytes in 0 blocks
==5568== still reachable: 1 bytes in 1 blocks
==5568== suppressed: 0 bytes in 0 blocks
==5568== Rerun with --leak-check=full to see details of leaked memory
==5568==
==5568== For counts of detected and suppressed errors, rerun with: -v
==5568== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)
What's wrong with line 36:
Code:
fgets(name, 100, fp);
? Looks good. But couple of lines above we see
Code:
if ((fp = fopen(".hometown.map", "r")) == NULL)
printf("Error opening hometown.map\r\n");
And we also see your error message "Error opening hometown.map". There are no such file as ".hometown.map" in my /tmp directory, naturally. The program tells us that there are no such file and just continues execution! Not good!
Code:
if ((fp = fopen(".hometown.map", "r")) == NULL) {
printf("Error opening hometown.map\r\n");
free(ch); // to calm down valgrind
exit(EXIT_FAILURE);
}
Fixed.
Continue:
Code:
$ touch .hometown.map
$ gcc -g crash.c && valgrind ./a.out
crash.c: In function ‘main’:
crash.c:81:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
crash.c:82:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
==5610== Memcheck, a memory error detector
==5610== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==5610== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==5610== Command: ./a.out
==5610==
==5610== Invalid write of size 8
==5610== at 0x400B72: main (crash.c:68)
==5610== Address 0x51f1318 is 0 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400BAE: main (crash.c:69)
==5610== Address 0x51f1318 is 0 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400BC4: main (crash.c:70)
==5610== Address 0x51f1318 is 0 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid write of size 1
==5610== at 0x400BFC: main (crash.c:70)
==5610== Address 0x51f1364 is 0 bytes after a block of size 4 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B67: main (crash.c:68)
==5610==
==5610== Invalid write of size 8
==5610== at 0x400C45: main (crash.c:72)
==5610== Address 0x51f1320 is 8 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400C87: main (crash.c:73)
==5610== Address 0x51f1320 is 8 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400C9D: main (crash.c:74)
==5610== Address 0x51f1320 is 8 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid write of size 1
==5610== at 0x400CD8: main (crash.c:74)
==5610== Address 0x51f13b5 is 0 bytes after a block of size 5 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400C3A: main (crash.c:72)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400D27: main (crash.c:79)
==5610== Address 0x51f1318 is 0 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Conditional jump or move depends on uninitialised value(s)
==5610== at 0x4E7D3D1: vfprintf (vfprintf.c:1630)
==5610== by 0x4E858F8: printf (printf.c:35)
==5610== by 0x400D3F: main (crash.c:79)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400D47: main (crash.c:80)
==5610== Address 0x51f1320 is 8 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Conditional jump or move depends on uninitialised value(s)
==5610== at 0x4E7D3D1: vfprintf (vfprintf.c:1630)
==5610== by 0x4E858F8: printf (printf.c:35)
==5610== by 0x400D5F: main (crash.c:80)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400DEE: main (crash.c:83)
==5610== Address 0x51f1320 is 8 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
==5610== Invalid read of size 8
==5610== at 0x400E0B: main (crash.c:85)
==5610== Address 0x51f1318 is 0 bytes after a block of size 8 alloc'd
==5610== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5610== by 0x400B06: main (crash.c:63)
==5610==
top->width = -1
top->height = 0
top->name = NUL
top->description = NULL
length of name: 3
length of desc: 4
descr freed
name freed
top freed
==5610==
==5610== HEAP SUMMARY:
==5610== in use at exit: 0 bytes in 0 blocks
==5610== total heap usage: 5 allocs, 5 frees, 586 bytes allocated
==5610==
==5610== All heap blocks were freed -- no leaks are possible
==5610==
==5610== For counts of detected and suppressed errors, rerun with: -v
==5610== Use --track-origins=yes to see where uninitialised values come from
==5610== ERROR SUMMARY: 14 errors from 14 contexts (suppressed: 2 from 2)
Oh no, not again..
Line 68:
Code:
top->name = malloc(strlen(name) + 1);
Can we write to top->name? Did we allocated space for it?
Code:
top = malloc(sizeof(top));
Hmm.. Yes? NO! What is top?
It is a pointer, its size is something like 4 or 8 bytes. Hardly enough.. One should write
Code:
top = malloc(sizeof(struct areas));
Fixed.
Continue:
Code:
$ gcc -g crash.c && valgrind ./a.out
crash.c: In function ‘main’:
crash.c:81:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
crash.c:82:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
==5866== Memcheck, a memory error detector
==5866== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==5866== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==5866== Command: ./a.out
==5866==
==5866== Invalid write of size 1
==5866== at 0x400BFC: main (crash.c:70)
==5866== Address 0x51f1374 is 0 bytes after a block of size 4 alloc'd
==5866== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5866== by 0x400B67: main (crash.c:68)
==5866==
==5866== Invalid write of size 1
==5866== at 0x400CD8: main (crash.c:74)
==5866== Address 0x51f13c5 is 0 bytes after a block of size 5 alloc'd
==5866== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5866== by 0x400C3A: main (crash.c:72)
==5866==
==5866== Conditional jump or move depends on uninitialised value(s)
==5866== at 0x4E7D3D1: vfprintf (vfprintf.c:1630)
==5866== by 0x4E858F8: printf (printf.c:35)
==5866== by 0x400D3F: main (crash.c:79)
==5866==
==5866== Conditional jump or move depends on uninitialised value(s)
==5866== at 0x4E7D3D1: vfprintf (vfprintf.c:1630)
==5866== by 0x4E858F8: printf (printf.c:35)
==5866== by 0x400D5F: main (crash.c:80)
==5866==
top->width = -1
top->height = 0
top->name = NUL
top->description = NULL
length of name: 3
length of desc: 4
descr freed
name freed
top freed
==5866==
==5866== HEAP SUMMARY:
==5866== in use at exit: 0 bytes in 0 blocks
==5866== total heap usage: 5 allocs, 5 frees, 610 bytes allocated
==5866==
==5866== All heap blocks were freed -- no leaks are possible
==5866==
==5866== For counts of detected and suppressed errors, rerun with: -v
==5866== Use --track-origins=yes to see where uninitialised values come from
==5866== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 2 from 2)
Invalid writes on lines 70, 74:
Code:
top->name = malloc(strlen(name) + 1);
strncpy(top->name, name, strlen(name));
top->name[strlen(name) + 1] = 0x00; // line 70
top->descr = malloc(strlen(desc) + 1);
strncpy(top->descr, desc, strlen(desc));
top->descr[strlen(desc) + 1] = 0x00; // line 74
Again, you allocate array of strlen(name)+1 bytes and write to the last element, which should be (srtlen(name)+1)-1 = strlen(name):
Code:
top->name = malloc(strlen(name) + 1);
strncpy(top->name, name, strlen(name));
top->name[strlen(name) ] = 0x00; // line 70
top->descr = malloc(strlen(desc) + 1);
strncpy(top->descr, desc, strlen(desc));
top->descr[strlen(desc) ] = 0x00; // line 74
Fixed:
Code:
$ gcc -g crash.c && valgrind ./a.out
crash.c: In function ‘main’:
crash.c:81:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
crash.c:82:6: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t’ [-Wformat]
==5915== Memcheck, a memory error detector
==5915== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==5915== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==5915== Command: ./a.out
==5915==
top->width = -1
top->height = 0
top->name = NUL
top->description = NULL
length of name: 3
length of desc: 4
descr freed
name freed
top freed
==5915==
==5915== HEAP SUMMARY:
==5915== in use at exit: 0 bytes in 0 blocks
==5915== total heap usage: 5 allocs, 5 frees, 610 bytes allocated
==5915==
==5915== All heap blocks were freed -- no leaks are possible
==5915==
==5915== For counts of detected and suppressed errors, rerun with: -v
==5915== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
Hurrah!
|