From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qOidHUJzgF+CCgAAWB0awg (envelope-from ) for ; Fri, 09 Oct 2020 10:27:14 -0400 Received: by simark.ca (Postfix, from userid 112) id 6B1201EF6F; Fri, 9 Oct 2020 10:27:14 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 886261E58C for ; Fri, 9 Oct 2020 10:27:13 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1F0B7385040C; Fri, 9 Oct 2020 14:27:13 +0000 (GMT) Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) by sourceware.org (Postfix) with ESMTPS id 60951385702E for ; Fri, 9 Oct 2020 14:27:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 60951385702E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x441.google.com with SMTP id x7so1914795wrl.3 for ; Fri, 09 Oct 2020 07:27:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=CNBMlfik4YydKlT3upntrg58bTwV5/8A8O2FuoctTuk=; b=J5NqygaUmoMUS0cMvrCCMdu0BcorSHZo65bbkMC0OxhEVwHvRv35w3pXUGYghbJkKq C3/1T7m0xlNoVVNN5ApZP8JZtZBMXG3Gldgh2Vl2X+T1RWI/K+S1k5gN+/LliWyVFs6c 7tSM5R0yJksvuYmcqKTkj8ckS8ZUWNOIVGb2KJiKA1+Hw1vr7r1YPQcLUSP+OOEdkmt0 G4DzUJtJn/2a4CJiKWBavSMzMqWxdCRx2pT2Lpbh6z0gLdJwZioPsB8lVI9cCRcYoza0 7MQW6WFT4qH9IDuu16d5xE99/2MaSleU2ldOQxBi+KSd2LpDRkShIGwFblnRcjpMmEiy QseQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=CNBMlfik4YydKlT3upntrg58bTwV5/8A8O2FuoctTuk=; b=l3WFDED9u5sK2MaOtqp1ItLgjQTWibRsWbdHW8/I1R8/QR5W3GLTBzGxTH38YHzhIQ oUr27jpjFt4ix1gbCCYteTQy14z19MWW8APnpc/lKNISxroYVCTDZ6e68aWK8himB+Xc 82PqVfRdD+j3J6QfnM9jmW6ENga1TJLFkdydEmhba82LFa78e56OUcafKw50/qh62yjI hHiwOwCIuKDGKrAZi/Cfsjl/v0U8iKbEahyZgS2joH8/FDLg8lkHFiMAsSnf+O/eJoDF AwN07kYlsMcdq+g0jaq2cqikDDU92Gs5CbldjHeCQorN3+vrpUYEMW+xEiq5/yzfml4M 6AzA== X-Gm-Message-State: AOAM530XwbsjHBISofPYZhmUDSlL36H470kgKc1M0nEZ9ukHSZPAqXsf 5LBzAYXofon0CAG7EhS+gdkEBg== X-Google-Smtp-Source: ABdhPJyR+G3agETFHPH2VauZSs7zneWcMHuI1/H5HJHUszjwMWJyrmLb5zNJ2DrxaStPRTCQOe5Sbw== X-Received: by 2002:adf:818b:: with SMTP id 11mr14922831wra.74.1602253629360; Fri, 09 Oct 2020 07:27:09 -0700 (PDT) Received: from localhost (host109-148-134-240.range109-148.btcentralplus.com. [109.148.134.240]) by smtp.gmail.com with ESMTPSA id g14sm12355885wrx.22.2020.10.09.07.27.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 09 Oct 2020 07:27:08 -0700 (PDT) Date: Fri, 9 Oct 2020 15:27:07 +0100 From: Andrew Burgess To: Tom Tromey Subject: Re: [PATCH] Fix bit offset regression Message-ID: <20201009142707.GR605036@embecosm.com> References: <20201009134130.3785956-1-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201009134130.3785956-1-tromey@adacore.com> X-Operating-System: Linux/5.8.12-100.fc31.x86_64 (x86_64) X-Uptime: 15:24:02 up 6 days, 6:36, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Tom Tromey [2020-10-09 07:41:30 -0600]: > The type-safe attribute patch introduced a regression that can occur > when the DW_AT_bit_offset value is negative. This can happen with > some Ada programs. > > This patch fixes the problem. It also fixes a minor oddity in the > existing scalar storage test -- this test was intended to assign a > smaller number of bits to the field. > > gdb/ChangeLog > 2020-10-09 Tom Tromey > > * dwarf2/read.c (dwarf2_add_field): Handle signed offsets. > > gdb/testsuite/ChangeLog > 2020-10-09 Tom Tromey > > * gdb.ada/scalar_storage/storage.adb (Another_Range): New type. > (Rec): Add field. Fix range. > * gdb.ada/scalar_storage.exp: Update. > --- > gdb/ChangeLog | 4 ++++ > gdb/dwarf2/read.c | 10 +++++----- > gdb/testsuite/ChangeLog | 6 ++++++ > gdb/testsuite/gdb.ada/scalar_storage.exp | 4 ++-- > gdb/testsuite/gdb.ada/scalar_storage/storage.adb | 9 ++++++--- > 5 files changed, 23 insertions(+), 10 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index eedfea112d9..2ec3789135d 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -15050,7 +15050,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die, > /* Get bit offset of field. */ > handle_data_member_location (die, cu, fp); > attr = dwarf2_attr (die, DW_AT_bit_offset, cu); > - if (attr != nullptr && attr->form_is_unsigned ()) > + if (attr != nullptr && attr->form_is_constant ()) > { > if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG) > { > @@ -15060,7 +15060,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die, > have to do anything special since we don't need to > know the size of the anonymous object. */ > SET_FIELD_BITPOS (*fp, (FIELD_BITPOS (*fp) > - + attr->as_unsigned ())); > + + attr->constant_value (0))); I wondered what the `0` was all about here, so I took a look... In attribute::constant_value it says: /* For DW_FORM_data16 see attribute::form_is_constant. */ complaint (_("Attribute value is not a constant (%s)"), dwarf_form_name (form)); return default_value; Hmm, and the comment on form_is_constant says: DW_FORM_data16 is not considered as constant_value cannot handle that. */ I'm not sure I'm any more enlightened about the default value... None of which has anything to do with your patch, which looks fine. Thanks, Andrew > } > else > { > @@ -15071,15 +15071,15 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die, > the field itself. The result is the bit offset of > the LSB of the field. */ > int anonymous_size; > - int bit_offset = attr->as_unsigned (); > + int bit_offset = attr->constant_value (0); > > attr = dwarf2_attr (die, DW_AT_byte_size, cu); > - if (attr != nullptr && attr->form_is_unsigned ()) > + if (attr != nullptr && attr->form_is_constant ()) > { > /* The size of the anonymous object containing > the bit field is explicit, so use the > indicated size (in bytes). */ > - anonymous_size = attr->as_unsigned (); > + anonymous_size = attr->constant_value (0); > } > else > { > diff --git a/gdb/testsuite/gdb.ada/scalar_storage.exp b/gdb/testsuite/gdb.ada/scalar_storage.exp > index b5e634c615b..952d7fd136e 100644 > --- a/gdb/testsuite/gdb.ada/scalar_storage.exp > +++ b/gdb/testsuite/gdb.ada/scalar_storage.exp > @@ -34,5 +34,5 @@ if ![runto "storage.adb:$bp_location" ] then { > return > } > > -gdb_test "print V_LE" "= \\(value => 126\\)" > -gdb_test "print V_BE" "= \\(value => 126\\)" > +gdb_test "print V_LE" "= \\(value => 126, another_value => 12\\)" > +gdb_test "print V_BE" "= \\(value => 126, another_value => 12\\)" > diff --git a/gdb/testsuite/gdb.ada/scalar_storage/storage.adb b/gdb/testsuite/gdb.ada/scalar_storage/storage.adb > index 608425d9dd1..741718e4e51 100644 > --- a/gdb/testsuite/gdb.ada/scalar_storage/storage.adb > +++ b/gdb/testsuite/gdb.ada/scalar_storage/storage.adb > @@ -18,13 +18,16 @@ with System.Storage_Elements; use System.Storage_Elements; > > procedure Storage is > subtype Some_Range is Natural range 0..127; > + subtype Another_Range is Natural range 0..15; > > type Rec is record > Value : Some_Range; > + Another_Value : Another_Range; > end record; > > for Rec use record > - Value at 0 range 0..127; > + Value at 0 range 0..6; > + Another_Value at 0 range 7..10; > end record; > > type Rec_LE is new Rec; > @@ -39,8 +42,8 @@ procedure Storage is > V_BE : Rec_BE; > > begin > - V_LE.Value := 126; > - V_BE.Value := 126; > + V_LE := (126, 12); > + V_BE := (126, 12); > > Do_Nothing (V_LE'Address); -- START > Do_Nothing (V_BE'Address); > -- > 2.26.2 >