[RFA:] valgrind annotations for ggc-page.c, ggc-common.c
Hans-Peter Nilsson
hp@bitrange.com
Wed Nov 20 12:08:00 GMT 2002
On Tue, 19 Nov 2002, Richard Henderson wrote:
> On Mon, Nov 18, 2002 at 10:14:20PM -0500, Hans-Peter Nilsson wrote:
> > * ggc-common.c (ggc_realloc) [ENABLE_VALGRIND_CHECKING]: Update
> > annotations.
> > * ggc-page.c (alloc_anon, free_page, ggc_alloc, poison_pages)
> > [ENABLE_VALGRIND_CHECKING]: Add machinery to annotate memory.
>
> Ok.
Thanks for the review.
Likewise as for cppfiles.c, I committed this instead, as an
obvious change. Tested together with the cppfiles.c change.
* ggc-common.c [!ENABLE_VALGRIND_CHECKING] (VALGRIND_DISCARD):
Define as empty.
(ggc_realloc): Update valgrind annotations.
* ggc-page.c [!ENABLE_VALGRIND_CHECKING] (VALGRIND_DISCARD):
Define as empty.
(alloc_anon, free_page, ggc_alloc, poison_pages): Add machinery to
valgrind-annotate memory.
Index: ggc-page.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-page.c,v
retrieving revision 1.56
diff -p -c -r1.56 ggc-page.c
*** ggc-page.c 12 Nov 2002 00:27:31 -0000 1.56
--- ggc-page.c 20 Nov 2002 00:02:38 -0000
*************** Software Foundation, 59 Temple Place - S
*** 29,34 ****
--- 29,40 ----
#include "ggc.h"
#include "timevar.h"
#include "params.h"
+ #ifdef ENABLE_VALGRIND_CHECKING
+ #include <valgrind.h>
+ #else
+ /* Avoid #ifdef:s when we can help it. */
+ #define VALGRIND_DISCARD(x)
+ #endif
/* Prefer MAP_ANON(YMOUS) to /dev/zero, since we don't need to keep a
file open. Prefer either to valloc. */
*************** alloc_anon (pref, size)
*** 524,529 ****
--- 530,540 ----
/* Remember that we allocated this memory. */
G.bytes_mapped += size;
+ /* Pretend we don't have access to the allocated pages. We'll enable
+ access to smaller pieces of the area in ggc_alloc. Discard the
+ handle to avoid handle leak. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS (page, size));
+
return page;
}
#endif
*************** free_page (entry)
*** 750,755 ****
--- 761,770 ----
"Deallocating page at %p, data %p-%p\n", (PTR) entry,
entry->page, entry->page + entry->bytes - 1);
+ /* Mark the page as inaccessible. Discard the handle to avoid handle
+ leak. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS (entry->page, entry->bytes));
+
set_page_table_entry (entry->page, NULL);
#ifdef USING_MALLOC_PAGE_GROUPS
*************** ggc_alloc (size)
*** 943,953 ****
--- 958,984 ----
result = entry->page + object_offset;
#ifdef ENABLE_GC_CHECKING
+ /* Keep poisoning-by-writing-0xaf the object, in an attempt to keep the
+ exact same semantics in presence of memory bugs, regardless of
+ ENABLE_VALGRIND_CHECKING. We override this request below. Drop the
+ handle to avoid handle leak. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_WRITABLE (result, OBJECT_SIZE (order)));
+
/* `Poison' the entire allocated object, including any padding at
the end. */
memset (result, 0xaf, OBJECT_SIZE (order));
+
+ /* Make the bytes after the end of the object unaccessible. Discard the
+ handle to avoid handle leak. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS ((char *) result + size,
+ OBJECT_SIZE (order) - size));
#endif
+ /* Tell Valgrind that the memory is there, but its content isn't
+ defined. The bytes at the end of the object are still marked
+ unaccessible. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_WRITABLE (result, size));
+
/* Keep track of how many bytes are being allocated. This
information is used in deciding when to collect. */
G.allocated += OBJECT_SIZE (order);
*************** poison_pages ()
*** 1433,1439 ****
word = i / HOST_BITS_PER_LONG;
bit = i % HOST_BITS_PER_LONG;
if (((p->in_use_p[word] >> bit) & 1) == 0)
! memset (p->page + i * size, 0xa5, size);
}
}
}
--- 1464,1482 ----
word = i / HOST_BITS_PER_LONG;
bit = i % HOST_BITS_PER_LONG;
if (((p->in_use_p[word] >> bit) & 1) == 0)
! {
! char *object = p->page + i * size;
!
! /* Keep poison-by-write when we expect to use Valgrind,
! so the exact same memory semantics is kept, in case
! there are memory errors. We override this request
! below. */
! VALGRIND_DISCARD (VALGRIND_MAKE_WRITABLE (object, size));
! memset (object, 0xa5, size);
!
! /* Drop the handle to avoid handle leak. */
! VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS (object, size));
! }
}
}
}
Index: ggc-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ggc-common.c,v
retrieving revision 1.56
diff -p -c -r1.56 ggc-common.c
*** ggc-common.c 16 Sep 2002 18:33:18 -0000 1.56
--- ggc-common.c 20 Nov 2002 00:02:38 -0000
*************** Software Foundation, 59 Temple Place - S
*** 30,35 ****
--- 30,41 ----
#include "varray.h"
#include "ggc.h"
#include "langhooks.h"
+ #ifdef ENABLE_VALGRIND_CHECKING
+ #include <valgrind.h>
+ #else
+ /* Avoid #ifdef:s when we can help it. */
+ #define VALGRIND_DISCARD(x)
+ #endif
/* Statistics about the allocation. */
static ggc_statistics *ggc_stats;
*************** ggc_realloc (x, size)
*** 155,164 ****
old_size = ggc_get_size (x);
if (size <= old_size)
! return x;
r = ggc_alloc (size);
memcpy (r, x, old_size);
return r;
}
--- 161,196 ----
old_size = ggc_get_size (x);
if (size <= old_size)
! {
! /* Mark the unwanted memory as unaccessible. We also need to make
! the "new" size accessible, since ggc_get_size returns the size of
! the pool, not the size of the individually allocated object, the
! size which was previously made accessible. Unfortunately, we
! don't know that previously allocated size. Without that
! knowledge we have to lose some initialization-tracking for the
! old parts of the object. An alternative is to mark the whole
! old_size as reachable, but that would lose tracking of writes
! after the end of the object (by small offsets). Discard the
! handle to avoid handle leak. */
! VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS ((char *) x + size,
! old_size - size));
! VALGRIND_DISCARD (VALGRIND_MAKE_READABLE (x, size));
! return x;
! }
r = ggc_alloc (size);
+
+ /* Since ggc_get_size returns the size of the pool, not the size of the
+ individually allocated object, we'd access parts of the old object
+ that were marked invalid with the memcpy below. We lose a bit of the
+ initialization-tracking since some of it may be uninitialized. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_READABLE (x, old_size));
+
memcpy (r, x, old_size);
+
+ /* The old object is not supposed to be used anymore. */
+ VALGRIND_DISCARD (VALGRIND_MAKE_NOACCESS (x, old_size));
+
return r;
}
brgds, H-P
More information about the Gcc-patches
mailing list