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 20E2C3857734 for ; Wed, 24 May 2023 09:31:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 20E2C3857734 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=1684920713; 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=qBMliHnzy+kQAlC7Be7rKqAzbZCODNejIvMQvyfbMic=; b=EqPTET1PbjyqOZujlNm3t9nreC9ZBon5uW7/Qn/OHAwGQJRjf8mtoKKL3aZP7IfJNKurRD Z9p/1UQQXO7IvlU4InFAbsHcLI9BQZP+ro/vwPbYo/fxQZMfH51DePaMLzmKvOFlDhAhel V2FPfAmHxXOYAnJ6FdzOdY0bEefEbQ8= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-121-G0DDj7O4Mhm02z45uc2wSw-1; Wed, 24 May 2023 05:31:52 -0400 X-MC-Unique: G0DDj7O4Mhm02z45uc2wSw-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-30a938fcfb5so348934f8f.0 for ; Wed, 24 May 2023 02:31:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684920711; x=1687512711; 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=qBMliHnzy+kQAlC7Be7rKqAzbZCODNejIvMQvyfbMic=; b=gpNPYTL0o6A0bU1ZWp5uicWa/SdszbbAp0OeQ3SAq8kJd6C92QuAzvqxBghZBFzU6O GrUKPlaWZOna+M8Ar2e+wLcYiXRhpsfF6WjpPj1oMHo34KZY1xhi9qpq4bea7afDZY7q XpmjI9nwcsvpR2IqFT2CSpcVfQZG19TvxpYJFd75ESqs4pMv9hJ/Vx7qc6pAvSLAYaxF 4YqeXGZuK4peE9hTBJuMBQwW7ATc9nFK8UMaOLVtumT97FIghodek7SrnaGO+rs7niZZ nAxAfFqicMn63LBw3Zx2KhMgq5yuHEfSGZNEP80bMNjvViYT+ZTTiOYSK2j1mSo/uNlH Zkgg== X-Gm-Message-State: AC+VfDz2D7ULo64zs3GBaZ5OTVdNhbld5QNtZ5FUs/FyjOa2nGol2nHx RQcX+VepkmhM7z+uvZgyV8rfNj7xO95heEmHU4La9mW2BRLi9fhba4Ocb4xRwOhfsKHaEuWcssg oPVxk/LGUqbcwkaAo5iFc3lpwqBhBy/pOhFx/n8RODmkxBXYaodtCU4swazQ6APgdBNnVLf18Zw Euh+pApQ== X-Received: by 2002:adf:dcc2:0:b0:309:38f4:fb52 with SMTP id x2-20020adfdcc2000000b0030938f4fb52mr12426265wrm.9.1684920710750; Wed, 24 May 2023 02:31:50 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5b+eaBSc7StdckbWSq8sutSJ/3fbnjQM7DJMjnEgFl+kAqFaG+cyV64FbJ8cO66/L3GqZzTA== X-Received: by 2002:adf:dcc2:0:b0:309:38f4:fb52 with SMTP id x2-20020adfdcc2000000b0030938f4fb52mr12426236wrm.9.1684920710309; Wed, 24 May 2023 02:31:50 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id i18-20020adffc12000000b0030631a599a0sm13776296wrr.24.2023.05.24.02.31.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 May 2023 02:31:49 -0700 (PDT) From: Andrew Burgess To: Aaron Merey via Gdb-patches , gdb-patches@sourceware.org Cc: Aaron Merey Subject: Re: [PATCH 2/7] gdb: add 'lazy' setting for command 'set debuginfod enabled' In-Reply-To: <20230227194212.348003-2-amerey@redhat.com> References: <20230227194212.348003-1-amerey@redhat.com> <20230227194212.348003-2-amerey@redhat.com> Date: Wed, 24 May 2023 10:31:48 +0100 Message-ID: <871qj6848r.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,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: > 'set debuginfod enabled lazy' turns on debuginfod downloading like > 'set debuginfod enabled on' but also enables ELF/DWARFs section > downloading via debuginfod_section_query. If I understand correctly, 'lazy' can still download the full files (when needed), but might first download some of the sections (e.g. the index), which might allow us to skip downloading the full files? I'm not sure that adding the 'lazy' setting as another option is the correct way to go. I'll explain more below... > > If support for debuginfod section queries was not found at configure > time, 'set debuginfod enabled lazy' will print an error message > indicating the missing support and default to 'set debuginfod enabled on'. > > Also update the help text and gdb.texinfo section for 'set debuginfod enabled' > with information on the lazy setting. > --- > gdb/debuginfod-support.c | 20 +++++++++++++++++--- > gdb/doc/gdb.texinfo | 9 +++++++-- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c > index f909742362f..12025fcf0c0 100644 > --- a/gdb/debuginfod-support.c > +++ b/gdb/debuginfod-support.c > @@ -34,12 +34,14 @@ static cmd_list_element *show_debuginfod_prefix_list; > static const char debuginfod_on[] = "on"; > static const char debuginfod_off[] = "off"; > static const char debuginfod_ask[] = "ask"; > +static const char debuginfod_lazy[] = "lazy"; > > static const char *debuginfod_enabled_enum[] = > { > debuginfod_on, > debuginfod_off, > debuginfod_ask, > + debuginfod_lazy, > nullptr > }; > > @@ -411,7 +413,7 @@ debuginfod_section_query (const unsigned char *build_id, > return scoped_fd (-ENOSYS); > #else > > - if (!debuginfod_is_enabled ()) > + if (debuginfod_enabled != debuginfod_lazy || !debuginfod_is_enabled ()) > return scoped_fd (-ENOSYS); Because you check for debuginfod_lazy first, and due to the short-circuit evaluation, then if debuginfod_enabled is set to 'on', 'off', or 'ask' we will always return -ENOSYS. As the default is 'ask' this feature is off by default, which I think is a shame. Even if we could call debuginfod_is_enabled here, and it asked the user, it would just set the value to 'on', which apparently still isn't good enough. I would suggest that this option (debuginfod_enabled) should remain as a yes/no/ask choice. If the user selects 'yes', and section downloading is supported by the current version of debuginfod then we should just do that. I don't think this level of control is something that users will normally care about -- they'll either be OK with downloading stuff (and select 'on'), or they are against downloading stuff (and select 'off'). I doubt there are many users who say ... I'm OK downloading full debug files, but not the individual sections. You could possibly add a new setting to individually control downloading the sections, though I'm so convinced that this isn't something a user will care about that I would suggest this should be a maintenance setting: maintenance set debuginfod download-sections on maintenance set debuginfod download-sections off maintenance show debuginfod download-sections We can always move maintenance commands into the user space later if needed, > > debuginfod_client *c = get_debuginfod_client (); > @@ -451,6 +453,14 @@ static void > set_debuginfod_enabled (const char *value) > { > #if defined(HAVE_LIBDEBUGINFOD) > +#if !defined(HAVE_DEBUGINFOD_FIND_SECTION) > + if (value == debuginfod_lazy) > + { > + debuginfod_enabled = debuginfod_on; > + error (_("Support for lazy downloading is not compiled into GDB. " \ > +"Defaulting to \"on\".")); > + } > +#endif > debuginfod_enabled = value; > #else > /* Disabling debuginfod when gdb is not built with it is a no-op. */ > @@ -550,8 +560,12 @@ _initialize_debuginfod () > _("Set whether to use debuginfod."), > _("Show whether to use debuginfod."), > _("\ > -When on, enable the use of debuginfod to download missing debug info and\n\ > -source files."), > +When set to \"on\", enable the use of debuginfod to download missing\n\ > +debug info and source files. \"off\" disables the use of debuginfod.\n\ > +When set to \"ask\", a prompt may ask whether to enable or disable\n\ > +debuginfod. When set to \"lazy\", debug info downloading will be\n\ > +deferred until it is required. GDB may also download components of\n\ > +debug info instead of entire files." ), > set_debuginfod_enabled, > get_debuginfod_enabled, > show_debuginfod_enabled, > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index c1ca45521ea..ac0636e3115 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -48757,10 +48757,15 @@ debug info or source files. By default, @code{debuginfod enabled} is set to > attempting to perform the next query. By default, @code{debuginfod enabled} > is set to @code{ask} for interactive sessions. > > +@item set debuginfod enabled lazy > +@value{GDBN} will attempt to defer downloading entire debug info files until > +necessary. @value{GDBN} may instead download individual components of the > +debug info files such as @code{.gdb_index}. Given how the code is currently written I would expect some changes to 'on' to make it clear that some functionality is missing in this mode. But if you take my advice then the docs would end up different anyway.. Thanks, Andrew > + > @kindex show debuginfod enabled > @item show debuginfod enabled > -Display whether @code{debuginfod enabled} is set to @code{on}, @code{off} or > -@code{ask}. > +Display whether @code{debuginfod enabled} is set to @code{on}, @code{off}, > +@code{ask} or @code{lazy}. > > @kindex set debuginfod urls > @cindex configure debuginfod URLs > -- > 2.39.2