From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KbuHEBZFqWX4uD8AWB0awg (envelope-from ) for ; Thu, 18 Jan 2024 10:34:46 -0500 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ONzficEA; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 2D7361E0C3; Thu, 18 Jan 2024 10:34:46 -0500 (EST) Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id F1DCF1E092 for ; Thu, 18 Jan 2024 10:34:43 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E56683858401 for ; Thu, 18 Jan 2024 15:34:42 +0000 (GMT) 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 4F5A23858C2F for ; Thu, 18 Jan 2024 15:34:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4F5A23858C2F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4F5A23858C2F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705592057; cv=none; b=MmVL6GbQwb2leUsWyd02NqxcmiR0MbLK+nin1jm781+4y8StEFYpbOsDRs/5fFYoZm/zT+UOcyvUnIzpiPOpoiJAprXoNUnQhB+bvMDTHb4v8fH5IV9cKqxM0VY4/bjfd+F2p6UitUWtwt5FirpDmwxSw1T++B+H/WZSHmi6e24= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705592057; c=relaxed/simple; bh=jNrEAE8GesVV6Tg6bqaFUltpQB5vnaJIQAnYdJ6IYG0=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=rwF83oEWHAxZ+be5kr9S06DiKRkwVYASc0EpsvhggRAwx2jah41YZMNWj1oBrDLYoxVCqlAe8mZilPnhJZYMDZukEMpaRVwUzgCNQcgoVzTpHQ0ulYYBlkcu7p6OZ7+1/fpHcOW8rc2UtJl8z+Ihcg3MRt9o9a8qACcnJ0hDPLc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705592051; 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=D72ykoW5QBaaKssWnVghaaI9ZxnN5hZIVMD9kYV+RVg=; b=ONzficEAm6+jxQz0p6s/iAwpQ0PzWDzASY6QRZFSHfhH6cx9cjJ6QMaecx0gdhYTPAXvOM bboGZwl4EpzEJ8zmSMuZQ1+7e7O6QaWDDyCIAmsmFoka8/5Y7Fs4gLjW0tW2rAFAqh8PtM /U07zysBNTm8o/RBQ/zVddfSi5KO/ZQ= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-584-Hn1Wp8NNOHKKAiDnrH5YqQ-1; Thu, 18 Jan 2024 10:34:09 -0500 X-MC-Unique: Hn1Wp8NNOHKKAiDnrH5YqQ-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-40e4f70af99so76653545e9.2 for ; Thu, 18 Jan 2024 07:34:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705592048; x=1706196848; 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=rcvpcD8SMpB2auLI6PtTYyUn4c6GqMpgJdyuZHHhKR4=; b=EHhUcESIyZ1dFh9CTbm2HGl015dHSxU/9jYxYoaDWEfp16K3/pgxs16pEnn/GTwrYp en330UzSffOn7tNWe/R22oIXmM+L8iy99mZNUdXXzOWJY6KMGLUApTLleKpyuc46D5Kt 2MmU3ayVz9z3wf9CfNJXe5yFPmx1q2n/iTPG3rWtcNF68V/DJiyKS06+6LYL0tx4GYFJ jaEfPrqmxZqFPNuq4CfLiO0FpGKch7xOjq7cabL5M/C8NLIb7mn9vqeV+iPwKZTrloAa KO/NZZVHB/lQ4koPXXShBEQ1v99wSxXpqEDQ+Cpd6ir/2PaZYMGrKuqkrlmzk0eb3fkK IqhA== X-Gm-Message-State: AOJu0YzlFF4O4Tk7Z70a9BENOdNRJpEdIb3ALEJ2cbBDRXa9ivU/ok0n oZ2LC348uSon505qVp7GhreTazG3L4jq/fpUfq0OBW7Clny9kxoRCXKP/Q2iUF4CzCrlA1CYi8Q z+idn/+Swdk3laaFe2z8mRHV5OYwTutS8sM6Zl3x/tMt88zUiVDmq//N++4c= X-Received: by 2002:a05:600c:143:b0:40e:52f1:7eea with SMTP id w3-20020a05600c014300b0040e52f17eeamr651083wmm.30.1705592047775; Thu, 18 Jan 2024 07:34:07 -0800 (PST) X-Google-Smtp-Source: AGHT+IEWq9+j0ilm7PWl43nvRfviG9lZQfHwfcs059i/S385rOYLDYtGBZe+X5WXAf2/SQIUAnp1kA== X-Received: by 2002:a05:600c:143:b0:40e:52f1:7eea with SMTP id w3-20020a05600c014300b0040e52f17eeamr651068wmm.30.1705592046712; Thu, 18 Jan 2024 07:34:06 -0800 (PST) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id s4-20020a05600c45c400b0040e6ff60057sm17465611wmo.48.2024.01.18.07.34.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jan 2024 07:34:06 -0800 (PST) From: Andrew Burgess To: Aaron Merey , gdb-patches@sourceware.org Cc: thiago.bauermann@linaro.org, Aaron Merey Subject: Re: [PATCH 3/4 v5] gdb/debuginfod: Support on-demand debuginfo downloading In-Reply-To: <20240117154855.2057850-4-amerey@redhat.com> References: <20240117154855.2057850-1-amerey@redhat.com> <20240117154855.2057850-4-amerey@redhat.com> Date: Thu, 18 Jan 2024 15:34:05 +0000 Message-ID: <87jzo664gi.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.8 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Aaron Merey writes: > v5 changes: Some tweaks needed rebase onto > objfile::find_and_add_separate_symbol_file changes in commit 661d98a. > v5 also implements suggestions from Thiago's review: > https://sourceware.org/pipermail/gdb-patches/2023-December/205506.html > > At the beginning of a session, gdb may attempt to download debuginfo > for all shared libraries associated with the process or core file > being debugged. This can be a waste of time and storage space when much > of the debuginfo ends up not being used during the session. > > To reduce the gdb's startup latency and to download only the debuginfo > that is really needed, this patch adds on-demand downloading of debuginfo. > > 'set debuginfo enabled on' now causes gdb to attempt to download a .gdb_index > for each shared library instead of its full debuginfo. Each corresponding > separate debuginfo will be deferred until gdb needs to expand symtabs > associated with the debuginfo's index. > > Because these indices are significantly smaller than their corresponding > debuginfo, this generally reduces the total amount of data gdb downloads. > Reductions of 80%-95% have been observed when debugging large GUI programs. > > (gdb) set debuginfod enabled on > (gdb) start > Downloading section .gdb_index for /lib64/libcurl.so.4 > [...] > 1826 client->server_mhandle = curl_multi_init (); > (gdb) step > Downloading separate debug info for /lib64/libcurl.so.4 > Downloading separate debug info for [libcurl dwz] > Downloading source file /usr/src/debug/curl-7.85.0-6.fc37.x86_64/build-full/lib/../../lib/multi.c > curl_multi_init () at ../../lib/multi.c:457 > 457 { > (gdb) > > Some of the key functions below include dwarf2_has_separate_index which > downloads the separate .gdb_index. If successful, the shared library > objfile owns the index until the separate debug objfile is downloaded > or confirmed to not be available. > > read_full_dwarf_from_debuginfod downloads the full debuginfo and > initializes the separate debug objfile. It is called by functions > such as dwarf2_gdb_index::expand_symtabs_matching and > dwarf2_base_index_functions::find_pc_sect_compunit_symtab when symtab > expansion is required. > --- > gdb/dwarf2/frame.c | 13 ++ > gdb/dwarf2/frame.h | 4 + > gdb/dwarf2/index-cache.c | 33 ++++ > gdb/dwarf2/index-cache.h | 13 ++ > gdb/dwarf2/public.h | 7 + > gdb/dwarf2/read-gdb-index.c | 125 +++++++++++++-- > gdb/dwarf2/read.c | 147 +++++++++++++++++- > gdb/dwarf2/read.h | 10 ++ > gdb/dwarf2/section.c | 3 +- > gdb/elfread.c | 3 +- > gdb/frame.c | 7 + > gdb/objfile-flags.h | 4 + > gdb/objfiles.h | 20 +++ > gdb/quick-symbol.h | 4 + > gdb/symfile-debug.c | 11 ++ > gdb/symfile.c | 13 +- > gdb/symtab.c | 18 ++- > gdb/testsuite/gdb.debuginfod/libsection1.c | 43 ++++++ > gdb/testsuite/gdb.debuginfod/libsection2.c | 37 +++++ > gdb/testsuite/gdb.debuginfod/section.c | 29 ++++ > gdb/testsuite/gdb.debuginfod/section.exp | 172 +++++++++++++++++++++ > gdb/testsuite/lib/debuginfod-support.exp | 27 +++- > gdb/testsuite/lib/gdb.exp | 8 +- > 23 files changed, 725 insertions(+), 26 deletions(-) > create mode 100644 gdb/testsuite/gdb.debuginfod/libsection1.c > create mode 100644 gdb/testsuite/gdb.debuginfod/libsection2.c > create mode 100644 gdb/testsuite/gdb.debuginfod/section.c > create mode 100644 gdb/testsuite/gdb.debuginfod/section.exp > > diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c > index d3d1ecdf1f5..9e290af79f7 100644 > --- a/gdb/dwarf2/frame.c > +++ b/gdb/dwarf2/frame.c > @@ -1620,6 +1620,19 @@ set_comp_unit (struct objfile *objfile, struct comp_unit *unit) > return dwarf2_frame_bfd_data.set (abfd, unit); > } > > +/* See frame.h. */ > + > +void > +dwarf2_clear_frame_data (struct objfile *objfile) > +{ > + bfd *abfd = objfile->obfd.get (); > + > + if (gdb_bfd_requires_relocations (abfd)) > + dwarf2_frame_objfile_data.clear (objfile); > + else > + dwarf2_frame_bfd_data.clear (abfd); > +} > + > /* Find the FDE for *PC. Return a pointer to the FDE, and store the > initial location associated with it into *PC. */ > > diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h > index 5643e557513..2391e313e7c 100644 > --- a/gdb/dwarf2/frame.h > +++ b/gdb/dwarf2/frame.h > @@ -238,6 +238,10 @@ void dwarf2_append_unwinders (struct gdbarch *gdbarch); > extern const struct frame_base * > dwarf2_frame_base_sniffer (frame_info_ptr this_frame); > > +/* Delete OBJFILEs comp_unit. */ > + > +extern void dwarf2_clear_frame_data (struct objfile * objfile); > + > /* Compute the DWARF CFA for a frame. */ > > CORE_ADDR dwarf2_frame_cfa (frame_info_ptr this_frame); > diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c > index 69f70642dc6..8c969ecd590 100644 > --- a/gdb/dwarf2/index-cache.c > +++ b/gdb/dwarf2/index-cache.c > @@ -240,6 +240,33 @@ index_cache::lookup_gdb_index (const bfd_build_id *build_id, > return {}; > } > > +/* See index-cache.h. */ > + > +gdb::array_view > +index_cache::lookup_gdb_index_debuginfod (const char *index_path, > + std::unique_ptr *resource) > +{ > + try > + { > + /* Try to map that file. */ > + index_cache_resource_mmap *mmap_resource > + = new index_cache_resource_mmap (index_path); > + > + /* Hand the resource to the caller. */ > + resource->reset (mmap_resource); > + > + return gdb::array_view > + ((const gdb_byte *) mmap_resource->mapping.get (), > + mmap_resource->mapping.size ()); > + } > + catch (const gdb_exception_error &except) > + { > + warning (_("Unable to read %s: %s"), index_path, except.what ()); > + } > + > + return {}; > +} > + > #else /* !HAVE_SYS_MMAN_H */ > > /* See dwarf-index-cache.h. This is a no-op on unsupported systems. */ > @@ -251,6 +278,12 @@ index_cache::lookup_gdb_index (const bfd_build_id *build_id, > return {}; > } > > +gdb::array_view > +index_cache::lookup_gdb_index_debuginfod (const char *index_path, > + std::unique_ptr *resource) > +{ > + return {}; > +} > #endif > > /* See dwarf-index-cache.h. */ > diff --git a/gdb/dwarf2/index-cache.h b/gdb/dwarf2/index-cache.h > index 023fc86fc89..0d845ebcadd 100644 > --- a/gdb/dwarf2/index-cache.h > +++ b/gdb/dwarf2/index-cache.h > @@ -90,6 +90,19 @@ class index_cache > lookup_gdb_index (const bfd_build_id *build_id, > std::unique_ptr *resource); > > + /* Look for an index file located at INDEX_PATH in the debuginfod cache. > + Unlike lookup_gdb_index, this function does not exit early if the > + index cache has not been enabled. > + > + If found, return the contents as an array_view and store the underlying > + resources (allocated memory, mapped file, etc) in RESOURCE. The returned > + array_view is valid as long as RESOURCE is not destroyed. > + > + If no matching index file is found, return an empty array view. */ > + gdb::array_view > + lookup_gdb_index_debuginfod (const char *index_path, > + std::unique_ptr *resource); > + > /* Return the number of cache hits. */ > unsigned int n_hits () const > { return m_n_hits; } > diff --git a/gdb/dwarf2/public.h b/gdb/dwarf2/public.h > index efb754b5fe8..4d3776465c3 100644 > --- a/gdb/dwarf2/public.h > +++ b/gdb/dwarf2/public.h > @@ -44,4 +44,11 @@ extern bool dwarf2_initialize_objfile > > extern void dwarf2_build_frame_info (struct objfile *); > > +/* Query debuginfod for the .gdb_index associated with OBJFILE. If > + successful, create an objfile to hold the .gdb_index information > + and act as a placeholder until the full debuginfo needs to be > + downloaded. */ > + > +extern bool dwarf2_has_separate_index (struct objfile *); > + > #endif /* DWARF2_PUBLIC_H */ > diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c > index 4ca44540296..55b04391eb5 100644 > --- a/gdb/dwarf2/read-gdb-index.c > +++ b/gdb/dwarf2/read-gdb-index.c > @@ -139,6 +139,8 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions > gdb.dwarf2/gdb-index.exp testcase. */ > void dump (struct objfile *objfile) override; > > + /* Calls do_expand_symtabs_matching and triggers debuginfo downloading > + if necessary. */ > bool expand_symtabs_matching > (struct objfile *objfile, > gdb::function_view file_matcher, > @@ -148,8 +150,60 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions > block_search_flags search_flags, > domain_enum domain, > enum search_domain kind) override; > + > + /* Expand symtabs matching a given symbol or file. */ > + bool do_expand_symtabs_matching > + (struct objfile *objfile, > + gdb::function_view file_matcher, > + const lookup_name_info *lookup_name, > + gdb::function_view symbol_matcher, > + gdb::function_view expansion_notify, > + block_search_flags search_flags, > + domain_enum domain, > + enum search_domain kind); > + > + /* Calls dwarf2_base_index_functions::expand_all_symtabs and downloads > + debuginfo if necessary. */ > + void expand_all_symtabs (struct objfile *objfile) override; > + > + /* Calls dwarf2_base_index_functions::find_last_source_symtab and downloads > + debuginfo if necessary. */ > + struct symtab *find_last_source_symtab (struct objfile *objfile) override; > }; > > +void > +dwarf2_gdb_index::expand_all_symtabs (struct objfile *objfile) > +{ > + try > + { > + dwarf2_base_index_functions::expand_all_symtabs (objfile); > + } > + catch (const gdb_exception &e) > + { > + if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > + exception_print (gdb_stderr, e); > + else > + read_full_dwarf_from_debuginfod (objfile, this); > + } > +} > + > +struct symtab * > +dwarf2_gdb_index::find_last_source_symtab (struct objfile *objfile) > +{ > + try > + { > + return dwarf2_base_index_functions::find_last_source_symtab (objfile); > + } > + catch (const gdb_exception &e) > + { > + if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > + exception_print (gdb_stderr, e); > + else > + read_full_dwarf_from_debuginfod (objfile, this); > + return nullptr; > + } > +} > + > /* This dumps minimal information about the index. > It is called via "mt print objfiles". > One use is to verify .gdb_index has been loaded by the > @@ -268,7 +322,7 @@ dw2_expand_marked_cus > } > > bool > -dwarf2_gdb_index::expand_symtabs_matching > +dwarf2_gdb_index::do_expand_symtabs_matching > (struct objfile *objfile, > gdb::function_view file_matcher, > const lookup_name_info *lookup_name, > @@ -317,6 +371,39 @@ dwarf2_gdb_index::expand_symtabs_matching > return result; > } > > +bool > +dwarf2_gdb_index::expand_symtabs_matching > + (struct objfile *objfile, > + gdb::function_view file_matcher, > + const lookup_name_info *lookup_name, > + gdb::function_view symbol_matcher, > + gdb::function_view expansion_notify, > + block_search_flags search_flags, > + domain_enum domain, > + enum search_domain kind) > +{ > + if (objfile->flags & OBJF_READNEVER) Should be: if ((objfile->flags & OBJF_READNEVER) != 0) > + return false; > + > + try > + { > + return do_expand_symtabs_matching (objfile, file_matcher, lookup_name, > + symbol_matcher, expansion_notify, > + search_flags, domain, kind); > + } > + catch (const gdb_exception &e) > + { > + if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > + { > + exception_print (gdb_stderr, e); > + return false; > + } > + > + read_full_dwarf_from_debuginfod (objfile, this); > + return true; > + } > +} > + > quick_symbol_functions_up > mapped_gdb_index::make_quick_functions () const > { > @@ -652,28 +739,32 @@ dwarf2_read_gdb_index > > /* If there is a .dwz file, read it so we can get its CU list as > well. */ > - dwz = dwarf2_get_dwz_file (per_bfd); > - if (dwz != NULL) > + if (get_gdb_index_contents_dwz != nullptr) > { > mapped_gdb_index dwz_map; > const gdb_byte *dwz_types_ignore; > offset_type dwz_types_elements_ignore; > + dwz = dwarf2_get_dwz_file (per_bfd); > > - gdb::array_view dwz_index_content > - = get_gdb_index_contents_dwz (objfile, dwz); > - > - if (dwz_index_content.empty ()) > - return 0; > - > - if (!read_gdb_index_from_buffer (bfd_get_filename (dwz->dwz_bfd.get ()), > - 1, dwz_index_content, &dwz_map, > - &dwz_list, &dwz_list_elements, > - &dwz_types_ignore, > - &dwz_types_elements_ignore)) > + if (dwz != nullptr) > { > - warning (_("could not read '.gdb_index' section from %s; skipping"), > - bfd_get_filename (dwz->dwz_bfd.get ())); > - return 0; > + gdb::array_view dwz_index_content > + = get_gdb_index_contents_dwz (objfile, dwz); > + > + if (dwz_index_content.empty ()) > + return 0; > + > + if (!read_gdb_index_from_buffer (bfd_get_filename > + (dwz->dwz_bfd.get ()), > + 1, dwz_index_content, &dwz_map, > + &dwz_list, &dwz_list_elements, > + &dwz_types_ignore, > + &dwz_types_elements_ignore)) > + { > + warning (_("could not read '.gdb_index' section from %s; skipping"), > + bfd_get_filename (dwz->dwz_bfd.get ())); > + return 0; > + } > } > } > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index 76dbeb8d536..8725a2fbcdb 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -35,6 +35,7 @@ > #include "dwarf2/attribute.h" > #include "dwarf2/comp-unit-head.h" > #include "dwarf2/cu.h" > +#include "dwarf2/frame.h" > #include "dwarf2/index-cache.h" > #include "dwarf2/index-common.h" > #include "dwarf2/leb.h" > @@ -96,6 +97,8 @@ > #include "split-name.h" > #include "gdbsupport/thread-pool.h" > #include "run-on-main-thread.h" > +#include "inferior.h" > +#include "debuginfod-support.h" > > /* When == 1, print basic high level tracing messages. > When > 1, be more verbose. > @@ -3003,7 +3006,7 @@ dwarf2_base_index_functions::find_per_cu (dwarf2_per_bfd *per_bfd, > } > > struct compunit_symtab * > -dwarf2_base_index_functions::find_pc_sect_compunit_symtab > +dwarf2_base_index_functions::do_find_pc_sect_compunit_symtab > (struct objfile *objfile, > struct bound_minimal_symbol msymbol, > CORE_ADDR pc, > @@ -3034,6 +3037,32 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab > return result; > } > > +struct compunit_symtab * > +dwarf2_base_index_functions::find_pc_sect_compunit_symtab > + (struct objfile *objfile, > + struct bound_minimal_symbol msymbol, > + CORE_ADDR pc, > + struct obj_section *section, > + int warn_if_readin) > +{ > + if (objfile->flags & OBJF_READNEVER) Should be: if ((objfile->flags & OBJF_READNEVER) != 0) > + return nullptr; > + > + try > + { > + return do_find_pc_sect_compunit_symtab (objfile, msymbol, pc, > + section, warn_if_readin); > + } > + catch (const gdb_exception &e) > + { > + if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > + exception_print (gdb_stderr, e); > + else > + read_full_dwarf_from_debuginfod (objfile, this); > + return nullptr; > + } > +} > + > void > dwarf2_base_index_functions::map_symbol_filenames > (struct objfile *objfile, > @@ -3190,6 +3219,29 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz) > return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res); > } > > +/* Query debuginfod for the .gdb_index matching OBJFILE's build-id. Return the > + contents if successful. */ > + > +static gdb::array_view > +get_gdb_index_contents_from_debuginfod (objfile *objfile, dwarf2_per_bfd *per_bfd) > +{ > + const bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ()); > + if (build_id == nullptr) > + return {}; > + > + gdb::unique_xmalloc_ptr index_path; > + scoped_fd fd = debuginfod_section_query (build_id->data, build_id->size, > + bfd_get_filename > + (objfile->obfd.get ()), > + ".gdb_index", > + &index_path); > + if (fd.get () < 0) > + return {}; > + > + return global_index_cache.lookup_gdb_index_debuginfod > + (index_path.get (), &per_bfd->index_cache_res); > +} > + > static quick_symbol_functions_up make_cooked_index_funcs > (dwarf2_per_objfile *); > > @@ -3200,7 +3252,8 @@ dwarf2_initialize_objfile (struct objfile *objfile, > const struct dwarf2_debug_sections *names, > bool can_copy) > { > - if (!dwarf2_has_info (objfile, names, can_copy)) > + if (!dwarf2_has_info (objfile, names, can_copy) > + && (objfile->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > return false; > > dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile); > @@ -3262,6 +3315,16 @@ dwarf2_initialize_objfile (struct objfile *objfile, > global_index_cache.hit (); > objfile->qf.push_front (per_bfd->index_table->make_quick_functions ()); > } > + /* Try to read just a separately downloaded gdb index. */ > + else if ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) Should be: (objfile->flags & OBJF_DOWNLOAD_DEFERRED) != 0 > + && dwarf2_read_gdb_index (per_objfile, > + get_gdb_index_contents_from_debuginfod, > + nullptr)) > + { > + dwarf_read_debug_printf ("found .gdb_index from debuginfod"); > + objfile->qf.push_front (per_bfd->index_table->make_quick_functions ()); > + objfile->qf.begin ()->get ()->from_separate_index = true; > + } > else > { > global_index_cache.miss (); > @@ -3270,6 +3333,86 @@ dwarf2_initialize_objfile (struct objfile *objfile, > return true; > } > > +/* See read.h. */ > + > +void > +read_full_dwarf_from_debuginfod (struct objfile *objfile, > + dwarf2_base_index_functions *fncs) > +{ > + gdb_assert (objfile->flags & OBJF_DOWNLOAD_DEFERRED); Should be: gdb_assert ((objfile->flags & OBJF_DOWNLOAD_DEFERRED) != 0); OK, I'm going to stop commenting on these now, but there's a few more of them in the rest of the patch. GDB style is not to rely on implicit int -> bool conversion. > + > + SCOPE_EXIT { objfile->remove_deferred_status (); }; > + > + const struct bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ()); > + const char *filename; > + gdb_bfd_ref_ptr debug_bfd; > + gdb::unique_xmalloc_ptr symfile_path; > + scoped_fd fd; Of these, filename, debug_bfd, and fd can be declared where they are first defined. You can probably move symfile_path closer to where it's used too. > + > + if (build_id == nullptr) > + return; > + > + filename = bfd_get_filename (objfile->obfd.get ()); > + fd = debuginfod_debuginfo_query (build_id->data, build_id->size, > + filename, &symfile_path); > + if (fd.get () < 0) > + return; > + > + /* Separate debuginfo successfully retrieved from server. */ > + debug_bfd = symfile_bfd_open (symfile_path.get ()); > + if (debug_bfd == nullptr > + || !build_id_verify (debug_bfd.get (), build_id->size, build_id->data)) > + { > + warning (_("File \"%s\" from debuginfod cannot be opened as bfd"), > + filename); > + return; > + } > + > + /* Clear frame data so it can be recalculated using DWARF. */ > + dwarf2_clear_frame_data (objfile); > + > + /* This may also trigger a dwz download. */ > + symbol_file_add_separate (debug_bfd, symfile_path.get (), > + current_inferior ()->symfile_flags, objfile); > +} > + > +/* See public.h. */ > + > +bool > +dwarf2_has_separate_index (struct objfile *objfile) > +{ > + if (objfile->flags & OBJF_DOWNLOAD_DEFERRED) > + return true; > + if (objfile->flags & OBJF_MAINLINE) > + return false; Flag checks missing '!= 0' again. > + if (!IS_DIR_SEPARATOR (*objfile_filename (objfile))) > + return false; > + > + const bfd_build_id *build_id = build_id_bfd_get (objfile->obfd.get ()); > + > + if (build_id == nullptr) > + return false; > + > + gdb::unique_xmalloc_ptr index_path; > + scoped_fd fd = debuginfod_section_query (build_id->data, > + build_id->size, > + bfd_get_filename > + (objfile->obfd.get ()), > + ".gdb_index", > + &index_path); > + > + if (fd.get () < 0) > + return false; > + > + /* We found a separate .gdb_index file so a separate debuginfo file > + should exist, but we don't want to download it until necessary. > + Attach the index to this objfile and defer the debuginfo download > + until gdb needs to expand symtabs referenced by the index. */ > + objfile->flags |= OBJF_DOWNLOAD_DEFERRED; > + dwarf2_initialize_objfile (objfile); > + return true; > +} > + > > > /* Find the base address of the compilation unit for range lists and > diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h > index d6631d45a54..8b6f1864458 100644 > --- a/gdb/dwarf2/read.h > +++ b/gdb/dwarf2/read.h > @@ -900,6 +900,10 @@ struct dwarf2_base_index_functions : public quick_symbol_functions > CORE_ADDR pc, struct obj_section *section, int warn_if_readin) > override; > > + struct compunit_symtab *do_find_pc_sect_compunit_symtab > + (struct objfile *objfile, struct bound_minimal_symbol msymbol, > + CORE_ADDR pc, struct obj_section *section, int warn_if_readin); > + > struct compunit_symtab *find_compunit_symtab_by_address > (struct objfile *objfile, CORE_ADDR address) override > { > @@ -972,4 +976,10 @@ extern void create_all_units (dwarf2_per_objfile *per_objfile); > > extern htab_up create_quick_file_names_table (unsigned int nr_initial_entries); > > +/* If OBJFILE contains information from a separately downloaded .gdb_index, > + attempt to download the full debuginfo. */ > + > +extern void read_full_dwarf_from_debuginfod (struct objfile *, > + dwarf2_base_index_functions *); > + > #endif /* DWARF2READ_H */ > diff --git a/gdb/dwarf2/section.c b/gdb/dwarf2/section.c > index 1235f293f45..b674103c72f 100644 > --- a/gdb/dwarf2/section.c > +++ b/gdb/dwarf2/section.c > @@ -54,7 +54,8 @@ dwarf2_section_info::get_bfd_owner () const > section = get_containing_section (); > gdb_assert (!section->is_virtual); > } > - gdb_assert (section->s.section != nullptr); > + if (section->s.section == nullptr) > + error (_("Can't find owner of DWARF section.")); This seems like an interesting change. There's no test that triggers this case, but I guess you ran into this at some point. Could you explain more about what's going on here? > return section->s.section->owner; > } > > diff --git a/gdb/elfread.c b/gdb/elfread.c > index c5ba8837217..764e991dfc2 100644 > --- a/gdb/elfread.c > +++ b/gdb/elfread.c > @@ -1219,7 +1219,8 @@ elf_symfile_read_dwarf2 (struct objfile *objfile, > && objfile->separate_debug_objfile_backlink == NULL) > { > if (objfile->find_and_add_separate_symbol_file (symfile_flags)) > - gdb_assert (objfile->separate_debug_objfile != nullptr); > + gdb_assert (objfile->separate_debug_objfile != nullptr > + || objfile->flags & OBJF_DOWNLOAD_DEFERRED); > else > has_dwarf2 = false; > } > diff --git a/gdb/frame.c b/gdb/frame.c > index 41003cc7771..d3409c8fd97 100644 > --- a/gdb/frame.c > +++ b/gdb/frame.c > @@ -1889,6 +1889,13 @@ get_selected_frame (const char *message) > error (("%s"), message); > > lookup_selected_frame (selected_frame_id, selected_frame_level); > + > + /* It is possible for lookup_selected_frame to cause a new objfile > + to be loaded. Some objfile observers may choose to clear > + selected_frame when an objfile is loaded. Work around this by > + calling lookup_selected_frame again if the first try failed. */ > + if (selected_frame == nullptr) > + lookup_selected_frame (selected_frame_id, selected_frame_level); I notice this case wasn't tested -- I added an abort here and reran the test, and didn't see it hit. I ask because this seems both interesting, and this doesn't feel like the right solution... As a minimum you'd need a comment on lookup_selected_frame that says "hey, you will need to call this function twice to guarantee that it did what it promised". So then I'd suggest we should just rename lookup_selected_frame to lookup_selected_frame_1, and have lookup_selected_frame be a wrapper which does the double lookup. But I was kind of hoping that we'd have a test that hit this case so I could look at the stack trace and better understand what exactly is happening when the selected frame is cleared. > } > /* There is always a frame. */ > gdb_assert (selected_frame != NULL); > diff --git a/gdb/objfile-flags.h b/gdb/objfile-flags.h > index 74aea1a88d3..3f922d9e84f 100644 > --- a/gdb/objfile-flags.h > +++ b/gdb/objfile-flags.h > @@ -56,6 +56,10 @@ enum objfile_flag : unsigned > /* User requested that we do not read this objfile's symbolic > information. */ > OBJF_READNEVER = 1 << 6, > + > + /* A separate .gdb_index has been downloaded for this objfile. > + Debuginfo for this objfile can be downloaded when required. */ > + OBJF_DOWNLOAD_DEFERRED = 1 << 7, > }; > > DEF_ENUM_FLAGS_TYPE (enum objfile_flag, objfile_flags); > diff --git a/gdb/objfiles.h b/gdb/objfiles.h > index 34b3dac0b26..69222092b61 100644 > --- a/gdb/objfiles.h > +++ b/gdb/objfiles.h > @@ -622,6 +622,26 @@ struct objfile > domain_enum domain, > bool *symbol_found_p); > > + /* Indicate that the acquisition of this objfile's separate debug objfile > + is no longer deferred. Used when the debug objfile has been acquired > + or could not be found. */ > + void remove_deferred_status () > + { > + flags &= ~OBJF_DOWNLOAD_DEFERRED; > + > + /* Remove quick_symbol_functions derived from a separately downloaded > + index. If available the separate debug objfile's index will be used > + instead, since that objfile actually contains the symbols and CUs > + referenced in the index. > + > + No more than one element of qf should have from_separate_index set > + to true. */ > + qf.remove_if ([&] (const quick_symbol_functions_up &qf_up) > + { > + return qf_up->from_separate_index; > + }); > + } > + > /* Return the relocation offset applied to SECTION. */ > CORE_ADDR section_offset (bfd_section *section) const > { > diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h > index 4cd234162ae..01a228b446c 100644 > --- a/gdb/quick-symbol.h > +++ b/gdb/quick-symbol.h > @@ -193,6 +193,10 @@ struct quick_symbol_functions > virtual void compute_main_name (struct objfile *objfile) > { > } > + > + /* True if this quick_symbol_functions is derived from a separately > + downloaded index. */ > + bool from_separate_index = false; > }; > > typedef std::unique_ptr quick_symbol_functions_up; > diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c > index c684f9cdd43..3ed31661300 100644 > --- a/gdb/symfile-debug.c > +++ b/gdb/symfile-debug.c > @@ -37,6 +37,7 @@ > #include "cli/cli-style.h" > #include "build-id.h" > #include "debuginfod-support.h" > +#include "dwarf2/public.h" > > /* We need to save a pointer to the real symbol functions. > Plus, the debug versions are malloc'd because we have to NULL out the > @@ -617,6 +618,16 @@ objfile::find_and_add_separate_symbol_file (symfile_add_flags symfile_flags) > = simple_find_and_open_separate_symbol_file > (this, find_separate_debug_file_by_debuglink, &warnings); > > + /* Attempt to download only an index from the separate debug info. > + As with debuginfod_find_and_open_separate_symbol_file, only attempt > + this once. */ > + if (debug_bfd == nullptr && attempt == 0 > + && dwarf2_has_separate_index (this)) > + { > + has_dwarf2 = true; > + break; > + } > + > /* Only try debuginfod on the first attempt. Sure, we could imagine > an extension that somehow adds the required debug info to the > debuginfod server but, at least for now, we don't support this > diff --git a/gdb/symfile.c b/gdb/symfile.c > index bdcc27e7ee5..ec4b4da12ed 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -989,6 +989,10 @@ syms_from_objfile (struct objfile *objfile, > static void > finish_new_objfile (struct objfile *objfile, symfile_add_flags add_flags) > { > + struct objfile *parent = objfile->separate_debug_objfile_backlink; > + bool was_deferred > + = (parent != nullptr) && (parent->flags & OBJF_DOWNLOAD_DEFERRED); > + > /* If this is the main symbol file we have to clean up all users of the > old main symbol file. Otherwise it is sufficient to fixup all the > breakpoints that may have been redefined by this symbol file. */ > @@ -999,7 +1003,8 @@ finish_new_objfile (struct objfile *objfile, symfile_add_flags add_flags) > > clear_symtab_users (add_flags); > } > - else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0) > + else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0 > + && !was_deferred) > { > breakpoint_re_set (); > } > @@ -1120,6 +1125,12 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name, > if (objfile->sf != nullptr) > finish_new_objfile (objfile, add_flags); > > + /* Remove deferred status now in case any observers trigger symtab > + expansion. Otherwise gdb might try to read parent for psymbols > + when it should read the separate debug objfile instead. */ > + if (parent != nullptr && (parent->flags & OBJF_DOWNLOAD_DEFERRED)) > + parent->remove_deferred_status (); > + > gdb::observers::new_objfile.notify (objfile); > > return objfile; > diff --git a/gdb/symtab.c b/gdb/symtab.c > index cc0bdb80c7c..ac9513608d7 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -2926,14 +2926,30 @@ find_pc_sect_compunit_symtab (CORE_ADDR pc, struct obj_section *section) > if (best_cust != NULL) > return best_cust; > > + int warn_if_readin = 1; > + > /* Not found in symtabs, search the "quick" symtabs (e.g. psymtabs). */ > > for (objfile *objf : current_program_space->objfiles ()) > { > + bool was_deferred = objf->flags & OBJF_DOWNLOAD_DEFERRED; > + > struct compunit_symtab *result > - = objf->find_pc_sect_compunit_symtab (msymbol, pc, section, 1); > + = objf->find_pc_sect_compunit_symtab (msymbol, pc, section, > + warn_if_readin); > + > if (result != NULL) > return result; > + > + /* If OBJF's separate debug info was just acquired, disable > + warn_if_readin for the next iteration of this loop. This prevents > + a spurious warning in case an observer already triggered expansion > + of the separate debug objfile's symtabs. */ > + if (was_deferred && objf->separate_debug_objfile != nullptr > + && (objf->flags & OBJF_DOWNLOAD_DEFERRED) == 0) > + warn_if_readin = 0; > + else if (warn_if_readin == 0) > + warn_if_readin = 1; > } > > return NULL; > diff --git a/gdb/testsuite/gdb.debuginfod/libsection1.c b/gdb/testsuite/gdb.debuginfod/libsection1.c > new file mode 100644 > index 00000000000..7ad19c276c4 > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/libsection1.c > @@ -0,0 +1,43 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2023 Free Software Foundation, Inc. Remember to update your copyright dates to '2023-2024' throughout. > + > + 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 > +#include > +#include > + > +extern void libsection2_test (); > +extern void *libsection2_thread_test (void *); > + > +static volatile int flag = 0; > + > +void > +libsection1_test () > +{ > + pthread_t thr; > + > + printf ("In libsection1\n"); > + libsection2_test (); > + > + pthread_create (&thr, NULL, libsection2_thread_test, (void *) &flag); > + > + /* Give the new thread a chance to actually enter libsection2_thread_test. */ > + while (!flag) > + ; > + > + printf ("Cancelling thread\n"); > + pthread_cancel (thr); > +} > diff --git a/gdb/testsuite/gdb.debuginfod/libsection2.c b/gdb/testsuite/gdb.debuginfod/libsection2.c > new file mode 100644 > index 00000000000..20edd68166b > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/libsection2.c > @@ -0,0 +1,37 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 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 > +#include > + > +void > +libsection2_test () > +{ > + printf ("In libsection2\n"); > +} > + > +void * > +libsection2_thread_test (void *arg) > +{ > + int *flag = (int *) arg; > + printf ("In thread test\n"); > + > + while (1) > + *flag = 1; > + > + return NULL; > +} > diff --git a/gdb/testsuite/gdb.debuginfod/section.c b/gdb/testsuite/gdb.debuginfod/section.c > new file mode 100644 > index 00000000000..33e4faf2cac > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/section.c > @@ -0,0 +1,29 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 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 libsection1_test (); > + > +int > +main () > +{ > + libsection1_test (); > + printf ("in section exec\n"); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.debuginfod/section.exp b/gdb/testsuite/gdb.debuginfod/section.exp > new file mode 100644 > index 00000000000..c3872ecdfc9 > --- /dev/null > +++ b/gdb/testsuite/gdb.debuginfod/section.exp > @@ -0,0 +1,172 @@ > +# Copyright 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 . > + > +# Test debuginfod functionality > + > +standard_testfile > + > +load_lib debuginfod-support.exp > + > +require allow_debuginfod_tests > + > +set sourcetmp [standard_output_file tmp-${srcfile}] > +set outputdir [standard_output_file {}] Not sure what these two are for. Maybe a copy&paste issue? > + > +# SECTEXEC is an executable which calls a function from LIB_SL1. > +set sectfile "section" > +set sectsrc $srcdir/$subdir/section.c > +set sectexec [standard_output_file $sectfile] I'd have expected you to use ${srcfile} and ${binfile} here which are setup by `standard_testfile` based on the name of the test script. So section.exp will setup `section.c` and `/path/to/.../section` respectively. The source file doesn't have the directory prefix, but the compile procs know how to find the source file, so that's OK. standard_testfile also gives you `testfile` which is $binfile without the path prefix, if that's needed. It's good practice to use $binfile throughout the test unless there's some compelling reason not too everyone knows what $binfile is. > + > +# Solib LIB_SL1 calls functions from LIB_SL2. > +set libfile1 "libsection1" > +set libsrc1 $srcdir/$subdir/$libfile1.c > +set lib_sl1 [standard_output_file $libfile1.sl] > + > +set libfile2 "libsection2" > +set libsrc2 $srcdir/$subdir/$libfile2.c > +set lib_sl2 [standard_output_file $libfile2.sl] > + > +set lib_opts1 [list debug build-id shlib=$lib_sl2] > +set lib_opts2 [list debug build-id] > +set exec_opts [list debug build-id shlib=$lib_sl1 shlib=$lib_sl2] > + > +clean_restart > + > +if {[enable_section_downloads] == 0} { > + untested "GDB does not support debuginfod section downloads" > + return -1 > +} > + > +# Compile SECTEXEC, LIB_SL1 and LIB_SL2. > +if { [gdb_compile_shlib $libsrc2 $lib_sl2 $lib_opts2] != "" } { > + untested "failed to compile $libfile2" > + return -1 > +} > + > +if { [gdb_compile_shlib_pthreads $libsrc1 $lib_sl1 $lib_opts1] != "" } { > + untested "failed to compile $libfile1" > + return -1 > +} > + > +if { [gdb_compile $sectsrc $sectexec executable $exec_opts] != "" } { > + untested "failed to compile $sectfile" > + return -1 > +} Instead of calling all these compile worker functions directly, the better approach is to call build_executable and pass through the correct flags. This seems to work for me: if { [build_executable "build $libfile2" $lib_sl2 $libsrc2 \ {debug build-id shlib}] != 0 } { return -1 } if { [build_executable "build $libfile1" $lib_sl1 $libsrc1 \ [list debug build-id shlib_pthreads shlib=$lib_sl2]] != 0 } { return -1 } if { [build_executable "build executable" $sectexec $sectsrc \ [list debug build-id shlib=$lib_sl1 shlib=$lib_sl2]] != 0 } { return -1 } I've inlined the flag lists here. If you'd rather define them separately still, that's fine. Notice also that I removed the calls to 'untested', these are not needed, the testsuite will call 'untested' for you if the build fails. > + > +# Add .gdb_index to solibs. > +if { [ensure_gdb_index $lib_sl1 "" "libsection1"] != 1 } { > + untested "failed to add .gdb_index to $libfile1" > + return -1 > +} > + > +if { [ensure_gdb_index $lib_sl2 "" "libsection2"] != 1 } { > + untested "failed to add .gdb_index to $libfile2" > + return -1 > +} > + > +# Strip solib debuginfo into separate files. > +if { [gdb_gnu_strip_debug $lib_sl1 ""] != 0} { > + fail "strip $lib_sl1 debuginfo" > + return -1 > +} > + > +if { [gdb_gnu_strip_debug $lib_sl2 ""] != 0} { > + fail "strip $lib_sl2 debuginfo" > + return -1 > +} > + > +# Move debuginfo files into directory that debuginfod will serve from. > +set debugdir [standard_output_file "debug"] > +set debuginfo_sl1 [standard_output_file $libfile1.sl.debug] > +set debuginfo_sl2 [standard_output_file $libfile2.sl.debug] > + > +file mkdir $debugdir > +file rename -force $debuginfo_sl1 $debugdir > +file rename -force $debuginfo_sl2 $debugdir > + > +# Restart GDB and clear the debuginfod client cache. Then load BINFILE into > +# GDB and start running it. Match output with pattern RES and use TESTNAME > +# as the test name. > +proc_with_prefix clean_restart_with_prompt { binfile testname } { > + global cache > + > + # Delete client cache so debuginfo downloads again. > + file delete -force $cache > + clean_restart > + > + gdb_test "set debuginfod enabled on" "" "clean_restart enable $testname" You can use gdb_test_no_output here rather than passing an empty pattern. > + gdb_load $binfile > + > + runto_main > +} > + > +# Tests with no debuginfod server running. > +proc_with_prefix no_url { } { > + global sectexec libfile1 libfile2 > + > + gdb_load $sectexec > + if {![runto_main]} { > + return > + } > + > + # Check that no section is downloaded and no debuginfo is found. > + gdb_test "info sharedlibrary" ".*Yes \\(\\*\\).*$libfile1.*" \ > + "found no url lib1" > + gdb_test "info sharedlibrary" ".*Yes \\(\\*\\).*$libfile2.*" \ > + "found no url lib2" > +} > + > +# Tests with a debuginfod server running. > +proc_with_prefix local_url { } { > + global sectexec > + global libsrc1 lib_sl1 libfile1 > + global libsrc2 lib_sl2 libfile2 > + global debugdir db > + > + set url [start_debuginfod $db $debugdir] > + if { $url == "" } { > + unresolved "failed to start debuginfod server" > + return > + } > + > + # Point GDB to the server. > + setenv DEBUGINFOD_URLS $url > + > + clean_restart_with_prompt $sectexec "index" > + > + # Download debuginfo when stepping into a function. > + set res ".*separate debug info for $lib_sl1.*\"In ${libfile1}\\\\n\".*" > + gdb_test "step" $res "step" > + > + clean_restart_with_prompt $sectexec "break" > + > + # Download debuginfo when setting a breakpoint. > + set res "Download.*separate debug info for $lib_sl2.*" > + gdb_test "br libsection2_test" $res "break set" > + > + # Hit the breakpoint. > + set res ".*Breakpoint 2, libsection2_test.*\"In ${libfile2}\\\\n\".*" > + gdb_test "c" $res "break continue" > +} > + > +# Create CACHE and DB directories ready for debuginfod to use. > +prepare_for_debuginfod cache db > + > +with_debuginfod_env $cache { > + no_url > + local_url > +} > + > +stop_debuginfod > diff --git a/gdb/testsuite/lib/debuginfod-support.exp b/gdb/testsuite/lib/debuginfod-support.exp > index 50a8b512a4a..e0b3dc39f51 100644 > --- a/gdb/testsuite/lib/debuginfod-support.exp > +++ b/gdb/testsuite/lib/debuginfod-support.exp > @@ -113,6 +113,8 @@ proc with_debuginfod_env { cache body } { > proc start_debuginfod { db debugdir } { > global debuginfod_spawn_id spawn_id > > + set logfile [standard_output_file "server_log"] > + > # Find an unused port. > set port 7999 > set found false > @@ -127,7 +129,8 @@ proc start_debuginfod { db debugdir } { > set old_spawn_id $spawn_id > } > > - spawn debuginfod -vvvv -d $db -p $port -F $debugdir > + spawn sh -c "debuginfod -vvvv -d $db -p $port -F $debugdir 2>&1 \ > + | tee $logfile" > set debuginfod_spawn_id $spawn_id > > if { [info exists old_spawn_id] } { > @@ -194,3 +197,25 @@ proc stop_debuginfod { } { > unset debuginfod_spawn_id > } > } > + > +# Return 1 if gdb is configured to download ELF/DWARF sections from > +# debuginfod servers. Otherwise return 0. We can use 'true' and 'false' in TCL code, see allow_debuginfod_tests as an example. > +proc enable_section_downloads { } { I think this should be renamed to something like allow_section_downloads (though I about allow_debuginfod_section_downloads to make it clear it's a debuginfod function). Then, in the test script you would say: require allow_debuginfod_section_downloads Rather than the `if` check that you have right now. > + global gdb_prompt This line isn't needed, gdb_prompt isn't used. Thanks, Andrew > + > + set cmd "maint set debuginfod download-sections on" > + set msg "enable section downloads" > + > + gdb_test_multiple $cmd $msg { > + -re -wrap ".*not compiled into GDB.*" { > + return 0 > + } > + -re -wrap "^" { > + return 1 > + } > + -re -wrap "" { > + fail "$gdb_test_name (unexpected output)" > + return 0 > + } > + } > +} > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 9ac707be696..9b3c7e6a02c 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -9414,10 +9414,14 @@ proc get_index_type { objfile { testname "" } } { > # STYLE controls which style of index to add, if needed. The empty > # string (the default) means .gdb_index; "-dwarf-5" means .debug_names. > > -proc ensure_gdb_index { binfile {style ""} } { > +proc ensure_gdb_index { binfile {style ""} {testname_prefix ""} } { > set testfile [file tail $binfile] > - > set test "check if index present" > + > + if { $testname_prefix != "" } { > + set test "$testname_prefix $test" > + } > + > set index_type [get_index_type $testfile $test] > > if { $index_type eq "gdb" || $index_type eq "dwarf5" } { > -- > 2.43.0