Valgrind : invalid read of size

Version imprimable

Bonjour,

je développe en C un programme permettant de gérer des lieux possédant un lieu au nord, un au sud, un à l'ouest et un à l'est.
Pour cela, je créé une structure Place et deux fonctions (une pour créer et l'autre pour libérer l'espace mémoire alloué).
Le problème est que dans certains cas, valgrind détecte des "Invalid read of size" dans la fonction de libération.
Pouvez-vous me dire quelle(s) erreur(s) il y a dans ma fonction de libération?

Merci.

Voici le code :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
 
#include <stdlib.h> /* for malloc */
#include <stdio.h> /* for fprintf */
typedef struct place
{
	int freeing;
	struct place* n;
	struct place* s;
	struct place* w;
	struct place* e;
} Place;
Place* Place_New()
{
	Place* place;
	place = malloc(sizeof(Place));
	if(place == NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		return(NULL);
	}
	else
	{
		place->freeing = 0;
		place->n = NULL;
		place->s = NULL;
		place->w = NULL;
		place->e = NULL;
	}
	return(place);
}
int Place_Free(Place* place)
{
	if(place==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		return(-1);
	}
	printf("Free\n");
	place->freeing = 1;
	if(place->n != NULL && place->n->freeing == 0)
	{
		Place_Free(place->n);
		place->n = NULL;
	}
	if(place->s != NULL && place->s->freeing == 0)
	{
		Place_Free(place->s);
		place->s = NULL;
	}
	if(place->w != NULL && place->w->freeing == 0)
	{
		Place_Free(place->w);
		place->w = NULL;
	}
	if(place->e != NULL && place->e->freeing == 0)
	{
		Place_Free(place->e);
		place->e = NULL;
	}
	free(place);
	place = NULL;
	return(0);
}
int main(int argc, char *argv[])
{
	/* Create the Place objects */
	Place* place = Place_New();
	if(place==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n = Place_New();
	if(place_n==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s = Place_New();
	if(place_s==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_w = Place_New();
	if(place_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_e = Place_New();
	if(place_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n_w = Place_New();
	if(place_n_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n_e = Place_New();
	if(place_n_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s_w = Place_New();
	if(place_s_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s_e = Place_New();
	if(place_s_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	place->n = place_n;
	place->s = place_s;
	place->w = place_w;
	place->e = place_e;
	place->n->n = NULL;
	place->n->s = place;
	place->n->w = place_n_w;
	place->n->e = place_n_e;
	place->s->n = place;
	place->s->s = NULL;
	place->s->w = place_s_w;
	place->s->e = place_s_e;
	place->w->n = place_n_w;
	place->w->s = place_s_w;
	place->w->w = NULL;
	place->w->e = place;
	place->e->n = place_n_e;
	place->e->s = place_s_e;
	place->e->w = place;
	place->e->e = NULL;
	/* Free the Place objects */
	if(Place_Free(place)==-1)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	exit(EXIT_SUCCESS);
	return(0);
}

Voici les erreurs détectées par Valgrind :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
 
$ valgrind ./main.out
==9729== Memcheck, a memory error detector.
==9729== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==9729== Using LibVEX rev 1804, a library for dynamic binary translation.
==9729== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==9729== Using valgrind-3.3.0, a dynamic binary instrumentation framework.
==9729== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==9729== For more details, rerun with: -v
==9729==
Free
Free
Free
Free
Free
Free
Free
Free
==9729== Invalid read of size 4
==9729==    at 0x80485FA: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048672: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==  Address 0x4431190 is 0 bytes inside a block of size 20 free'd
==9729==    at 0x4023359: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==9729==    by 0x80486BC: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048672: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x804860E: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==
==9729== Invalid read of size 4
==9729==    at 0x804862C: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048672: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==  Address 0x4431220 is 0 bytes inside a block of size 20 free'd
==9729==    at 0x4023359: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==9729==    by 0x80486BC: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048672: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048640: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
Free
==9729==
==9729== Invalid read of size 4
==9729==    at 0x80485FA: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80486A4: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==  Address 0x44311d8 is 0 bytes inside a block of size 20 free'd
==9729==    at 0x4023359: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==9729==    by 0x80486BC: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80486A4: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x804860E: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==
==9729== Invalid read of size 4
==9729==    at 0x804862C: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80486A4: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==  Address 0x4431268 is 0 bytes inside a block of size 20 free'd
==9729==    at 0x4023359: free (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==9729==    by 0x80486BC: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80486A4: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x8048640: Place_Free (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==    by 0x80489E0: main (in /home/yugiohjcj/Documents/Programmes/Tests/C/test_free/main.out)
==9729==
==9729== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 37 from 1)
==9729== malloc/free: in use at exit: 0 bytes in 0 blocks.
==9729== malloc/free: 9 allocs, 9 frees, 180 bytes allocated.
==9729== For counts of detected errors, rerun with: -v
==9729== All heap blocks were freed -- no leaks are possible.

La construction est incorrecte : les pointeurs des "coins" ne sont pas initialisés

On obtient ceci (les cellules (en rouge) suivies de leur 4 pointeurs; Le pointeur NULL est noté .) symboliquement :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 n_w n n_e . . . . place . . n_w . . n_e . w place e n_w n n_e s_w s s_e . w place place e . s_w s s_e . place . . . . . s_w . . s_e .
L'ordre des destructions, sauf erreur, sera alors :
free() de n_w, n_e, n, s_w, s_e, s

puis appel de la fonction Place_Free() pour w : celle-ci tente de détruire n_w qui l'a déjà été :

Code:

if(place->n != NULL && place->n->freeing == 0)

accès à un élément libéré -> plantage

09/07/2010, 15h15
YuGiOhJCJ

OK.

Je pense comprendre pourquoi la fonction Place_Free() pour w tente de détruire n_w : le pointeur pour w vers le nord pointe vers la zone mémoire qui a été libérée précédemment.

Donc, il faut qu'au moment ou je libère n_w, je parvienne à mette à NULL tous les pointeurs vers ce lieu.

Je vais essayer de corriger cela...

Voilà j'ai trouvé une solution :
-j'ai corrigé les pointeurs des "coins" en les initialisant (comme diogene me l'avait fait remarquer)
-j'ai mis à NULL tous les pointeurs vers un lieu lorsqu'on le libère (comme je l'avais proposé)

Mais même si cela fonctionne (valgrind ne détecte plus d'erreur), j'ai du mal à comprendre pourquoi je n'ai pas pu laisser à NULL les pointeurs des "coins"...

Quelqu'un peut il m'expliquer à nouveau?

L'ordre de libération est donc (sauf erreur de ma part) :

n_w
n_e
n
s_w
s_e
s
w
e
place

Voici le code corrigé :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
 
#include <stdlib.h> /* for malloc */
#include <stdio.h> /* for fprintf */
typedef struct place
{
	int freeing;
	struct place* n;
	struct place* s;
	struct place* w;
	struct place* e;
} Place;
Place* Place_New()
{
	Place* place;
	place = malloc(sizeof(Place));
	if(place == NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		return(NULL);
	}
	else
	{
		place->freeing = 0;
		place->n = NULL;
		place->s = NULL;
		place->w = NULL;
		place->e = NULL;
	}
	return(place);
}
int Place_Free(Place* place)
{
	if(place==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		return(-1);
	}
	place->freeing = 1;
	if(place->n != NULL)
	{
		if(place->n->freeing == 0)
		{
			Place_Free(place->n);
		}
	}
	if(place->s != NULL)
	{
		if(place->s->freeing == 0)
		{
			Place_Free(place->s);
		}
	}
	if(place->w != NULL)
	{
		if(place->w->freeing == 0)
		{
			Place_Free(place->w);
		}
	}
	if(place->e != NULL)
	{
		if(place->e->freeing == 0)
		{
			Place_Free(place->e);
		}
	}
	printf("Free\n");
	if(place->n != NULL)
	{
		printf("n\n");
		place->n->s = NULL;
	}
	if(place->s != NULL)
	{
		printf("s\n");
		place->s->n = NULL;
	}
	if(place->w != NULL)
	{
		printf("w\n");
		place->w->e = NULL;
	}
	if(place->e != NULL)
	{
		printf("e\n");
		place->e->w = NULL;
	}
	free(place);
	place = NULL;
	return(0);
}
int main(int argc, char *argv[])
{
	/* Create the Place objects */
	Place* place = Place_New();
	if(place==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n = Place_New();
	if(place_n==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s = Place_New();
	if(place_s==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_w = Place_New();
	if(place_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_e = Place_New();
	if(place_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n_w = Place_New();
	if(place_n_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_n_e = Place_New();
	if(place_n_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s_w = Place_New();
	if(place_s_w==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	Place* place_s_e = Place_New();
	if(place_s_e==NULL)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	place->n = place_n;
	place->s = place_s;
	place->w = place_w;
	place->e = place_e;
	place->n->n = NULL;
	place->n->s = place;
	place->n->w = place_n_w;
	place->n->e = place_n_e;
	place->s->n = place;
	place->s->s = NULL;
	place->s->w = place_s_w;
	place->s->e = place_s_e;
	place->w->n = place_n_w;
	place->w->s = place_s_w;
	place->w->w = NULL;
	place->w->e = place;
	place->e->n = place_n_e;
	place->e->s = place_s_e;
	place->e->w = place;
	place->e->e = NULL;
	place->n->w->n = NULL;
	place->n->w->s = place_w;
	place->n->w->w = NULL;
	place->n->w->e = place_n;
	place->n->e->n = NULL;
	place->n->e->s = place_e;
	place->n->e->w = place_n;
	place->n->e->e = NULL;
	place->s->w->n = place_w;
	place->s->w->s = NULL;
	place->s->w->w = NULL;
	place->s->w->e = place_s;
	place->s->e->n = place_e;
	place->s->e->s = NULL;
	place->s->e->w = place_s;
	place->s->e->e = NULL;
	/* Free the Place objects */
	if(Place_Free(place)==-1)
	{
		fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, __PRETTY_FUNCTION__);
		exit(EXIT_FAILURE);
	}
	exit(EXIT_SUCCESS);
	return(0);
}

09/07/2010, 18h41
diogene
*

Citation:

Mais même si cela fonctionne (valgrind ne détecte plus d'erreur), j'ai du mal à comprendre pourquoi je n'ai pas pu laisser à NULL les pointeurs des "coins"...

Si on laisse à NULL tous les pointeurs des coins on va avoir la séquence :
free() : n_w impossible de remettre à NULL les pointeurs w de n (n->w) et n de w (w->n)
free() : n_e impossible de remettre à NULL les pointeurs e de n (n->e) et n de e (e->n)
free() : n comporte deux pointeurs (w et e) dans les décors. La remise à NULL de n->w->e et n->e->w. Conséquence : va planter

* De même, il n'est pas suffisant de mettre correctement les pointeurs des coins sans remettre à NULL les pointeurs vers les blocs détruits car on va faire des accès à des zones détruites.

* Avec le code que tu présentes, on devrait avoir l'ordre de libération suivant :

free() : n_e
free() : e
free() : s_e
free() : s
free() : s_w
free() : w
free() : n_w
free() : n
free() : place

* Remarque :
1- si le tableau est grand, attention à la profondeur de la récursivité .
2- le code serait plus simple si tu utilisais un tableau de pointeurs (plus souple, possibilité de faire des boucles) :
Code:

1 2 3 4 5 6 typedef enum {n,s,w,e} DIR; typedef struct place { int freeing; struct place* dir[4]; } Place;