From patchwork Fri May 23 01:21:23 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 1086 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id C0D3B360076 for ; Thu, 22 May 2014 18:23:57 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14314964) id 6FFC163D9596D; Thu, 22 May 2014 18:23:57 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id 4E6E163D55988 for ; Thu, 22 May 2014 18:23:57 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=BfPeRTqYVZhZYItJ 1E/b8ichdx0CQyXytM7B9qWFFGYQ39fzfTe3l+CawWtyKznTDOwCi7lP4/twldcE 7Yc7G9hMAH8SFtnWMtTszi0tyglwVrhmWTloda+ubTuETT7cRZrWLVFxq3lIjk0f jJ6NWj66KtnutFoOt2RthQKVbCY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; s=default; bh=uOVcG0G8uqbvDTb8uytIw8 L+r2k=; b=jHNb3pPu1rQPBMYMw6Udz5fHtgRK/eFFAujnaudMmdACmhpVV3XjKx PBcDxiLEnyCh8ITkqL0F5LAS/Wz4cx1JZk3iAa+hhflHfxA/b1tvrpTZcUSUxqxh OanAk7o3wF6CGbsnGKIv3v7hMC92bZNxrO0nDnP5xCeXNwZUp0mX0= Received: (qmail 6253 invoked by alias); 23 May 2014 01:23:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 6225 invoked by uid 89); 23 May 2014 01:23:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, T_FILL_THIS_FORM_SHORT autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 May 2014 01:23:50 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1WneD4-0005uN-T5 from Yao_Qi@mentor.com ; Thu, 22 May 2014 18:23:46 -0700 Received: from SVR-ORW-FEM-05.mgc.mentorg.com ([147.34.97.43]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 22 May 2014 18:23:46 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.2.247.3; Thu, 22 May 2014 18:23:01 -0700 Message-ID: <537EA293.2050000@codesourcery.com> Date: Fri, 23 May 2014 09:21:23 +0800 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Tom Tromey CC: Subject: Re: [PATCH 02/12] Generalize varobj iterator References: <1392367471-13527-1-git-send-email-yao@codesourcery.com> <1392367471-13527-3-git-send-email-yao@codesourcery.com> <874n0jkokw.fsf@fleche.redhat.com> In-Reply-To: <874n0jkokw.fsf@fleche.redhat.com> X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in On 05/22/2014 01:51 AM, Tom Tromey wrote: > Yao> +static void > Yao> +py_varobj_iter_dtor (struct varobj_iter *self) > Yao> +{ > Yao> + struct py_varobj_iter *dis = (struct py_varobj_iter *) self; > Yao> + > Yao> + Py_XDECREF (dis->iter); > > I think this has to acquire the GIL before calling Py_XDECREF. > PyGILState_Ensure and PyGILState_Release are added. > Yao> +static void > Yao> +py_varobj_iter_ctor (struct py_varobj_iter *self, > Yao> + struct varobj *var, PyObject *pyiter) > Yao> +{ > Yao> + self->base.var = var; > Yao> + self->base.ops = &py_varobj_iter_ops; > Yao> + self->base.next_raw_index = 0; > Yao> + self->iter = pyiter; > [...] > Yao> +static struct py_varobj_iter * > Yao> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter) > Yao> +{ > [...] > Yao> + py_varobj_iter_ctor (self, var, pyiter); > > I think these two functions should be documented as stealing a reference > to PYITER. It's helpful to see such comments later. > > Using CPYCHECKER_STEALS_REFERENCE_TO_ARG would also be nice, but I > appreciate that this isn't easy for everybody to test. I add CPYCHECKER_STEALS_REFERENCE_TO_ARG to these two functions, and verified by cpychecker. Some errors go away. > > Yao> + iter = PyObject_GetIter (children); > Yao> + if (iter == NULL) > Yao> + { > > The "if" looks misindented. > Fixed. > Yao> +struct varobj_iter; > Yao> +struct varobj; > Yao> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var, > Yao> + PyObject *printer); > Yao> #endif /* GDB_PYTHON_INTERNAL_H */ > > Please leave a blank line before the #endif. > Fixed. > Yao> +extern struct cleanup *varobj_ensure_python_env (struct varobj *var); > > Extra space after "extern". Fixed. diff --git a/gdb/Makefile.in b/gdb/Makefile.in index f2c16ec..da6437b 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -369,7 +369,8 @@ SUBDIR_PYTHON_OBS = \ py-threadevent.o \ py-type.o \ py-utils.o \ - py-value.o + py-value.o \ + py-varobj.o SUBDIR_PYTHON_SRCS = \ python/python.c \ @@ -405,7 +406,8 @@ SUBDIR_PYTHON_SRCS = \ python/py-threadevent.c \ python/py-type.c \ python/py-utils.c \ - python/py-value.c + python/py-value.c \ + python/py-varobj.c SUBDIR_PYTHON_DEPS = SUBDIR_PYTHON_LDFLAGS= SUBDIR_PYTHON_CFLAGS= @@ -857,7 +859,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \ ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \ exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \ i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \ -ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \ +ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \ +nto-tdep.h serial.h \ c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \ cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \ cli/cli-script.h macrotab.h symtab.h common/version.h \ @@ -2482,6 +2485,10 @@ py-value.o: $(srcdir)/python/py-value.c $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c $(POSTCOMPILE) +py-varobj.o: $(srcdir)/python/py-varobj.c + $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c + $(POSTCOMPILE) + # # Dependency tracking. Most of this is conditional on GNU Make being # found by configure; if GNU Make is not found, we fall back to a diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c new file mode 100644 index 0000000..50bea40 --- /dev/null +++ b/gdb/python/py-varobj.c @@ -0,0 +1,185 @@ +/* Copyright (C) 2013-2014 Free Software Foundation, Inc. + + This program 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 . */ + +#include "defs.h" +#include "python-internal.h" +#include "varobj.h" +#include "varobj-iter.h" + +/* A dynamic varobj iterator "class" for python pretty-printed + varobjs. This inherits struct varobj_iter. */ + +struct py_varobj_iter +{ + /* The 'base class'. */ + struct varobj_iter base; + + /* The python iterator returned by the printer's 'children' method, + or NULL if not available. */ + PyObject *iter; +}; + +/* Implementation of the 'dtor' method of pretty-printed varobj + iterators. */ + +static void +py_varobj_iter_dtor (struct varobj_iter *self) +{ + struct py_varobj_iter *dis = (struct py_varobj_iter *) self; + PyGILState_STATE state = PyGILState_Ensure (); + + Py_XDECREF (dis->iter); + PyGILState_Release (state); +} + +/* Implementation of the 'next' method of pretty-printed varobj + iterators. */ + +static varobj_item * +py_varobj_iter_next (struct varobj_iter *self) +{ + struct py_varobj_iter *t = (struct py_varobj_iter *) self; + struct cleanup *back_to; + PyObject *item; + + back_to = varobj_ensure_python_env (self->var); + + item = PyIter_Next (t->iter); + + if (item == NULL) + { + /* Normal end of iteration. */ + if (!PyErr_Occurred ()) + return NULL; + + /* If we got a memory error, just use the text as the item. */ + if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error)) + { + PyObject *type, *value, *trace; + char *name_str, *value_str; + + PyErr_Fetch (&type, &value, &trace); + value_str = gdbpy_exception_to_string (type, value); + Py_XDECREF (type); + Py_XDECREF (value); + Py_XDECREF (trace); + if (value_str == NULL) + { + gdbpy_print_stack (); + return NULL; + } + + name_str = xstrprintf ("", + self->next_raw_index++); + item = Py_BuildValue ("(ss)", name_str, value_str); + xfree (name_str); + xfree (value_str); + if (item == NULL) + { + gdbpy_print_stack (); + return NULL; + } + } + else + { + /* Any other kind of error. */ + gdbpy_print_stack (); + return NULL; + } + } + + self->next_raw_index++; + do_cleanups (back_to); + return item; +} + +/* The 'vtable' of pretty-printed python varobj iterators. */ + +static const struct varobj_iter_ops py_varobj_iter_ops = +{ + py_varobj_iter_dtor, + py_varobj_iter_next +}; + +/* Constructor of pretty-printed varobj iterators. VAR is the varobj + whose children the iterator will be iterating over. PYITER is the + python iterator actually responsible for the iteration. */ + +static void CPYCHECKER_STEALS_REFERENCE_TO_ARG (3) +py_varobj_iter_ctor (struct py_varobj_iter *self, + struct varobj *var, PyObject *pyiter) +{ + self->base.var = var; + self->base.ops = &py_varobj_iter_ops; + self->base.next_raw_index = 0; + self->iter = pyiter; +} + +/* Allocate and construct a pretty-printed varobj iterator. VAR is + the varobj whose children the iterator will be iterating over. + PYITER is the python iterator actually responsible for the + iteration. */ + +static struct py_varobj_iter * CPYCHECKER_STEALS_REFERENCE_TO_ARG (2) +py_varobj_iter_new (struct varobj *var, PyObject *pyiter) +{ + struct py_varobj_iter *self; + + self = XNEW (struct py_varobj_iter); + py_varobj_iter_ctor (self, var, pyiter); + return self; +} + +/* Return a new pretty-printed varobj iterator suitable to iterate + over VAR's children. */ + +struct varobj_iter * +py_varobj_get_iterator (struct varobj *var, PyObject *printer) +{ + PyObject *children; + int i; + PyObject *iter; + struct py_varobj_iter *py_iter; + struct cleanup *back_to = varobj_ensure_python_env (var); + + if (!PyObject_HasAttr (printer, gdbpy_children_cst)) + { + do_cleanups (back_to); + return NULL; + } + + children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst, + NULL); + if (children == NULL) + { + gdbpy_print_stack (); + error (_("Null value returned for children")); + } + + make_cleanup_py_decref (children); + + iter = PyObject_GetIter (children); + if (iter == NULL) + { + gdbpy_print_stack (); + error (_("Could not get children iterator")); + } + + py_iter = py_varobj_iter_new (var, iter); + + do_cleanups (back_to); + + return &py_iter->base; +} diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 07650f7..3aab045 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -520,4 +520,9 @@ int gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object) CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; +struct varobj_iter; +struct varobj; +struct varobj_iter *py_varobj_get_iterator (struct varobj *var, + PyObject *printer); + #endif /* GDB_PYTHON_INTERNAL_H */ diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h new file mode 100644 index 0000000..3a530bc --- /dev/null +++ b/gdb/varobj-iter.h @@ -0,0 +1,63 @@ +/* Iterator of varobj. + Copyright (C) 2013-2014 Free Software Foundation, Inc. + + This program 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 . */ + +struct varobj_iter_ops; + +typedef PyObject varobj_item; + +/* A dynamic varobj iterator "class". */ + +struct varobj_iter +{ + /* The 'vtable'. */ + const struct varobj_iter_ops *ops; + + /* The varobj this iterator is listing children for. */ + struct varobj *var; + + /* The next raw index we will try to check is available. If it is + equal to number_of_children, then we've already iterated the + whole set. */ + int next_raw_index; +}; + +/* The vtable of the varobj iterator class. */ + +struct varobj_iter_ops +{ + /* Destructor. Releases everything from SELF (but not SELF + itself). */ + void (*dtor) (struct varobj_iter *self); + + /* Returns the next object or NULL if it has reached the end. */ + varobj_item *(*next) (struct varobj_iter *self); +}; + +/* Returns the next varobj or NULL if it has reached the end. */ + +#define varobj_iter_next(ITER) (ITER)->ops->next (ITER) + +/* Delete a varobj_iter object. */ + +#define varobj_iter_delete(ITER) \ + do \ + { \ + if ((ITER) != NULL) \ + { \ + (ITER)->ops->dtor (ITER); \ + xfree (ITER); \ + } \ + } while (0) diff --git a/gdb/varobj.c b/gdb/varobj.c index 0467a79..02cce5a 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -41,6 +41,8 @@ typedef int PyObject; #endif +#include "varobj-iter.h" + /* Non-zero if we want to see trace of varobj level stuff. */ unsigned int varobjdebug = 0; @@ -140,14 +142,14 @@ struct varobj_dynamic /* The iterator returned by the printer's 'children' method, or NULL if not available. */ - PyObject *child_iter; + struct varobj_iter *child_iter; /* We request one extra item from the iterator, so that we can report to the caller whether there are more items than we have already reported. However, we don't want to install this value when we read it, because that will mess up future updates. So, we stash it here instead. */ - PyObject *saved_item; + varobj_item *saved_item; }; struct cpstack @@ -256,7 +258,7 @@ is_root_p (struct varobj *var) #ifdef HAVE_PYTHON /* Helper function to install a Python environment suitable for use during operations on VAR. */ -static struct cleanup * +struct cleanup * varobj_ensure_python_env (struct varobj *var) { return ensure_python_env (var->root->exp->gdbarch, @@ -773,6 +775,19 @@ dynamic_varobj_has_child_method (struct varobj *var) return result; } +/* A factory for creating dynamic varobj's iterators. Returns an + iterator object suitable for iterating over VAR's children. */ + +static struct varobj_iter * +varobj_get_iterator (struct varobj *var) +{ + if (var->dynamic->pretty_printer) + return py_varobj_get_iterator (var, var->dynamic->pretty_printer); + + gdb_assert_not_reached (_("\ +requested an iterator from a non-dynamic varobj")); +} + #endif static int @@ -788,9 +803,7 @@ update_dynamic_varobj_children (struct varobj *var, { #if HAVE_PYTHON struct cleanup *back_to; - PyObject *children; int i; - PyObject *printer = var->dynamic->pretty_printer; if (!gdb_python_initialized) return 0; @@ -798,37 +811,22 @@ update_dynamic_varobj_children (struct varobj *var, back_to = varobj_ensure_python_env (var); *cchanged = 0; - if (!PyObject_HasAttr (printer, gdbpy_children_cst)) - { - do_cleanups (back_to); - return 0; - } if (update_children || var->dynamic->child_iter == NULL) { - children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst, - NULL); + varobj_iter_delete (var->dynamic->child_iter); + var->dynamic->child_iter = varobj_get_iterator (var); - if (!children) - { - gdbpy_print_stack (); - error (_("Null value returned for children")); - } + Py_XDECREF (var->dynamic->saved_item); + var->dynamic->saved_item = NULL; - make_cleanup_py_decref (children); + i = 0; - Py_XDECREF (var->dynamic->child_iter); - var->dynamic->child_iter = PyObject_GetIter (children); if (var->dynamic->child_iter == NULL) { - gdbpy_print_stack (); - error (_("Could not get children iterator")); + do_cleanups (back_to); + return 0; } - - Py_XDECREF (var->dynamic->saved_item); - var->dynamic->saved_item = NULL; - - i = 0; } else i = VEC_length (varobj_p, var->children); @@ -838,7 +836,6 @@ update_dynamic_varobj_children (struct varobj *var, for (; to < 0 || i < to + 1; ++i) { PyObject *item; - int force_done = 0; /* See if there was a leftover from last time. */ if (var->dynamic->saved_item) @@ -847,52 +844,17 @@ update_dynamic_varobj_children (struct varobj *var, var->dynamic->saved_item = NULL; } else - item = PyIter_Next (var->dynamic->child_iter); - - if (!item) { - /* Normal end of iteration. */ - if (!PyErr_Occurred ()) - break; - - /* If we got a memory error, just use the text as the - item. */ - if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error)) - { - PyObject *type, *value, *trace; - char *name_str, *value_str; - - PyErr_Fetch (&type, &value, &trace); - value_str = gdbpy_exception_to_string (type, value); - Py_XDECREF (type); - Py_XDECREF (value); - Py_XDECREF (trace); - if (!value_str) - { - gdbpy_print_stack (); - break; - } - - name_str = xstrprintf ("", i); - item = Py_BuildValue ("(ss)", name_str, value_str); - xfree (name_str); - xfree (value_str); - if (!item) - { - gdbpy_print_stack (); - break; - } - - force_done = 1; - } - else - { - /* Any other kind of error. */ - gdbpy_print_stack (); - break; - } + item = varobj_iter_next (var->dynamic->child_iter); } + if (item == NULL) + { + /* Iteration is done. Remove iterator from VAR. */ + varobj_iter_delete (var->dynamic->child_iter); + var->dynamic->child_iter = NULL; + break; + } /* We don't want to push the extra child on any report list. */ if (to < 0 || i < to) { @@ -932,9 +894,6 @@ update_dynamic_varobj_children (struct varobj *var, element. */ break; } - - if (force_done) - break; } if (i < VEC_length (varobj_p, var->children)) @@ -953,9 +912,8 @@ update_dynamic_varobj_children (struct varobj *var, *cchanged = 1; var->num_children = VEC_length (varobj_p, var->children); - - do_cleanups (back_to); + do_cleanups (back_to); return 1; #else gdb_assert_not_reached ("should never be called if Python is not enabled"); @@ -1245,7 +1203,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor, Py_XDECREF (var->pretty_printer); var->pretty_printer = visualizer; - Py_XDECREF (var->child_iter); + varobj_iter_delete (var->child_iter); var->child_iter = NULL; } diff --git a/gdb/varobj.h b/gdb/varobj.h index 1199e0b..991c2e8 100644 --- a/gdb/varobj.h +++ b/gdb/varobj.h @@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to); extern int varobj_pretty_printed_p (struct varobj *var); +extern struct cleanup *varobj_ensure_python_env (struct varobj *var); + extern int varobj_default_value_is_changeable_p (struct varobj *var); extern int varobj_value_is_changeable_p (struct varobj *var);