[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Emacs-diffs] pdumper 3290cda: Fix conservative GC bugs
From: |
Daniel Colascione |
Subject: |
[Emacs-diffs] pdumper 3290cda: Fix conservative GC bugs |
Date: |
Wed, 14 Feb 2018 19:41:25 -0500 (EST) |
branch: pdumper
commit 3290cda6743d25f20e78312296f26fba3749d6f2
Author: Daniel Colascione <address@hidden>
Commit: Daniel Colascione <address@hidden>
Fix conservative GC bugs
---
src/alloc.c | 37 ++++++++++++++++++++++---------------
src/pdumper.c | 30 +++++++++++++++++++++++++-----
src/pdumper.h | 23 ++++++++++++++++++++---
3 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index be34371..0070b46 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 148dcd9..15a3c1b 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<<DUMP_RELOC_ALIGNMENT_BITS) == 0);
dump_remember_object (ctx, object, offset);
- dump_push (&ctx->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 3bf80d7..c407619 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 ();
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Emacs-diffs] pdumper 3290cda: Fix conservative GC bugs,
Daniel Colascione <=