[pypy-commit] pypy reverse-debugger: Go through the list of all llops and mark the ones that are unsafe.

arigo pypy.commits at gmail.com
Thu Jun 30 08:10:18 EDT 2016


Author: Armin Rigo <arigo at tunes.org>
Branch: reverse-debugger
Changeset: r85469:567dcdd8e1b6
Date: 2016-06-30 14:11 +0200
http://bitbucket.org/pypy/pypy/changeset/567dcdd8e1b6/

Log:	Go through the list of all llops and mark the ones that are unsafe.

diff --git a/rpython/memory/gctransform/boehm.py b/rpython/memory/gctransform/boehm.py
--- a/rpython/memory/gctransform/boehm.py
+++ b/rpython/memory/gctransform/boehm.py
@@ -16,11 +16,11 @@
         super(BoehmGCTransformer, self).__init__(translator, inline=inline)
         self.finalizer_funcptrs = {}
 
-        atomic_mh = mallocHelpers()
+        atomic_mh = mallocHelpers(gckind='gc')
         atomic_mh.allocate = lambda size: llop.boehm_malloc_atomic(llmemory.GCREF, size)
         ll_malloc_fixedsize_atomic = atomic_mh._ll_malloc_fixedsize
 
-        mh = mallocHelpers()
+        mh = mallocHelpers(gckind='gc')
         mh.allocate = lambda size: llop.boehm_malloc(llmemory.GCREF, size)
         ll_malloc_fixedsize = mh._ll_malloc_fixedsize
 
diff --git a/rpython/memory/gctransform/refcounting.py b/rpython/memory/gctransform/refcounting.py
--- a/rpython/memory/gctransform/refcounting.py
+++ b/rpython/memory/gctransform/refcounting.py
@@ -54,7 +54,7 @@
         def ll_no_pointer_dealloc(adr):
             llop.gc_free(lltype.Void, adr)
 
-        mh = mallocHelpers()
+        mh = mallocHelpers(gckind='gc')
         mh.allocate = llmemory.raw_malloc
         def ll_malloc_fixedsize(size):
             size = gc_header_offset + size
diff --git a/rpython/memory/gctransform/transform.py b/rpython/memory/gctransform/transform.py
--- a/rpython/memory/gctransform/transform.py
+++ b/rpython/memory/gctransform/transform.py
@@ -411,7 +411,7 @@
 
 # ________________________________________________________________
 
-def mallocHelpers():
+def mallocHelpers(gckind):
     class _MallocHelpers(object):
         def _freeze_(self):
             return True
@@ -442,9 +442,16 @@
     mh._ll_malloc_varsize_no_length = _ll_malloc_varsize_no_length
     mh.ll_malloc_varsize_no_length = _ll_malloc_varsize_no_length
 
+    if gckind == 'raw':
+        llopstore = llop.raw_store
+    elif gckind == 'gc':
+        llopstore = llop.gc_store
+    else:
+        raise AssertionError(gckind)
+
     def ll_malloc_varsize(length, size, itemsize, lengthoffset):
         result = mh.ll_malloc_varsize_no_length(length, size, itemsize)
-        llop.raw_store(lltype.Void, result, lengthoffset, length)
+        llopstore(lltype.Void, result, lengthoffset, length)
         return result
     mh.ll_malloc_varsize = ll_malloc_varsize
 
@@ -464,7 +471,7 @@
     def __init__(self, translator, inline=False):
         super(GCTransformer, self).__init__(translator, inline=inline)
 
-        mh = mallocHelpers()
+        mh = mallocHelpers(gckind='raw')
         mh.allocate = llmemory.raw_malloc
         ll_raw_malloc_fixedsize = mh._ll_malloc_fixedsize
         ll_raw_malloc_varsize_no_length = mh.ll_malloc_varsize_no_length
diff --git a/rpython/rtyper/llinterp.py b/rpython/rtyper/llinterp.py
--- a/rpython/rtyper/llinterp.py
+++ b/rpython/rtyper/llinterp.py
@@ -1128,9 +1128,26 @@
     def op_revdb_stop_point(self, *args):
         pass
 
