From: Daniel Colascione Date: Thu, 15 Feb 2018 00:41:03 +0000 (-0800) Subject: Fix conservative GC bugs X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=3290cda6743d25f20e78312296f26fba3749d6f2;p=emacs.git Fix conservative GC bugs --- diff --git a/src/alloc.c b/src/alloc.c index be34371eabe..0070b465f3a 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4120,13 +4120,13 @@ vector_marked_p (const struct Lisp_Vector *v) { if (pdumper_object_p (v)) { - /* TODO: look into using a range test against - hot_discardable_start instead of loading the pvec header. - We'll have already loaded the dump header cache line, after - all. */ - enum pvec_type pvectype = PSEUDOVECTOR_TYPE (v); - if (pvectype == PVEC_BOOL_VECTOR) - return true; + /* Look at cold_start first so that we don't have to fault in + the vector header just to tell that it's a bool vector. */ + if (pdumper_cold_object_p (v)) + { + eassert (PSEUDOVECTOR_TYPE (v) == PVEC_BOOL_VECTOR); + return true; + } return pdumper_marked_p (v); } return XVECTOR_MARKED_P (v); @@ -5014,7 +5014,10 @@ mark_maybe_object (Lisp_Object obj) definitely _don't_ have an object. */ if (pdumper_object_p (po)) { - if (pdumper_object_p_precise (po)) + /* Don't use pdumper_object_p_precise here! It doesn't check the + tag bits. OBJ here might be complete garbage, so we need to + verify both the pointer and the tag. */ + if (XTYPE (obj) == pdumper_find_object_type (po)) mark_object (obj); return; } @@ -6968,11 +6971,14 @@ mark_object (Lisp_Object arg) mark_char_table (ptr, (enum pvec_type) pvectype); break; - case PVEC_BOOL_VECTOR: - /* Do not mark bool vectors in a dump image: these objects - are "cold" and don't have mark bits. */ - if (!pdumper_object_p (ptr)) - set_vector_marked (ptr); + case PVEC_BOOL_VECTOR: + /* bool vectors in a dump are permanently "marked", since + they're in the old section and don't have mark bits. + If we're looking at a dumped bool vector, we should + have aborted above when we called vector_marked_p(), so + we should never get here. */ + eassert (!pdumper_object_p (ptr)); + set_vector_marked (ptr); break; case PVEC_SUBR: @@ -7097,8 +7103,9 @@ mark_object (Lisp_Object arg) CHECK_ALLOCATED_AND_LIVE (live_float_p); /* Do not mark floats stored in a dump image: these floats are "cold" and do not have mark bits. */ - if (!pdumper_object_p (XFLOAT (obj)) && - !XFLOAT_MARKED_P (XFLOAT (obj))) + if (pdumper_object_p (XFLOAT (obj))) + eassert (pdumper_cold_object_p (XFLOAT (obj))); + else if (!XFLOAT_MARKED_P (XFLOAT (obj))) XFLOAT_MARK (XFLOAT (obj)); break; diff --git a/src/pdumper.c b/src/pdumper.c index 148dcd9755e..15a3c1be8be 100644 --- a/src/pdumper.c +++ b/src/pdumper.c @@ -422,6 +422,11 @@ struct dump_flags referents normally, but dump an object itself separately, later. */ bool_bf dump_object_contents : 1; + /* Record object starts. We turn this flag off when writing to the + discardable section so that we don't trick conservative GC into + thinking we have objects there. Ignored (we never record object + starts) if dump_object_contents is false. */ + bool_bf dump_object_starts : 1; /* Pack objects tighter than GC memory alignment would normally require. Useful for objects copied into the Emacs image instead of used directly from the loaded dump. @@ -2146,6 +2151,7 @@ dump_misc_any (struct dump_context *ctx, struct Lisp_Misc_Any *misc_any) static dump_off dump_float (struct dump_context *ctx, const struct Lisp_Float *lfloat) { + eassert (ctx->header.cold_start); struct Lisp_Float out; dump_object_start (ctx, GCALIGNMENT, &out, sizeof (out)); DUMP_FIELD_COPY (&out, lfloat, u.data); @@ -2907,9 +2913,10 @@ dump_object_1 (struct dump_context *ctx, Lisp_Object object) { eassert (offset % (1<object_starts, - list2 (dump_off_to_lisp (XTYPE (object)), - dump_off_to_lisp (offset))); + if (ctx->flags.dump_object_starts) + dump_push (&ctx->object_starts, + list2 (dump_off_to_lisp (XTYPE (object)), + dump_off_to_lisp (offset))); } } @@ -3704,6 +3711,7 @@ types. */) circumstances below, we temporarily change this default behavior. */ ctx->flags.dump_object_contents = true; + ctx->flags.dump_object_starts = true; /* We want to consolidate certain object types that we know are very likely to be modified. */ @@ -3800,6 +3808,8 @@ types. */) This section consists of objects that need to be memcpy()ed into the Emacs data section instead of just used directly. */ ctx->header.discardable_start = ctx->offset; + ctx->flags.dump_object_starts = false; + dump_copied_objects (ctx); eassert (dump_queue_empty_p (&ctx->dump_queue)); eassert (NILP (ctx->copied_queue)); @@ -3807,6 +3817,10 @@ types. */) dump_align_output (ctx, dump_get_page_size ()); ctx->header.cold_start = ctx->offset; + /* Resume recording object starts, since the cold section will stick + around. */ + ctx->flags.dump_object_starts = true; + /* Start the cold section. This section contains bytes that should never change and so can be direct-mapped from the dump without special processing. */ @@ -4689,9 +4703,13 @@ dump_loaded_p (void) } bool -pdumper_object_p_precise_impl (const void *obj) +pdumper_cold_object_p_impl (const void *obj) { - return pdumper_find_object_type (obj) != PDUMPER_NO_OBJECT; + eassert (pdumper_object_p (obj)); + eassert (pdumper_object_p_precise (obj)); + dump_off offset = ptrdiff_t_to_dump_off ( + (intptr_t) obj - dump_public.start); + return offset >= dump_private.header.cold_start; } enum Lisp_Type @@ -4715,6 +4733,7 @@ pdumper_marked_p_impl (const void *obj) eassert (pdumper_object_p (obj)); ptrdiff_t offset = (intptr_t) obj - dump_public.start; eassert (offset % GCALIGNMENT == 0); + eassert (offset < dump_private.header.cold_start); eassert (offset < dump_private.header.discardable_start); ptrdiff_t bitno = offset / GCALIGNMENT; return dump_bitset_bit_set_p (&dump_private.mark_bits, bitno); @@ -4726,6 +4745,7 @@ pdumper_set_marked_impl (const void *obj) eassert (pdumper_object_p (obj)); ptrdiff_t offset = (intptr_t) obj - dump_public.start; eassert (offset % GCALIGNMENT == 0); + eassert (offset < dump_private.header.cold_start); eassert (offset < dump_private.header.discardable_start); ptrdiff_t bitno = offset / GCALIGNMENT; dump_bitset_set_bit (&dump_private.mark_bits, bitno); diff --git a/src/pdumper.h b/src/pdumper.h index 3bf80d7daef..c40761944d8 100644 --- a/src/pdumper.h +++ b/src/pdumper.h @@ -157,6 +157,25 @@ pdumper_object_p (const void *obj) #endif } +extern bool pdumper_cold_object_p_impl (const void *obj); + +/* Return whether the OBJ is in the cold section of the dump. + Only bool-vectors and floats should end up there. + pdumper_object_p() and pdumper_object_p_precise() must have + returned true for OBJ before calling this function. */ +INLINE _GL_ATTRIBUTE_CONST +bool +pdumper_cold_object_p (const void *obj) +{ +#ifdef HAVE_PDUMPER + return pdumper_cold_object_p_impl (obj); +#else + (void) obj; + return false; +#endif +} + + extern enum Lisp_Type pdumper_find_object_type_impl (const void *obj); /* Return the type of the dumped object that starts at OBJ. It is a @@ -174,8 +193,6 @@ pdumper_find_object_type (const void *obj) #endif } -extern bool pdumper_object_p_precise_impl (const void *obj); - /* Return whether OBJ points exactly to the start of some object in the loaded dump image. It is a programming error to call this routine for an OBJ for which pdumper_object_p would return @@ -185,7 +202,7 @@ bool pdumper_object_p_precise (const void *obj) { #ifdef HAVE_PDUMPER - return pdumper_object_p_precise_impl (obj); + return pdumper_find_object_type (obj) != PDUMPER_NO_OBJECT; #else (void) obj; emacs_abort ();