[pypy-commit] pypy default: Fix for 'rpy_fastgil' on shadowstack: see comments

[arigo]

Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r73015:8357f0cf25ea
Date: 2014-08-23 22:18 +0200
http://bitbucket.org/pypy/pypy/changeset/8357f0cf25ea/

Log:	Fix for 'rpy_fastgil' on shadowstack: see comments

diff --git a/rpython/jit/backend/x86/assembler.py b/rpython/jit/backend/x86/assembler.py
--- a/rpython/jit/backend/x86/assembler.py
+++ b/rpython/jit/backend/x86/assembler.py
@@ -1066,13 +1066,16 @@
         cb = callbuilder.CallBuilder(self, fnloc, arglocs)
         cb.emit_no_collect()
 
-    def _reload_frame_if_necessary(self, mc, align_stack=False):
+    def _reload_frame_if_necessary(self, mc, align_stack=False,
+                                   shadowstack_reg=None):
         gcrootmap = self.cpu.gc_ll_descr.gcrootmap
         if gcrootmap:
             if gcrootmap.is_shadow_stack:
-                rst = gcrootmap.get_root_stack_top_addr()
-                mc.MOV(ecx, heap(rst))
-                mc.MOV(ebp, mem(ecx, -WORD))
+                if shadowstack_reg is None:
+                    rst = gcrootmap.get_root_stack_top_addr()
+                    mc.MOV(ecx, heap(rst))
+                    shadowstack_reg = ecx
+                mc.MOV(ebp, mem(shadowstack_reg, -WORD))
         wbdescr = self.cpu.gc_ll_descr.write_barrier_descr
         if gcrootmap and wbdescr:
             # frame never uses card marking, so we enforce this is not
diff --git a/rpython/jit/backend/x86/callbuilder.py b/rpython/jit/backend/x86/callbuilder.py
--- a/rpython/jit/backend/x86/callbuilder.py
+++ b/rpython/jit/backend/x86/callbuilder.py
@@ -1,6 +1,7 @@
 import sys
 from rpython.rlib.clibffi import FFI_DEFAULT_ABI
 from rpython.rlib.objectmodel import we_are_translated
+from rpython.rtyper.lltypesystem import lltype, rffi
 from rpython.jit.metainterp.history import INT, FLOAT
 from rpython.jit.backend.x86.arch import (WORD, IS_X86_64, IS_X86_32,
                                           PASS_ON_MY_FRAME, FRAME_FIXED_SIZE)
@@ -21,6 +22,8 @@
 def align_stack_words(words):
     return (words + CALL_ALIGN - 1) & ~(CALL_ALIGN-1)
 
+NO_ARG_FUNC_PTR = lltype.Ptr(lltype.FuncType([], lltype.Void))
+
 
 class CallBuilderX86(AbstractCallBuilder):
 
@@ -87,13 +90,50 @@
         self.asm.push_gcmap(self.mc, gcmap, store=True)
 
     def pop_gcmap(self):
-        self.asm._reload_frame_if_necessary(self.mc)
+        ssreg = None
+        gcrootmap = self.asm.cpu.gc_ll_descr.gcrootmap
+        if gcrootmap:
+            if gcrootmap.is_shadow_stack and self.is_call_release_gil:
+                from rpython.jit.backend.x86.assembler import heap
+                from rpython.jit.backend.x86 import rx86
+                from rpython.rtyper.lltypesystem.lloperation import llop
+                #
+                # When doing a call_release_gil with shadowstack, there
+                # is the risk that the 'rpy_fastgil' was free but the
+                # current shadowstack can be the one of a different
+                # thread.  So here we check if the shadowstack pointer
+                # is still the same as before we released the GIL (saved
+                # in 'ebx'), and if not, we call 'thread_run'.
+                rst = gcrootmap.get_root_stack_top_addr()
+                mc = self.mc
+                mc.CMP(ebx, heap(rst))
+                mc.J_il8(rx86.Conditions['E'], 0)
+                je_location = mc.get_relative_pos()
+                # call 'thread_run'
+                t_run = llop.gc_thread_run_ptr(NO_ARG_FUNC_PTR)
+                mc.CALL(imm(rffi.cast(lltype.Signed, t_run)))
+                # patch the JE above
+                offset = mc.get_relative_pos() - je_location
+                assert 0 < offset <= 127
+                mc.overwrite(je_location-1, chr(offset))
+                ssreg = ebx
+        #
+        self.asm._reload_frame_if_necessary(self.mc, shadowstack_reg=ssreg)
         if self.change_extra_stack_depth:
             self.asm.set_extra_stack_depth(self.mc, 0)
         self.asm.pop_gcmap(self.mc)
 
     def call_releasegil_addr_and_move_real_arguments(self, fastgil):
         from rpython.jit.backend.x86.assembler import heap
+        assert self.is_call_release_gil
+        #
+        # Save this thread's shadowstack pointer into 'ebx',
+        # for later comparison
+        gcrootmap = self.asm.cpu.gc_ll_descr.gcrootmap
+        if gcrootmap:
+            if gcrootmap.is_shadow_stack:
+                rst = gcrootmap.get_root_stack_top_addr()
+                self.mc.MOV(ebx, heap(rst))
         #
         if not self.asm._is_asmgcc():
             # shadowstack: change 'rpy_fastgil' to 0 (it should be
diff --git a/rpython/memory/gctransform/framework.py b/rpython/memory/gctransform/framework.py
--- a/rpython/memory/gctransform/framework.py
+++ b/rpython/memory/gctransform/framework.py
@@ -977,6 +977,12 @@
             hop.genop("direct_call", [self.root_walker.thread_run_ptr])
             self.pop_roots(hop, livevars)
 
+    def gct_gc_thread_run_ptr(self, hop):
+        assert self.translator.config.translation.thread
+        assert hasattr(self.root_walker, 'thread_run_ptr')
+        hop.genop("same_as", [self.root_walker.thread_run_ptr],
+                  resultvar=hop.spaceop.result)
+
     def gct_gc_thread_start(self, hop):
         assert self.translator.config.translation.thread
         if hasattr(self.root_walker, 'thread_start_ptr'):
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
@@ -473,6 +473,7 @@
     'gc_set_max_heap_size': LLOp(),
     'gc_can_move'         : LLOp(sideeffects=False),
     'gc_thread_run'       : LLOp(),
+    'gc_thread_run_ptr'   : LLOp(sideeffects=False),
     'gc_thread_start'     : LLOp(),
     'gc_thread_die'       : LLOp(),
     'gc_thread_before_fork':LLOp(),   # returns an opaque address