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 eb0bf3ae6688b7 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.

1 files changed, 16 insertions, 5 deletions

diff --git a/src/hb-object-private.hh b/src/hb-object-private.hh
index 635d62dc..6b73ff92 100644
--- a/src/hb-object-private.hh
+++ b/src/hb-object-private.hh
@@ -47,8 +47,9 @@
 
 /* reference_count */
 
-#define HB_REFERENCE_COUNT_INVALID_VALUE -1
-#define HB_REFERENCE_COUNT_INIT {HB_ATOMIC_INT_INIT(HB_REFERENCE_COUNT_INVALID_VALUE)}
+#define HB_REFERENCE_COUNT_INERT_VALUE -1
+#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
 {
@@ -58,9 +59,10 @@ struct hb_reference_count_t
   inline int get_unsafe (void) const { return ref_count.get_unsafe (); }
   inline int inc (void) { return ref_count.inc (); }
   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>
 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>
 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);
   if (unlikely (!obj || hb_object_is_inert (obj)))
     return obj;
+  assert (hb_object_is_valid (obj));
   obj->header.ref_count.inc ();
   return obj;
 }
@@ -159,6 +167,7 @@ static inline bool hb_object_destroy (Type *obj)
   hb_object_trace (obj, HB_FUNC);
   if (unlikely (!obj || hb_object_is_inert (obj)))
     return false;
+  assert (hb_object_is_valid (obj));
   if (obj->header.ref_count.dec () != 1)
     return false;
 
@@ -175,6 +184,7 @@ static inline bool hb_object_set_user_data (Type               *obj,
 {
   if (unlikely (!obj || hb_object_is_inert (obj)))
     return false;
+  assert (hb_object_is_valid (obj));
   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)))
     return NULL;
+  assert (hb_object_is_valid (obj));
   return obj->header.user_data.get (key);
 }