diff options
author | Hans Boehm <boehm@acm.org> | 2018-02-12 11:24:54 +0300 |
---|---|---|
committer | Ivan Maidanski <ivmai@mail.ru> | 2018-02-12 11:24:54 +0300 |
commit | bc7d0756a2b1f832c26454c9b08cacb1e6ad06dc (patch) | |
tree | 4ab9005a9d362ba986817138e19e09ba09eac17b /reclaim.c | |
parent | 95c4f56183ec9be404857c575971ccfd972f2194 (diff) | |
download | bdwgc-bc7d0756a2b1f832c26454c9b08cacb1e6ad06dc.tar.gz |
Avoid potential race between realloc and clear_hdr_marks/reclaim_generic
GC_realloc might be changing the block size while GC_reclaim_block
or GC_clear_hdr_marks is examining it. The change to the size field
is benign, in that GC_reclaim (and GC_clear_hdr_marks) would work
correctly with either value, since we are not changing the number
of objects in the block. But seeing a half-updated value (though
unlikely to occur in practice) could be probably bad.
Using unordered atomic accesses on the size and hb_descr fields would
solve the issue.
* mallocx.c [AO_HAVE_store] (GC_realloc): Use AO_store() to update
hhdr->hb_sz and hhdr->hb_descr; add static assert that size of
hhdr->hb_sz matches that of AO_t; add comment.
* mallocx.c [!AO_HAVE_store] (GC_realloc): Add LOCK/UNLOCK around
hb_sz and hb_descr fields assignment.
* mark.c [AO_HAVE_load] (GC_clear_hdr_marks): Use AO_load() to get
hhdr->hb_sz value; add comment.
* reclaim.c (IS_PTRFREE_SAFE): New macro (uses AO_load() if available).
* reclaim.c (GC_reclaim_generic, GC_reclaim_block): Replace
(hhdr)->hb_descr==0 with IS_PTRFREE_SAFE(hhdr).
Diffstat (limited to 'reclaim.c')
-rw-r--r-- | reclaim.c | 27 |
1 files changed, 18 insertions, 9 deletions
@@ -291,6 +291,16 @@ STATIC void GC_reclaim_check(struct hblk *hbp, hdr *hhdr, word sz) } } +/* Is a pointer-free block? Same as IS_PTRFREE macro (in os_dep.c) but */ +/* uses unordered atomic access to avoid racing with GC_realloc. */ +#ifdef AO_HAVE_load +# define IS_PTRFREE_SAFE(hhdr) \ + (AO_load((volatile AO_t *)&(hhdr)->hb_descr) == 0) +#else + /* No race as GC_realloc holds the lock while updating hb_descr. */ +# define IS_PTRFREE_SAFE(hhdr) ((hhdr)->hb_descr == 0) +#endif + /* * Generic procedure to rebuild a free list in hbp. * Also called directly from GC_malloc_many. @@ -304,7 +314,7 @@ GC_INNER ptr_t GC_reclaim_generic(struct hblk * hbp, hdr *hhdr, size_t sz, GC_ASSERT(GC_find_header((ptr_t)hbp) == hhdr); # ifndef GC_DISABLE_INCREMENTAL - GC_remove_protection(hbp, 1, (hhdr)->hb_descr == 0 /* Pointer-free? */); + GC_remove_protection(hbp, 1, IS_PTRFREE_SAFE(hhdr)); # endif # ifdef ENABLE_DISCLAIM if ((hhdr -> hb_flags & HAS_DISCLAIM) != 0) { @@ -314,7 +324,7 @@ GC_INNER ptr_t GC_reclaim_generic(struct hblk * hbp, hdr *hhdr, size_t sz, /* else */ if (init || GC_debugging_started) { result = GC_reclaim_clear(hbp, hhdr, sz, list, count); } else { - GC_ASSERT((hhdr)->hb_descr == 0 /* Pointer-free block */); + GC_ASSERT(IS_PTRFREE_SAFE(hhdr)); result = GC_reclaim_uninit(hbp, hhdr, sz, list, count); } if (IS_UNCOLLECTABLE(hhdr -> hb_obj_kind)) GC_set_hdr_marks(hhdr); @@ -408,10 +418,10 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) # ifdef ENABLE_DISCLAIM in_use: # endif - if (hhdr -> hb_descr != 0) { - GC_composite_in_use += sz; - } else { + if (IS_PTRFREE_SAFE(hhdr)) { GC_atomic_in_use += sz; + } else { + GC_composite_in_use += sz; } } } else { @@ -453,11 +463,10 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) /* already have the right cache context here. Also */ /* doing it here avoids some silly lock contention in */ /* GC_malloc_many. */ - - if (hhdr -> hb_descr != 0) { - GC_composite_in_use += sz * hhdr -> hb_n_marks; - } else { + if (IS_PTRFREE_SAFE(hhdr)) { GC_atomic_in_use += sz * hhdr -> hb_n_marks; + } else { + GC_composite_in_use += sz * hhdr -> hb_n_marks; } } } |