-    def op_revdb_send_output(self, ll_string):
-        from rpython.rtyper.annlowlevel import hlstr
-        sys.stdout.write(hlstr(ll_string))
+    def op_revdb_send_answer(self, *args):
+        raise NotImplementedError
+    def op_revdb_breakpoint(self, *args):
+        raise NotImplementedError
+    def op_revdb_get_value(self, *args):
+        raise NotImplementedError
+    def op_revdb_get_unique_id(self, *args):
+        raise NotImplementedError
+
+    def op_revdb_watch_save_state(self, *args):
+        return False
+
+    def op_revdb_watch_restore_state(self, *args):
+        raise NotImplementedError
+    def op_revdb_weakref_create(self, *args):
+        raise NotImplementedError
+    def op_revdb_weakref_deref(self, *args):
+        raise NotImplementedError
+    def op_revdb_call_destructor(self, *args):
+        raise NotImplementedError
 
 
 class Tracer(object):
diff --git a/rpython/rtyper/lltypesystem/lloperation.py b/rpython/rtyper/lltypesystem/lloperation.py
--- a/rpython/rtyper/lltypesystem/lloperation.py
+++ b/rpython/rtyper/lltypesystem/lloperation.py
@@ -8,7 +8,8 @@
 class LLOp(object):
 
     def __init__(self, sideeffects=True, canfold=False, canraise=(),
-                 canmallocgc=False, canrun=False, tryfold=False):
+                 canmallocgc=False, canrun=False, tryfold=False,
+                 revdb_protect=False):
         # self.opname = ... (set afterwards)
 
         if canfold:
@@ -41,6 +42,9 @@
         # The operation can be run directly with __call__
         self.canrun = canrun or canfold
 
+        # RevDB: the operation must always be protected with RPY_REVDB_EMIT()
+        self.revdb_protect = revdb_protect
+
     # __________ make the LLOp instances callable from LL helpers __________
 
     __name__ = property(lambda self: 'llop_'+self.opname)
@@ -385,17 +389,19 @@
     'boehm_malloc_atomic':  LLOp(),
     'boehm_register_finalizer': LLOp(),
     'boehm_disappearing_link': LLOp(),
-    'raw_malloc':           LLOp(),
+    'raw_malloc':           LLOp(revdb_protect=True),
     'raw_malloc_usage':     LLOp(sideeffects=False),
-    'raw_free':             LLOp(),
-    'raw_memclear':         LLOp(),
-    'raw_memset':           LLOp(),
-    'raw_memcopy':          LLOp(),
-    'raw_memmove':          LLOp(),
-    'raw_load':             LLOp(sideeffects=False, canrun=True),
-    'raw_store':            LLOp(canrun=True),
-    'bare_raw_store':       LLOp(),
+    'raw_free':             LLOp(revdb_protect=True),
+    'raw_memclear':         LLOp(revdb_protect=True),
+    'raw_memset':           LLOp(revdb_protect=True),
+    'raw_memcopy':          LLOp(revdb_protect=True),
+    'raw_memmove':          LLOp(revdb_protect=True),
+    'raw_load':             LLOp(revdb_protect=True, sideeffects=False,
+                                                     canrun=True),
+    'raw_store':            LLOp(revdb_protect=True, canrun=True),
+    'bare_raw_store':       LLOp(revdb_protect=True),
     'gc_load_indexed':      LLOp(sideeffects=False, canrun=True),
+    'gc_store':             LLOp(canrun=True),
     'stack_malloc':         LLOp(), # mmh
     'track_alloc_start':    LLOp(),
     'track_alloc_stop':     LLOp(),
@@ -442,7 +448,7 @@
     'get_write_barrier_failing_case': LLOp(sideeffects=False),
     'get_write_barrier_from_array_failing_case': LLOp(sideeffects=False),
     'gc_get_type_info_group': LLOp(sideeffects=False),
-    'll_read_timestamp': LLOp(canrun=True),
+    'll_read_timestamp': LLOp(revdb_protect=True, canrun=True),
 
     # __________ GC operations __________
 
