Poison freed objects such that double-free is detected

Previously we were setting refcount of freed objects to the inert value, which
was harmful because it caused further destroy()s of the freed object to NOT
call free() and hence hide the bug.  Indeed, after eb0bf3ae66 test-object
was double-free'ing objects and this was never caught on Linux.  It only was
caught as crashing on Mac.

Now we poison refcount upon freeing and check that it's valid whenever reading
it.  Makes test-object fail now.
pull/148/head
Behdad Esfahbod 9 years ago
parent 6578575cc8
commit 326b5ebf57
  1. 21
      src/hb-object-private.hh

@ -47,8 +47,9 @@
/* reference_count */ /* reference_count */
#define HB_REFERENCE_COUNT_INVALID_VALUE -1 #define HB_REFERENCE_COUNT_INERT_VALUE -1
#define HB_REFERENCE_COUNT_INIT {HB_ATOMIC_INT_INIT(HB_REFERENCE_COUNT_INVALID_VALUE)} #define HB_REFERENCE_COUNT_POISON_VALUE -0x0000DEAD
#define HB_REFERENCE_COUNT_INIT {HB_ATOMIC_INT_INIT(HB_REFERENCE_COUNT_INERT_VALUE)}
struct hb_reference_count_t struct hb_reference_count_t
{ {
@ -58,9 +59,10 @@ struct hb_reference_count_t
inline int get_unsafe (void) const { return ref_count.get_unsafe (); } inline int get_unsafe (void) const { return ref_count.get_unsafe (); }
inline int inc (void) { return ref_count.inc (); } inline int inc (void) { return ref_count.inc (); }
inline int dec (void) { return ref_count.dec (); } inline int dec (void) { return ref_count.dec (); }
inline void finish (void) { ref_count.set_unsafe (HB_REFERENCE_COUNT_INVALID_VALUE); } inline void finish (void) { ref_count.set_unsafe (HB_REFERENCE_COUNT_POISON_VALUE); }
inline bool is_invalid (void) const { return ref_count.get_unsafe () == HB_REFERENCE_COUNT_INVALID_VALUE; } inline bool is_inert (void) const { return ref_count.get_unsafe () == HB_REFERENCE_COUNT_INERT_VALUE; }
inline bool is_valid (void) const { return ref_count.get_unsafe () > 0; }
}; };
@ -142,7 +144,12 @@ static inline void hb_object_init (Type *obj)
template <typename Type> template <typename Type>
static inline bool hb_object_is_inert (const Type *obj) static inline bool hb_object_is_inert (const Type *obj)
{ {
return unlikely (obj->header.ref_count.is_invalid ()); return unlikely (obj->header.ref_count.is_inert ());
}
template <typename Type>
static inline bool hb_object_is_valid (const Type *obj)
{
return likely (obj->header.ref_count.is_valid ());
} }
template <typename Type> template <typename Type>
static inline Type *hb_object_reference (Type *obj) static inline Type *hb_object_reference (Type *obj)
@ -150,6 +157,7 @@ static inline Type *hb_object_reference (Type *obj)
hb_object_trace (obj, HB_FUNC); hb_object_trace (obj, HB_FUNC);
if (unlikely (!obj || hb_object_is_inert (obj))) if (unlikely (!obj || hb_object_is_inert (obj)))
return obj; return obj;
assert (hb_object_is_valid (obj));
obj->header.ref_count.inc (); obj->header.ref_count.inc ();
return obj; return obj;
} }
@ -159,6 +167,7 @@ static inline bool hb_object_destroy (Type *obj)
hb_object_trace (obj, HB_FUNC); hb_object_trace (obj, HB_FUNC);
if (unlikely (!obj || hb_object_is_inert (obj))) if (unlikely (!obj || hb_object_is_inert (obj)))
return false; return false;
assert (hb_object_is_valid (obj));
if (obj->header.ref_count.dec () != 1) if (obj->header.ref_count.dec () != 1)
return false; return false;
@ -175,6 +184,7 @@ static inline bool hb_object_set_user_data (Type *obj,
{ {
if (unlikely (!obj || hb_object_is_inert (obj))) if (unlikely (!obj || hb_object_is_inert (obj)))
return false; return false;
assert (hb_object_is_valid (obj));
return obj->header.user_data.set (key, data, destroy, replace); return obj->header.user_data.set (key, data, destroy, replace);
} }
@ -184,6 +194,7 @@ static inline void *hb_object_get_user_data (Type *obj,
{ {
if (unlikely (!obj || hb_object_is_inert (obj))) if (unlikely (!obj || hb_object_is_inert (obj)))
return NULL; return NULL;
assert (hb_object_is_valid (obj));
return obj->header.user_data.get (key); return obj->header.user_data.get (key);
} }

Loading…
Cancel
Save