From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KJYMDY3kAWq2BDAAWB0awg (envelope-from ) for ; Mon, 11 May 2026 10:15:41 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=OduquYPR; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 1927E1E0C3; Mon, 11 May 2026 10:15:41 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H2,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D2ACC1E067 for ; Mon, 11 May 2026 10:15:39 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 475BD4BAD15C for ; Mon, 11 May 2026 14:15:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 475BD4BAD15C Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=OduquYPR Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 5FC5B4BABF15 for ; Mon, 11 May 2026 14:15:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5FC5B4BABF15 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5FC5B4BABF15 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1778508900; cv=none; b=X9vS5HPq2Pa4hqQtQitgCPBEP5eGTwd/szyOKgFJGpcRdJr0BWHp1Cm4iG3ELPBU5V+vvCnSgz+b6BUG2sBPr7zP8OCZDTMqSiJJvDO7OeBH9V46JxhYnhCvZNoevV0NryvW2fRu+TacfO+bLIBavaUGKvBIYQYJZEEeIjQF59w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1778508900; c=relaxed/simple; bh=2AiR/bQh6a96i7i/gZ7wQpaiSIe9xognk+DX3Vm/sM0=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=qnu+R/dxVxRdWzxg02gD7iZ1Quo0jm9C3zrzpKmzEODYABT+hcu8OcSIK4txmNt0L1cooZ0phGunPatacSjxnc7gCcakPtYNn+EE16IT2af16LReiJ+AUWmGpGd1b3FL8tThAqMZ/3VrI2EJ8zC8S5lyOBDJXRV4+ntj63i0oYA= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=OduquYPR DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5FC5B4BABF15 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778508899; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ME/d4uqvHAbPqWVsuwDB0P2KzU2bVvP8cfPOd2muG5M=; b=OduquYPRVD9+zregX/2DtCtl4JXRvCfnBFLUF3ciGtpJzXRKnim7kBiavdUkaH0omqUwXP GCzVHX5zc7m7o+sXlpwmh7mmtNPKfb5t5iJ7YTBU1N1iepC0lqquxiICp2Z3xJ8DpxsSxI lgSS/yYA7YGfash2+hxJroJwnCKHyy8= Received: from mail-dl1-f70.google.com (mail-dl1-f70.google.com [74.125.82.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-7-FkOfyJx4OKWsi5jVtNU5WQ-1; Mon, 11 May 2026 10:14:58 -0400 X-MC-Unique: FkOfyJx4OKWsi5jVtNU5WQ-1 X-Mimecast-MFC-AGG-ID: FkOfyJx4OKWsi5jVtNU5WQ_1778508897 Received: by mail-dl1-f70.google.com with SMTP id a92af1059eb24-12df8bc580cso2752110c88.0 for ; Mon, 11 May 2026 07:14:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778508897; x=1779113697; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ME/d4uqvHAbPqWVsuwDB0P2KzU2bVvP8cfPOd2muG5M=; b=IBh8AfVC1kcXaKYvCFaOvoVNmZKcx/g198Np6d3kPpG4g4TRBzq5JPfkfqN0c5KNXq dHI5KwWlfYu49qp5/aHH1afPhE/5jY72Q8dtlY7PNiK4FIp5NzS3sZVb/ZHYCuoHzj0b 5i/skFWpH3VifyU1mreAiHRVs8/Zn9zlzymavbG8YtAui8ue+ZkqquPupQLpWJh6dhzO nMipof1SfBC8jHgF6J0e8maSzOersZkB0zj9Tj4moDs2WjXmlGdcmw0UdU43/GFmn1Dy Af0tc3vBnPrHcpZ73piig+7XksafP/4M3FfMohcYQfIZVxNHLvtSrFOOqI4AiQEBn7M4 eiJw== X-Forwarded-Encrypted: i=1; AFNElJ+pAC5QeL1HhviVFm8G51AJ/J/b4CtbrnlRYAEKnjHEfM055isAqMyb/L8dfeSdEfS+U495HSC/D6iOmg==@sourceware.org X-Gm-Message-State: AOJu0YyMFsDDSCA2Lgc+wNFiOAs/5zB/O0Xc8ya/adbXW+fyTaUSLOqZ dRSWnsIMk0IROpYn8mAn3PPQ1U5gsXWkqrY4PhOFaLmcSxS7bjdXOBR70e3MV11kZU0dr0xlckR FRqwbrkwT8a7B1fIiIEeuvND/vpr9dOpbuacwGRPnZOf436ZGy5VnAUqywc/O6rQO2KhdVEQ= X-Gm-Gg: Acq92OE54b0Rpw7knU0sxpDhLQB8pg7jmN+y0dirq7+e6s5ZzvzJ3n3UPPh9GzM5/lI Xw90Vt3saOfOmqd8HFTIRdpF7jOek4NnlLW3RFRWIXTYMPpD3rGkDhXWULqwsc1cihBfqZz9Bwt ZDfR029D0CWI98JjFz0l/wwv5Qg0r+a5Ha4YGutCpYkkdQNWLZ6YgLQLoNtBTToB5+b/P0kzCX6 e4QMQnfqenT+sGjZinKz8lTNxU8F1dwYfYVmaRgB/bA3HIJoq3h6bLqbrffllerrXC0jBVv2/71 qmHOE1i9RX/QWT26Gir9+yKPU9EdnFkt/S7ok1PkDACDKxZshsIF9g69YW3c23IpwZQn2/RgewP qqp4iPjNQETknJtJ+eYMLSSXLBwdJERI= X-Received: by 2002:a05:7022:6707:b0:130:8ed9:204c with SMTP id a92af1059eb24-1323ac98250mr8929031c88.10.1778508896569; Mon, 11 May 2026 07:14:56 -0700 (PDT) X-Received: by 2002:a05:7022:6707:b0:130:8ed9:204c with SMTP id a92af1059eb24-1323ac98250mr8928930c88.10.1778508894323; Mon, 11 May 2026 07:14:54 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:993e::75d? ([2804:14d:8084:993e::75d]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-1327810ffb9sm20319061c88.2.2026.05.11.07.14.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 May 2026 07:14:53 -0700 (PDT) Message-ID: <350b3a8d-3174-4703-b532-4ba49fe5ea05@redhat.com> Date: Mon, 11 May 2026 11:14:49 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] Use "output-radix" setting to format function offsets To: Zander Work , gdb-patches@sourceware.org Cc: tom@tromey.com References: <20260509044543.558625-2-zdw@google.com> From: Guinevere Larsen In-Reply-To: <20260509044543.558625-2-zdw@google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 6LKKfGqd5MG9DodyLMf-WKn7674Qx5fO_FT3-cYD7bk_1778508897 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 5/9/26 1:45 AM, Zander Work wrote: > Updates since v1: > > * Addressed feedback in the test suite by more robustly handling the > GDB I/O and improved the regex patterns used to match on the offset > values. > * Updated the accompanying test program to have a minimum level of > complexity to avoid compiler optimizations trimming out too much > function body, and fixed the formatting and copyright date > * Added a comment on the `format_pc_offset()` implementation and > addressed comments in that function. > > I believe the only open items are: > > * Should MI consumers have the string radix-formatted offset value, or > continue having an int value (this is the current impl in the patch)? > * I still need to do an FSF copyright assignment. > > Please let me know if I missed anything else to address. Thanks! Hi Zander! Thanks for the quick v2 for this patch. When sending another version of a patch, we also add the commit message that will be in the git repo, so that it can receive comments without needing to find the v1. For other reviewers' convenience, here's the original email: https://inbox.sourceware.org/gdb-patches/CAB3ousCKmytOaWJMNC4kBm8oBqmUWOywg_bEnTutmm9M6vty_Q@mail.gmail.com/T/#m20c344a2b51c35bcb4f678a209f5c5eb986fbb1b I only have one formatting nit I didn't notice on v1, but there's no need to send a v3 just for that. Feel free to add my review tag to the end of the commit message! Reviewed-By: Guinevere Larsen This isn't enough to push the commit, I hope a global maintainer approves this patch soon. > --- > gdb/disasm.c | 18 ++++++++++--- > gdb/printcmd.c | 2 +- > gdb/testsuite/gdb.base/radix.c | 35 ++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/radix.exp | 43 ++++++++++++++++++++++++++++++++ > gdb/valprint.c | 13 ++++++++++ > gdb/valprint.h | 6 +++++ > 6 files changed, 112 insertions(+), 5 deletions(-) > create mode 100644 gdb/testsuite/gdb.base/radix.c > > diff --git a/gdb/disasm.c b/gdb/disasm.c > index 81c466c188a..5d19dea72b6 100644 > --- a/gdb/disasm.c > +++ b/gdb/disasm.c > @@ -375,10 +375,20 @@ gdb_pretty_print_disassembler::pretty_print_insn (const struct disasm_insn *insn > m_uiout->field_string ("func-name", name, > function_name_style.style ()); > /* For negative offsets, avoid displaying them as +-N; the sign of > - the offset takes the place of the "+" here. */ > - if (offset >= 0) > - m_uiout->text ("+"); > - m_uiout->field_signed ("offset", offset); > + the offset takes the place of the "+" here. For MI consumers, > + emit the integer value; otherwise, print the formatted offset based > + on the current 'output-radix'. */ Small nit that I didn't notice in the first version, but this is not lined up correctly, it should be 1 tab and 3 spaces. > + if (m_uiout->is_mi_like_p ()) > + { > + if (offset >= 0) > + m_uiout->text ("+"); > + m_uiout->field_signed ("offset", offset); > + } > + else > + { > + std::string s = format_pc_offset (offset); > + m_uiout->field_string ("offset", s.c_str ()); I'm going to add this for reference for other reviewers: It seems that radix is respected in other commands, like print. So I think it would be reasonable to make it respected in the disas command as well, but since I don't know how MI consumers work, I also don't know if something would break here... so yeah, I think it might be fine, don't really know enough but the possibility of MI consumers breaking makes me cautious. > + } > m_uiout->text (">:\t"); > } > else > diff --git a/gdb/printcmd.c b/gdb/printcmd.c > index ae498395436..5fb8c666447 100644 > --- a/gdb/printcmd.c > +++ b/gdb/printcmd.c > @@ -567,7 +567,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr, > gdb_puts ("<", stream); > fputs_styled (name.c_str (), function_name_style.style (), stream); > if (offset != 0) > - gdb_printf (stream, "%+d", offset); > + gdb_puts (format_pc_offset (offset).c_str (), stream); > > /* Append source filename and line number if desired. Give specific > line # of this addr, if we have it; else line # of the nearest symbol. */ > diff --git a/gdb/testsuite/gdb.base/radix.c b/gdb/testsuite/gdb.base/radix.c > new file mode 100644 > index 00000000000..8a4d1287208 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/radix.c > @@ -0,0 +1,35 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2026 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 > +#include > + > +static int v; > + > +int > +main (void) > +{ > + v = 0; > + > + puts ("hello world"); > + > + printf ("this is another string\n"); > + > + v += 3; > + > + return v; > +} > diff --git a/gdb/testsuite/gdb.base/radix.exp b/gdb/testsuite/gdb.base/radix.exp > index 7a4320bbf36..4b8e2d74b49 100644 > --- a/gdb/testsuite/gdb.base/radix.exp > +++ b/gdb/testsuite/gdb.base/radix.exp > @@ -17,6 +17,11 @@ > # This file was written by Fred Fish. (fnf@cygnus.com) > # And rewritten by Michael Chastain (mec.gnu@mindspring.com) > > +standard_testfile > + > +if {[build_executable "failed to prepare" $testfile $srcfile debug]} { > + return -1 > +} > > # Start with a fresh gdb. > > @@ -189,3 +194,41 @@ gdb_test "set radix 7" \ > gdb_test "show output-radix" \ > "Default output radix for printing of values is 10\\." \ > "output radix unchanged after rejection through set radix command" > + > +with_test_prefix "pc offset radix" { > + clean_restart $testfile > + > + if { ![runto_main] } { > + return -1 > + } > + > + proc test_pc_offset_radix { oradix offset_re } { > + global gdb_prompt > + > + gdb_test "set output-radix $oradix" \ > + "Output radix now set to decimal $oradix.*\\." > + > + set test "x/i with output-radix $oradix" > + > + gdb_test_multiple "x/i \$pc" "$test 1" { > + -re -wrap ":.*" { > + pass $gdb_test_name > + } > + } > + > + gdb_test "ni 3" "\[0-9\]+.*" "Next instruction for radix $oradix" > + > + gdb_test_multiple "x/i \$pc" "$test 2" { > + -re -wrap ":.*" { > + pass $gdb_test_name > + } > + } > + } > + > + test_pc_offset_radix 8 {0[0-7]+} > + test_pc_offset_radix 10 {[1-9][0-9]*} > + test_pc_offset_radix 16 {0x[0-9a-f]+} > + > + gdb_test "set output-radix 10" "Output radix now set to decimal 10.*\\." \ > + "restore output-radix" > +} > diff --git a/gdb/valprint.c b/gdb/valprint.c > index 62b1b33bb66..d0d0472ca7a 100644 > --- a/gdb/valprint.c > +++ b/gdb/valprint.c > @@ -171,6 +171,19 @@ show_output_radix (struct ui_file *file, int from_tty, > value); > } > > +/* See valprint.h. */ > + > +std::string > +format_pc_offset (int offset) > +{ > + std::string sign = (offset < 0) ? "-" : "+"; > + > + std::string body = int_string (offset < 0 ? -offset : offset, output_radix, > + 0, 0, 1); > + > + return sign + body; > +} > + > /* By default we print arrays without printing the index of each element in > the array. This behavior can be changed by setting PRINT_ARRAY_INDEXES. */ > > diff --git a/gdb/valprint.h b/gdb/valprint.h > index 0ce3e0781f6..5511707cba3 100644 > --- a/gdb/valprint.h > +++ b/gdb/valprint.h > @@ -320,6 +320,12 @@ extern int build_address_symbolic (struct gdbarch *, > int *line, > int *unmapped); > > +/* Format OFFSET, the offset portion of a "" display, as > + a string with an explicit sign prefix ("+" or "-"). The numeric > + portion is rendered using the current "output-radix". */ > + > +extern std::string format_pc_offset (int offset); > + > /* Check to see if RECURSE is greater than or equal to the allowed > printing max-depth (see 'set print max-depth'). If it is then print an > ellipsis expression to STREAM and return true, otherwise return false. -- Cheers, Guinevere Larsen It/she