@@ -456,8 +462,8 @@
     # see rlib/objectmodel for gc_identityhash and gc_id
     'gc_identityhash':      LLOp(sideeffects=False, canmallocgc=True),
     'gc_id':                LLOp(sideeffects=False, canmallocgc=True),
-    'gc_obtain_free_space': LLOp(),
-    'gc_set_max_heap_size': LLOp(),
+    'gc_obtain_free_space': LLOp(revdb_protect=True),
+    'gc_set_max_heap_size': LLOp(revdb_protect=True),
     'gc_can_move'         : LLOp(sideeffects=False),
     'gc_thread_run'       : LLOp(),
     'gc_thread_start'     : LLOp(),
@@ -523,7 +529,7 @@
 
     # __________ misc operations __________
 
-    'stack_current':        LLOp(sideeffects=False),
+    'stack_current':        LLOp(revdb_protect=True, sideeffects=False),
     'keepalive':            LLOp(),
     'same_as':              LLOp(canfold=True),
     'hint':                 LLOp(),
diff --git a/rpython/rtyper/lltypesystem/opimpl.py b/rpython/rtyper/lltypesystem/opimpl.py
--- a/rpython/rtyper/lltypesystem/opimpl.py
+++ b/rpython/rtyper/lltypesystem/opimpl.py
@@ -706,6 +706,7 @@
     TVAL = lltype.typeOf(newvalue)
     p = rffi.cast(rffi.CArrayPtr(TVAL), p + ofs)
     p[0] = newvalue
+op_gc_store = op_raw_store
 
 def op_raw_load(TVAL, p, ofs):
     from rpython.rtyper.lltypesystem import rffi
diff --git a/rpython/translator/c/funcgen.py b/rpython/translator/c/funcgen.py
--- a/rpython/translator/c/funcgen.py
+++ b/rpython/translator/c/funcgen.py
@@ -266,7 +266,8 @@
     def gen_op(self, op):
         macro = 'OP_%s' % op.opname.upper()
         line = None
-        if op.opname.startswith('gc_') and op.opname != 'gc_load_indexed':
+        if (op.opname.startswith('gc_') and op.opname != 'gc_load_indexed'
+                                        and op.opname != 'gc_store'):
             meth = getattr(self.gcpolicy, macro, None)
             if meth:
                 line = meth(self, op)
@@ -278,6 +279,11 @@
             lst = [self.expr(v) for v in op.args]
             lst.append(self.expr(op.result))
             line = '%s(%s);' % (macro, ', '.join(lst))
+        if self.db.reverse_debugger:
+            from rpython.translator.revdb import gencsupp
+            if op.opname in gencsupp.set_revdb_protected:
+                line = gencsupp.emit(line, self.lltypename(op.result),
+                                     self.expr(op.result))
         if "\n" not in line:
             yield line
         else:
@@ -435,7 +441,7 @@
         return 'abort();  /* jit_conditional_call */'
 
     # low-level operations
-    def generic_get(self, op, sourceexpr):
+    def generic_get(self, op, sourceexpr, accessing_mem=True):
         T = self.lltypemap(op.result)
         newvalue = self.expr(op.result, special_case_void=False)
         result = '%s = %s;' % (newvalue, sourceexpr)
@@ -445,7 +451,8 @@
             S = self.lltypemap(op.args[0]).TO
             if (S._gckind != 'gc' and not S._hints.get('is_excdata')
                     and not S._hints.get('static_immutable')
-                    and not S._hints.get('ignore_revdb')):
+                    and not S._hints.get('ignore_revdb')
+                    and accessing_mem):
                 from rpython.translator.revdb import gencsupp
                 result = gencsupp.emit(result, self.lltypename(op.result),
                                        newvalue)
@@ -464,7 +471,7 @@
                 result = gencsupp.emit_void(result)
         return result
 
