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 /mallocx.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 'mallocx.c')
-rw-r--r-- | mallocx.c | 38 |
1 files changed, 32 insertions, 6 deletions
@@ -106,13 +106,39 @@ GC_API void * GC_CALL GC_realloc(void * p, size_t lb) if (sz > MAXOBJBYTES) { /* Round it up to the next whole heap block */ - word descr; + word descr = GC_obj_kinds[obj_kind].ok_descriptor; + + sz = (sz + HBLKSIZE-1) & ~HBLKMASK; + if (GC_obj_kinds[obj_kind].ok_relocate_descr) + descr += sz; + /* 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. (The alternate solution might */ + /* be to initially overallocate large objects, so we do not */ + /* have to adjust the size in GC_realloc, if they still fit. */ + /* But that is probably more expensive, since we may end up */ + /* scanning a bunch of zeros during GC.) */ +# ifdef AO_HAVE_store + GC_STATIC_ASSERT(sizeof(hhdr->hb_sz) == sizeof(AO_t)); + AO_store((volatile AO_t *)&hhdr->hb_sz, (AO_t)sz); + AO_store((volatile AO_t *)&hhdr->hb_descr, (AO_t)descr); +# else + { + DCL_LOCK_STATE; + + LOCK(); + hhdr -> hb_sz = sz; + hhdr -> hb_descr = descr; + UNLOCK(); + } +# endif - sz = (sz+HBLKSIZE-1) & (~HBLKMASK); - hhdr -> hb_sz = sz; - descr = GC_obj_kinds[obj_kind].ok_descriptor; - if (GC_obj_kinds[obj_kind].ok_relocate_descr) descr += sz; - hhdr -> hb_descr = descr; # ifdef MARK_BIT_PER_OBJ GC_ASSERT(hhdr -> hb_inv_sz == LARGE_INV_SZ); # endif |