From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 6BF57385842C for ; Mon, 21 Feb 2022 17:27:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6BF57385842C Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-653-urXato9AP321vhs5F8_MeA-1; Mon, 21 Feb 2022 12:27:24 -0500 X-MC-Unique: urXato9AP321vhs5F8_MeA-1 Received: by mail-wr1-f70.google.com with SMTP id e1-20020adfa741000000b001e2e74c3d4eso7651573wrd.12 for ; Mon, 21 Feb 2022 09:27:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=yRgq3SHewMu2EDUuRfKBglxF1aR9ZbJaeeFUewe40QE=; b=TCKKZ3Rd5gOGwCXzNvf1cxyS6zHj13WX1I9ZzJsPeKGFKZPUrXkBmTjKVM3zDbiKAN /WNfyNR5fQ21pKT2drwknUbCy5SjIJSDP3DM/WZWLPf7VxrMop2Wb6NVcZ0O56er+RsJ HEirhgXO0YJ3janLpiL6okzxJET8nhWKtuifhxCGxLWC1b+49VqbIwY7OA2GYmZwkzqC A/KBpXpm62lSGSzmjxMAp96+px8kMcRe5qOEwRfVXq7+B+bA5xEf0SExDynTUZUf6xg5 4vypod0Ys5ermZzPt/t/4tdeibKAAuYJvlobq5S6L9z31XrX241EiWBenBL3c3ZZIa9n hWaw== X-Gm-Message-State: AOAM5330AiuuK1f8WRzzNp6oHvXdufMTJ+HtBHWvS2wBZ0BdzVrCKdkH RrJmPyhRGwuC29F6Bvcqx2Z4XgAFFvy356HIIzrq+tOU7MaM5EiSm1VoHrGBhFURhKofkRrZCHR jrQLnhz2DIcoSsIpaTmVYrQ== X-Received: by 2002:adf:a41a:0:b0:1e3:b1c:d5a7 with SMTP id d26-20020adfa41a000000b001e30b1cd5a7mr16186553wra.511.1645464443151; Mon, 21 Feb 2022 09:27:23 -0800 (PST) X-Google-Smtp-Source: ABdhPJys8m61NSBkNbKXI9betvWx/NcLGQj94JV0ry4lbZP1E5DBW1Of7whdOaThl2W+CoGLkexYtg== X-Received: by 2002:adf:a41a:0:b0:1e3:b1c:d5a7 with SMTP id d26-20020adfa41a000000b001e30b1cd5a7mr16186537wra.511.1645464442885; Mon, 21 Feb 2022 09:27:22 -0800 (PST) Received: from localhost (host86-169-131-29.range86-169.btcentralplus.com. [86.169.131.29]) by smtp.gmail.com with ESMTPSA id d29sm651090wrc.105.2022.02.21.09.27.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Feb 2022 09:27:22 -0800 (PST) From: Andrew Burgess To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/python: add gdb.Architecture.format_address In-Reply-To: <83leyhs07f.fsf@gnu.org> References: <20220211161721.3252422-1-aburgess@redhat.com> <83leyhs07f.fsf@gnu.org> Date: Mon, 21 Feb 2022 17:27:21 +0000 Message-ID: <878ru4cepy.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Feb 2022 17:27:31 -0000 Eli Zaretskii via Gdb-patches writes: >> Date: Fri, 11 Feb 2022 16:17:21 +0000 >> From: Andrew Burgess via Gdb-patches >> Cc: Andrew Burgess >> >> diff --git a/gdb/NEWS b/gdb/NEWS >> index e173d38c3a1..4f4f0c2af6d 100644 >> --- a/gdb/NEWS >> +++ b/gdb/NEWS >> @@ -187,6 +187,11 @@ GNU/Linux/LoongArch loongarch*-*-linux* >> >> GNU/Linux/OpenRISC or1k*-*-linux* >> >> + ** New function gdb.Architecture.format_address(ADDRESS), which >> + takes an address in the currently selected inferior's address >> + space, and returns a string representing the address. The format >> + of the returned string is '0x.... '. >> + >> *** Changes in GDB 11 >> >> * The 'set disassembler-options' command now supports specifying options >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi >> index c1a3f5f2a7e..50443f7b704 100644 >> --- a/gdb/doc/python.texi >> +++ b/gdb/doc/python.texi >> @@ -6016,6 +6016,25 @@ >> @code{gdb.Architecture}. >> @end defun >> >> +@defun Architecture.format_address (@var{address}) >> +Returns @var{address}, an address within the currently selected > > Our style is to say "Return", not "Returns". > > Also, saying "return ADDRESS" basically misses the main rationale of > this function, I think; see below. I've completely rewritten both the /doc/ entry, and the NEWS entry based on your feedback. > >> +inferior's address space, formatted as a string. When a suitable >> +symbol can be found to associate with @var{address} this will be >> +included in the returned string, formatted like this: >> + >> +@smallexample >> +0x00001042 >> +@end smallexample >> + >> +If there is no symbol that @value{GDBN} can find to associate with >> +@var{address} then the returned string will just contain >> +@var{address}. >> + >> +If @var{address} is not accessible within the current inferior's >> +address space, this function will still return a string containing >> +@var{address}. >> +@end defun > > More generally, I wonder whether the name "format_address" is the best > one we could come up with. Isn't this the equivalent of "info symbol" > CLI command? If so, why not call it "address_to_symbol" or somesuch? Given we have an actual gdb.Symbol class, I'm reluctant to use address_to_symbol because I think that might give the unrealistic expectation that this function returns a gdb.Symbol object. This function is really a wrapper around the internal function 'print_address', but, as the Python version doesn't print anything, I ended up with format_address. I'd also be reluctant to go with address_to_string as that might give the impression that it "just" converts a number to an string, which obviously would be a really weird thing to have as a separate function in Python. I'm certainly not against renaming, if we can come up with a better name... maybe 'format_address_info'? I don't know... I still kind of like 'format_address'... > > This goes back to the documentation: saying that a method takes its > argument and returns it as a string makes the reader wonder why would > we need such a trivial method. So the documentation should start by > saying that the method returns SYMBOL+OFFSET that corresponds to > ADDRESS, and only mention that it returns ADDRESS as a string as the > fallback, when SYMBOL cannot be found. Thanks, I took this advice. I'm not sure if my use of @samp{} is acceptable in the new docs - maybe there's better formatting constructs I could/should use. Thanks, Andrew --- commit eba54d7150c8d87b34db41594ab4f6aef95cd847 Author: Andrew Burgess Date: Sat Oct 23 09:59:25 2021 +0100 gdb/python: add gdb.Architecture.format_address Add a new method gdb.Architecture.format_address, which is a wrapper around GDB's print_address function. This method takes an address, and returns a string with the format: ADDRESS Where, ADDRESS is the original address, formatted as hexadecimal, and padded with zeros on the left up to the width of an address in the current architecture. SYMBOL is a symbol whose address range covers ADDRESS, and OFFSET is the offset from SYMBOL to ADDRESS in decimal. If there's no SYMBOL whose address range covers ADDRESS, then the part is not included. This is useful if a user wants to write a Python script that pretty-print addresses, the user no longer needs to do manual symbol lookup, and additionally, things like the zero padding on addresses will be consistent with the builtin GDB behaviour. diff --git a/gdb/NEWS b/gdb/NEWS index 9da74e71796..c4e31188fdb 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -185,6 +185,12 @@ GNU/Linux/LoongArch loongarch*-*-linux* set styling'). When false, which is the default if the argument is not given, then no styling is applied to the returned string. + ** New function gdb.Architecture.format_address(ADDRESS), that takes + an address, and returns a string formatted as: + ADDRESS + This is the same format that GDB uses when printing address, + symbol, and offset information from the disassembler. + * New features in the GDB remote stub, GDBserver ** GDBserver is now supported on OpenRISC GNU/Linux. diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi index c1a3f5f2a7e..a095d055807 100644 --- a/gdb/doc/python.texi +++ b/gdb/doc/python.texi @@ -6016,6 +6016,30 @@ @code{gdb.Architecture}. @end defun +@defun Architecture.format_address (@var{address}) +Return a string in the format @samp{ADDRESS }, where +@samp{ADDRESS} is @var{address} formatted in hexadecimal, +@samp{SYMBOL} is a symbol, the address range of which, covers +@var{address}, and @samp{OFFSET} is the offset from @samp{SYMBOL} to +@var{address} in decimal. This is the same format that @value{GDBN} +uses when printing address, symbol, and offset information, for +example, within disassembler output. + +If no @samp{SYMBOL} has an address range that covers @var{address}, +then the @samp{} part is not included in the returned +string, instead the returned string will just contain the +@var{address} formatted as hexadecimal. + +In all cases, the @samp{ADDRESS} component will be padded with leading +zeros based on the width of an address for the current architecture. + +An example of the returned string is: + +@smallexample +0x00001042 +@end smallexample +@end defun + @node Registers In Python @subsubsection Registers In Python @cindex Registers In Python diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c index 0f273b344e4..95ae931e73e 100644 --- a/gdb/python/py-arch.c +++ b/gdb/python/py-arch.c @@ -348,6 +348,31 @@ gdbpy_all_architecture_names (PyObject *self, PyObject *args) return list.release (); } +/* Implement gdb.architecture.format_address(ADDR). Provide access to + GDB's print_address function from Python. The returned address will + have the format '0x..... '. */ + +static PyObject * +archpy_format_address (PyObject *self, PyObject *args, PyObject *kw) +{ + static const char *keywords[] = { "address", nullptr }; + PyObject *addr_obj; + CORE_ADDR addr; + struct gdbarch *gdbarch = nullptr; + + ARCHPY_REQUIRE_VALID (self, gdbarch); + + if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &addr_obj)) + return nullptr; + + if (get_addr_from_python (addr_obj, &addr) < 0) + return nullptr; + + string_file buf; + print_address (gdbarch, addr, &buf); + return PyString_FromString (buf.c_str ()); +} + void _initialize_py_arch (); void _initialize_py_arch () @@ -391,6 +416,12 @@ group GROUP-NAME." }, METH_NOARGS, "register_groups () -> Iterator.\n\ Return an iterator over all of the register groups in this architecture." }, + { "format_address", (PyCFunction) archpy_format_address, + METH_VARARGS | METH_KEYWORDS, + "format_address (ADDRESS) -> String.\n\ +Format ADDRESS, an address within the currently selected inferior's\n\ +address space, as a string. The format of the returned string is\n\ +'ADDRESS ' without the quotes." }, {NULL} /* Sentinel */ }; diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp index b55778b0b72..c4854033d8c 100644 --- a/gdb/testsuite/gdb.python/py-arch.exp +++ b/gdb/testsuite/gdb.python/py-arch.exp @@ -127,3 +127,18 @@ foreach a $arch_names b $py_arch_names { } } gdb_assert { $lists_match } + +# Check the gdb.Architecture.format_address method. +set main_addr [get_hexadecimal_valueof "&main" "UNKNOWN"] +gdb_test "python print(\"Got: \" + gdb.selected_inferior().architecture().format_address($main_addr))" \ + "Got: $main_addr
" \ + "gdb.Architecture.format_address, result should have no offset" +set next_addr [format 0x%x [expr $main_addr + 1]] +gdb_test "python print(\"Got: \" + gdb.selected_inferior().architecture().format_address($next_addr))" \ + "Got: $next_addr " \ + "gdb.Architecture.format_address, result should have an offset" +if {![is_address_zero_readable]} { + gdb_test "python print(\"Got: \" + gdb.selected_inferior().architecture().format_address(0))" \ + "Got: 0x0" \ + "gdb.Architecture.format_address for address 0" +}