-    def OP_GETFIELD(self, op, ampersand=''):
+    def OP_GETFIELD(self, op, ampersand='', accessing_mem=True):
         assert isinstance(op.args[1], Constant)
         STRUCT = self.lltypemap(op.args[0]).TO
         structdef = self.db.gettypedefnode(STRUCT)
@@ -472,7 +479,7 @@
         expr = ampersand + structdef.ptr_access_expr(self.expr(op.args[0]),
                                                      op.args[1].value,
                                                      baseexpr_is_const)
-        return self.generic_get(op, expr)
+        return self.generic_get(op, expr, accessing_mem=accessing_mem)
 
     def OP_BARE_SETFIELD(self, op):
         assert isinstance(op.args[1], Constant)
@@ -488,9 +495,9 @@
         RESULT = self.lltypemap(op.result).TO
         if (isinstance(RESULT, FixedSizeArray) or
                 (isinstance(RESULT, Array) and barebonearray(RESULT))):
-            return self.OP_GETFIELD(op, ampersand='')
+            return self.OP_GETFIELD(op, ampersand='', accessing_mem=False)
         else:
-            return self.OP_GETFIELD(op, ampersand='&')
+            return self.OP_GETFIELD(op, ampersand='&', accessing_mem=False)
 
     def OP_GETARRAYSIZE(self, op):
         ARRAY = self.lltypemap(op.args[0]).TO
@@ -498,8 +505,7 @@
             return '%s = %d;' % (self.expr(op.result),
                                  ARRAY.length)
         else:
-            return '%s = %s->length;' % (self.expr(op.result),
-                                         self.expr(op.args[0]))
+            return self.generic_get(op, '%s->length;' % self.expr(op.args[0]))
 
     def OP_GETARRAYITEM(self, op):
         ARRAY = self.lltypemap(op.args[0]).TO
@@ -563,8 +569,7 @@
             return '%s = %d;'%(self.expr(op.result), ARRAY.length)
         else:
             assert isinstance(ARRAY, Array)
-            return '%s = %s.length;'%(self.expr(op.result), expr)
-
+            return self.generic_get(op, '%s.length;' % expr)
 
 
     def OP_PTR_NONZERO(self, op):
@@ -608,18 +613,14 @@
         return 'GC_REGISTER_FINALIZER(%s, (GC_finalization_proc)%s, NULL, NULL, NULL);' \
                % (self.expr(op.args[0]), self.expr(op.args[1]))
 
-    def OP_RAW_MALLOC(self, op):
-        eresult = self.expr(op.result)
-        esize = self.expr(op.args[0])
-        return "OP_RAW_MALLOC(%s, %s, void *);" % (esize, eresult)
-
     def OP_STACK_MALLOC(self, op):
-        eresult = self.expr(op.result)
-        esize = self.expr(op.args[0])
-        return "OP_STACK_MALLOC(%s, %s, void *);" % (esize, eresult)
+        #eresult = self.expr(op.result)
+        #esize = self.expr(op.args[0])
+        #return "OP_STACK_MALLOC(%s, %s, void *);" % (esize, eresult)
+        raise NotImplementedError
 
     def OP_DIRECT_FIELDPTR(self, op):
-        return self.OP_GETFIELD(op, ampersand='&')
+        return self.OP_GETFIELD(op, ampersand='&', accessing_mem=False)
 
     def OP_DIRECT_ARRAYITEMS(self, op):
         ARRAY = self.lltypemap(op.args[0]).TO
@@ -672,8 +673,10 @@
 
     def OP_CAST_PTR_TO_INT(self, op):
         if self.db.reverse_debugger:
-            from rpython.translator.revdb import gencsupp
-            return gencsupp.cast_ptr_to_int(self, op)
+            TSRC = self.lltypemap(op.args[0])
+            if isinstance(TSRC, Ptr) and TSRC.TO._gckind == 'gc':
+                from rpython.translator.revdb import gencsupp
+                return gencsupp.cast_gcptr_to_int(self, op)
         return self.OP_CAST_POINTER(op)
 
     def OP_LENGTH_OF_SIMPLE_GCARRAY_FROM_OPAQUE(self, op):
