Author: Devin Jeanpierre <[email protected]>
Branch: gc-forkfriendly
Changeset: r85051:79a9cecb91c5
Date: 2016-06-09 00:35 -0700
http://bitbucket.org/pypy/pypy/changeset/79a9cecb91c5/
Log: Fixing little to nothing, make incminimark_remoteheader more
correctish, maybe.
e.g. test_zrpy_gc, these two fail (with an assert failure and
segfault resp.):
TestRemoteHeaderShadowStack.test_compile_framework_7_interior
TestRemoteHeaderShadowStack.test_compile_framework_9
Some rambling notes on the above tests:
* test_compile_framework_7_interior fails with an assert error:
PyPy assertion failed at rpython_memory_gc.c:11879: in
pypy_g_IncrementalMiniMarkGCBase__collect_obj: non-pinned nursery
obj in _collect_obj
I find this really confusing and disturbing. Nothing I do to
nursery objects should be any different. In fact, I tried changing
set_flags/add_flags/etc. to check is_in_nursery, like this:
if self.is_in_nursery(obj): return
incminimark.IncrementalMiniMarkGCBase.add_flags(self, obj, flags)
Even this didn't solve it, even though it meant that every
operation was on the tid field for nursery objects, and should be
identical to incminimark!
If I unconditionally called
incminimark.IncrementalMiniMarkGCBase.add_flags etc., then it only
passed tests if I did it for *all three* mutating methods.
My conclusion is that the remote_flags for some object in the
nursery is ignored, and we were setting flags on it at some point
even though it was is_in_nursery reported it was not in the nursery.
(e.g.: maybe it was copied to the nursery, but the remote_flags
field wasn't included. But why?)
Maybe it's time to step through in rr/gdb again.
* test_compile_framework_9 fails with a segfault.
I think this is purely because I have not found all the ways this
interacts poorly with the JIT compiler, and so it is solvable. For
example, it is fixed if I do the "if self.is_in_nursery(obj):"
checks above. My guess is that somewhere, nursery objects are
allocated with a garbage remote_flags (like the one in the JIT I
think I fixed in this commit), and I just need to catch that case,
or else completely stop using remote_flags for young objects.
diff --git a/rpython/jit/backend/llsupport/gc.py
b/rpython/jit/backend/llsupport/gc.py
--- a/rpython/jit/backend/llsupport/gc.py
+++ b/rpython/jit/backend/llsupport/gc.py
@@ -146,6 +146,7 @@
round_up = False
write_barrier_descr = None
fielddescr_tid = None
+ fielddescr_remote_flags = None
gcrootmap = None
str_type_id = 0
unicode_type_id = 0
@@ -393,6 +394,10 @@
def _setup_tid(self):
self.fielddescr_tid = get_field_descr(self, self.GCClass.HDR, 'tid')
+ if self.GCClass.has_remote_flags:
+ self.fielddescr_remote_flags = get_field_descr(self,
self.GCClass.HDR, 'remote_flags')
+ else:
+ self.fielddescr_remote_flags = None
frame_tid = self.layoutbuilder.get_type_id(jitframe.JITFRAME)
self.translator._jit2gc['frame_tid'] = frame_tid
diff --git a/rpython/jit/backend/llsupport/rewrite.py
b/rpython/jit/backend/llsupport/rewrite.py
--- a/rpython/jit/backend/llsupport/rewrite.py
+++ b/rpython/jit/backend/llsupport/rewrite.py
@@ -868,6 +868,11 @@
# produce a SETFIELD to initialize the GC header
self.emit_setfield(v_newgcobj, ConstInt(tid),
descr=self.gc_ll_descr.fielddescr_tid)
+ if self.gc_ll_descr.fielddescr_remote_flags is not None:
+ # produce a SETFIELD to initialize the rest of the GC header
+ # (Only present in _remoteheader GC)
+ self.emit_setfield(v_newgcobj, ConstInt(0),
+ descr=self.gc_ll_descr.fielddescr_remote_flags)
def gen_initialize_len(self, v_newgcobj, v_length, arraylen_descr):
# produce a SETFIELD to initialize the array length
diff --git a/rpython/jit/backend/llsupport/test/zrpy_gc_test.py
b/rpython/jit/backend/llsupport/test/zrpy_gc_test.py
--- a/rpython/jit/backend/llsupport/test/zrpy_gc_test.py
+++ b/rpython/jit/backend/llsupport/test/zrpy_gc_test.py
@@ -84,6 +84,8 @@
#
t = TranslationContext()
t.config.translation.gc = gc
+ # TODO: re-enable inlining when this test passes
+ t.config.translation.backendopt.inline = False
if gc != 'boehm':
t.config.translation.gcremovetypeptr = True
for name, value in kwds.items():
diff --git a/rpython/jit/backend/x86/test/test_zrpy_gc.py
b/rpython/jit/backend/x86/test/test_zrpy_gc.py
--- a/rpython/jit/backend/x86/test/test_zrpy_gc.py
+++ b/rpython/jit/backend/x86/test/test_zrpy_gc.py
@@ -4,3 +4,8 @@
class TestShadowStack(CompileFrameworkTests):
gcrootfinder = "shadowstack"
gc = "incminimark"
+
+
+class TestRemoteHeaderShadowStack(CompileFrameworkTests):
+ gcrootfinder = "shadowstack"
+ gc = "incminimark_remoteheader"
diff --git a/rpython/jit/backend/x86/test/test_zrpy_vmprof.py
b/rpython/jit/backend/x86/test/test_zrpy_vmprof.py
--- a/rpython/jit/backend/x86/test/test_zrpy_vmprof.py
+++ b/rpython/jit/backend/x86/test/test_zrpy_vmprof.py
@@ -2,6 +2,9 @@
from rpython.jit.backend.llsupport.test.zrpy_vmprof_test import
CompiledVmprofTest
class TestZVMprof(CompiledVmprofTest):
-
gcrootfinder = "shadowstack"
- gc = "incminimark"
\ No newline at end of file
+ gc = "incminimark"
+
+class TestRemoteHeaderZVMprof(CompiledVmprofTest):
+ gcrootfinder = "shadowstack"
+ gc = "incminimark_remoteheader"
diff --git a/rpython/jit/backend/x86/test/test_zvmprof.py
b/rpython/jit/backend/x86/test/test_zvmprof.py
--- a/rpython/jit/backend/x86/test/test_zvmprof.py
+++ b/rpython/jit/backend/x86/test/test_zvmprof.py
@@ -2,6 +2,9 @@
from rpython.jit.backend.llsupport.test.zrpy_vmprof_test import
CompiledVmprofTest
class TestZVMprof(CompiledVmprofTest):
-
gcrootfinder = "shadowstack"
- gc = "incminimark"
\ No newline at end of file
+ gc = "incminimark"
+
+class TestRemoteHeaderZVMprof(CompiledVmprofTest):
+ gcrootfinder = "shadowstack"
+ gc = "incminimark_remoteheader"
diff --git a/rpython/memory/gc/base.py b/rpython/memory/gc/base.py
--- a/rpython/memory/gc/base.py
+++ b/rpython/memory/gc/base.py
@@ -21,6 +21,7 @@
can_usually_pin_objects = False
object_minimal_size = 0
gcflag_extra = 0 # or a real GC flag that is always 0 when not collecting
+ has_remote_flags = False
def __init__(self, config, chunk_size=DEFAULT_CHUNK_SIZE,
translated_to_c=True):
diff --git a/rpython/memory/gc/incminimark.py b/rpython/memory/gc/incminimark.py
--- a/rpython/memory/gc/incminimark.py
+++ b/rpython/memory/gc/incminimark.py
@@ -625,6 +625,7 @@
#
# Get the memory from the nursery. If there is not enough space
# there, do a collect first.
+ ll_assert(self.nursery_free != llmemory.NULL, "malloc_fixedsize()
with uninitialized nursery")
result = self.nursery_free
self.nursery_free = new_free = result + totalsize
if new_free > self.nursery_top:
@@ -685,6 +686,7 @@
#
# Get the memory from the nursery. If there is not enough space
# there, do a collect first.
+ ll_assert(self.nursery_free != llmemory.NULL, "malloc_varsize()
without initializing nursery")
result = self.nursery_free
self.nursery_free = new_free = result + totalsize
if new_free > self.nursery_top:
@@ -1088,6 +1090,7 @@
# have been chosen to allow 'flags' to be zero in the common
# case (hence the 'NO' in their name).
hdr = llmemory.cast_adr_to_ptr(addr, lltype.Ptr(self.HDR))
+ ll_assert(hdr != lltype.nullptr(self.HDR), "init_gc_object():
addr==NULL")
hdr.tid = self.combine(typeid16, flags)
def init_gc_object_immortal(self, addr, typeid16, flags=0):
@@ -1120,7 +1123,8 @@
Implemented a bit obscurely by checking an unrelated flag
that can never be set on a young object -- except if tid == -42.
"""
- assert self.is_in_nursery(obj)
+ ll_assert(self.is_in_nursery(obj), "is_forwarded(): object not in
nursery!")
+
tid = self.get_flags(obj)
result = (tid & GCFLAG_FINALIZATION_ORDERING != 0)
if result:
@@ -1439,7 +1443,8 @@
self.add_flags(addr_array, GCFLAG_CARDS_SET)
remember_young_pointer_from_array2._dont_inline_ = True
- assert self.card_page_indices > 0
+ ll_assert(self.card_page_indices > 0,
+ "_init_writebarrier_with_card_marker(): card_page_indices <=
0")
self.remember_young_pointer_from_array2 = (
remember_young_pointer_from_array2)
@@ -1523,7 +1528,8 @@
def manually_copy_card_bits(self, source_addr, dest_addr, length):
# manually copy the individual card marks from source to dest
- assert self.card_page_indices > 0
+ ll_assert(self.card_page_indices > 0,
+ "_init_writebarrier_with_card_marker(): card_page_indices <=
0")
bytes = self.card_marking_bytes_for_length(length)
#
anybyte = 0
@@ -1684,12 +1690,13 @@
nursery_barriers = self.AddressDeque()
prev = self.nursery
self.surviving_pinned_objects.sort()
- assert self.pinned_objects_in_nursery == \
- self.surviving_pinned_objects.length()
+ ll_assert(self.pinned_objects_in_nursery == \
+ self.surviving_pinned_objects.length(),
+ "_minor_collection(): self.pinned_objects_in_nursery !=
self.surviving_pinned_objects.length()")
while self.surviving_pinned_objects.non_empty():
#
cur = self.surviving_pinned_objects.pop()
- assert cur >= prev
+ ll_assert(cur >= prev, "_minor_collection(): cur < prev")
#
# clear the arena between the last pinned object (or arena start)
# and the pinned object
@@ -1747,7 +1754,7 @@
debug_stop("gc-minor")
def _reset_flag_old_objects_pointing_to_pinned(self, obj, ignore):
- assert self.get_flags(obj) & GCFLAG_PINNED_OBJECT_PARENT_KNOWN
+ ll_assert(self.get_flags(obj) & GCFLAG_PINNED_OBJECT_PARENT_KNOWN,
"_reset_flag_old_objects_pointing_to_pinned(): not
GCFLAG_PINNED_OBJECT_PARENT_KNOWN")
self.remove_flags(obj, GCFLAG_PINNED_OBJECT_PARENT_KNOWN)
def _visit_old_objects_pointing_to_pinned(self, obj, ignore):
@@ -3056,7 +3063,6 @@
gc.finalize_header(hdr)
return True
-
class IncrementalMiniMarkGC(IncrementalMiniMarkGCBase):
HDR = lltype.Struct('header', ('tid', lltype.Signed))
# During a minor collection, the objects in the nursery that are
diff --git a/rpython/memory/gc/incminimark_remoteheader.py
b/rpython/memory/gc/incminimark_remoteheader.py
--- a/rpython/memory/gc/incminimark_remoteheader.py
+++ b/rpython/memory/gc/incminimark_remoteheader.py
@@ -4,15 +4,19 @@
from rpython.memory.gc import incminimark
from rpython.rlib.rarithmetic import LONG_BIT
from rpython.rtyper.lltypesystem import rffi, lltype, llmemory
+from rpython.rlib.debug import ll_assert
SIGNEDP = lltype.Ptr(lltype.FixedSizeArray(lltype.Signed, 1))
class IncrementalMiniMarkRemoteHeaderGC(incminimark.IncrementalMiniMarkGCBase):
# The GC header is similar to incminimark, except that the flags can be
# placed anywhere, not just in the bits of tid.
- HDR = lltype.Struct('header',
- ('tid', lltype.Signed),
- ('remote_flags', SIGNEDP))
+ HDR = lltype.Struct(
+ 'header',
+ ('tid', lltype.Signed),
+ ('remote_flags', SIGNEDP)
+ )
+ has_remote_flags = True
minimal_size_in_nursery = (
llmemory.sizeof(HDR) + llmemory.sizeof(llmemory.Address))
@@ -35,21 +39,22 @@
# This gets compiled to nonsense like (&pypy_g_header_1433.h_tid)
# at the top level (global variable initialization). Instead, we set
# it to NULL and lazily initialize it later.
- ## hdr.remote_flags = lltype.direct_fieldptr(hdr, 'tid')
hdr.remote_flags = lltype.nullptr(SIGNEDP.TO)
def make_forwardstub(self, obj, forward_to):
- assert (self.header(obj).remote_flags
- == lltype.direct_fieldptr(self.header(obj), 'tid')), \
- "Nursery objects should not have separately-allocated flags."
+ hdr = self.header(obj)
+ ll_assert(
+ hdr.remote_flags == lltype.nullptr(SIGNEDP.TO)
+ or hdr.remote_flags == lltype.direct_fieldptr(hdr, 'tid'),
+ "Nursery objects should not have separately-allocated flags.")
incminimark.IncrementalMiniMarkGCBase.make_forwardstub(self, obj,
forward_to)
- hdr = self.header(obj)
- hdr.remote_flags = lltype.direct_fieldptr(hdr, 'tid')
def copy_header(self, src, dest):
dest_hdr = self.header(dest)
dest_hdr.tid = self.get_flags(src)
- dest_hdr.remote_flags = lltype.direct_fieldptr(dest_hdr, 'tid')
+ ll_assert(
+ not self.is_in_nursery(dest),
+ "Copying headers to another nursery element?")
self.__extract_flags_to_pointer(dest_hdr)
def __extract_flags_to_pointer(self, hdr):
@@ -57,9 +62,6 @@
Expects the object to not use inline tid-flags.
"""
- assert (hdr.remote_flags == lltype.nullptr(SIGNEDP.TO)
- or hdr.remote_flags == lltype.direct_fieldptr(hdr, 'tid')), \
- "leaking old remote_flags!"
size = llmemory.sizeof(lltype.Signed)
adr = self.__ac_for_flags.malloc(size)
hdr.remote_flags = llmemory.cast_adr_to_ptr(adr, SIGNEDP)
@@ -67,7 +69,8 @@
def finalize_header(self, adr):
hdr = llmemory.cast_adr_to_ptr(adr, lltype.Ptr(self.HDR))
- if hdr.remote_flags != lltype.nullptr(SIGNEDP.TO):
+ if (hdr.remote_flags != lltype.nullptr(SIGNEDP.TO)
+ and hdr.remote_flags != lltype.direct_fieldptr(hdr, 'tid')):
# If it points to allocated memory, this will be picked up by
# __free_flags_if_finalized.
hdr.remote_flags[0] |= incminimark.GCFLAG_DEAD
@@ -84,31 +87,28 @@
# Manipulate flags through a pointer.
- def __lazy_init_flags(self, hdr):
+ def __lazy_init_flags(self, obj):
+ hdr = self.header(obj)
# XXX Is there anywhere I can initialize this only once without having
# to check for null on EVERY access?
if hdr.remote_flags == lltype.nullptr(SIGNEDP.TO):
hdr.remote_flags = lltype.direct_fieldptr(hdr, 'tid')
def get_flags(self, obj):
- hdr = self.header(obj)
- self.__lazy_init_flags(hdr)
- return hdr.remote_flags[0]
+ self.__lazy_init_flags(obj)
+ return self.header(obj).remote_flags[0]
def set_flags(self, obj, flags):
- hdr = self.header(obj)
- self.__lazy_init_flags(hdr)
- hdr.remote_flags[0] = flags
+ self.__lazy_init_flags(obj)
+ self.header(obj).remote_flags[0] = flags
def add_flags(self, obj, flags):
- hdr = self.header(obj)
- self.__lazy_init_flags(hdr)
- hdr.remote_flags[0] |= flags
+ self.__lazy_init_flags(obj)
+ self.header(obj).remote_flags[0] |= flags
def remove_flags(self, obj, flags):
- hdr = self.header(obj)
- self.__lazy_init_flags(hdr)
- hdr.remote_flags[0] &= ~flags
+ self.__lazy_init_flags(obj)
+ self.header(obj).remote_flags[0] &= ~flags
def _free_flags_if_finalized(adr, unused_arg):
diff --git a/rpython/translator/c/test/test_newgc.py
b/rpython/translator/c/test/test_newgc.py
--- a/rpython/translator/c/test/test_newgc.py
+++ b/rpython/translator/c/test/test_newgc.py
@@ -1660,7 +1660,7 @@
assert res == 42
-class TestIncrementalMiniMarkRemoteHeadersGC(TestIncrementalMiniMarkGC):
+class TestIncrementalMiniMarkRemoteHeaderGC(TestIncrementalMiniMarkGC):
gcpolicy = "incminimark_remoteheader"
@@ -1760,5 +1760,5 @@
class TestIncrementalMiniMarkGCMostCompact(TaggedPointersTest,
TestIncrementalMiniMarkGC):
removetypeptr = True
-class TestIncrementalMiniMarkRemoteHeadersGCMostCompact(TaggedPointersTest,
TestIncrementalMiniMarkRemoteHeadersGC):
+class TestIncrementalMiniMarkRemoteHeaderGCMostCompact(TaggedPointersTest,
TestIncrementalMiniMarkRemoteHeaderGC):
removetypeptr = True
_______________________________________________
pypy-commit mailing list
[email protected]
https://mail.python.org/mailman/listinfo/pypy-commit
