From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id FppjAOv7A2Y+HxgAWB0awg (envelope-from ) for ; Wed, 27 Mar 2024 06:58:51 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=I/6cfvUB; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id D5B341E0C0; Wed, 27 Mar 2024 06:58:50 -0400 (EDT) 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 AE4781E030 for ; Wed, 27 Mar 2024 06:58:48 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 14476386480F for ; Wed, 27 Mar 2024 10:58:48 +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 0866C3864825 for ; Wed, 27 Mar 2024 10:58:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0866C3864825 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 0866C3864825 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=1711537106; cv=none; b=xRIvCyJxLjhVuSviFHPf3niwPjUutGVduS9D6Uz0tZyqBzx+I1ui4DRLkYEFSpmtmHH661AtMDblNLsbYif7IRBfCDCn1Yt5Wvnupuz7G/1z7LxoSIEGxvPGsJbTyjbBZMgMj3ikRc005zXFGUxMIaHMH39ULtWwtpsA6rNQjE4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711537106; c=relaxed/simple; bh=ac5u8QMzflYU8YWIBfsVM8jIM24ivL33l+Bo70iALVo=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=PwOzykQFNMz7Vu/RedEgJ5UuwhksmKiVm8VWoOsaXSbpQ1iX5f7xD9l1Umy2FJatyUowaFgP/l34/PFdHolyyDzljTkbUnEW4WIgwGpjSIiTtStACymHDAcKGxMCJkjrCei7mmOI/pImSBjp5Dgsb7fZHQLz4WVmX9Icc42X0Bk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711537103; 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=LMM6OhiliLWRcPdqee77dQB6JxzjmFhE4yArrIahPHc=; b=I/6cfvUBBGHYC/hs8K4u5Wq73FvPgXI1RiOlBOBLb0oUTM9BzcckTVwn10Xt1Vyq3FdP3g RJwo/TCobYTdeQNdiG0VygvdIVF0JDR8ZrUPfPc3HgTBnoEZtANKTK2LxZLJnqwbWPdpAf Nfew7sM7WYdeLtoKpLN4OFkiyuxP6sY= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-680-5nFEBq7NNziggZfYEiptQQ-1; Wed, 27 Mar 2024 06:58:22 -0400 X-MC-Unique: 5nFEBq7NNziggZfYEiptQQ-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-a466c7b0587so468768566b.0 for ; Wed, 27 Mar 2024 03:58:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711537101; x=1712141901; 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=LMM6OhiliLWRcPdqee77dQB6JxzjmFhE4yArrIahPHc=; b=B6g3YjusRoqER4qpfBdl1RBBMcEUHcDv03j/CRY/z8s2mXbVh/KP6tR+/I1Qi1vvq5 BpHcoyvZoAuYOmqXQezwxt7KkFNhH19pGM0nSmmZLkUd11eMV2rjctY+bl/bFFRenIVI GhDpKZL3G9mAFQuOddNPeOjHAETQB3Q1CM2fG932zz1kSLfmw0RKBTMxMDAwvoiO2AlF L9UXBBWlTplT2Ua7e2aDaY58VL+EHD/ISvbWATtOtWX9isvKmYa8xsQDZ0M0FVZSEcgs KJeimXRXeoCuWYkhR9YpPg3kAYFgINDcDjnCkD1wSt8XqMdzoNKagWEJKGJoKbdF5AN8 u1Tg== X-Gm-Message-State: AOJu0YyzXZ6iHkC2aw8fdz5jOPxD4Uqgt3AuQY+107Akzi4L7YK/1im6 gbBJ5KUyYy+KgYKy4F4ayfPzui/tAI24qvnP4GZsrzojuPB36toN8fENAa2IWDRP+4kwjRPe8W2 erPXVnb6bTbjaAJtNjrjs8ZKXjZAsOC+zA0zH0iVXDjVEjPQKek1oN97/gHY= X-Received: by 2002:a17:906:1c06:b0:a4d:b0e9:efb8 with SMTP id k6-20020a1709061c0600b00a4db0e9efb8mr2656658ejg.24.1711537100836; Wed, 27 Mar 2024 03:58:20 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGY+RMuKAqrXgy8hJhbq+zRnDYnW5yZB52qtY5HLPJIxGW8jlgg38/b086E5DGhsSa62ak2pg== X-Received: by 2002:a17:906:1c06:b0:a4d:b0e9:efb8 with SMTP id k6-20020a1709061c0600b00a4db0e9efb8mr2656642ejg.24.1711537100239; Wed, 27 Mar 2024 03:58:20 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id bo11-20020a170906d04b00b00a46d049ff63sm5213287ejb.21.2024.03.27.03.58.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Mar 2024 03:58:19 -0700 (PDT) From: Andrew Burgess To: Aaron Merey Cc: gdb-patches@sourceware.org, thiago.bauermann@linaro.org, Aaron Merey Subject: Re: [PATCH 3/4 v5] gdb/debuginfod: Support on-demand debuginfo downloading In-Reply-To: <20240119051213.2502965-1-amerey@redhat.com> References: <87jzo664gi.fsf@redhat.com> <20240119051213.2502965-1-amerey@redhat.com> Date: Wed, 27 Mar 2024 10:58:19 +0000 Message-ID: <87plvg3pro.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=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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.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: > 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 {}; > +} I wonder if the core of this function should be merged with lookup_gdb_index? The core functionality should be identical, but there's already a little divergence. I'd be tempted to just rename lookup_gdb_index_debuginfod to lookup_gdb_index and have it be an overload, one that finds the index in a named file, and the other that takes a build-id, creates a filename, and then calls your new version. > 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 *); Unless I'm missing something then this comment is not correct. This function doesn't create an objfile to held the .gdb_index does it? > + > #endif /* DWARF2_PUBLIC_H */ > diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c > index 4ca44540296..f4d77ed2d04 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); This is the part that I don't really understand about this patch, why is the download and read of the full dwarf done in the exception path? How do we know that the exception that was thrown indicates we should go fetch the dwarf and not that some other error has occurred? I couldn't find any obvious path from which read_full_dwarf_from_debuginfod might throw an exception, but if it does then I guess we're not going to get the behaviour we expect. I guess my expectation, going into this patch, was that you would have generalised the index checking code in some way so that when we check the index and find a match, instead of reading the full DWARF, we'd download the info, and then read the full DWARF. I'm not saying that what you have is wrong, or that it needs to change, just that I found this approach surprising, and I don't really understand the design -- but I'm open to being convinced. Thanks, Andrew