[pypy-commit] pypy stm-thread-2: stm: don't turn inevitable before raw-mallocs and raw-accesses/frees to the freshly allocated memory

[Raemi]

Author: Remi Meier <meierrem at student.ethz.ch>
Branch: stm-thread-2
Changeset: r59396:9cf6b9415c82
Date: 2012-11-23 15:35 +0100
http://bitbucket.org/pypy/pypy/changeset/9cf6b9415c82/

Log:	stm: don't turn inevitable before raw-mallocs and raw-accesses/frees
	to the freshly allocated memory this leaks memory in case the
	transaction performing malloc aborts

diff --git a/pypy/translator/stm/inevitable.py b/pypy/translator/stm/inevitable.py
--- a/pypy/translator/stm/inevitable.py
+++ b/pypy/translator/stm/inevitable.py
@@ -26,10 +26,9 @@
 SETTERS = set(['setfield', 'setarrayitem', 'setinteriorfield'])
 MALLOCS = set(['malloc', 'malloc_varsize',
                'malloc_nonmovable', 'malloc_nonmovable_varsize'])
-
 # ____________________________________________________________
 
-def should_turn_inevitable_getter_setter(op):
+def should_turn_inevitable_getter_setter(op, fresh_mallocs):
     # Getters and setters are allowed if their first argument is a GC pointer.
     # If it is a RAW pointer, and it is a read from a non-immutable place,
     # and it doesn't use the hint 'stm_dont_track_raw_accesses', then they
@@ -41,9 +40,9 @@
         return False
     if S._hints.get('stm_dont_track_raw_accesses', False):
         return False
-    return True
+    return not fresh_mallocs.is_fresh_malloc(op.args[0])
 
-def should_turn_inevitable(op, block):
+def should_turn_inevitable(op, block, fresh_mallocs):
     # Always-allowed operations never cause a 'turn inevitable'
     if op.opname in ALWAYS_ALLOW_OPERATIONS:
         return False
@@ -52,16 +51,24 @@
     if op.opname in GETTERS:
         if op.result.concretetype is lltype.Void:
             return False
-        return should_turn_inevitable_getter_setter(op)
+        return should_turn_inevitable_getter_setter(op, fresh_mallocs)
     if op.opname in SETTERS:
         if op.args[-1].concretetype is lltype.Void:
             return False
-        return should_turn_inevitable_getter_setter(op)
+        return should_turn_inevitable_getter_setter(op, fresh_mallocs)
     #
-    # Mallocs
+    # Mallocs & Frees
     if op.opname in MALLOCS:
-        flags = op.args[1].value
-        return flags['flavor'] != 'gc'
+        # flags = op.args[1].value
+        # return flags['flavor'] != 'gc'
+        return False # XXX: Produces memory leaks on aborts
+    if op.opname == 'free':
+        # We can only run a CFG in non-inevitable mode from start
+        # to end in one transaction (every free gets called once
+        # for every fresh malloc). No need to turn inevitable.
+        # If the transaction is splitted, the remaining parts of the
+        # CFG will always run in inevitable mode anyways.
+        return not fresh_mallocs.is_fresh_malloc(op.args[0])
 
     #
     # Function calls
@@ -97,10 +104,12 @@
     return SpaceOperation('stm_become_inevitable', [c_info],
                           varoftype(lltype.Void))
 
-def insert_turn_inevitable(translator, graph):
+def insert_turn_inevitable(graph):
+    from pypy.translator.backendopt.writeanalyze import FreshMallocs
+    fresh_mallocs = FreshMallocs(graph)
     for block in graph.iterblocks():
         for i in range(len(block.operations)-1, -1, -1):
             op = block.operations[i]
-            if should_turn_inevitable(op, block):
+            if should_turn_inevitable(op, block, fresh_mallocs):
                 inev_op = turn_inevitable_op(op.opname)
                 block.operations.insert(i, inev_op)
diff --git a/pypy/translator/stm/test/test_inevitable.py b/pypy/translator/stm/test/test_inevitable.py
--- a/pypy/translator/stm/test/test_inevitable.py
+++ b/pypy/translator/stm/test/test_inevitable.py
@@ -20,7 +20,7 @@
         interp, self.graph = get_interpreter(fn, args, view=False)
         interp.frame_class = LLSTMInevFrame
         self.translator = interp.typer.annotator.translator
-        insert_turn_inevitable(self.translator, self.graph)
+        insert_turn_inevitable(self.graph)
         if option.view:
             self.translator.view()
         #
@@ -113,7 +113,19 @@
             lltype.free(p, flavor='raw')
 
         res = self.interpret_inevitable(f1, [])
-        assert res == 'malloc'
+        assert res is None
+        assert 0, """we do not turn inevitable before
+        raw-mallocs which causes leaks on aborts"""
+
+    def test_unknown_raw_free(self):
+        X = lltype.Struct('X', ('foo', lltype.Signed))
+        def f2():
+            return lltype.malloc(X, flavor='raw')
+        def f1():
+            lltype.free(f2(), flavor='raw')
+
+        res = self.interpret_inevitable(f1, [])
+        assert res == 'free'
 
 
     def test_ext_direct_call_safe(self):
diff --git a/pypy/translator/stm/transform2.py b/pypy/translator/stm/transform2.py
--- a/pypy/translator/stm/transform2.py
+++ b/pypy/translator/stm/transform2.py
@@ -25,7 +25,7 @@
         from pypy.translator.stm.inevitable import insert_turn_inevitable
         #
         for graph in self.translator.graphs:
-            insert_turn_inevitable(self.translator, graph)
+            insert_turn_inevitable(graph)
 
     def start_log(self):
         from pypy.translator.c.support import log