@@ -723,6 +726,7 @@
            '((%(typename)s) (((char *)%(addr)s) + %(offset)s))[0] = %(value)s;'
            % locals())
     OP_BARE_RAW_STORE = OP_RAW_STORE
+    OP_GC_STORE = OP_RAW_STORE     # the difference is only in 'revdb_protect'
 
     def OP_RAW_LOAD(self, op):
         addr = self.expr(op.args[0])
diff --git a/rpython/translator/c/src/mem.h b/rpython/translator/c/src/mem.h
--- a/rpython/translator/c/src/mem.h
+++ b/rpython/translator/c/src/mem.h
@@ -8,8 +8,8 @@
 #define OP_STACK_CURRENT(r)  r = (Signed)&r
 
 
-#define OP_RAW_MALLOC(size, r, restype)  {				\
-	r = (restype) malloc(size);				\
+#define OP_RAW_MALLOC(size, r)  {				        \
+	r = malloc(size);				                \
 	if (r != NULL) {						\
 	    COUNT_MALLOC;						\
 	}								\
diff --git a/rpython/translator/revdb/gencsupp.py b/rpython/translator/revdb/gencsupp.py
--- a/rpython/translator/revdb/gencsupp.py
+++ b/rpython/translator/revdb/gencsupp.py
@@ -1,5 +1,6 @@
 import py
 from rpython.rtyper.lltypesystem import lltype, llmemory, rffi, rstr
+from rpython.rtyper.lltypesystem.lloperation import LL_OPERATIONS
 from rpython.translator.c.support import cdecl
 from rpython.rlib import exports, revdb
 
@@ -25,10 +26,13 @@
     return 'rpy_reverse_db_register_destructor(%s, %s);' % (
         funcgen.expr(op.args[0]), funcgen.expr(op.args[1]))
 
-def cast_ptr_to_int(funcgen, op):
+def cast_gcptr_to_int(funcgen, op):
     return '%s = RPY_REVDB_CAST_PTR_TO_INT(%s);' % (
         funcgen.expr(op.result), funcgen.expr(op.args[0]))
 
+set_revdb_protected = set(opname for opname, opdesc in LL_OPERATIONS.items()
+                                 if opdesc.revdb_protect)
+
 
 def prepare_database(db):
     FUNCPTR = lltype.Ptr(lltype.FuncType([revdb._CMDPTR, lltype.Ptr(rstr.STR)],
diff --git a/rpython/translator/revdb/src-revdb/revdb.c b/rpython/translator/revdb/src-revdb/revdb.c
--- a/rpython/translator/revdb/src-revdb/revdb.c
+++ b/rpython/translator/revdb/src-revdb/revdb.c
@@ -966,7 +966,7 @@
         memcpy(future_ids, extra, cmd->extra_size);
         future_ids[cmd->extra_size / sizeof(uint64_t)] = 0;
         uid_break = *future_ids;
-        attach_gdb();
+        //attach_gdb();
     }
     future_next_id = future_ids;
 }
@@ -1274,8 +1274,6 @@
 
 static void replay_call_destructors(void)
 {
-    fq_trigger();
-
     /* Re-enable fetching (disabled when we saw ASYNC_FINALIZER_TRIGGER),
        and fetch the uid's of dying objects with old-style destructors.
     */
@@ -1295,6 +1293,18 @@
         obj = _ftree_pop(&destructor_tree, uid, &callback);
         callback(obj);
     }
+
+    /* Now we're back in normal mode.  We trigger the finalizer 
+       queues here. */
+    fq_trigger();
 }
 
 /* ------------------------------------------------------------ */
+
+
+RPY_EXTERN
+void seeing_uid(uint64_t uid)
+{
+    if (uid == 1895569)
+        attach_gdb();
+}
diff --git a/rpython/translator/revdb/src-revdb/revdb_include.h b/rpython/translator/revdb/src-revdb/revdb_include.h
--- a/rpython/translator/revdb/src-revdb/revdb_include.h
+++ b/rpython/translator/revdb/src-revdb/revdb_include.h
@@ -37,8 +37,9 @@
             ((unsigned long long)_e) & ((2ULL << (8*sizeof(_e)-1)) - 1))
 #endif
 
