Revise GC extensions implementation in accordance with code review.

* Adjustments to conservative marking.
* Remove symbol concatenation from gc_invoke_callbacks().
* Clean up handling of jl_buff_tag objects.
* Add comments.
* Fix formatting.

src/gc.c

@@ -24,13 +24,13 @@ static jl_gc_callback_list_t *gc_cblist_post_gc;
 static jl_gc_callback_list_t *gc_cblist_notify_external_alloc;
 static jl_gc_callback_list_t *gc_cblist_notify_external_free;
 
-#define gc_invoke_callbacks(kind, args) \
+#define gc_invoke_callbacks(ty, list, args) \
  do { \
- for (jl_gc_callback_list_t *cb = gc_cblist_##kind; \
+ for (jl_gc_callback_list_t *cb = list; \
  cb != NULL; \
  cb = cb->next) \
  { \
- ((jl_gc_cb_##kind##_t)(cb->func)) args; \
+ ((ty)(cb->func)) args; \
  } \
  } while (0)
 
@@ -116,6 +116,10 @@ JL_DLLEXPORT void jl_gc_set_cb_notify_external_free(jl_gc_cb_notify_external_fre
 static jl_mutex_t finalizers_lock;
 static jl_mutex_t gc_cache_lock;
 
+// Flag that tells us whether we need to support conservative marking
+// of objects.
+static int support_conservative_marking = 0;
+
 /**
  * Note about GC synchronization:
  *
@@ -457,7 +461,7 @@ static void gc_sweep_foreign_objs_in_list(arraylist_t *objs)
 {
  size_t p = 0;
  for (size_t i = 0; i < objs->len; i++) {
- jl_value_t *v = (jl_value_t *)(objs->items[i++]);
+ jl_value_t *v = (jl_value_t *)(objs->items[i]);
  jl_datatype_t *t = (jl_datatype_t *)(jl_typeof(v));
  const jl_datatype_layout_t *layout = t->layout;
  jl_fielddescdyn_t *desc = (jl_fielddescdyn_t*)jl_dt_layout_fields(layout);
@@ -473,7 +477,7 @@ static void gc_sweep_foreign_objs_in_list(arraylist_t *objs)
 
 static void gc_sweep_foreign_objs(void)
 {
- for (int i = 0;i < jl_n_threads;i++) {
+ for (int i = 0;i < jl_n_threads; i++) {
  jl_ptls_t ptls2 = jl_all_tls_states[i];
  gc_sweep_foreign_objs_in_list(&ptls2->sweep_objs);
  }
@@ -873,7 +877,8 @@ JL_DLLEXPORT jl_value_t *jl_gc_big_alloc(jl_ptls_t ptls, size_t sz)
  bigval_t *v = (bigval_t*)malloc_cache_align(allocsz);
  if (v == NULL)
  jl_throw(jl_memory_exception);
- gc_invoke_callbacks(notify_external_alloc, (v, allocsz));
+ gc_invoke_callbacks(jl_gc_cb_notify_external_alloc_t,
+ gc_cblist_notify_external_alloc, (v, allocsz));
 #ifdef JULIA_ENABLE_THREADING
  jl_atomic_fetch_add(&gc_num.allocd, allocsz);
 #else
@@ -922,7 +927,8 @@ static bigval_t **sweep_big_list(int sweep_full, bigval_t **pv) JL_NOTSAFEPOINT
 #ifdef MEMDEBUG
  memset(v, 0xbb, v->sz&~3);
 #endif
- gc_invoke_callbacks(notify_external_free, (v));
+ gc_invoke_callbacks(jl_gc_cb_notify_external_free_t,
+ gc_cblist_notify_external_free, (v));
  jl_free_aligned(v);
  }
  gc_time_count_big(old_bits, bits);
@@ -1051,6 +1057,9 @@ static inline jl_taggedvalue_t *reset_page(const jl_gc_pool_t *p, jl_gc_pagemeta
  // This prevents unnecessary fragmentation from multiple pages
  // being allocated from at the same time. Instead, objects will
  // only ever be allocated from the first object in the list.
+ // This is specifically being relied on by the implementation
+ // of jl_gc_internal_obj_base_ptr() so that the function does
+ // not have to traverse the entire list.
  jl_taggedvalue_t *flpage = (jl_taggedvalue_t *)gc_page_data(fl);
  next->next = flpage->next;
  flpage->next = beg;
@@ -2287,7 +2296,8 @@ mark: {
  int16_t tid = ta->tid;
  jl_ptls_t ptls2 = jl_all_tls_states[tid];
  ptls->last_gc_mark_sp = &sp;
- gc_invoke_callbacks(task_scanner, (ta, ta == ptls2->root_task));
+ gc_invoke_callbacks(jl_gc_cb_task_scanner_t,
+ gc_cblist_task_scanner, (ta, ta == ptls2->root_task));
 #ifdef COPY_STACKS
  if (stkbuf && ta->copy_stack)
  gc_setmark_buf_(ptls, stkbuf, bits, ta->bufsz);
@@ -2385,7 +2395,6 @@ mark: {
  else if (layout->fielddesc_type == 2) {
  // This is very uncommon
  // Do not do store to load forwarding to save some code size
- assert(layout->fielddesc_type == 2);
  jl_fielddesc32_t *desc = (jl_fielddesc32_t*)jl_dt_layout_fields(layout);
  assert(first < nfields);
  gc_mark_obj32_t markdata = {new_obj, desc + first, desc + nfields, nptr};
@@ -2635,7 +2644,8 @@ static int _jl_gc_collect(jl_ptls_t ptls, int full)
  // 3. walk roots
  mark_roots(gc_cache, &sp);
  ptls->last_gc_mark_sp = &sp;
- gc_invoke_callbacks(root_scanner, (full));
+ gc_invoke_callbacks(jl_gc_cb_root_scanner_t,
+ gc_cblist_root_scanner, (full));
  gc_mark_loop(ptls, sp);
  gc_mark_sp_init(gc_cache, &sp);
  gc_num.since_sweep += gc_num.allocd + (int64_t)gc_num.interval;
@@ -2666,7 +2676,9 @@ static int _jl_gc_collect(jl_ptls_t ptls, int full)
  // so that the objects are not incorrectly reset.
  gc_mark_loop(ptls, sp);
  gc_mark_sp_init(gc_cache, &sp);
- mark_reset_age = 1;
+ // Conservative marking relies on age to tell allocated objects
+ // and freelist entries apart.
+ mark_reset_age = !support_conservative_marking;
  // Reset the age and old bit for any unmarked objects referenced by the
  // `to_finalize` list. These objects are only reachable from this list
  // and should not be referenced by any old objects so this won't break
@@ -2800,7 +2812,8 @@ JL_DLLEXPORT void jl_gc_collect(int full)
  // TODO (concurrently queue objects)
  // no-op for non-threading
  jl_gc_wait_for_the_world();
- gc_invoke_callbacks(pre_gc, (full));
+ gc_invoke_callbacks(jl_gc_cb_pre_gc_t,
+ gc_cblist_pre_gc, (full));
 
  if (!jl_gc_disable_counter) {
  JL_LOCK_NOGC(&finalizers_lock);
@@ -2825,7 +2838,8 @@ JL_DLLEXPORT void jl_gc_collect(int full)
  run_finalizers(ptls);
  ptls->in_finalizer = was_in_finalizer;
  }
- gc_invoke_callbacks(post_gc, (full));
+ gc_invoke_callbacks(jl_gc_cb_post_gc_t,
+ gc_cblist_post_gc, (full));
 }
 
 void gc_mark_queue_all_roots(jl_ptls_t ptls, gc_mark_sp_t *sp)
@@ -3214,6 +3228,36 @@ JL_DLLEXPORT jl_value_t *jl_gc_alloc_3w(void)
  return jl_gc_alloc(ptls, sizeof(void*) * 3, NULL);
 }
 
+JL_DLLEXPORT int jl_gc_enable_conservative_gc_support(void)
+{
+ static jl_mutex_t conservative_gc_lock;
+ static_assert(jl_buff_tag % GC_PAGE_SZ == 0,
+ "jl_buff_tag must be a multiple of GC_PAGE_SZ");
+ if (jl_is_initialized()) {
+ JL_LOCK_NOGC(&conservative_gc_lock);
+ int result = support_conservative_marking;
+ support_conservative_marking = 1;
+ JL_UNLOCK_NOGC(&conservative_gc_lock);
+ if (!result) {
+ // Do a full collection to ensure that age bits are updated
+ // properly. We don't have to worry about race conditions
+ // for this part, as allocation itself is unproblematic and
+ // a collection will wait for safepoints.
+ jl_gc_collect(1);
+ }
+ return result;
+ } else {
+ int result = support_conservative_marking;
+ support_conservative_marking = 1;
+ return result;
+ }
+}
+
+JL_DLLEXPORT int jl_gc_conservative_gc_support_enabled(void)
+{
+ return support_conservative_marking;
+}
+
 JL_DLLEXPORT jl_value_t *jl_gc_internal_obj_base_ptr(void *p)
 {
  p = (char *) p - 1;
@@ -3295,9 +3339,11 @@ JL_DLLEXPORT jl_value_t *jl_gc_internal_obj_base_ptr(void *p)
  return NULL;
  }
  // Not a freelist entry, therefore a valid object.
-  valid_object:
+ valid_object:
  // We have to treat objects with type `jl_buff_tag` differently,
  // as they must not be passed to the usual marking functions.
+ // Note that jl_buff_tag is a multiple of GC_PAGE_SZ, thus it
+ // cannot be a type reference.
  if ((cell->header & ~(uintptr_t) 3) == jl_buff_tag)
  return NULL;
  return jl_valueof(cell);

src/julia_gcext.h

@@ -43,10 +43,6 @@ JL_DLLEXPORT jl_datatype_t *jl_new_foreign_type(
 JL_DLLEXPORT size_t jl_gc_max_internal_obj_size(void);
 JL_DLLEXPORT size_t jl_gc_external_obj_hdr_size(void);
 
-// Returns the base address of a memory block, assuming it
-// is stored in a julia memory pool. Return NULL otherwise.
-JL_DLLEXPORT jl_value_t *jl_gc_internal_obj_base_ptr(void *p);
-
 // Field layout descriptor for custom types that do
 // not fit Julia layout conventions. This is associated with
 // jl_datatype_t instances where fielddesc_type == 3.
@@ -56,11 +52,59 @@ typedef struct {
  jl_sweepfunc_t sweepfunc;
 } jl_fielddescdyn_t;
 
+// Allocate an object with a foreign type.
 JL_DLLEXPORT void *jl_gc_alloc_typed(jl_ptls_t ptls, size_t sz, void *ty);
+// Queue an object or array of objects for scanning by the garbage collector.
+// These functions must only be called from within a root scanner callback
+// or from within a custom mark function.
 JL_DLLEXPORT int jl_gc_mark_queue_obj(jl_ptls_t ptls, jl_value_t *obj);
 JL_DLLEXPORT void jl_gc_mark_queue_objarray(jl_ptls_t ptls, jl_value_t *parent,
  jl_value_t **objs, size_t nobjs);
 
+// Calling this function on an object of a foreign type will cause the
+// sweep function to be called during garbage collection. This function
+// must be called at most once per object. If not enabled for an object,
+// the sweep function will not be called. Normally, this should happen
+// right after allocating such an object.
 JL_DLLEXPORT void jl_gc_schedule_foreign_sweepfunc(jl_ptls_t ptls, jl_value_t * bj);
 
+// CAUTION: The following functions enable support for conservative
+// marking. Conservative marking will treat any machine word that points
+// to an object (potentially including its interior) as a valid
+// reference, regardless of whether it actually is one or just an
+// integer that happens to match an actual address.
+//
+// This is a sharp tool and should only be used as a measure of last
+// resort. The user should be familiar with the risk of memory leaks
+// (especially on 32-bit machines) if used inappropriately and how
+// optimizing compilers can hide references from conservative stack
+// scanning. In particular, arrays must be kept explicitly visible to
+// the GC (by using JL_GC_PUSH1(), storing them in a location marked by
+// the Julia GC, etc.) while their contents are being accessed. The
+// reason is that array contents aren't marked separately, so if the
+// object itself is not visible to the GC, neither are the contents.
+
+// Enable support for conservative scanning. The function returns
+// whether support was already enabled. The function may implicitly
+// trigger a full garbage collection to properly update all internal
+// data structures.
+JL_DLLEXPORT int jl_gc_enable_conservative_gc_support(void);
+
+// This function returns whether support for conservative scanning has
+// been enabled. The return values are the same as for
+// jl_gc_enable_conservative_gc_support().
+JL_DLLEXPORT int jl_gc_conservative_gc_support_enabled(void);
+
+// Returns the base address of a memory block, assuming it is stored in
+// a julia memory pool. Return NULL otherwise.
+//
+// NOTE: This will only work for internal pool allocations. For external
+// allocations, the user must track allocations using the notification
+// callbacks above and verify that they are valid objects. Note that
+// external allocations may not all be valid objects and that for those,
+// the user *must* validate that they have a proper type, i.e. that
+// jl_typeof(obj) is an actual type object.
+JL_DLLEXPORT jl_value_t *jl_gc_internal_obj_base_ptr(void *p);
+
+
 #endif // _JULIA_GCEXT_H

src/julia_internal.h

@@ -281,7 +281,9 @@ JL_DLLEXPORT jl_value_t *jl_gc_alloc(jl_ptls_t ptls, size_t sz, void *ty);
 # define jl_gc_alloc(ptls, sz, ty) jl_gc_alloc_(ptls, sz, ty)
 #endif
 
-#define jl_buff_tag ((uintptr_t)0x4eade000)
+// jl_buff_tag must be a multiple of GC_PAGE_SZ so that it can't be
+// confused for an actual type reference.
+#define jl_buff_tag ((uintptr_t)0x4eadc000)
 typedef void jl_gc_tracked_buffer_t; // For the benefit of the static analyzer
 STATIC_INLINE jl_gc_tracked_buffer_t *jl_gc_alloc_buf(jl_ptls_t ptls, size_t sz)
 {