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.133.124]) by sourceware.org (Postfix) with ESMTPS id CED4F385B83D for ; Thu, 17 Feb 2022 23:17:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CED4F385B83D Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-490-Ok_GfDDNMrmdNL5k6MTPsg-1; Thu, 17 Feb 2022 18:17:34 -0500 X-MC-Unique: Ok_GfDDNMrmdNL5k6MTPsg-1 Received: by mail-wr1-f72.google.com with SMTP id v17-20020adfc5d1000000b001e6405c2b56so2957308wrg.7 for ; Thu, 17 Feb 2022 15:17:34 -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=yy6mDydDi3ML1GWt7Gg2465/E+VcztO2jK1XdsKVVIo=; b=5C6/olfgSW+NnLSdEmYJAB9nW7YAAarDQvpKQH++84dzf9QM94DEFCgqKZ9zXGex4h wXSpfeSJvo1+Hf6VNGtngb5uEPiMkqZWNGbJ5FKu6EpPBYYWiWHuz+d9MUzn1Fs7wlIx SOftmowOWMVTvN7da26SNPiLqLbqq1TQ40p4FqEvQM3s3fs89eS9/GqOAmSFB/lgrBhZ JET/5z/xScM3qrUwvhRfcqlvhzz4w/mJdtDi3Bs2eMcLgVIUNHtZW7TiTUAvIca45S8z UPeOzhS4Aq40ifKbc6tKUI0FdI6P+PXwbuQPeXCdnoszgpyCULQQ3cVXukE8XhEyPOiQ OhVw== X-Gm-Message-State: AOAM530FTIadAjEh8G6gPY7lLsHLNjfgj0KQ/C8k/eAivfPm29Zj5DA2 LlEFh456JyF8hl5yn9S9kLlWlKHvHtlAiwNmD1ewl1iaq4gy+1h0CK2xwYfw7glNescZt6a0HeA tLuPjQK46ElCPku9lYGbxBn2QlSloVzoyd7xfTHX2YVyxa9tWXkKQcBX2uMCKaiS4rA68A+LVNg == X-Received: by 2002:a5d:668b:0:b0:1e4:a643:7146 with SMTP id l11-20020a5d668b000000b001e4a6437146mr3874179wru.340.1645139852856; Thu, 17 Feb 2022 15:17:32 -0800 (PST) X-Google-Smtp-Source: ABdhPJz9Z/UPkBIxLj39JQOlCpXdAlHNcbNsbXVMb5GzxwmJf9iB4WA/OwK6EwG3FeNHpRbKTtWixg== X-Received: by 2002:a5d:668b:0:b0:1e4:a643:7146 with SMTP id l11-20020a5d668b000000b001e4a6437146mr3874163wru.340.1645139852395; Thu, 17 Feb 2022 15:17:32 -0800 (PST) Received: from localhost (host86-134-151-224.range86-134.btcentralplus.com. [86.134.151.224]) by smtp.gmail.com with ESMTPSA id s7sm10468438wro.104.2022.02.17.15.17.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Feb 2022 15:17:32 -0800 (PST) From: Andrew Burgess To: Tom Tromey via Gdb-patches , gdb-patches@sourceware.org Cc: Tom Tromey Subject: Re: [PATCH] Change how "print/x" displays floating-point value In-Reply-To: <20220217212957.1747537-1-tromey@adacore.com> References: <20220217212957.1747537-1-tromey@adacore.com> Date: Thu, 17 Feb 2022 23:17:30 +0000 Message-ID: <874k4xdqwl.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=-10.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, 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: Thu, 17 Feb 2022 23:17:44 -0000 Tom Tromey via Gdb-patches writes: > Currently, "print/x" will display a floating-point value by first > casting it to an integer type. This yields weird results like: > > (gdb) print/x 1.5 > $1 = 0x1 > > This has confused users multiple times -- see PR gdb/16242, where > there are several dups. I've also seen some confusion from this > internally at AdaCore. > > The manual says: > > 'x' > Regard the bits of the value as an integer, and print the integer > in hexadecimal. > > ... which seems more useful. So, perhaps what happened is that this > was incorrectly implemented (or maybe correctly implemented and then > regressed, as there don't seem to be any tests). > > This patch fixes the bug. > > There was a previous discussion where we agreed to preserve the old > behavior: > > https://sourceware.org/legacy-ml/gdb-patches/2017-06/msg00314.html > > However, I think it makes more sense to follow the manual. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16242 > --- > gdb/printcmd.c | 9 ++------- > gdb/testsuite/gdb.base/printcmds.exp | 7 +++++-- > gdb/testsuite/gdb.base/return-nodebug.exp | 7 ++++++- > 3 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/gdb/printcmd.c b/gdb/printcmd.c > index 6f9be820b0c..30de1927d39 100644 > --- a/gdb/printcmd.c > +++ b/gdb/printcmd.c > @@ -426,19 +426,14 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type, > len = newlen; > } > > - /* Historically gdb has printed floats by first casting them to a > - long, and then printing the long. PR cli/16242 suggests changing > - this to using C-style hex float format. > - > - Biased range types and sub-word scalar types must also be handled > + /* Biased range types and sub-word scalar types must be handled > here; the value is correctly computed by unpack_long. */ > gdb::byte_vector converted_bytes; > /* Some cases below will unpack the value again. In the biased > range case, we want to avoid this, so we store the unpacked value > here for possible use later. */ > gdb::optional val_long; > - if (((type->code () == TYPE_CODE_FLT > - || is_fixed_point_type (type)) > + if ((is_fixed_point_type (type) > && (options->format == 'o' > || options->format == 'x' > || options->format == 't' > diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp > index 37632985a07..2d0aafd6d8b 100644 > --- a/gdb/testsuite/gdb.base/printcmds.exp > +++ b/gdb/testsuite/gdb.base/printcmds.exp > @@ -158,8 +158,11 @@ proc test_float_rejected {} { > # Regression test for PR gdb/21675 > proc test_radices {} { > gdb_test "print/o 16777211" " = 077777773" > - gdb_test "print/d 1.5" " = 1" > - gdb_test "print/u 1.5" " = 1" > + > + # See PR gdb/16242 for this. > + gdb_test "print/d 1.5f" " = 1069547520" > + gdb_test "print/u 1.5f" " = 1069547520" > + gdb_test "print/x 1.5f" " = 0x3fc00000" I noticed the reference to PR gdb/21675 at the top of this hunk, which was an interesting read. Would it be possible to increase the testing to cover printing doubles as well as floats, and also to test /o and /t format? FWIW, I think this change makes sense. Though I wonder if it is worth having a NEWS entry as this might look like a bug to someone who is used to the old behaviour? Also, maybe I'm just overly paranoid, but I wonder if it is worth really stressing the point in the manual (@node Output Format) that the value is reinterpreted, rather than cast. Especially for /d /u /o, etc the manual is rather vague on how a float will be handled, e.g.: @item d Print as integer in signed decimal. Which doesn't really tell me much... Thanks, Andrew > > gdb_test "print/u (char) -1" " = 255" > gdb_test "print/d (unsigned char) -1" " = -1" > diff --git a/gdb/testsuite/gdb.base/return-nodebug.exp b/gdb/testsuite/gdb.base/return-nodebug.exp > index 6fd41bee884..1cce09d2fc4 100644 > --- a/gdb/testsuite/gdb.base/return-nodebug.exp > +++ b/gdb/testsuite/gdb.base/return-nodebug.exp > @@ -38,7 +38,12 @@ proc do_test {type} { > "advance to marker" > > # And if it returned the full width of the result. > - gdb_test "print /d t" " = -1" "full width of the returned result" > + if {$type == "float" || $type == "double"} { > + set flag "" > + } else { > + set flag "/d" > + } > + gdb_test "print $flag t" " = -1" "full width of the returned result" > } > } > } > -- > 2.31.1