From 9d99463219d3125e87c7a07cf6aba5624e16aae6 Mon Sep 17 00:00:00 2001 From: ivmai Date: Fri, 6 May 2011 04:43:49 +0000 Subject: [PATCH] 2011-05-06 Ivan Maidanski * dbg_mlc.c (GC_has_other_debug_info): Change return type to int; return -1 if the object has (or had) debugging info but was marked deallocated. * include/private/dbg_mlc.h (GC_has_other_debug_info): Ditto. * dbg_mlc.c (GC_has_other_debug_info): Update documentation; remove "ohdr" local variable. * dbg_mlc.c (GC_debug_free): Don't call GC_free if the object has probably been deallocated. * dbg_mlc.c (GC_debug_free): Don't actually free the object even in the leak-finding mode if GC_findleak_delay_free. * dbg_mlc.c (GC_print_all_smashed_proc): Print a trailing blank line. * dbg_mlc.c (GC_check_leaked): New function (only unless SHORT_DBG_HDRS). * doc/README.environment (GC_FINDLEAK_DELAY_FREE): Document. * doc/README.macros (GC_FINDLEAK_DELAY_FREE): Ditto. * include/private/dbg_mlc.h (START_FLAG, END_FLAG): Use GC_WORD_C on 64-bit architectures. * include/private/dbg_mlc.h (NOT_MARKED): Remove redundant parentheses. * include/private/dbg_mlc.h (GC_HAS_DEBUG_INFO): Update (due to GC_has_other_debug_info change). * include/private/gc_priv.h (GC_findleak_delay_free): New global variable declaration (unless SHORT_DBG_HDRS). * misc.c (GC_findleak_delay_free): New global variable; recognize GC_FINDLEAK_DELAY_FREE. * misc.c (GC_init): Recognize GC_FINDLEAK_DELAY_FREE environment variable (unless SHORT_DBG_HDRS). * reclaim.c (GC_check_leaked): Declare (unless SHORT_DBG_HDRS). * reclaim.c (GC_add_leaked): Don't add the object to leaked list if marked as deallocated. --- ChangeLog | 34 +++++++ dbg_mlc.c | 78 +++++++++++----- doc/README.environment | 4 + doc/README.macros | 5 + include/private/dbg_mlc.h | 187 +++++++++++++++++++------------------- include/private/gc_priv.h | 7 ++ misc.c | 13 +++ reclaim.c | 9 ++ 8 files changed, 224 insertions(+), 113 deletions(-) diff --git a/ChangeLog b/ChangeLog index d6278469..91d1afd2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,37 @@ +2011-05-06 Ivan Maidanski + + * dbg_mlc.c (GC_has_other_debug_info): Change return type to int; + return -1 if the object has (or had) debugging info but was + marked deallocated. + * include/private/dbg_mlc.h (GC_has_other_debug_info): Ditto. + * dbg_mlc.c (GC_has_other_debug_info): Update documentation; + remove "ohdr" local variable. + * dbg_mlc.c (GC_debug_free): Don't call GC_free if the object has + probably been deallocated. + * dbg_mlc.c (GC_debug_free): Don't actually free the object even + in the leak-finding mode if GC_findleak_delay_free. + * dbg_mlc.c (GC_print_all_smashed_proc): Print a trailing blank + line. + * dbg_mlc.c (GC_check_leaked): New function (only unless + SHORT_DBG_HDRS). + * doc/README.environment (GC_FINDLEAK_DELAY_FREE): Document. + * doc/README.macros (GC_FINDLEAK_DELAY_FREE): Ditto. + * include/private/dbg_mlc.h (START_FLAG, END_FLAG): Use GC_WORD_C + on 64-bit architectures. + * include/private/dbg_mlc.h (NOT_MARKED): Remove redundant + parentheses. + * include/private/dbg_mlc.h (GC_HAS_DEBUG_INFO): Update (due to + GC_has_other_debug_info change). + * include/private/gc_priv.h (GC_findleak_delay_free): New global + variable declaration (unless SHORT_DBG_HDRS). + * misc.c (GC_findleak_delay_free): New global variable; recognize + GC_FINDLEAK_DELAY_FREE. + * misc.c (GC_init): Recognize GC_FINDLEAK_DELAY_FREE environment + variable (unless SHORT_DBG_HDRS). + * reclaim.c (GC_check_leaked): Declare (unless SHORT_DBG_HDRS). + * reclaim.c (GC_add_leaked): Don't add the object to leaked list + if marked as deallocated. + 2011-05-05 Ivan Maidanski * dbg_mlc.c (GC_has_other_debug_info): Fix punctuation in the diff --git a/dbg_mlc.c b/dbg_mlc.c index b2671c1c..c3d9e77f 100644 --- a/dbg_mlc.c +++ b/dbg_mlc.c @@ -32,27 +32,27 @@ GC_INNER void GC_default_print_heap_obj_proc(ptr_t p); /* odd, which is added by the GC_HAS_DEBUG_INFO macro. */ /* Note that if DBG_HDRS_ALL is set, uncollectable objects */ /* on free lists may not have debug information set. Thus it's */ - /* not always safe to return TRUE, even if the client does */ - /* its part. */ - GC_INNER GC_bool GC_has_other_debug_info(ptr_t p) + /* not always safe to return TRUE (1), even if the client does */ + /* its part. Return -1 if the object with debug info has been */ + /* marked as deallocated. */ + GC_INNER int GC_has_other_debug_info(ptr_t p) { - oh * ohdr = (oh *)p; - ptr_t body = (ptr_t)(ohdr + 1); - word sz = GC_size((ptr_t) ohdr); + ptr_t body = (ptr_t)((oh *)p + 1); + word sz = GC_size(p); - if (HBLKPTR((ptr_t)ohdr) != HBLKPTR((ptr_t)body) + if (HBLKPTR(p) != HBLKPTR((ptr_t)body) || sz < DEBUG_BYTES + EXTRA_BYTES) { - return(FALSE); + return 0; } - if (ohdr -> oh_sz == sz) { - /* Object may have had debug info, but has been deallocated */ - return(FALSE); + if (((oh *)p) -> oh_sf != (START_FLAG ^ (word)body) + && ((word *)p)[BYTES_TO_WORDS(sz)-1] != (END_FLAG ^ (word)body)) { + return 0; } - if (ohdr -> oh_sf == (START_FLAG ^ (word)body)) return(TRUE); - if (((word *)ohdr)[BYTES_TO_WORDS(sz)-1] == (END_FLAG ^ (word)body)) { - return(TRUE); + if (((oh *)p)->oh_sz == sz) { + /* Object may have had debug info, but has been deallocated */ + return -1; } - return(FALSE); + return 1; } #endif /* !SHORT_DBG_HDRS */ @@ -771,17 +771,26 @@ GC_API void GC_CALL GC_debug_free(void * p) ptr_t clobbered = GC_check_annotated_obj((oh *)base); word sz = GC_size(base); if (clobbered != 0) { - GC_print_smashed_obj(((oh *)base) -> oh_sz == sz ? - "GC_debug_free: found previously deallocated (?) object at" : - "GC_debug_free: found smashed location at", - p, clobbered); GC_have_errors = TRUE; + if (((oh *)base) -> oh_sz == sz) { + GC_print_smashed_obj( + "GC_debug_free: found previously deallocated (?) object at", + p, clobbered); + return; /* ignore double free */ + } else { + GC_print_smashed_obj("GC_debug_free: found smashed location at", + p, clobbered); + } } - /* Invalidate size */ + /* Invalidate size (mark the object as deallocated) */ ((oh *)base) -> oh_sz = sz; # endif /* SHORT_DBG_HDRS */ } - if (GC_find_leak) { + if (GC_find_leak +# ifndef SHORT_DBG_HDRS + && ((ptr_t)p - (ptr_t)base != sizeof(oh) || !GC_findleak_delay_free) +# endif + ) { GC_free(base); } else { hdr * hhdr = HDR(p); @@ -914,6 +923,7 @@ STATIC void GC_print_all_smashed_proc(void) GC_smashed[i] = 0; } GC_n_smashed = 0; + GC_err_printf("\n"); } /* Check all marked objects in the given block for validity */ @@ -951,6 +961,32 @@ STATIC void GC_check_heap_proc(void) GC_apply_to_all_blocks(GC_check_heap_block, 0); } +GC_INNER GC_bool GC_check_leaked(ptr_t base) +{ + size_t i; + size_t obj_sz; + word *p; + + if ( +# if defined(KEEP_BACK_PTRS) || defined(MAKE_BACK_GRAPH) + (*(word *)base & 1) != 0 && +# endif + GC_has_other_debug_info(base) >= 0) + return TRUE; /* object has leaked */ + + /* Validate freed object's content. */ + p = (word *)(base + sizeof(oh)); + obj_sz = BYTES_TO_WORDS(HDR(base)->hb_sz - sizeof(oh)); + for (i = 0; i < obj_sz; ++i) + if (p[i] != GC_FREED_MEM_MARKER) { + GC_set_mark_bit(base); /* do not reclaim it in this cycle */ + GC_add_smashed((ptr_t)(&p[i])); /* alter-after-free detected */ + break; /* don't report any other smashed locations in the object */ + } + + return FALSE; /* GC_debug_free() has been called */ +} + #endif /* !SHORT_DBG_HDRS */ struct closure { diff --git a/doc/README.environment b/doc/README.environment index d9995f59..866e9055 100644 --- a/doc/README.environment +++ b/doc/README.environment @@ -160,6 +160,10 @@ GC_FIND_LEAK - Turns on GC_find_leak and thus leak detection. Forces a collection at program termination to detect leaks that would otherwise occur after the last GC. +GC_FINDLEAK_DELAY_FREE - Turns on deferred freeing of objects in the + leak-finding mode (see the corresponding macro + description for more information). + GC_ABORT_ON_LEAK - Causes the application to be terminated once leaked or smashed objects are found. diff --git a/doc/README.macros b/doc/README.macros index c4a33c4c..4fd5c840 100644 --- a/doc/README.macros +++ b/doc/README.macros @@ -87,6 +87,11 @@ FIND_LEAK Causes GC_find_leak to be initially set. This causes the explicitly deallocated, and reports exceptions. Finalization and the test program are not usable in this mode. +GC_FINDLEAK_DELAY_FREE Turns on deferred freeing of objects in the + leak-finding mode letting the collector to detect alter-object-after-free + errors as well as detect leaked objects sooner (instead of only when program + terminates). Has no effect if SHORT_DBG_HDRS. + GC_ABORT_ON_LEAK Causes the application to be terminated once leaked or smashed (corrupted on use-after-free) objects are found (after printing the information about that objects). diff --git a/include/private/dbg_mlc.h b/include/private/dbg_mlc.h index a6f2ef6a..7b2969d2 100644 --- a/include/private/dbg_mlc.h +++ b/include/private/dbg_mlc.h @@ -25,92 +25,96 @@ #ifndef _DBG_MLC_H #define _DBG_MLC_H -# include "gc_priv.h" -# ifdef KEEP_BACK_PTRS -# include "gc_backptr.h" -# endif +#include "gc_priv.h" +#ifdef KEEP_BACK_PTRS +# include "gc_backptr.h" +#endif -# define START_FLAG ((word)0xfedcedcb) -# define END_FLAG ((word)0xbcdecdef) +#if CPP_WORDSZ == 32 +# define START_FLAG (word)0xfedcedcb +# define END_FLAG (word)0xbcdecdef +#else +# define START_FLAG GC_WORD_C(0xFEDCEDCBfedcedcb) +# define END_FLAG GC_WORD_C(0xBCDECDEFbcdecdef) +#endif /* Stored both one past the end of user object, and one before */ /* the end of the object as seen by the allocator. */ -# if defined(KEEP_BACK_PTRS) || defined(PRINT_BLACK_LIST) \ - || defined(MAKE_BACK_GRAPH) - /* Pointer "source"s that aren't real locations. */ - /* Used in oh_back_ptr fields and as "source" */ - /* argument to some marking functions. */ -# define NOT_MARKED (ptr_t)(0) -# define MARKED_FOR_FINALIZATION ((ptr_t)(word)2) - /* Object was marked because it is finalizable. */ -# define MARKED_FROM_REGISTER ((ptr_t)(word)4) - /* Object was marked from a register. Hence the */ - /* source of the reference doesn't have an address. */ -# endif /* KEEP_BACK_PTRS || PRINT_BLACK_LIST */ +#if defined(KEEP_BACK_PTRS) || defined(PRINT_BLACK_LIST) \ + || defined(MAKE_BACK_GRAPH) + /* Pointer "source"s that aren't real locations. */ + /* Used in oh_back_ptr fields and as "source" */ + /* argument to some marking functions. */ +# define NOT_MARKED (ptr_t)0 +# define MARKED_FOR_FINALIZATION ((ptr_t)(word)2) + /* Object was marked because it is finalizable. */ +# define MARKED_FROM_REGISTER ((ptr_t)(word)4) + /* Object was marked from a register. Hence the */ + /* source of the reference doesn't have an address. */ +#endif /* KEEP_BACK_PTRS || PRINT_BLACK_LIST */ /* Object header */ typedef struct { -# if defined(KEEP_BACK_PTRS) || defined(MAKE_BACK_GRAPH) - /* We potentially keep two different kinds of back */ - /* pointers. KEEP_BACK_PTRS stores a single back */ - /* pointer in each reachable object to allow reporting */ - /* of why an object was retained. MAKE_BACK_GRAPH */ - /* builds a graph containing the inverse of all */ - /* "points-to" edges including those involving */ - /* objects that have just become unreachable. This */ - /* allows detection of growing chains of unreachable */ - /* objects. It may be possible to eventually combine */ - /* both, but for now we keep them separate. Both */ - /* kinds of back pointers are hidden using the */ - /* following macros. In both cases, the plain version */ - /* is constrained to have an least significant bit of 1,*/ - /* to allow it to be distinguished from a free list */ - /* link. This means the plain version must have an */ - /* lsb of 0. */ - /* Note that blocks dropped by black-listing will */ - /* also have the lsb clear once debugging has */ - /* started. */ - /* We're careful never to overwrite a value with lsb 0. */ -# if ALIGNMENT == 1 - /* Fudge back pointer to be even. */ -# define HIDE_BACK_PTR(p) GC_HIDE_POINTER(~1 & (GC_word)(p)) -# else -# define HIDE_BACK_PTR(p) GC_HIDE_POINTER(p) -# endif - -# ifdef KEEP_BACK_PTRS - GC_hidden_pointer oh_back_ptr; -# endif -# ifdef MAKE_BACK_GRAPH - GC_hidden_pointer oh_bg_ptr; -# endif -# if defined(KEEP_BACK_PTRS) != defined(MAKE_BACK_GRAPH) - /* Keep double-pointer-sized alignment. */ - word oh_dummy; -# endif +# if defined(KEEP_BACK_PTRS) || defined(MAKE_BACK_GRAPH) + /* We potentially keep two different kinds of back */ + /* pointers. KEEP_BACK_PTRS stores a single back */ + /* pointer in each reachable object to allow reporting */ + /* of why an object was retained. MAKE_BACK_GRAPH */ + /* builds a graph containing the inverse of all */ + /* "points-to" edges including those involving */ + /* objects that have just become unreachable. This */ + /* allows detection of growing chains of unreachable */ + /* objects. It may be possible to eventually combine */ + /* both, but for now we keep them separate. Both */ + /* kinds of back pointers are hidden using the */ + /* following macros. In both cases, the plain version */ + /* is constrained to have an least significant bit of 1, */ + /* to allow it to be distinguished from a free list */ + /* link. This means the plain version must have an */ + /* lsb of 0. */ + /* Note that blocks dropped by black-listing will */ + /* also have the lsb clear once debugging has */ + /* started. */ + /* We're careful never to overwrite a value with lsb 0. */ +# if ALIGNMENT == 1 + /* Fudge back pointer to be even. */ +# define HIDE_BACK_PTR(p) GC_HIDE_POINTER(~1 & (GC_word)(p)) +# else +# define HIDE_BACK_PTR(p) GC_HIDE_POINTER(p) +# endif +# ifdef KEEP_BACK_PTRS + GC_hidden_pointer oh_back_ptr; +# endif +# ifdef MAKE_BACK_GRAPH + GC_hidden_pointer oh_bg_ptr; # endif - const char * oh_string; /* object descriptor string */ - word oh_int; /* object descriptor integers */ -# ifdef NEED_CALLINFO - struct callinfo oh_ci[NFRAMES]; +# if defined(KEEP_BACK_PTRS) != defined(MAKE_BACK_GRAPH) + /* Keep double-pointer-sized alignment. */ + word oh_dummy; # endif -# ifndef SHORT_DBG_HDRS - word oh_sz; /* Original malloc arg. */ - word oh_sf; /* start flag */ -# endif /* SHORT_DBG_HDRS */ +# endif + const char * oh_string; /* object descriptor string */ + word oh_int; /* object descriptor integers */ +# ifdef NEED_CALLINFO + struct callinfo oh_ci[NFRAMES]; +# endif +# ifndef SHORT_DBG_HDRS + word oh_sz; /* Original malloc arg. */ + word oh_sf; /* start flag */ +# endif /* SHORT_DBG_HDRS */ } oh; /* The size of the above structure is assumed not to de-align things, */ /* and to be a multiple of the word length. */ #ifdef SHORT_DBG_HDRS -# define DEBUG_BYTES (sizeof (oh)) -# define UNCOLLECTABLE_DEBUG_BYTES DEBUG_BYTES +# define DEBUG_BYTES (sizeof (oh)) +# define UNCOLLECTABLE_DEBUG_BYTES DEBUG_BYTES #else - /* Add space for END_FLAG, but use any extra space that was already */ - /* added to catch off-the-end pointers. */ - /* For uncollectable objects, the extra byte is not added. */ -# define UNCOLLECTABLE_DEBUG_BYTES (sizeof (oh) + sizeof (word)) -# define DEBUG_BYTES (UNCOLLECTABLE_DEBUG_BYTES - EXTRA_BYTES) + /* Add space for END_FLAG, but use any extra space that was already */ + /* added to catch off-the-end pointers. */ + /* For uncollectable objects, the extra byte is not added. */ +# define UNCOLLECTABLE_DEBUG_BYTES (sizeof (oh) + sizeof (word)) +# define DEBUG_BYTES (UNCOLLECTABLE_DEBUG_BYTES - EXTRA_BYTES) #endif /* Round bytes to words without adding extra byte at end. */ @@ -122,42 +126,41 @@ typedef struct { /* PRINT_CALL_CHAIN prints the call chain stored in an object */ /* to stderr. It requires that we do not hold the lock. */ #if defined(SAVE_CALL_CHAIN) - struct callinfo; - GC_INNER void GC_save_callers(struct callinfo info[NFRAMES]); - GC_INNER void GC_print_callers(struct callinfo info[NFRAMES]); -# define ADD_CALL_CHAIN(base, ra) GC_save_callers(((oh *)(base)) -> oh_ci) -# define PRINT_CALL_CHAIN(base) GC_print_callers(((oh *)(base)) -> oh_ci) + struct callinfo; + GC_INNER void GC_save_callers(struct callinfo info[NFRAMES]); + GC_INNER void GC_print_callers(struct callinfo info[NFRAMES]); +# define ADD_CALL_CHAIN(base, ra) GC_save_callers(((oh *)(base)) -> oh_ci) +# define PRINT_CALL_CHAIN(base) GC_print_callers(((oh *)(base)) -> oh_ci) #elif defined(GC_ADD_CALLER) - struct callinfo; - GC_INNER void GC_print_callers(struct callinfo info[NFRAMES]); -# define ADD_CALL_CHAIN(base, ra) ((oh *)(base)) -> oh_ci[0].ci_pc = (ra) -# define PRINT_CALL_CHAIN(base) GC_print_callers(((oh *)(base)) -> oh_ci) + struct callinfo; + GC_INNER void GC_print_callers(struct callinfo info[NFRAMES]); +# define ADD_CALL_CHAIN(base, ra) ((oh *)(base)) -> oh_ci[0].ci_pc = (ra) +# define PRINT_CALL_CHAIN(base) GC_print_callers(((oh *)(base)) -> oh_ci) #else -# define ADD_CALL_CHAIN(base, ra) -# define PRINT_CALL_CHAIN(base) +# define ADD_CALL_CHAIN(base, ra) +# define PRINT_CALL_CHAIN(base) #endif -# ifdef GC_ADD_CALLER -# define OPT_RA ra, -# else -# define OPT_RA -# endif - +#ifdef GC_ADD_CALLER +# define OPT_RA ra, +#else +# define OPT_RA +#endif /* Check whether object with base pointer p has debugging info */ /* p is assumed to point to a legitimate object in our part */ /* of the heap. */ #ifdef SHORT_DBG_HDRS -# define GC_has_other_debug_info(p) TRUE +# define GC_has_other_debug_info(p) 1 #else - GC_INNER GC_bool GC_has_other_debug_info(ptr_t p); + GC_INNER int GC_has_other_debug_info(ptr_t p); #endif #if defined(KEEP_BACK_PTRS) || defined(MAKE_BACK_GRAPH) # define GC_HAS_DEBUG_INFO(p) \ - ((*((word *)p) & 1) && GC_has_other_debug_info(p)) + ((*((word *)p) & 1) && GC_has_other_debug_info(p) > 0) #else -# define GC_HAS_DEBUG_INFO(p) GC_has_other_debug_info(p) +# define GC_HAS_DEBUG_INFO(p) (GC_has_other_debug_info(p) > 0) #endif #endif /* _DBG_MLC_H */ diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 2b7113ef..c02def1b 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -1790,6 +1790,13 @@ GC_EXTERN void (*GC_print_heap_obj)(ptr_t p); /* Print an address map of the process. */ #endif +#ifndef SHORT_DBG_HDRS + GC_EXTERN GC_bool GC_findleak_delay_free; + /* Do not immediately deallocate object on */ + /* free() in the leak-finding mode, just mark */ + /* it as freed (and deallocate it after GC). */ +#endif + GC_EXTERN GC_bool GC_have_errors; /* We saw a smashed or leaked object. */ /* Call error printing routine */ /* occasionally. It is ok to read it */ diff --git a/misc.c b/misc.c index bd8730f0..50d55e04 100644 --- a/misc.c +++ b/misc.c @@ -114,6 +114,14 @@ GC_bool GC_quiet = 0; /* used also in pcr_interface.c */ int GC_find_leak = 0; #endif +#ifndef SHORT_DBG_HDRS +# ifdef GC_FINDLEAK_DELAY_FREE + GC_INNER GC_bool GC_findleak_delay_free = TRUE; +# else + GC_INNER GC_bool GC_findleak_delay_free = FALSE; +# endif +#endif /* !SHORT_DBG_HDRS */ + #ifdef ALL_INTERIOR_POINTERS int GC_all_interior_pointers = 1; #else @@ -789,6 +797,11 @@ GC_API void GC_CALL GC_init(void) if (0 != GETENV("GC_FIND_LEAK")) { GC_find_leak = 1; } +# ifndef SHORT_DBG_HDRS + if (0 != GETENV("GC_FINDLEAK_DELAY_FREE")) { + GC_findleak_delay_free = TRUE; + } +# endif if (0 != GETENV("GC_ALL_INTERIOR_POINTERS")) { GC_all_interior_pointers = 1; } diff --git a/reclaim.c b/reclaim.c index f9bfd851..c9e8e530 100644 --- a/reclaim.c +++ b/reclaim.c @@ -41,8 +41,17 @@ STATIC unsigned GC_n_leaked = 0; GC_INNER GC_bool GC_have_errors = FALSE; +#ifndef SHORT_DBG_HDRS + GC_INNER GC_bool GC_check_leaked(ptr_t base); /* from dbg_mlc.c */ +#endif + GC_INLINE void GC_add_leaked(ptr_t leaked) { +# ifndef SHORT_DBG_HDRS + if (GC_findleak_delay_free && !GC_check_leaked(leaked)) + return; +# endif + GC_have_errors = TRUE; /* FIXME: Prevent adding an object while printing leaked ones. */ if (GC_n_leaked < MAX_LEAKED) { -- 2.40.0