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 BD4503858404 for ; Thu, 13 Oct 2022 07:59:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BD4503858404 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-35-9ikAb8N3PNuFCBDhflCPFg-1; Thu, 13 Oct 2022 03:59:10 -0400 X-MC-Unique: 9ikAb8N3PNuFCBDhflCPFg-1 Received: by mail-wm1-f72.google.com with SMTP id fc12-20020a05600c524c00b003b5054c70d3so743874wmb.5 for ; Thu, 13 Oct 2022 00:59:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=yOZ9LpsgPmSeNlK3C1w0wBRVnknzJCe/OzP6tAllhiA=; b=5Zp+sYvzYgvL/XCRrNLeMNgfdQW3/vjF90HUQAx4TczrKG7eI6nELzHdHHuOqSiuoe xIgRZcmSj8VZgoAK1jaHHjV+g7DRDgamZnZxypw4QuilmRSy2v1UKhl6BHyZf+AsmZ5t ZhceDCWyXljjvIQVEGuJ057Nkl2ge7oDxQLQECK17jL/imuDosORH7w2RQ6FufBMSu3c G8eKdhIMYc1j+8OHMy05Ri1e9yEDud1xS73FMXCbG20Ujq3ElwEQf5sdYZYW4QKYDFo6 UdzHGiu1TD3ioRcuszb9qHCiajhM4kdVLnI5vOCr+7zas4AC/oKJGnAzPeag2ZKM6RaL rQHg== X-Gm-Message-State: ACrzQf21G6XAMdNVOUNfJ3lonwqb1U1Cg9V1jQc+ofj/AlDshwIftkPL r2xIkSx2huvWpMwGiP0SufJcapdS3ZMlgBlnJPIJeLkICuhF56DNpFVuv3XAYv50zAW6/f08IkH Ue6BO5nndAQKFw9WWdJkQFQ== X-Received: by 2002:a5d:59a7:0:b0:230:3652:1aa with SMTP id p7-20020a5d59a7000000b00230365201aamr13107372wrr.308.1665647948955; Thu, 13 Oct 2022 00:59:08 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4jCXHoFSxHzt1jSK8lJXs+0jE3ACIG4OJklJFy9OBesVjyg/zr9joARcMVpyWD0IFFGHdKpw== X-Received: by 2002:a5d:59a7:0:b0:230:3652:1aa with SMTP id p7-20020a5d59a7000000b00230365201aamr13107360wrr.308.1665647948643; Thu, 13 Oct 2022 00:59:08 -0700 (PDT) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id x16-20020a5d4910000000b00232251d71c7sm1383291wrq.68.2022.10.13.00.59.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Oct 2022 00:59:07 -0700 (PDT) Message-ID: Date: Thu, 13 Oct 2022 09:59:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set. To: Aaron Merey , gdb-patches@sourceware.org References: <20221007234428.12845-1-amerey@redhat.com> <20221012204916.467707-1-amerey@redhat.com> From: Bruno Larsen In-Reply-To: <20221012204916.467707-1-amerey@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP 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, 13 Oct 2022 07:59:17 -0000 On 12/10/2022 22:49, Aaron Merey via Gdb-patches wrote: > Hi, > > I'm reposting this patch with a testcase added to > testsuite/gdb.debuginfod/fetch_src_and_symbols.exp. I tried finding a > way to reproduce the error without debuginfod by using 'set > substitute-path' but the substitution rules always converted the symtab > fullname to the filename which avoided the error. Since I don't know > of any other way to trigger the error other than with source files > downloaded from debuginfod, the gdb.debuginfod seems like the best (only?) > place for the test. Another option would be making a unit test, so you can programmatically control the state from within GDB. That said, your test does trigger the problem and shows that the patch fixes it, so I'm ok with this test. Just a minor nit. > Aaron > > --- > > During breakpoint re-setting, the source_filename of an > explicit_location_spec is used to lookup the symtabs associated with > the breakpoint being re-set. This source_filename is compared with each > known symtab filename in order to retrieve the breakpoint's symtabs. > > However the source_filename may have been originally copied from a > symtab's fullname (the path where GDB found the source file) when the > breakpoint was first created. If a breakpoint symtab's filename and > fullname differ and there is no substitute-path setting that converts > the fullname to the filename, this will cause a NOT_FOUND_ERROR to be > thrown during re-setting. > > Fix this by using a symtab's filename to set the explicit_location_spec > source_filename instead of the symtab's fullname. > --- > gdb/linespec.c | 4 ++-- > gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 7 +++++-- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/gdb/linespec.c b/gdb/linespec.c > index 3db35998f7e..805c98ca201 100644 > --- a/gdb/linespec.c > +++ b/gdb/linespec.c > @@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls) > /* Make sure we have a filename for canonicalization. */ > if (ls->explicit_loc.source_filename == NULL) > { > - const char *fullname = symtab_to_fullname (state->default_symtab); > + const char *filename = state->default_symtab->filename; > > /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab > form so that displaying SOURCE_FILENAME can follow the current > FILENAME_DISPLAY_STRING setting. But as it is used only rarely > it has been kept for code simplicity only in absolute form. */ > - ls->explicit_loc.source_filename = xstrdup (fullname); > + ls->explicit_loc.source_filename = xstrdup (filename); > } > } > else > diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > index 8bb9203686d..9e7d4321913 100644 > --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > @@ -213,8 +213,11 @@ proc_with_prefix local_url { } { > gdb_test "file $binfile" "" "file [file tail $binfile]" "Enable debuginfod?.*" "y" > gdb_test_no_output "set substitute-path $outputdir /dev/null" \ > "set substitute-path" > - gdb_test "br main" "Breakpoint 1 at.*file.*" > - gdb_test "l" ".*This program is distributed in the hope.*" > + gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd" > + gdb_test "list 1" ".*This program is distributed in the hope.*" > + gdb_test "break 25" "Breakpoint 1 at.*file.*" > + gdb_test "run" "Breakpoint 1,.*" \ > + "file [file tail $binfile] set breakpoint]" extra ']' at the end Also, would be nice to have a small comment mentioning that there could be an error when resetting the breakpoint, so we have some context when looking at this test in the future. With these changes, I'm ok with the patch going in, but I can't approve it for pushing. Don't forget to add the tag to the commit :) Reviewed-By: Bruno Larsen Cheers, Bruno > > # GDB should now find the executable file. > clean_restart