From: Dag Sverre Seljebotn Date: Thu, 24 Jul 2008 14:32:46 +0000 (+0200) Subject: Buffer refactor nearly done, but there is a tricky segfaulting bug somewhere... X-Git-Tag: 0.9.8.1~49^2~76 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=0b65efc4c6b871d262bc971b4103b046bad76543;p=cython.git Buffer refactor nearly done, but there is a tricky segfaulting bug somewhere... --- diff --git a/Cython/Compiler/Buffer.py b/Cython/Compiler/Buffer.py index 960995f6..0aa46908 100644 --- a/Cython/Compiler/Buffer.py +++ b/Cython/Compiler/Buffer.py @@ -8,6 +8,103 @@ from Cython.Compiler.Errors import CompileError import PyrexTypes from sets import Set as set +def used_buffer_aux_vars(entry): + buffer_aux = entry.buffer_aux + buffer_aux.buffer_info_var.used = True + for s in buffer_aux.shapevars: s.used = True + for s in buffer_aux.stridevars: s.used = True + +def put_unpack_buffer_aux_into_scope(buffer_aux, code): + bufstruct = buffer_aux.buffer_info_var.cname + + code.putln(" ".join(["%s = %s.strides[%d];" % + (s.cname, bufstruct, idx) + for idx, s in enumerate(buffer_aux.stridevars)])) + code.putln(" ".join(["%s = %s.shape[%d];" % + (s.cname, bufstruct, idx) + for idx, s in enumerate(buffer_aux.shapevars)])) + +def put_zero_buffer_aux_into_scope(buffer_aux, code): + # If new buffer is None, set up to access 0 + # for a "safer segfault" on access + code.putln("%s.buf = 0;" % buffer_aux.buffer_info_var.cname) + code.putln(" ".join(["%s = 0;" % s.cname + for s in buffer_aux.stridevars])) + code.putln(" ".join(["%s = 0;" % s.cname + for s in buffer_aux.shapevars])) + +def put_acquire_arg_buffer(entry, code, pos): + buffer_aux = entry.buffer_aux + cname = entry.cname + bufstruct = buffer_aux.buffer_info_var.cname + flags = '0' + # Acquire any new buffer + code.put('if (%s != Py_None) ' % cname) + code.begin_block() + code.putln('%s.buf = 0;' % bufstruct) # PEP requirement + code.put(code.error_goto_if( + 'PyObject_GetBuffer(%s, &%s, %s) == -1' % ( + cname, bufstruct, flags), pos)) + # An exception raised in arg parsing cannot be catched, so no + # need to do care about the buffer then. + put_unpack_buffer_aux_into_scope(buffer_aux, code) + code.end_block() + +def put_release_buffer(entry, code): + code.putln("if (%s != Py_None) PyObject_ReleaseBuffer(%s, &%s);" % ( + entry.cname, entry.cname, entry.buffer_aux.buffer_info_var.cname)) + +def put_assign_to_buffer(lhs_cname, rhs_cname, buffer_aux, is_initialized, pos, code): + bufstruct = buffer_aux.buffer_info_var.cname + flags = '0' + + if is_initialized: + # Release any existing buffer + code.put('if (%s != Py_None) ' % lhs_cname) + code.begin_block(); + code.putln('PyObject_ReleaseBuffer(%s, &%s);' % ( + lhs_cname, bufstruct)) + code.end_block() + # Acquire any new buffer + code.put('if (%s != Py_None) ' % rhs_cname) + code.begin_block() + code.putln('%s.buf = 0;' % bufstruct) # PEP requirement + code.put('if (%s) ' % code.unlikely( + 'PyObject_GetBuffer(%s, &%s, %s) == -1' % ( + rhs_cname, + bufstruct, + flags) + + ' || %s((char*)%s.format) == NULL' % ( + buffer_aux.tschecker.cname, bufstruct + ))) + code.begin_block() + # If acquisition failed, attempt to reacquire the old buffer + # before raising the exception. A failure of reacquisition + # will cause the reacquisition exception to be reported, one + # can consider working around this later. + if is_initialized: + put_zero_buffer_aux_into_scope(buffer_aux, code) + code.put('if (%s != Py_None && PyObject_GetBuffer(%s, &%s, %s) == -1) ' % ( + lhs_cname, lhs_cname, bufstruct, flags)) + code.begin_block() + put_zero_buffer_aux_into_scope(buffer_aux, code) + code.end_block() + else: + # our entry had no previous vaule, so set to None when acquisition fails + code.putln('%s = Py_None; Py_INCREF(Py_None);' % lhs_cname) + code.putln(code.error_goto(pos)) + code.end_block() # acquisition failure + # Unpack indices + put_unpack_buffer_aux_into_scope(buffer_aux, code) + code.putln('} else {') + # If new buffer is None, set up to access 0 + # for a "safer segfault" on access + put_zero_buffer_aux_into_scope(buffer_aux, code) + code.end_block() + + # Everything is ok, assign object variable + code.putln("%s = %s;" % (lhs_cname, rhs_cname)) + class PureCFuncNode(Node): child_attrs = [] @@ -103,27 +200,25 @@ class IntroduceBufferAuxiliaryVars(CythonTransform): tschecker = self.tschecker(buftype.dtype) # Declare auxiliary vars - bufinfo = scope.declare_var(temp_name_handle(u"%s_bufinfo" % name), - self.bufstruct_type, node.pos) - - temp_var = scope.declare_var(temp_name_handle(u"%s_tmp" % name), - entry.type, node.pos) + cname = scope.mangle(Naming.bufstruct_prefix, name) + bufinfo = scope.declare_var(name="$%s" % cname, cname=cname, + type=self.bufstruct_type, pos=node.pos) + + bufinfo.used = True + + def var(prefix, idx): + cname = scope.mangle(prefix, "%d_%s" % (idx, name)) + result = scope.declare_var("$%s" % cname, PyrexTypes.c_py_ssize_t_type, + node.pos, cname=cname, is_cdef=True) + result.init = "0" + if entry.is_arg: + result.used = True + return result + stridevars = [var(Naming.bufstride_prefix, i) for i in range(entry.type.ndim)] + shapevars = [var(Naming.bufshape_prefix, i) for i in range(entry.type.ndim)] + entry.buffer_aux = Symtab.BufferAux(bufinfo, stridevars, shapevars, tschecker) - stridevars = [] - shapevars = [] - for idx in range(buftype.ndim): - # stride - varname = temp_name_handle(u"%s_%s%d" % (name, "stride", idx)) - var = scope.declare_var(varname, PyrexTypes.c_int_type, node.pos, is_cdef=True) - stridevars.append(var) - # shape - varname = temp_name_handle(u"%s_%s%d" % (name, "shape", idx)) - var = scope.declare_var(varname, PyrexTypes.c_uint_type, node.pos, is_cdef=True) - shapevars.append(var) - entry.buffer_aux = Symtab.BufferAux(bufinfo, stridevars, - shapevars, tschecker) - entry.buffer_aux.temp_var = temp_var scope.buffer_entries = bufvars self.scope = scope diff --git a/Cython/Compiler/Code.py b/Cython/Compiler/Code.py index ce615cf1..4dfe0a5f 100644 --- a/Cython/Compiler/Code.py +++ b/Cython/Compiler/Code.py @@ -211,7 +211,7 @@ class CCodeWriter: #print "...private and not definition, skipping" ### return if not entry.used and visibility == "private": - #print "not used and private, skipping" ### + #print "not used and private, skipping", entry.cname ### return storage_class = "" if visibility == 'extern': @@ -352,12 +352,15 @@ class CCodeWriter: pos[1], cinfo, lbl) - - def error_goto_if(self, cond, pos): + + def unlikely(self, cond): if Options.gcc_branch_hints: - return "if (unlikely(%s)) %s" % (cond, self.error_goto(pos)) + return 'unlikely(%s)' % cond else: - return "if (%s) %s" % (cond, self.error_goto(pos)) + return cond + + def error_goto_if(self, cond, pos): + return "if (%s) %s" % (self.unlikely(cond), self.error_goto(pos)) def error_goto_if_null(self, cname, pos): return self.error_goto_if("!%s" % cname, pos) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index e9e321c3..5e77b410 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -894,10 +894,14 @@ class NameNode(AtomicExprNode): self.type = PyrexTypes.error_type self.entry.used = 1 if self.entry.type.is_buffer: - # Need some temps - - print self.dump() - + # Have an rhs temp just in case. All rhs I could + # think of had a single symbol result_code but better + # safe than sorry. Feel free to change this. + import Buffer + self.new_buffer_temp = Symtab.new_temp(self.entry.type) + self.temps = [self.new_buffer_temp] + Buffer.used_buffer_aux_vars(self.entry) + def analyse_rvalue_entry(self, env): #print "NameNode.analyse_rvalue_entry:", self.name ### #print "Entry:", self.entry.__dict__ ### @@ -1012,7 +1016,7 @@ class NameNode(AtomicExprNode): entry = self.entry if entry is None: return # There was an error earlier - + # is_pyglobal seems to be True for module level-globals only. # We use this to access class->tp_dict if necessary. if entry.is_pyglobal: @@ -1054,11 +1058,26 @@ class NameNode(AtomicExprNode): code.put_xdecref(self.result_code, self.ctype()) else: code.put_decref(self.result_code, self.ctype()) - code.putln('%s = %s;' % (self.result_code, rhs.result_as(self.ctype()))) + if self.type.is_buffer: + self.generate_acquire_buffer(rhs, code) + else: + code.putln('%s = %s;' % (self.result_code, rhs.result_as(self.ctype()))) if debug_disposal_code: print("NameNode.generate_assignment_code:") print("...generating post-assignment code for %s" % rhs) rhs.generate_post_assignment_code(code) + + def generate_acquire_buffer(self, rhs, code): + rhstmp = self.new_buffer_temp.cname + buffer_aux = self.entry.buffer_aux + bufstruct = buffer_aux.buffer_info_var.cname + code.putln('%s = %s;' % (rhstmp, rhs.result_as(self.ctype()))) + + import Buffer + Buffer.put_assign_to_buffer(self.result_code, rhstmp, buffer_aux, + is_initialized=not self.skip_assignment_decref, + pos=self.pos, code=code) + code.putln("%s = 0;" % rhstmp) def generate_deletion_code(self, code): if self.entry is None: diff --git a/Cython/Compiler/Naming.py b/Cython/Compiler/Naming.py index 4296bf71..30484168 100644 --- a/Cython/Compiler/Naming.py +++ b/Cython/Compiler/Naming.py @@ -31,6 +31,10 @@ prop_set_prefix = pyrex_prefix + "setprop_" type_prefix = pyrex_prefix + "t_" typeobj_prefix = pyrex_prefix + "type_" var_prefix = pyrex_prefix + "v_" +bufstruct_prefix = pyrex_prefix + "bstruct_" +bufstride_prefix = pyrex_prefix + "bstride_" +bufshape_prefix = pyrex_prefix + "bshape_" +bufoffset_prefix = pyrex_prefix + "boffset_" vtable_prefix = pyrex_prefix + "vtable_" vtabptr_prefix = pyrex_prefix + "vtabptr_" vtabstruct_prefix = pyrex_prefix + "vtabstruct_" diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index 302d6689..00ebbbf7 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -828,6 +828,7 @@ class FuncDefNode(StatNode, BlockNode): return lenv def generate_function_definitions(self, env, code, transforms): + import Buffer # Generate C code for header and body of function code.init_labels() lenv = self.local_scope @@ -876,7 +877,9 @@ class FuncDefNode(StatNode, BlockNode): for entry in lenv.arg_entries: if entry.type.is_pyobject and lenv.control_flow.get_state((entry.name, 'source')) != 'arg': code.put_var_incref(entry) - # ----- Initialise local variables + if entry.type.is_buffer: + Buffer.put_acquire_arg_buffer(entry, code, self.pos) + # ----- Initialise local variables for entry in lenv.var_entries: if entry.type.is_pyobject and entry.init_to_none and entry.used: code.put_init_var_to_py_none(entry) @@ -925,6 +928,8 @@ class FuncDefNode(StatNode, BlockNode): for entry in lenv.var_entries: if lenv.control_flow.get_state((entry.name, 'initalized')) is not True: entry.xdecref_cleanup = 1 + for entry in lenv.buffer_entries: + Buffer.put_release_buffer(entry, code) code.put_var_decrefs(lenv.var_entries, used_only = 1) # Decref any increfed args for entry in lenv.arg_entries: diff --git a/tests/run/bufaccess.pyx b/tests/run/bufaccess.pyx index d30baafb..e905bfc4 100644 --- a/tests/run/bufaccess.pyx +++ b/tests/run/bufaccess.pyx @@ -1,5 +1,17 @@ cimport __cython__ +__doc__ = u""" + >>> A = MockBuffer("i", range(10), label="A") + >>> B = MockBuffer("i", range(10), label="B") + >>> E = ErrorBuffer("E") + + >>> acquire_release(A, B) + acquired A + released A + acquired B + released B +""" + __doc__ = u""" >>> A = MockBuffer("i", range(10), label="A") >>> B = MockBuffer("i", range(10), label="B") @@ -63,6 +75,13 @@ __doc__ = u""" released """ +__sdfdoc__ = """ + >>> printbuf_float(MockBuffer("f", [1.0, 1.25, 0.75, 1.0]), (4,)) + acquired + 1.0 1.25 0.75 1.0 + released +""" + ctypedef char* (*write_func_ptr)(char*, object) cdef char* write_float(char* buf, object value): (buf)[0] = value @@ -89,6 +108,7 @@ def acquire_release(o1, o2): def acquire_raise(o): cdef object[int] buf buf = o + o.printlog() raise Exception("on purpose") def as_argument(object[int] bufarg, int n):