Skip to content

Commit

Permalink
Temporary fix for the Julia GC
Browse files Browse the repository at this point in the history 
This addresses a problem where the Julia GC during a partial sweep frees
some, but not all objects of an unreachable data structure. If an
address to a remaining object of such a data structure is still randomly
hit during conservative stack scanning, it may erroneously try to mark
the deallocated objects, too.

This temporary fix works by validating each pointer before it is marked.
Because this is potentially expensive, the eventual solution should fix
the root cause, which is conservative stack scanning resurrecting
pointers to dead objects that have been freed during a partial
collection.
  • Loading branch information
@rbehrends
rbehrends committed Apr 16, 2019
1 parent 65993f6 commit df48487
Showing 1 changed file with 57 additions and 17 deletions.
74 changes: 57 additions & 17 deletions src/julia_gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,13 @@
** Various options controlling special features of the Julia GC code follow
*/

// if DISABLE_BIGVAL_TRACKING is defined, we don't track the location of
// large bags; this speeds up some things, but may expose bugs in GAP code
// which incorrectly holds pointers into bags over a GC.
// #define DISABLE_BIGVAL_TRACKING
// if SCAN_STACK_FOR_MPTRS_ONLY is defined, stack scanning will only
// look for references to master pointers, but not bags themselves.
// This should be safe, as GASMAN uses the same mechanism, but it is
// at least theoretically possible that an optimizing compiler will
// remove the last master pointer reference before the last reference
// to the bag itself.
#define SCAN_STACK_FOR_MPTRS_ONLY

// if REQUIRE_PRECISE_MARKING is defined, we assume that all marking
// functions are precise, i.e., they only invoke MarkBag on valid bags,
Expand All @@ -54,6 +57,12 @@
// if MARKING_STRESS_TEST is defined, we stress test the TryMark code
// #define MARKING_STRESS_TEST

// if VALIDATE_MARKING is defined, the program is aborted if we ever
// encounter a reference during marking that does not meet additional
// validation criteria. These tests are compararively expensive and
// should not be enabled by default.
// #define VALIDATE_MARKING


/****************************************************************************
**
Expand Down Expand Up @@ -124,7 +133,7 @@ static void JFinalizer(jl_value_t * obj)
TabFreeFuncBags[tnum]((Bag)&contents);
}

#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)

/****************************************************************************
**
Expand Down Expand Up @@ -196,7 +205,7 @@ static inline void * align_ptr(void * p)
return (void *)u;
}

#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)

typedef struct treap_t {
struct treap_t *left, *right;
Expand Down Expand Up @@ -403,7 +412,7 @@ static UInt StackAlignBags;
static Bag * GapStackBottom;
static jl_ptls_t JuliaTLS, SaveTLS;
static size_t max_pool_obj_size;
#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)
static size_t bigval_startoffset;
#endif
static UInt YoungRef;
Expand Down Expand Up @@ -448,11 +457,32 @@ void InitMarkFuncBags(UInt type, TNumMarkFuncBags mark_func)
TabMarkFuncBags[type] = mark_func;
}

static inline int JMarkGapObjSafe(void * obj)
{
// only traverse objects internally used by GAP
void *ty = jl_typeof(obj);
if (ty != datatype_mptr && ty != datatype_bag
&& ty != datatype_largebag && ty != jl_weakref_type)
return 0;
return jl_gc_mark_queue_obj(JuliaTLS, (jl_value_t *)obj);
}

static inline int JMark(void * obj)
{
#ifdef VALIDATE_MARKING
// Validate that `obj` is still allocated and not on a
// free list already. We verify this by checking that the
// type is a pool object of type `jl_datatype_type`.
jl_value_t *ty = jl_typeof(obj);
if (jl_gc_internal_obj_base_ptr(ty) != ty)
abort();
if (!jl_typeis(ty, jl_datatype_type))
abort();
#endif
return jl_gc_mark_queue_obj(JuliaTLS, (jl_value_t *)obj);
}


// Overview of conservative stack scanning
//
// A key aspect of conservative marking is that (1) we need to determine
Expand Down Expand Up @@ -482,7 +512,7 @@ static void TryMark(void * p)
{
jl_value_t * p2 = jl_gc_internal_obj_base_ptr(p);
if (!p2) {
#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)
// It is possible for p to point past the end of
// the object, so we subtract one word from the
// address. This is safe, as the object is preceded
Expand Down Expand Up @@ -513,7 +543,12 @@ static void TryMark(void * p)
#endif
}
if (p2) {
JMark(p2);
#ifdef SCAN_STACK_FOR_MPTRS_ONLY
if (jl_typeis(p2, datatype_mptr))
JMark(p2);
#else
JMarkGapObjSafe(p2);
#endif
}
}

Expand Down Expand Up @@ -575,7 +610,7 @@ static void GapRootScanner(int full)
{
// mark our Julia module (this contains references to our custom data
// types, which thus also will not be collected prematurely)
JMark(Module);
jl_gc_mark_queue_obj(JuliaTLS, (jl_value_t *) Module);
jl_task_t * task = JuliaTLS->current_task;
size_t size;
int tid;
Expand Down Expand Up @@ -687,7 +722,11 @@ static uintptr_t JMarkMPtr(jl_ptls_t ptls, jl_value_t * obj)
{
if (!*(void **)obj)
return 0;
if (JMark(BAG_HEADER((Bag)obj)))
void *header = BAG_HEADER((Bag)obj);
void *ty = jl_typeof(header);
if (ty != datatype_bag && ty != datatype_largebag)
return 0;
if (JMark(header))
return 1;
return 0;
}
Expand All @@ -712,7 +751,7 @@ void InitBags(UInt initial_size, Bag * stack_bottom, UInt stack_align)
TabMarkFuncBags[i] = MarkAllSubBags;
// These callbacks need to be set before initialization so
// that we can track objects allocated during `jl_init()`.
#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)
jl_gc_set_cb_notify_external_alloc(alloc_bigval, 1);
jl_gc_set_cb_notify_external_free(free_bigval, 1);
bigval_startoffset = jl_gc_external_obj_hdr_size();
Expand Down Expand Up @@ -825,7 +864,7 @@ Bag NewBag(UInt type, UInt size)
if (size == 0)
alloc_size++;

#if defined(DISABLE_BIGVAL_TRACKING)
#if defined(SCAN_STACK_FOR_MPTRS_ONLY)
bag = jl_gc_alloc_typed(JuliaTLS, sizeof(void *), datatype_mptr);
SET_PTR_BAG(bag, 0);
#endif
Expand All @@ -837,7 +876,7 @@ Bag NewBag(UInt type, UInt size)
header->size = size;


#if !defined(DISABLE_BIGVAL_TRACKING)
#if !defined(SCAN_STACK_FOR_MPTRS_ONLY)
// allocate the new masterpointer
bag = jl_gc_alloc_typed(JuliaTLS, sizeof(void *), datatype_mptr);
SET_PTR_BAG(bag, DATA(header));
Expand Down Expand Up @@ -943,18 +982,19 @@ inline void MarkBag(Bag bag)
// relies on Julia internals. It is functionally equivalent
// to:
//
// if (JMark(p)) YoungRef++;
// if (JMarkGapObjSafe(p)) YoungRef++;
//
switch (jl_astaggedvalue(p)->bits.gc) {
case 0:
if (JMark(p))
if (JMarkGapObjSafe(p))
YoungRef++;
break;
case 1:
YoungRef++;
break;
case 2:
JMark(p);
JMarkGapObjSafe(p);
break;
case 3:
break;
}
Expand Down

0 comments on commit df48487

Please sign in to comment.