From f5867e64730cb2b327c70944baaaa750b3dd6008 Mon Sep 17 00:00:00 2001 From: Dag Sverre Seljebotn Date: Fri, 25 Jul 2008 12:16:50 +0200 Subject: [PATCH] Buffer bounds checking etc. --- Cython/Compiler/Buffer.py | 54 +++++++++++++++++++++++++++++++++++- Cython/Compiler/Code.py | 12 ++++---- Cython/Compiler/ExprNodes.py | 26 ++++++++--------- Cython/Compiler/Symtab.py | 1 + tests/run/bufaccess.pyx | 39 ++++++++++++++++++++------ 5 files changed, 102 insertions(+), 30 deletions(-) diff --git a/Cython/Compiler/Buffer.py b/Cython/Compiler/Buffer.py index 0aa46908..3a742e44 100644 --- a/Cython/Compiler/Buffer.py +++ b/Cython/Compiler/Buffer.py @@ -90,7 +90,7 @@ def put_assign_to_buffer(lhs_cname, rhs_cname, buffer_aux, is_initialized, pos, 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 + # our entry had no previous value, 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 @@ -105,6 +105,58 @@ def put_assign_to_buffer(lhs_cname, rhs_cname, buffer_aux, is_initialized, pos, # Everything is ok, assign object variable code.putln("%s = %s;" % (lhs_cname, rhs_cname)) + +def put_access(entry, index_types, index_cnames, tmp_cname, pos, code): + """Returns a c string which can be used to access the buffer + for reading or writing""" + bufaux = entry.buffer_aux + bufstruct = bufaux.buffer_info_var.cname + # Check bounds and fix negative indices + boundscheck = True + nonegs = True + if boundscheck: + code.putln("%s = -1;" % tmp_cname) + code.putln("//HERE") + for idx, (type, cname, shape) in enumerate(zip(index_types, index_cnames, + bufaux.shapevars)): + if type.signed != 0: + nonegs = False + # not unsigned, deal with negative index + if idx > 0: code.put("else ") + code.putln("if (%s < 0) {" % cname) + code.putln("%s += %s;" % (cname, shape.cname)) + if boundscheck: + code.putln("if (%s) %s = %d;" % ( + code.unlikely("%s < 0" % cname), tmp_cname, idx)) + code.put("} else ") + else: + if idx > 0: code.put("} else ") + if boundscheck: + # check bounds in positive direction + code.putln("if (%s) %s = %d;" % ( + code.unlikely("%s >= %s" % (cname, shape.cname)), + tmp_cname, idx)) +# if boundscheck or not nonegs: +# code.putln("}") + if boundscheck: + code.put("if (%s) " % code.unlikely("%s != -1" % tmp_cname)) + code.begin_block() + code.putln('PyErr_Format(PyExc_IndexError, ' + + '"Index out of range (buffer lookup, axis %%d)", %s);' % + tmp_cname); + code.putln(code.error_goto(pos)) + code.end_block() + + # Create buffer lookup and return it + + offset = " + ".join(["%s * %s" % (idx, stride.cname) + for idx, stride in + zip(index_cnames, bufaux.stridevars)]) + ptrcode = "(%s.buf + %s)" % (bufstruct, offset) + valuecode = "*%s" % entry.type.buffer_ptr_type.cast_code(ptrcode) + return valuecode + + class PureCFuncNode(Node): child_attrs = [] diff --git a/Cython/Compiler/Code.py b/Cython/Compiler/Code.py index 4dfe0a5f..2fc9770e 100644 --- a/Cython/Compiler/Code.py +++ b/Cython/Compiler/Code.py @@ -337,6 +337,12 @@ class CCodeWriter: self.putln("#ifndef %s" % guard) self.putln("#define %s" % guard) + def unlikely(self, cond): + if Options.gcc_branch_hints: + return 'unlikely(%s)' % cond + else: + return cond + def error_goto(self, pos): lbl = self.error_label self.use_label(lbl) @@ -353,12 +359,6 @@ class CCodeWriter: cinfo, lbl) - def unlikely(self, cond): - if Options.gcc_branch_hints: - return 'unlikely(%s)' % cond - else: - return cond - def error_goto_if(self, cond, pos): return "if (%s) %s" % (self.unlikely(cond), self.error_goto(pos)) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index 5e77b410..fef53037 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -1363,7 +1363,9 @@ class IndexNode(ExprNode): self.type = self.base.type.dtype self.is_buffer_access = True self.index_temps = [Symtab.new_temp(i.type) for i in indices] - self.temps = self.index_temps + self.tmpint = Symtab.new_temp(PyrexTypes.c_int_type) + + self.temps = self.index_temps + [self.tmpint] if getting: # we only need a temp because result_code isn't refactored to # generation time, but this seems an ok shortcut to take @@ -1440,7 +1442,8 @@ class IndexNode(ExprNode): if self.index is not None: self.index.generate_disposal_code(code) else: - for i in self.indices: i.generate_disposal_code(code) + for i in self.indices: + i.generate_disposal_code(code) def generate_result_code(self, code): if self.is_buffer_access: @@ -1512,20 +1515,15 @@ class IndexNode(ExprNode): self.generate_subexpr_disposal_code(code) def buffer_access_code(self, code): - # 1. Assign indices to temps + # Assign indices to temps for temp, index in zip(self.index_temps, self.indices): code.putln("%s = %s;" % (temp.cname, index.result_code)) - # 2. Output code to do bounds checking on these - - # 3. Return a code fragment string which does buffer - # lookup, which can be used on lhs or rhs of an assignment - # in the caller depending on the scenario. - bufaux = self.base.entry.buffer_aux - offset = " + ".join(["%s * %s" % (idx.cname, stride.cname) - for idx, stride in - zip(self.index_temps, bufaux.stridevars)]) - ptrcode = "(%s.buf + %s)" % (bufaux.buffer_info_var.cname, offset) - valuecode = "*%s" % self.base.type.buffer_ptr_type.cast_code(ptrcode) + # Generate buffer access code using these temps + import Buffer + valuecode = Buffer.put_access(entry=self.base.entry, + index_types=[i.type for i in self.index_temps], + index_cnames=[i.cname for i in self.index_temps], + pos=self.pos, tmp_cname=self.tmpint.cname, code=code) return valuecode diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py index 48c0fada..e0c6f8ad 100644 --- a/Cython/Compiler/Symtab.py +++ b/Cython/Compiler/Symtab.py @@ -225,6 +225,7 @@ class Scope: self.num_to_entry = {} self.obj_to_entry = {} self.pystring_entries = [] + self.buffer_entries = [] self.control_flow = ControlFlow.LinearControlFlow() def start_branching(self, pos): diff --git a/tests/run/bufaccess.pyx b/tests/run/bufaccess.pyx index e905bfc4..949d15e9 100644 --- a/tests/run/bufaccess.pyx +++ b/tests/run/bufaccess.pyx @@ -23,14 +23,14 @@ __doc__ = u""" acquired B released B - Apparently, doctest won't handle mixed exceptions and print - stats, so need to circumvent this. - >>> A.resetlog() - >>> acquire_raise(A) +Apparently, doctest won't handle mixed exceptions and print +stats, so need to circumvent this. + >>> #A.resetlog() + >>> #acquire_raise(A) Traceback (most recent call last): ... Exception: on purpose - >>> A.printlog() + >>> #A.printlog() acquired A released A @@ -52,7 +52,7 @@ __doc__ = u""" 0 1 2 3 4 5 released A - #>>> forin_assignment([A, B, A], 3) + >>> #forin_assignment([A, B, A], 3) acquired A 3 released A @@ -63,16 +63,35 @@ __doc__ = u""" 3 released A - >>> printbuf_float(MockBuffer("f", [1.0, 1.25, 0.75, 1.0]), (4,)) + >>> #printbuf_float(MockBuffer("f", [1.0, 1.25, 0.75, 1.0]), (4,)) acquired 1.0 1.25 0.75 1.0 released - - >>> printbuf_int_2d(MockBuffer("i", range(6), (2,3)), (2,3)) + + >>> #C = MockBuffer("i", range(6), (2,3)), (2,3) + >>> #printbuf_int_2d(C) acquired 0 1 2 3 4 5 released + +Check negative indexing: + >>> get_int_2d(C, 1, 1) + 4 + >>> get_int_2d(C, -1, 0) + 3 + >>> get_int_2d(C, -1, -2) + 4 + >>> get_int_2d(C, -2, -3) + 0 + +Out-of-bounds errors: + >>> get_int_2d(C, 2, 0) + Traceback (most recent call last): + ... + IndexError: on purpose + + """ __sdfdoc__ = """ @@ -190,6 +209,8 @@ cdef class MockBuffer: for i, x in enumerate(strides): self.strides[i] = x self.shape = stdlib.malloc(self.ndim * sizeof(Py_ssize_t)) + for i, x in enumerate(shape): + self.shape[i] = x def __getbuffer__(MockBuffer self, Py_buffer* buffer, int flags): global log -- 2.26.2