commit aa205b53a3a2de5b1c6ce81d29f096ded9fd003b Author: David Malcolm dmalcolm@redhat.com Date: Fri Sep 2 12:36:44 2011 -0400

cpychecker: when PyArg_ParseTuple with "O" succeeds, a sane non-NULL PyObject* is written back

Introduce MyState.make_sane_object(), and use it to generalize the initialization of a sane ob_type from just within MyState.mkstate_new_ref to also within MyState.mkstate_borrowed_ref

Use make_sane_object to special-case the "O" format code.

libcpychecker/refcounts.py | 102 +++++++++++++------- .../refcounts/PyArg_ParseTuple/correct_O/input.c | 56 +++++++++++ .../refcounts/PyArg_ParseTuple/correct_O/script.py | 22 ++++ .../PyArg_ParseTuple/correct_O/stdout.txt | 43 ++++++++ .../PyDict_GetItemString/correct/stdout.txt | 2 +- .../PyDict_GetItemString/incorrect/stdout.txt | 2 +- .../refcounts/module_handling/stdout.txt | 2 +- 7 files changed, 193 insertions(+), 36 deletions(-) --- diff --git a/libcpychecker/refcounts.py b/libcpychecker/refcounts.py index 526459c..e90fd21 100644 --- a/libcpychecker/refcounts.py +++ b/libcpychecker/refcounts.py @@ -42,6 +42,7 @@ from libcpychecker.utils import log # the prefix "t_" means a Transition # the prefix "s_" means a State # the prefix "v_" means an AbstractValue +# the prefix "r_" means a Region

def stmt_is_assignment_to_count(stmt): if hasattr(stmt, 'lhs'): @@ -422,6 +423,49 @@ class MyState(State): newstate.loc = self.loc.next_loc() return Transition(self, newstate, 'calling %s()' % fnname)

+ def make_sane_object(self, stmt, name, v_refcount, r_typeobj=None): + """ + Modify this State, adding a new object. + + The ob_refcnt is set to the given value. + + The object has ob_type set to either the given typeobj, + or a sane new one. + + Returns r_nonnull, a Region + """ + check_isinstance(stmt, gcc.Gimple) + check_isinstance(name, str) + check_isinstance(v_refcount, RefcountValue) + + # Claim a Region for the object: + r_nonnull = self.make_heap_region(name, stmt) + + # Set up ob_refcnt to the given value: + r_ob_refcnt = self.make_field_region(r_nonnull, + 'ob_refcnt') # FIXME: this should be a memref and fieldref + self.value_for_region[r_ob_refcnt] = v_refcount + + # Ensure that the new object has a sane ob_type: + if r_typeobj is None: + # If no specific type object provided by caller, supply one: + r_typeobj = Region('PyTypeObject for %s' % name, None) + # it is its own region: + self.region_for_var[r_typeobj] = r_typeobj + + # Set up obj->ob_type: + ob_type = self.make_field_region(r_nonnull, 'ob_type') + self.value_for_region[ob_type] = PointerToRegion(get_PyTypeObject().pointer, + stmt.loc, + r_typeobj) + # Set up obj->ob_type->tp_dealloc: + tp_dealloc = self.make_field_region(r_typeobj, 'tp_dealloc') + type_of_tp_dealloc = gccutils.get_field_by_name(get_PyTypeObject().type, + 'tp_dealloc').type + self.value_for_region[tp_dealloc] = GenericTpDealloc(type_of_tp_dealloc, + stmt.loc) + return r_nonnull + def mkstate_new_ref(self, stmt, name, typeobjregion=None): """ Make a new State, in which a new ref to some object has been @@ -432,52 +476,31 @@ class MyState(State): newstate = self.copy() newstate.loc = self.loc.next_loc()

- nonnull = newstate.make_heap_region(name, stmt) - ob_refcnt = newstate.make_field_region(nonnull, 'ob_refcnt') # FIXME: this should be a memref and fieldref - newstate.value_for_region[ob_refcnt] = RefcountValue.new_ref() + r_nonnull = newstate.make_sane_object(stmt, name, + RefcountValue.new_ref(), + typeobjregion) if stmt.lhs: newstate.assign(stmt.lhs, PointerToRegion(stmt.lhs.type, stmt.loc, - nonnull), + r_nonnull), stmt.loc) - # Ensure that the new object has a sane ob_type: - if typeobjregion is None: - # If no specific type object provided by caller, supply one: - typeobjregion = Region('PyTypeObject for %s' % name, None) - # it is its own region: - self.region_for_var[typeobjregion] = typeobjregion + # FIXME + return newstate, r_nonnull

