From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75940 invoked by alias); 26 Jul 2019 01:38:29 -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 75930 invoked by uid 89); 26 Jul 2019 01:38:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-31.7 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=sk:simark, PyCFunction, meth_varargs, simark@simark.ca X-HELO: mail-ot1-f65.google.com Received: from mail-ot1-f65.google.com (HELO mail-ot1-f65.google.com) (209.85.210.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 26 Jul 2019 01:38:26 +0000 Received: by mail-ot1-f65.google.com with SMTP id s7so7637861oth.7 for ; Thu, 25 Jul 2019 18:38:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KesjdJIXTfosFhiuhMWA935RqJG3LGCjWJlasbsU6qw=; b=PnJWTUdzdOO7Ufcje9ZoM+2bbsCRgu7JuADzhW2lESGwyYljBeonqgx/klZiZXYy9K gvEFLQF2PHCNlBkB2PRH8IUKn+PreKTbR8YgmJll85p/NV5RWxLQWHzSmqvBK2k2qPOa TMeM+203462UouSgSsN2bkzfxabwHq6VKRfE/hrkWLIc6kNQAfKuuEGHQGqInAhttZyW Nri3HdCeyafUsxROGsPOmrzp1ccccqsO9xNQnOtaq2xjJE9Huw1QM10izf4ZfBq3ykQQ Tv8hup/yA1m+7rjdfIKxXAMg22fzts+pXuKa+kfzNhD6+EfsRlIshi4eNFnO9W93EaDC JgEg== MIME-Version: 1.0 References: <20190625220832.247935-1-cbiesinger@google.com> <20190722205121.89189-1-cbiesinger@google.com> <54adbe2b-3501-52ea-fdac-5a44f7869391@simark.ca> In-Reply-To: <54adbe2b-3501-52ea-fdac-5a44f7869391@simark.ca> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Fri, 26 Jul 2019 01:38:00 -0000 Message-ID: Subject: Re: [PATCH] Add an Objfile.lookup_global_symbol function To: Simon Marchi Cc: Christian Biesinger via gdb-patches , Eli Zaretskii Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2019-07/txt/msg00565.txt.bz2 On Mon, Jul 22, 2019 at 6:30 PM Simon Marchi wrote: > What did you end up deciding, to let Python code search for static symbols? I decided to follow your suggestion and add a lookup_static_symbol function (to Objfile and gdb). I haven't had time to implement it yet but hopefully I can do that in the next couple of days. Would you like me to do Objfile.lookup_static_symbol as part of this patch or together with gdb.lookup_static_symbol? > I just have a few nits on the patch below (and some questions about the doc, > which is why I added Eli in CC). Will send a new patch momentarily. > On 2019-07-22 4:51 p.m., Christian Biesinger via gdb-patches wrote: > > Per the discussion in the thread about my gdb.lookup_global_symbol > > patch, I've changed this patch to only look in GLOBAL_SCOPE. > > > > This is essentially the inverse of Symbol.objfile. This allows > > handling different symbols with the same name (but from different > > objfiles) and can also be faster if the objfile is known. > > > > gdb/ChangeLog: > > > > 2019-06-25 Christian Biesinger > > > > * NEWS: Mention new function Objfile.lookup_global_symbol. > > * python/py-objfile.c (objfpy_lookup_global_symbol): New function > > Objfile.lookup_global_symbol. > > > > gdb/doc/ChangeLog: > > > > 2019-06-25 Christian Biesinger > > > > * python.texi (Objfiles In Python): Document new function > > Objfile.lookup_global_symbol. > > > > gdb/testsuite/ChangeLog: > > > > 2019-06-25 Christian Biesinger > > > > * gdb.python/py-objfile.c: Add global and static vars. > > * gdb.python/py-objfile.exp: Test new function Objfile. > > lookup_global_symbol. > > --- > > gdb/NEWS | 3 ++ > > gdb/doc/python.texi | 12 ++++++++ > > gdb/python/py-objfile.c | 40 ++++++++++++++++++++++++- > > gdb/testsuite/gdb.python/py-objfile.c | 3 ++ > > gdb/testsuite/gdb.python/py-objfile.exp | 7 +++++ > > 5 files changed, 64 insertions(+), 1 deletion(-) > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc1d58520d..c57cb4576e 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -36,6 +36,9 @@ > > ** gdb.Type has a new property 'objfile' which returns the objfile the > > type was defined in. > > > > + ** gdb.Objfile has a new method 'lookup_global_symbol' to lookup a symbol > > + from this objfile only. > > + > > * New commands > > > > | [COMMAND] | SHELL_COMMAND > > diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi > > index 034623513b..80da607824 100644 > > --- a/gdb/doc/python.texi > > +++ b/gdb/doc/python.texi > > @@ -4441,6 +4441,18 @@ searches then this function can be used to add a debug info file > > from a different place. > > @end defun > > > > +@defun Objfile.lookup_global_symbol (name @r{[}, domain@r{]}) > > +Searches for a global symbol named @var{name} in this objfile. Optionally, the > > +search scope can be restricted with the @var{domain} argument. > > +The @var{domain} argument must be a domain constant defined in the @code{gdb} > > +module and described in @ref{Symbols In Python}. This function is similar to > > +@code{gdb.lookup_global_symbol}, except that the search is limited to this > > +objfile. > > + > > +The result is a @code{gdb.Symbol} object or @code{None} if the symbol > > +is not found. > > +@end defun > > + > > @node Frames In Python > > @subsubsection Accessing inferior stack frames from Python > > > > diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c > > index 199c567a04..a9a6ae6725 100644 > > --- a/gdb/python/py-objfile.c > > +++ b/gdb/python/py-objfile.c > > @@ -406,7 +406,7 @@ objfpy_is_valid (PyObject *self, PyObject *args) > > Py_RETURN_TRUE; > > } > > > > -/* Implementation of gdb.Objfile.add_separate_debug_file (self) -> Boolean. */ > > +/* Implementation of gdb.Objfile.add_separate_debug_file (self, string) -> None. */ > > This appears to be an unrelated (but valid) change. Can you please make a separate patch > and push it as obvious? Just mind the 80 columns. If the function returns None, I think > we could just omit the return type and say: > > /* Implementation of gdb.Objfile.add_separate_debug_file (self, string). */ > > ... and it will fit. > > So just push a patch with this (including the ChangeLog entry), and post it to the list > with the "PATCH obvious" prefix, or something like that, to let people know. OK, done. (Oops, I see you already answered my question here that I asked you again on IRC) > > static PyObject * > > objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw) > > @@ -434,6 +434,39 @@ objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw) > > Py_RETURN_NONE; > > } > > > > +/* Implementation of gdb.Objfile.lookup_global_symbol (self, string [, domain]) -> gdb.Symbol. */ > > > + > > +static PyObject * > > +objfpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw) > > +{ > > + static const char *keywords[] = { "name", "domain", NULL }; > > + objfile_object *obj = (objfile_object *) self; > > + const char *symbol_name; > > + int domain = VAR_DOMAIN; > > + > > + OBJFPY_REQUIRE_VALID (obj); > > + > > + if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &symbol_name, > > + &domain)) > > + return nullptr; > > + > > + try > > + { > > + struct symbol *sym = lookup_global_symbol_from_objfile > > + (obj->objfile, symbol_name, (domain_enum) domain).symbol; > > + if (sym == nullptr) > > + Py_RETURN_NONE; > > + > > + return symbol_to_symbol_object (sym); > > + } > > + catch (const gdb_exception &except) > > + { > > + GDB_PY_HANDLE_EXCEPTION (except); > > + } > > + > > + Py_RETURN_NONE; > > +} > > + > > /* Implement repr() for gdb.Objfile. */ > > > > static PyObject * > > @@ -652,6 +685,11 @@ Return true if this object file is valid, false if not." }, > > "add_separate_debug_file (file_name).\n\ > > Add FILE_NAME to the list of files containing debug info for the objfile." }, > > > > + { "lookup_global_symbol", (PyCFunction) objfpy_lookup_global_symbol, > > + METH_VARARGS | METH_KEYWORDS, > > + "lookup_global_symbol (name [, domain]).\n\ > > +Looks up a global symbol in this objfile and returns it." }, > > + > > To be consistent with the rest of the Python function help strings, I think it should > be > > "Look up a global symbol in this objfile and return it." > > i.e. not at the third person of the singular. Eli, could you tell which form is preferred? > The change to python.texi might also need to be changed to that form as well. > > > { NULL } > > }; > > > > diff --git a/gdb/testsuite/gdb.python/py-objfile.c b/gdb/testsuite/gdb.python/py-objfile.c > > index ac41491234..6d751bddae 100644 > > --- a/gdb/testsuite/gdb.python/py-objfile.c > > +++ b/gdb/testsuite/gdb.python/py-objfile.c > > @@ -15,6 +15,9 @@ > > You should have received a copy of the GNU General Public License > > along with this program. If not, see . */ > > > > +int global_var = 42; > > +static int static_var = 50; > > + > > int > > main () > > { > > diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp > > index b5e0c5e760..2e8358bd76 100644 > > --- a/gdb/testsuite/gdb.python/py-objfile.exp > > +++ b/gdb/testsuite/gdb.python/py-objfile.exp > > @@ -55,6 +55,13 @@ gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").filename)" \ > > gdb_test "python print (gdb.lookup_objfile (\"junk\"))" \ > > "Objfile not found\\.\r\n${python_error_text}" > > > > +gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").lookup_global_symbol (\"global_var\").name)" \ > > + "global_var" "lookup_global_symbol finds a valid symbol" > > +gdb_test "python print gdb.lookup_objfile (\"${testfile}\").lookup_global_symbol (\"static_var\") is None" \ > > + "True" "lookup_global_symbol does not find static symbols" > > Please add parentheses to this print so it works with Python 3. > > > +gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").lookup_global_symbol (\"stdout\"))" \ > > + "None" "lookup_global_symbol doesn't find symbol in other objfile" > > + > > set binfile_build_id [get_build_id $binfile] > > if [string compare $binfile_build_id ""] { > > verbose -log "binfile_build_id = $binfile_build_id" > > > > Simon