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 C4F6C384B831 for ; Thu, 19 May 2022 18:48:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C4F6C384B831 Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-267-ip6mMjUoPV2FB5VvOteuZQ-1; Thu, 19 May 2022 14:48:35 -0400 X-MC-Unique: ip6mMjUoPV2FB5VvOteuZQ-1 Received: by mail-ed1-f70.google.com with SMTP id o10-20020aa7c7ca000000b0042a4f08405fso4115197eds.22 for ; Thu, 19 May 2022 11:48:35 -0700 (PDT) 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=5St2Jgjq8VfK9GTfL+C6MsBTJ2Zbn5+piWiWP3aFWpc=; b=NOvkW2V1O67aR+nYpdLcbuCpFo0OVpYxzVB2RoFEtnPhCrPIjIKvZDprL+/fkNyhNu aL209gF8Rle0jOM4eDNox0O6Bj0rszIi5F0iD2AS46lZVjSPrfOsqtg9+hd2PZ7i4s6V 6NTLbHZ3zjbklJXavEBigpvYzpHac4uCbXvWFad54A3B7X/helOt0ljERQ2ornZCwtw1 yXndNY8znVJV9RE7VyB+zN3izTTbGmVd0doBKoUdHqYFJBXkMVrCI4U2FvJY8mVOBZ9G 4zuLsU1/HCYdUx3GRNxVDq109MPBwjF40/AEedBrCb049Tku/gfSBMmTK8HQlcSMKsmc pRGw== X-Gm-Message-State: AOAM531IsWc2AYTK70T4q/RA2Yw2VlEcJaCsH2Rt1OpBMM2YfGe38Vut tBSN3DKd/RvAKQ0xr+Y5et8N04u5oLzjqu+9/6RrBXiAIoPfLaXMvOpenWGQ5pidNn6QaSN8rHF 7ZcR+6/vJl3mPBHanehFd4A== X-Received: by 2002:a17:907:6090:b0:6fa:14ca:fba2 with SMTP id ht16-20020a170907609000b006fa14cafba2mr5893500ejc.564.1652986114472; Thu, 19 May 2022 11:48:34 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyPAs9FzTQ0Po+EeceE017P5WIP9mKbBlm055leYVeZP31BI02K8m59SvXpmlYEY+DV0Uv/6Q== X-Received: by 2002:a17:907:6090:b0:6fa:14ca:fba2 with SMTP id ht16-20020a170907609000b006fa14cafba2mr5893487ejc.564.1652986114233; Thu, 19 May 2022 11:48:34 -0700 (PDT) Received: from localhost (92.40.178.42.threembb.co.uk. [92.40.178.42]) by smtp.gmail.com with ESMTPSA id gz20-20020a170907a05400b006fea2b678basm263090ejc.89.2022.05.19.11.48.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 May 2022 11:48:33 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Cc: Tom Tromey , Pedro Alves Subject: Re: [PATCH][gdb/testsuite] Fix printing of 0x10000000000000000 In-Reply-To: <20220518122555.GA24256@delia> References: <20220518122555.GA24256@delia> Date: Thu, 19 May 2022 19:48:31 +0100 Message-ID: <87v8u1icjk.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.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_ABUSEAT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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, 19 May 2022 18:48:46 -0000 Tom de Vries via Gdb-patches writes: > Hi, > > When printing large literal constants we have an unexpected result for > 0x10000000000000000: > ... > $ gdb -q -batch \ > -ex "p /x 0xffffffffffffffff" \ > -ex "p /x 0x10000000000000000" \ > -ex "p /x 0x10000000000000001" > $1 = 0xffffffffffffffff > $2 = 0x0 > Numeric constant too large. > ... > > Fix this by fixing overflow detecting in parse_number, such that we have: > ... > $1 = 0xffffffffffffffff > Numeric constant too large. > Numeric constant too large. > ... > > For rust, I've used as overflow string: "Integer literal is too large", based > on what I found at > /src/test/ui/parser/int-literal-too-large-span.rs > but perhaps someone has a better idea. > > Don't fix this for modula-2, it makes more sense to do this as part of a > parse_number rewrite to use ULONGEST, skip for now. > > Tested on x86_64-linux, with a build with --enable-targets=all. > > Any comments? This all looks good to me, just one minor comment... > > Thanks, > - Tom > > [gdb/testsuite] Fix printing of 0x10000000000000000 > > --- > gdb/c-exp.y | 18 ++++++------------ > gdb/f-exp.y | 15 +++++---------- > gdb/go-exp.y | 16 +++++----------- > gdb/p-exp.y | 16 +++++----------- > gdb/rust-parse.c | 5 ++++- > gdb/testsuite/gdb.base/parse_number.exp | 17 +++++++++++++++++ > 6 files changed, 42 insertions(+), 45 deletions(-) > > diff --git a/gdb/c-exp.y b/gdb/c-exp.y > index 72f8dd32d93..84bc0568e8f 100644 > --- a/gdb/c-exp.y > +++ b/gdb/c-exp.y > @@ -2095,18 +2095,12 @@ parse_number (struct parser_state *par_state, > if (i >= base) > return ERROR; /* Invalid digit in this base */ > > - /* Portably test for overflow (only works for nonzero values, so make > - a second check for zero). FIXME: Can't we just make n and prevn > - unsigned and avoid this? */ > - if (c != 'l' && c != 'u' && c != 'i' && (prevn >= n) && n != 0) > - unsigned_p = 1; /* Try something unsigned */ > - > - /* Portably test for unsigned overflow. > - FIXME: This check is wrong; for example it doesn't find overflow > - on 0x123456789 when LONGEST is 32 bits. */ > - if (c != 'l' && c != 'u' && c != 'i' && n != 0) > - { > - if (unsigned_p && prevn >= n) > + if (c != 'l' && c != 'u' && c != 'i') > + { > + /* Test for overflow. */ > + if (prevn == 0 && n == 0) > + ; > + else if (prevn >= n) > error (_("Numeric constant too large.")); > } > prevn = n; > diff --git a/gdb/f-exp.y b/gdb/f-exp.y > index 90cc2c65c7b..124dfcb1b8f 100644 > --- a/gdb/f-exp.y > +++ b/gdb/f-exp.y > @@ -1076,16 +1076,11 @@ parse_number (struct parser_state *par_state, > n *= base; > n += i; > } > - /* Portably test for overflow (only works for nonzero values, so make > - a second check for zero). */ > - if ((prevn >= n) && n != 0) > - unsigned_p=1; /* Try something unsigned */ > - /* If range checking enabled, portably test for unsigned overflow. */ > - if (RANGE_CHECK && n != 0) > - { > - if ((unsigned_p && prevn >= n)) > - range_error (_("Overflow on numeric constant.")); > - } > + /* Test for overflow. */ > + if (prevn == 0 && n == 0) > + ; > + else if (RANGE_CHECK && prevn >= n) > + range_error (_("Overflow on numeric constant.")); > prevn = n; > } > > diff --git a/gdb/go-exp.y b/gdb/go-exp.y > index 1b9b6ea7e90..8c9a558262f 100644 > --- a/gdb/go-exp.y > +++ b/gdb/go-exp.y > @@ -777,18 +777,12 @@ parse_number (struct parser_state *par_state, > if (i >= base) > return ERROR; /* Invalid digit in this base. */ > > - /* Portably test for overflow (only works for nonzero values, so make > - a second check for zero). FIXME: Can't we just make n and prevn > - unsigned and avoid this? */ > - if (c != 'l' && c != 'u' && (prevn >= n) && n != 0) > - unsigned_p = 1; /* Try something unsigned. */ > - > - /* Portably test for unsigned overflow. > - FIXME: This check is wrong; for example it doesn't find overflow > - on 0x123456789 when LONGEST is 32 bits. */ > - if (c != 'l' && c != 'u' && n != 0) > + if (c != 'l' && c != 'u') > { > - if ((unsigned_p && prevn >= n)) > + /* Test for overflow. */ > + if (n == 0 && prevn == 0) > + ; > + else if (prevn >= n) > error (_("Numeric constant too large.")); > } > prevn = n; > diff --git a/gdb/p-exp.y b/gdb/p-exp.y > index 7c88df65e69..7e119963759 100644 > --- a/gdb/p-exp.y > +++ b/gdb/p-exp.y > @@ -919,18 +919,12 @@ parse_number (struct parser_state *par_state, > if (i >= base) > return ERROR; /* Invalid digit in this base. */ > > - /* Portably test for overflow (only works for nonzero values, so make > - a second check for zero). FIXME: Can't we just make n and prevn > - unsigned and avoid this? */ > - if (c != 'l' && c != 'u' && (prevn >= n) && n != 0) > - unsigned_p = 1; /* Try something unsigned. */ > - > - /* Portably test for unsigned overflow. > - FIXME: This check is wrong; for example it doesn't find overflow > - on 0x123456789 when LONGEST is 32 bits. */ > - if (c != 'l' && c != 'u' && n != 0) > + if (c != 'l' && c != 'u') > { > - if (unsigned_p && prevn >= n) > + /* Test for overflow. */ > + if (prevn == 0 && n == 0) > + ; > + else if (prevn >= n) > error (_("Numeric constant too large.")); > } > prevn = n; > diff --git a/gdb/rust-parse.c b/gdb/rust-parse.c > index 7d7d882872c..836f49108f8 100644 > --- a/gdb/rust-parse.c > +++ b/gdb/rust-parse.c > @@ -1024,7 +1024,10 @@ rust_parser::lex_number () > } > } > > - value = strtoulst (number.c_str () + offset, NULL, radix); > + const char *trailer; > + value = strtoulst (number.c_str () + offset, &trailer, radix); > + if (*trailer != '\0') > + error ("Integer literal is too large"); > if (implicit_i32 && value >= ((uint64_t) 1) << 31) > type = get_type ("i64"); > > diff --git a/gdb/testsuite/gdb.base/parse_number.exp b/gdb/testsuite/gdb.base/parse_number.exp > index 444f5d0534b..60526f5612a 100644 > --- a/gdb/testsuite/gdb.base/parse_number.exp > +++ b/gdb/testsuite/gdb.base/parse_number.exp > @@ -68,6 +68,16 @@ proc test_parse_numbers {arch} { > > gdb_test_no_output "set language $lang" > > + if { $lang == "modula-2" || $lang == "fortran" } { > + set re_overflow "Overflow on numeric constant\\." > + } elseif { $lang == "ada" } { > + set re_overflow "Integer literal out of range" > + } elseif { $lang == "rust" } { > + set re_overflow "Integer literal is too large" > + } else { > + set re_overflow "Numeric constant too large\\." > + } > + > set val "0xffffffffffffffff" > set val [hex_for_lang $lang $val] > if {$lang == "fortran"} { > @@ -98,6 +108,13 @@ proc test_parse_numbers {arch} { > gdb_test "ptype $val" " = $8B_type" > } > } > + > + if { $lang == "modula-2" } { > + continue > + } Maybe you're working on modula-2 right now, in which case, ignore this. But, if you're not, then it might be better to raise a bug for the module-2 issues - even if it's just "parse_number needs a rewrite", and then use setup_xfail here instead of continue. But otherwise, this all looks OK to me. Thanks, Andrew > + > + set val [hex_for_lang $lang "10000000000000000"] > + gdb_test "p/x $val" $re_overflow > } > } >