- # Set up obj->ob_type: - ob_type = newstate.make_field_region(nonnull, 'ob_type') - newstate.value_for_region[ob_type] = PointerToRegion(get_PyTypeObject().pointer, - stmt.loc, - typeobjregion) - # Set up obj->ob_type->tp_dealloc: - tp_dealloc = newstate.make_field_region(typeobjregion, 'tp_dealloc') - type_of_tp_dealloc = gccutils.get_field_by_name(get_PyTypeObject().type, - 'tp_dealloc').type - newstate.value_for_region[tp_dealloc] = GenericTpDealloc(type_of_tp_dealloc, - stmt.loc) - return newstate, nonnull - - def mkstate_borrowed_ref(self, stmt, name): + def mkstate_borrowed_ref(self, stmt, name, r_typeobj=None): """Make a new State, giving a borrowed ref to some object""" newstate = self.copy() newstate.loc = self.loc.next_loc()

- nonnull = newstate.make_heap_region(name, stmt) - ob_refcnt = newstate.make_field_region(nonnull, 'ob_refcnt') # FIXME: this should be a memref and fieldref - newstate.value_for_region[ob_refcnt] = RefcountValue.borrowed_ref() - #ob_type = newstate.make_field_region(nonnull, 'ob_type') - #newstate.value_for_region[ob_type] = PointerToRegion(get_PyTypeObject().pointer, - # stmt.loc, - # typeobjregion) + r_nonnull = newstate.make_sane_object(stmt, name, + RefcountValue.borrowed_ref(), + r_typeobj) if stmt.lhs: newstate.assign(stmt.lhs, PointerToRegion(stmt.lhs.type, stmt.loc, - nonnull), + r_nonnull), stmt.loc) return newstate

@@ -604,6 +627,17 @@ class MyState(State): if isinstance(v_fmt.region, RegionForStringConstant): return v_fmt.region.text

+ def _get_new_value_for_vararg(unit, exptype): + if unit.code == 'O': + # non-NULL sane PyObject*: + return PointerToRegion(exptype.dereference, + stmt.loc, + self.make_sane_object(stmt, 'object from arg "O"', + RefcountValue.borrowed_ref())) + + # Unknown value: + return UnknownValue(exptype.dereference, stmt.loc) + def _handle_successful_parse(fmt): exptypes = fmt.iter_exp_types() for v_vararg, (unit, exptype) in zip(v_varargs, exptypes): @@ -611,7 +645,9 @@ class MyState(State): # print(' unit: %r' % unit) # print(' exptype: %r %s' % (exptype, exptype)) if isinstance(v_vararg, PointerToRegion): - s_success.value_for_region[v_vararg.region] = UnknownValue(exptype.dereference, stmt.loc) + v_new = _get_new_value_for_vararg(unit, exptype) + check_isinstance(v_new, AbstractValue) + s_success.value_for_region[v_vararg.region] = v_new

