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.133.124]) by sourceware.org (Postfix) with ESMTPS id 07B173858418 for ; Wed, 24 May 2023 10:12:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 07B173858418 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684923144; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=q+eASTSSrBX8XGNEdqUWBlzjDU0W3pJEfRpJ3UlfS5M=; b=Lsr9/CaWGm/dIGR2qziNfGhH4Te0tydjXcNBnIryrfEpXydIKze3nWmKUXutpQLS7MV2cQ X3DQdSorMlTb4J4N/6tR66vMIKN1I+AtGL+WWKnZinVYpwzhuDchhL0a+2qchWQDlcHpSW uaP4JdknY41eXcUwn9A+VdnCYvE+YaU= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-472--Q5H4sASMzS8yJ5g0mQtoA-1; Wed, 24 May 2023 06:12:23 -0400 X-MC-Unique: -Q5H4sASMzS8yJ5g0mQtoA-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3093b0cf714so240906f8f.2 for ; Wed, 24 May 2023 03:12:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684923142; x=1687515142; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=q+eASTSSrBX8XGNEdqUWBlzjDU0W3pJEfRpJ3UlfS5M=; b=YOd6Cjp0gQJgb2NbjTpKyf6ZKX/76xFooauUQUkD8xEQyeG2jPokjRFVhUr8MwxPZD KcOqi0BdX8kGg1xFafV5Ty/Uc3dNjOeD1ap0zltcueWsrc8CQ0nTNnaWIoZyJoJv3EgN mWacMdwKeTeqKGEQzhBY17dssPjqDnCdoJt97X1yTji4Xb57nYcrZ+ZQEdtnmoT3F966 SsSrdvaNFFxxtGblS9oGMlhMbIfG7k0X7Yxcy8lbVQt0dlRMmG+y1ltr2P7ITL0NNuU/ L96oBkk+A5QjX4Rv4pCBdcHMN/vdghEAB83x2JFKU9583sUANmED7J07pmxWZJCf7LHO mG3w== X-Gm-Message-State: AC+VfDwVU+xN4szVPTv6Cynmgo3+Vzz9ti/AChoV/zztay8LXYPxXoCp +W6o4jT6mvgEBTPeIvmzPxcvlD/s8uFlUGCQy12ppqv6mQzEEmKDntjPVJrGGJzC1FlGK7L/93Y v9MRy996XFJyp8C1kdnEbsjU3O6PHyw6Ursi27GAT8nXmnElAwZt38FpekLD7z70KTm8UVO4oOz V5D5lLaA== X-Received: by 2002:adf:e70d:0:b0:2f0:2dfe:e903 with SMTP id c13-20020adfe70d000000b002f02dfee903mr11796162wrm.69.1684923142238; Wed, 24 May 2023 03:12:22 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6SMlSbGCN4Mnof5hhE6YWhb3PHO7DX3Kg9JJdRD1KFBqFAbBJrxBz7yrNobLbrEY7BkEs1gQ== X-Received: by 2002:adf:e70d:0:b0:2f0:2dfe:e903 with SMTP id c13-20020adfe70d000000b002f02dfee903mr11796139wrm.69.1684923141800; Wed, 24 May 2023 03:12:21 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id u2-20020a05600c210200b003f42314832fsm1798390wml.18.2023.05.24.03.12.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 May 2023 03:12:21 -0700 (PDT) From: Andrew Burgess To: Aaron Merey via Gdb-patches , gdb-patches@sourceware.org Cc: Aaron Merey Subject: Re: [PATCH 6/7] gdb/testsuite/gdb.debuginfod: Add lazy downloading tests In-Reply-To: <20230227194212.348003-6-amerey@redhat.com> References: <20230227194212.348003-1-amerey@redhat.com> <20230227194212.348003-6-amerey@redhat.com> Date: Wed, 24 May 2023 11:12:20 +0100 Message-ID: <87pm6q6nsr.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.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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 List-Id: Aaron Merey via Gdb-patches writes: > Add some tests for 'set debuginfod enabled lazy', .gdb_index downloading > and deferred debuginfo downloading. > --- > .../gdb.debuginfod/fetch_src_and_symbols.exp | 58 +++++++++++++++++++ > gdb/testsuite/gdb.debuginfod/libsection.c | 24 ++++++++ > gdb/testsuite/gdb.debuginfod/section.c | 28 +++++++++ > 3 files changed, 110 insertions(+) > create mode 100644 gdb/testsuite/gdb.debuginfod/libsection.c > create mode 100644 gdb/testsuite/gdb.debuginfod/section.c > > diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > index 8158c5c3cc6..fc7e4b685dc 100644 > --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp > @@ -42,6 +42,23 @@ if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug build-id}] != "" > return -1 > } > > +set sectfile "section" > +set sectsrc $srcdir/$subdir/section.c > +set sectexec [standard_output_file $sectfile] > + > +set libfile "libsection" > +set libsrc $srcdir/$subdir/$libfile.c > +set lib_sl [standard_output_file $libfile.sl] > + > +set lib_opts [list debug build-id ] > +set exec_opts [list debug build-id shlib=$lib_sl] > + > +if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != "" > + || [gdb_compile $sectsrc $sectexec executable $exec_opts] != "" } { > + untested "failed to compile" > + return -1 > +} I'm really not a fan of either having a separate "add tests" commit, this makes it so much harder in the future to track down which test corresponds to a particular change. Nor am I a fan of having this one test script fetch_src_and_symbols.exp, into which all debuginfod tests just keep getting added. I already made an attempt to factor out some of the support functionality into lib/debuginfod-support.exp, and it would be great to see more functionality added in there so that it becomes easier to add new debuginfod tests. I mean, it's fine, if the new test makes use of an existing binary and you're just running some extra commands as part of an existing test script, but for me the red flag is that you have a whole new source file which you compile, that indicates (to me) that this should be a new test script. Also, this approach of adding tests in a single commit makes it really easy to miss testing some of the changes -- for example, I see no use of backtrace anywhere in fetch_src_and_symbols.exp, so that patch isn't being tested at all. > + > # Write some assembly that just has a .gnu_debugaltlink section. > # Copied from testsuite/gdb.dwarf2/dwzbuildid.exp. > proc write_just_debugaltlink {filename dwzname buildid} { > @@ -94,6 +111,7 @@ proc write_dwarf_file {filename buildid {value 99}} { > } > > set corefile [standard_output_file "corefile"] > +set lazy_support -1 > > # Setup the global variable DEBUGDIR as a directory containing the > # debug information for the test executable. > @@ -103,6 +121,8 @@ set corefile [standard_output_file "corefile"] > # running. > proc_with_prefix no_url { } { > global binfile outputdir debugdir > + global sectexec lib_sl libfile lazy_support > + global gdb_prompt > > setenv DEBUGINFOD_URLS "" > > @@ -119,11 +139,18 @@ proc_with_prefix no_url { } { > return -1 > } > > + if { [gdb_gnu_strip_debug $lib_sl ""] != 0} { > + fail "strip shlib debuginfo" > + return -1 > + } > + > set debugdir [standard_output_file "debug"] > set debuginfo [standard_output_file "fetch_src_and_symbols.debug"] > + set debuginfo_shlib [standard_output_file $libfile.sl.debug] > > file mkdir $debugdir > file rename -force $debuginfo $debugdir > + file rename -force $debuginfo_shlib $debugdir > > # Test that GDB cannot find symbols without debuginfod. > clean_restart $binfile > @@ -171,6 +198,25 @@ proc_with_prefix no_url { } { > > clean_restart > gdb_test "core $::corefile" ".*in ?? ().*" "file [file tail $::corefile]" > + clean_restart > + > + # Check for lazy downloading support. > + gdb_test_multiple "set debuginfod enabled lazy" "check for lazy" { > + -re ".*lazy downloading is not compiled into GDB.*\n.*${gdb_prompt} $" { > + set lazy_support 0 > + } > + -re ".*${gdb_prompt} $" { > + set lazy_support 1 > + } > + } As one example, this would be a great candidate for moving into lib/debuginfod-support.exp. Thanks, Andrew > + > + if {$lazy_support == 1} { > + gdb_test "file $sectexec" "" "file [file tail $sectexec] file no url" > + gdb_test "start" "" "file [file tail $sectexec] start no url" > + gdb_test "info sharedlibrary" ".*Yes \\(\\*\\).*libsection.*" "lib no debug" > + } else { > + untested "lazy support no_url" > + } > } > > # Test that GDB prints the debuginfod URLs when loading files. URLS > @@ -208,6 +254,7 @@ proc disable_delete_breakpoints {} { > # expected debug information. > proc_with_prefix local_url { } { > global binfile outputdir debugdir db > + global sectexec lib_sl libfile lazy_support > > set url [start_debuginfod $db $debugdir] > if { $url == "" } { > @@ -256,6 +303,17 @@ proc_with_prefix local_url { } { > "file [file tail ${binfile}_alt.o]" \ > $enable_debuginfod_question "y" > > + if {$lazy_support == 1} { > + # GDB should now download .gdb_index. > + clean_restart > + gdb_test "set debuginfod enabled lazy" "" "set lazy urls" > + gdb_test "file $sectexec" "" "file [file tail $sectexec] file urls" > + set res ".*Download.*\.gdb_index.*libsection.*\r\nDownload.*debug info.*libsection.*" > + gdb_test "start $sectexec" $res "file [file tail $sectexec] start urls" > + } else { > + untested "lazy support urls" > + } > + > # Configure debuginfod with commands. > unsetenv DEBUGINFOD_URLS > clean_restart > diff --git a/gdb/testsuite/gdb.debuginfod/libsection.c b/gdb/testsuite/gdb.debuginfod/libsection.c > new file mode 100644 > index 00000000000..510f9205282 > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/libsection.c > @@ -0,0 +1,24 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020-2023 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 . */ > + > +#include > + > +void > +libsection_test () > +{ > + printf ("In libsection\n"); > +} > diff --git a/gdb/testsuite/gdb.debuginfod/section.c b/gdb/testsuite/gdb.debuginfod/section.c > new file mode 100644 > index 00000000000..3598896c1bc > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/section.c > @@ -0,0 +1,28 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020-2023 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 . */ > + > +#include > + > +extern void libsection_test (); > + > +int > +main() > +{ > + (void) open ("", 0); > + libsection_test (); > + return 0; > +} > -- > 2.39.2