From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13240 invoked by alias); 29 Aug 2014 13:39:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13231 invoked by uid 89); 29 Aug 2014 13:39:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,DATE_IN_PAST_12_24,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pa0-f73.google.com Received: from mail-pa0-f73.google.com (HELO mail-pa0-f73.google.com) (209.85.220.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 29 Aug 2014 13:39:19 +0000 Received: by mail-pa0-f73.google.com with SMTP id kx10so1374003pab.2 for ; Fri, 29 Aug 2014 06:39:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=Ub26rM20o8q80Xay0osQDScMxDYrLDbTVIsneFrlB3Y=; b=PqMMF2UcGgD9iLPkXmmavhtT5Yad4y3wTHM/hbmutgNEV+nAoYi7eIdVtZVxZytp1U 7m1CH2g/+rHGL1F8bHXzforMQJOMQ096XYBMCcFj5lx/QCixJI7mVE1gcy/doFqZc929 1+v1HAf/K0J1W5qsYoHnGtDOidglE/Q5amJ3hSV3OU7po0mcwM4zKWr2wlBkCA455/Y3 6InHPB5tb6x1Q+dPgwelkGj610tNjJXHXxb2DtDRSHOvXGNn7uvKMaBRFIWdJ3d/aaaY Ka5tUmFr23JnFuwF9/08N+aU6DbgY4O68a1Edty8bbbJyLxVZVq71gtm0rRWt1mAsc71 1YQA== X-Gm-Message-State: ALoCoQlq0Trbi8kKZ7LXtQMuuoh08rxsUBDbUYAWLAoA7J6coJbUvBh0LleVNWmTWA6pZWt0l0IX X-Received: by 10.70.89.97 with SMTP id bn1mr5823253pdb.5.1409319556948; Fri, 29 Aug 2014 06:39:16 -0700 (PDT) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id m14si1273yhm.7.2014.08.29.06.39.16 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 29 Aug 2014 06:39:16 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id 230565A57D8; Thu, 28 Aug 2014 10:30:14 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21503.26406.286858.300183@ruffy2.mtv.corp.google.com> Date: Fri, 29 Aug 2014 13:39:00 -0000 To: Alexander Smundak Cc: gdb-patches Subject: Re: [PATCH] Add Frame.read_register to Python API In-Reply-To: References: <83oay128ca.fsf@gnu.org> <87ioo7uuqm.fsf@fleche.redhat.com> <21494.15883.93664.480097@ruffy2.mtv.corp.google.com> X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00644.txt.bz2 Alexander Smundak writes: > I've fixed the problems you pointed out, please take another look. > Sasha > > > The ability to read registers is needed to use Frame Filter API to > display the frames created by JIT compilers. > > gdb/Changelog > 2014-08-26 Sasha Smundak > > * python/py-frame.c (frapy_read_register): New function. > > 2014-08-26 Sasha Smundak > > * python.texi (Frames in Python): Add read_register description. > > 2014-08-26 Sasha Smundak > > * gdb.python/py-frame.exp: Test Frame.read_register. Hi. Nit: See https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages for new guidelines on how commit messages should be formatted. The Changelog part of it is a little different than what's written above. [I don't want to be excessively picky about such things during review. Just want to make sure that when this gets checked in the format is right: the above is missing directory names for each section.] > diff --git a/gdb/NEWS b/gdb/NEWS > index d603cf7..46c6a87 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -3,6 +3,9 @@ > > *** Changes since GDB 7.8 > > +* Python Scripting > + You can now access frame registers from Python scripts. > + > * On resume, GDB now always passes the signal the program had stopped > for to the thread the signal was sent to, even if the user changed > threads before resuming. Previously GDB would often (but not > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > index 4688783..3cb6bf8 100644 > --- a/gdb/doc/python.texi > +++ b/gdb/doc/python.texi > @@ -3589,6 +3589,13 @@ Return the frame's symtab and line object. > @xref{Symbol Tables In Python}. > @end defun > > +@defun Frame.read_register (register) > +Return the value of @var{register} in this frame. The @var{register} > +argument must be a string (e.g., @code{'sp'} or @code{'rax'}). > +Returns a @code{Gdb.Value} object. Throws an exception if @var{register} > +does not exist. > +@end defun > + > @defun Frame.read_var (variable @r{[}, block@r{]}) > Return the value of @var{variable} in this frame. If the optional > argument @var{block} is provided, search for the variable from that > diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c > index 120e147..88e9114 100644 > --- a/gdb/python/py-frame.c > +++ b/gdb/python/py-frame.c > @@ -28,6 +28,7 @@ > #include "python-internal.h" > #include "symfile.h" > #include "objfiles.h" > +#include "user-regs.h" > > typedef struct { > PyObject_HEAD > @@ -235,6 +236,40 @@ frapy_pc (PyObject *self, PyObject *args) > return gdb_py_long_from_ulongest (pc); > } > > +/* Implementation of gdb.Frame.read_register (self, register) -> gdb.Value. > + Returns the value of a register in this frame. */ > + > +static PyObject * > +frapy_read_register (PyObject *self, PyObject *args) > +{ > + volatile struct gdb_exception except; > + const char *regnum_str; > + struct value *val = NULL; > + > + if (!PyArg_ParseTuple (args, "s", ®num_str)) > + return NULL; > + > + TRY_CATCH (except, RETURN_MASK_ALL) > + { > + struct frame_info *frame; > + int regnum = -1; Nit: I suspect the "= -1" is no longer needed. > + > + FRAPY_REQUIRE_VALID (self, frame); > + > + regnum = user_reg_map_name_to_regnum (get_frame_arch (frame), > + regnum_str, > + strlen (regnum_str)); > + if (regnum >= 0) > + val = value_of_register (regnum, frame); > + > + if (val == NULL) > + PyErr_SetString (PyExc_ValueError, _("Unknown register.")); > + } > + GDB_PY_HANDLE_EXCEPTION (except); > + > + return val == NULL ? NULL : value_to_value_object (val); > +} > + > /* Implementation of gdb.Frame.block (self) -> gdb.Block. > Returns the frame's code block. */ > > @@ -674,6 +709,9 @@ Return the reason why it's not possible to find frames older than this." }, > { "pc", frapy_pc, METH_NOARGS, > "pc () -> Long.\n\ > Return the frame's resume address." }, > + { "read_register", frapy_read_register, METH_VARARGS, > + "read_register (register_name) -> gdb.Value\n\ > +Return the value of the register in the frame." }, > { "block", frapy_block, METH_NOARGS, > "block () -> gdb.Block.\n\ > Return the frame's code block." }, > diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp > index 3517824..e47f340 100644 > --- a/gdb/testsuite/gdb.python/py-frame.exp > +++ b/gdb/testsuite/gdb.python/py-frame.exp > @@ -94,3 +94,20 @@ gdb_test "python print ('result = %s' % f0.read_var ('variable_which_surely_does > gdb_test "python print ('result = %s' % f0.read_var ('a'))" " = 1" "test Frame.read_var - success" > > gdb_test "python print ('result = %s' % (gdb.selected_frame () == f1))" " = True" "test gdb.selected_frame" > + > +# Can read SP register. > +gdb_test "python print ('result = %s' % (gdb.selected_frame ().read_register ('sp') == gdb.parse_and_eval ('\$sp')))" \ > + " = True" \ > + "test Frame.read_register(sp)" > + > +# PC value obtained via read_register is as expected. > +gdb_test "python print ('result = %s' % (f0.read_register('pc') == f0.pc()))" \ > + " = True" \ > + "test Frame.read_register(pc)" > + > +# On x86-64, PC is in $rip register. > +if {[istarget x86_64-*]} { > + gdb_test "python print ('result = %s' % (f0.read_register('pc') == f0.read_register('rip')))" \ > + " = True" \ > + "test Frame.read_register(rip)" > +} LGTM with the above nits addressed.