fmt_string = _get_format_string(v_fmt) if fmt_string: diff --git a/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c new file mode 100644 index 0000000..aa16d01 --- /dev/null +++ b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c @@ -0,0 +1,56 @@ +/* + Copyright 2011 David Malcolm dmalcolm@redhat.com + Copyright 2011 Red Hat, Inc. + + This is free software: you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see + http://www.gnu.org/licenses/. +*/ + +#include <Python.h> + +/* + Test of correct reference-handling in a call to PyArg_ParseTuple + that uses the "O" format code +*/ + +PyObject * +test(PyObject *self, PyObject *args) +{ + PyObject *obj; + + if (!PyArg_ParseTuple(args, "O:test", + &obj)) { + return NULL; + } + + /* + We now have a borrowed non-NULL ref to "obj". + + To correctly use it as the return value, we need to INCREF it: + */ + Py_INCREF(obj); + return obj; +} +static PyMethodDef test_methods[] = { + {"test_method", test, METH_VARARGS, NULL}, + {NULL, NULL, 0, NULL} /* Sentinel */ +}; + +/* + PEP-7 +Local variables: +c-basic-offset: 4 +indent-tabs-mode: nil +End: +*/ diff --git a/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/script.py b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/script.py new file mode 100644 index 0000000..fdd5ba3 --- /dev/null +++ b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/script.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +# Copyright 2011 David Malcolm dmalcolm@redhat.com +# Copyright 2011 Red Hat, Inc. +# +# This is free software: you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see +# http://www.gnu.org/licenses/. + +from libcpychecker import main +main(verify_refcounting=True, + dump_traces=True, + show_traces=False) diff --git a/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/stdout.txt b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/stdout.txt new file mode 100644 index 0000000..05e8767 --- /dev/null +++ b/tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/stdout.txt @@ -0,0 +1,43 @@ +Trace 0: + Transitions: + 'PyArg_ParseTuple() succeeds' + 'taking False path' + 'returning' + Return value: + repr(): PointerToRegion(gcctype='struct PyObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=32), region=RegionOnHeap('object from arg "O"', gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=32))) + str(): (struct PyObject *)&RegionOnHeap('object from arg "O"', gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=32)) from tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c:32 + r->ob_refcnt: unknown Py_ssize_t from tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c:42 + r->ob_type: None + Region("region-for-arg-gcc.ParmDecl('self')"): + repr(): Region("region-for-arg-gcc.ParmDecl('self')") + str(): Region("region-for-arg-gcc.ParmDecl('self')") + r->ob_refcnt: refs: 0 + N where N >= 1 + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=28), region=Region("region-for-type-of-arg-gcc.ParmDecl('self')")) + Region("region-for-arg-gcc.ParmDecl('args')"): + repr(): Region("region-for-arg-gcc.ParmDecl('args')") + str(): Region("region-for-arg-gcc.ParmDecl('args')") + r->ob_refcnt: refs: 0 + N where N >= 1 + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=28), region=Region("region-for-type-of-arg-gcc.ParmDecl('args')")) + Exception: + (struct PyObject *)0 from tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c:29 + +Trace 1: + Transitions: + 'PyArg_ParseTuple() fails' + 'taking True path' + 'returning' + Return value: + repr(): ConcreteValue(gcctype='struct PyObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=34), value=0) + str(): (struct PyObject *)0 from tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c:34 + Region("region-for-arg-gcc.ParmDecl('self')"): + repr(): Region("region-for-arg-gcc.ParmDecl('self')") + str(): Region("region-for-arg-gcc.ParmDecl('self')") + r->ob_refcnt: refs: 0 + N where N >= 1 + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=28), region=Region("region-for-type-of-arg-gcc.ParmDecl('self')")) + Region("region-for-arg-gcc.ParmDecl('args')"): + repr(): Region("region-for-arg-gcc.ParmDecl('args')") + str(): Region("region-for-arg-gcc.ParmDecl('args')") + r->ob_refcnt: refs: 0 + N where N >= 1 + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyArg_ParseTuple/correct_O/input.c', line=28), region=Region("region-for-type-of-arg-gcc.ParmDecl('args')")) + Exception: + RegionForGlobal(gcc.VarDecl('PyExc_TypeError')) diff --git a/tests/cpychecker/refcounts/PyDict_GetItemString/correct/stdout.txt b/tests/cpychecker/refcounts/PyDict_GetItemString/correct/stdout.txt index 98ace2f..d3fc08c 100644 --- a/tests/cpychecker/refcounts/PyDict_GetItemString/correct/stdout.txt +++ b/tests/cpychecker/refcounts/PyDict_GetItemString/correct/stdout.txt @@ -7,7 +7,7 @@ Trace 0: repr(): PointerToRegion(gcctype='struct PyObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/correct/input.c', line=32), region=RegionOnHeap('PyDict_GetItemString', gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/correct/input.c', line=32))) str(): (struct PyObject *)&RegionOnHeap('PyDict_GetItemString', gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/correct/input.c', line=32)) from tests/cpychecker/refcounts/PyDict_GetItemString/correct/input.c:32 r->ob_refcnt: refs: 1 + N where N >= 1 - r->ob_type: None + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/correct/input.c', line=32), region=Region('PyTypeObject for PyDict_GetItemString')) Region("region-for-arg-gcc.ParmDecl('self')"): repr(): Region("region-for-arg-gcc.ParmDecl('self')") str(): Region("region-for-arg-gcc.ParmDecl('self')") diff --git a/tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/stdout.txt b/tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/stdout.txt index f741ffb..715aa1c 100644 --- a/tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/stdout.txt +++ b/tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/stdout.txt @@ -6,7 +6,7 @@ Trace 0: repr(): PointerToRegion(gcctype='struct PyObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/input.c', line=35), region=RegionOnHeap('PyDict_GetItemString', gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/input.c', line=35))) str(): (struct PyObject *)&RegionOnHeap('PyDict_GetItemString', gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/input.c', line=35)) from tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/input.c:35 r->ob_refcnt: refs: 0 + N where N >= 1 - r->ob_type: None + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/PyDict_GetItemString/incorrect/input.c', line=35), region=Region('PyTypeObject for PyDict_GetItemString')) Region("region-for-arg-gcc.ParmDecl('self')"): repr(): Region("region-for-arg-gcc.ParmDecl('self')") str(): Region("region-for-arg-gcc.ParmDecl('self')") diff --git a/tests/cpychecker/refcounts/module_handling/stdout.txt b/tests/cpychecker/refcounts/module_handling/stdout.txt index 8f04a7d..eb35af6 100644 --- a/tests/cpychecker/refcounts/module_handling/stdout.txt +++ b/tests/cpychecker/refcounts/module_handling/stdout.txt @@ -6,7 +6,7 @@ Trace 0: repr(): RegionOnHeap('output from Py_InitModule4', gcc.Location(file='tests/cpychecker/refcounts/module_handling/input.c', line=46)) str(): output from Py_InitModule4 allocated at tests/cpychecker/refcounts/module_handling/input.c:46 r->ob_refcnt: refs: 0 + N where N >= 1 - r->ob_type: None + r->ob_type: PointerToRegion(gcctype='struct PyTypeObject *', loc=gcc.Location(file='tests/cpychecker/refcounts/module_handling/input.c', line=46), region=Region('PyTypeObject for output from Py_InitModule4')) Exception: (struct PyObject *)0 from tests/cpychecker/refcounts/module_handling/input.c:41