-#if 1    /* enable to print all allocs to stderr */
+#if 0    /* enable to print all allocs to stderr */
 #  define _RPY_REVDB_PRUID()                                    \
+    seeing_uid(uid);                                            \
     fprintf(stderr,                                             \
             "%s:%d: obj %llu\n",                                \
             __FILE__, __LINE__, (unsigned long long) uid)
diff --git a/rpython/translator/revdb/test/test_basic.py b/rpython/translator/revdb/test/test_basic.py
--- a/rpython/translator/revdb/test/test_basic.py
+++ b/rpython/translator/revdb/test/test_basic.py
@@ -83,6 +83,11 @@
     def done(self):
         return self.cur == len(self.buffer)
 
+    def write_call(self, expected_string):
+        x = self.next()   # raw_malloc: the pointer we got
+        x = self.next(); assert x == len(expected_string)
+        x = self.next('i'); assert x == 0      # errno
+
 
 def compile(self, entry_point, backendopt=True,
             withsmallfuncsets=None):
@@ -131,9 +136,7 @@
         self.compile(main, backendopt=False)
         assert self.run('abc d') == '[abc, d]\n'
         rdb = self.fetch_rdb([self.exename, 'abc', 'd'])
-        # write() call
-        x = rdb.next(); assert x == len('[abc, d]\n')
-        x = rdb.next('i'); assert x == 0      # errno
+        rdb.write_call('[abc, d]\n')
         x = rdb.next('q'); assert x == 0      # number of stop points
         # that's all we should get from this simple example
         assert rdb.done()
@@ -151,9 +154,7 @@
         hash_value = int(match.group(1))
         rdb = self.fetch_rdb([self.exename, 'Xx'])
         # compute_identity_hash() doesn't record anything
-        # write() call
-        x = rdb.next(); assert x == len(out)
-        x = rdb.next('i'); assert x == 0      # errno
+        rdb.write_call(out)
         # done
         x = rdb.next('q'); assert x == 0      # number of stop points
         assert rdb.done()
@@ -174,8 +175,7 @@
         # write() call (it used to be the case that vtable reads where
         # recorded too; the single byte fetched from the vtable from
         # the '.x' in main() would appear here)
-        x = rdb.next(); assert x == len(out)
-        x = rdb.next('i'); assert x == 0      # errno
+        rdb.write_call(out)
         # done
         x = rdb.next('q'); assert x == 0      # number of stop points
         assert rdb.done()
@@ -195,8 +195,7 @@
         assert out == '41\n'
         rdb = self.fetch_rdb([self.exename, 'Xx'])
         # write() call
-        x = rdb.next(); assert x == len(out)
-        x = rdb.next('i'); assert x == 0      # errno
+        rdb.write_call(out)
         # done
         x = rdb.next('q'); assert x == 0      # number of stop points
         assert rdb.done()
@@ -230,8 +229,7 @@
             assert out == expected_output
             rdb = self.fetch_rdb([self.exename] + input.split())
             # write() call
-            x = rdb.next(); assert x == len(out)
-            x = rdb.next('i'); assert x == 0      # errno
+            rdb.write_call(out)
             x = rdb.next('q'); assert x == 0      # number of stop points
             assert rdb.done()
 
diff --git a/rpython/translator/revdb/test/test_weak.py b/rpython/translator/revdb/test/test_weak.py
--- a/rpython/translator/revdb/test/test_weak.py
+++ b/rpython/translator/revdb/test/test_weak.py
@@ -251,9 +251,7 @@
             x = rdb.next()
             assert x == len(seen_uids)
         assert len(seen_uids) == int(out)
-        # write() call
-        x = rdb.next(); assert x == len(out)
-        x = rdb.next('i'); assert x == 0      # errno
+        rdb.write_call(out)
         x = rdb.next('q'); assert x == 3000    # number of stop points
 
 


More information about the pypy-commit mailing list