From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 4D5FE3858C60 for ; Thu, 7 Oct 2021 15:21:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4D5FE3858C60 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 197FLW6w014563 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 7 Oct 2021 11:21:37 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 197FLW6w014563 Received: from [172.16.0.95] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0E9891E4A3; Thu, 7 Oct 2021 11:21:31 -0400 (EDT) Subject: Re: [PATCH v2] gdb: add accessors for field (and call site) location To: Simon Marchi , gdb-patches@sourceware.org References: <20210924213445.3413303-1-simon.marchi@efficios.com> From: Simon Marchi Message-ID: <9e8a7ffe-8ba3-c683-369a-d53ee9f78900@polymtl.ca> Date: Thu, 7 Oct 2021 11:21:31 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20210924213445.3413303-1-simon.marchi@efficios.com> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 7 Oct 2021 15:21:32 +0000 X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TRACKER_ID, TXREP 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, 07 Oct 2021 15:21:42 -0000 On 2021-09-24 5:34 p.m., Simon Marchi wrote: > From: Simon Marchi > > Changes in v2: > > - Undo adding a FIELD_LOC_KIND_UNSET > - Fix other regressions (described below) > > Add accessors for the various location values in struct field. This > lets us assert that when we get a location value of a certain kind (say, > bitpos), the field's location indeed contains a value of that kind. > > Remove the SET_FIELD_* macros, instead use the new setters directly. > Update the FIELD_* macros used to access field locations to go through > the getters. They will be removed in a subsequent patch. > > There are places where the FIELD_* macros are used on call_site_target > structures, because it contains members of the same name (loc_kind and > loc). For now, I have replicated the getters/setters in > call_site_target. But we could perhaps eventually factor them in a > "location" structure that can be used at both places. > > Note that the field structure, being zero-initialized, defaults to a > bitpos location with value 0. While writing this patch, I tried to make > it default to an "unset" location, to catch places where we would miss > setting a field's location. However, I found that some places relied on > the default being "bitpos 0", so I left it as-is. This change could > always be done as follow-up work, making these places explicitly set the > "bitpos 0" location. > > I found two issues to fix: > > - I got some failures in the gdb.base/infcall-nested-structs-c++.exp > test. They were caused by two functions in amd64-tdep.c using > TYPE_FIELD_BITPOS before checking if the location is of the bitpos > kind, which they do indirectly through `field_is_static`. Simply > move getting the bitpos below the field_is_static call. > > - I got a failure in gdb.xml/tdesc-regs.exp. It turns out that in > make_gdb_type_enum, we set enum field values using SET_FIELD_BITPOS, > and later access them through FIELD_ENUMVAL. Fix that by using > set_loc_enumval to set the value. > > Change-Id: I53d3734916c46457576ba11dd77df4049d2fc1e8 I pushed this. Simon