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 40F8B385ED71 for ; Wed, 26 Jan 2022 19:18:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 40F8B385ED71 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 20QJIdbo009290 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 26 Jan 2022 14:18:43 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 20QJIdbo009290 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (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 86B341EE18; Wed, 26 Jan 2022 14:18:38 -0500 (EST) Message-ID: Date: Wed, 26 Jan 2022 14:18:38 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] gdb: work around negative DW_AT_data_member_location GCC 11 bug Content-Language: en-US To: Keith Seitz , Simon Marchi , gdb-patches@sourceware.org References: <20211129153725.1499053-1-simon.marchi@efficios.com> <844ce501-7a0d-2806-2a57-d08c71e8bcb4@polymtl.ca> <53f71b5f-a989-7eef-0178-9a96a414ece8@redhat.com> From: Simon Marchi In-Reply-To: <53f71b5f-a989-7eef-0178-9a96a414ece8@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 26 Jan 2022 19:18:39 +0000 X-Spam-Status: No, score=-3039.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, 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: Wed, 26 Jan 2022 19:18:52 -0000 On 2022-01-26 13:17, Keith Seitz wrote: >>> The difficult part would be if GCC 11 ever emits a legitimate >>> DW_AT_data_member_location value of -1 in other situations, then we >>> would need to identify when the -1 is legitimate and when it is >>> not. > We'll cross that bridge when/if we get to it? ;-) > > I just have two tiny nits... > >>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >>> index 737d8a4c81b..0c66a6daf97 100644 >>> --- a/gdb/dwarf2/read.c >>> +++ b/gdb/dwarf2/read.c >>> @@ -14489,6 +14489,16 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu, >>> if (attr->form_is_constant ()) >>> { >>> LONGEST offset = attr->constant_value (0); >>> + >>> + /* Work around this GCC 11 bug, where it would erroneously use -1 >>> + data member locations, instead of 0: >>> + >>> + Negative DW_AT_data_member_location >>> + https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378 >>> + */ >>> + if (offset == -1) >>> + offset = 0; >>> + > > Kevin and I discussed this briefly, and he convinced me that a complaint here might > be useful. What do you think? Would that work for you? if (offset == -1) { complaint (_("DW_AT_data_member_location value of -1, assuming 0")); offset = 0; } > member-location.exp >>> new file mode 100644 >>> index 00000000000..664c4e47acc >>> --- /dev/null >>> +++ b/gdb/testsuite/gdb.dwarf2/negative-data-member-location.exp >>> @@ -0,0 +1,76 @@ >>> +# Copyright 2021 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 . >>> + >>> +# Test our workaround for a GCC 11 bug, where it sometimes puts a -1 value for >>> +# DW_AT_data_member_location: >>> +# >>> +# Negative DW_AT_data_member_location >>> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378 >>> + >>> +load_lib dwarf.exp >>> + >>> +# This test can only be run on targets which support DWARF-2 and use gas. >>> +if ![dwarf2_support] { >>> + return 0 >>> +} >>> + > > Please pardon my Tclish fetish, but since this is a new file (and likely to > be used as a model for future contributions), may I ask that we be more > pedantic about the formatting? I.e., all conditional expressions enclosed in > curly brackets, no use of "then" in "if" statements? Those are so > last-century. [:-)] No problem, I'm all for consistency. Would you like to list those rules here? https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards It's always good to have something to refer to. > I think this is go to go regardless. I recommend you approve your patch. I'll go read your other email. Simon