From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) by sourceware.org (Postfix) with ESMTPS id 8D54D3948829 for ; Tue, 24 May 2022 18:38:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8D54D3948829 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=debian.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f41.google.com with SMTP id x12so2694290wrg.2 for ; Tue, 24 May 2022 11:38:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:date:in-reply-to :references:user-agent:mime-version; bh=Q+gaD/MJekUw18jXDt2sAZkfZoeWc//NKlBecngPX9Y=; b=Z2O+KYRzjKjmfh9iAo9W1GuxxSJgUD0JftL6TT9qzdRaKgz/6bH61m2dzN46HyopjC Dmn1YLVP8zcUUpuhHtxD7xvruutG2Eau+u3nJmWHAuSw5ClskmywTYcKsqwTsEG/lPu2 lkFiWC9VuA8JCg3JQBXZTkUOFvbn9HjQc5O/on1tHS/+hRCYEWx0AbfQrKhoN8CbZzG0 BlT3E//LbSa8hXifsyVoGYoBBbFa607CKmC8PtIDrwJfs9h5N2V71uuiWUtKpnk2l15q DhtqEPhNIU+ReDSDbCVLGVeZ32zE3xIJp6lv45xdkmaljIb5mkenyCnCnAjUTZAII+9d HbyQ== X-Gm-Message-State: AOAM5305GiZFmuh/PsBly9cNe5AILQkF+pUI1zzVa8MjSIJ8+HaFA3fC xtDssOD7bTrVUeIz2O240ktvNHqkw5Oqjw== X-Google-Smtp-Source: ABdhPJy0ttajdBRspfkZ4xOnuFoPr2YptcnCrfZHgsIUI6rJzk/PxXHXvQqhz5KsQA1B7AwZ9qugfA== X-Received: by 2002:adf:f90f:0:b0:20e:5fd4:5d06 with SMTP id b15-20020adff90f000000b0020e5fd45d06mr25154399wrr.371.1653417506322; Tue, 24 May 2022 11:38:26 -0700 (PDT) Received: from localhost ([137.220.125.106]) by smtp.gmail.com with ESMTPSA id y6-20020a05600015c600b0020cfed0bb7fsm166586wry.53.2022.05.24.11.38.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 May 2022 11:38:25 -0700 (PDT) Message-ID: <693aa125072ed68a25d1e57aeb87bea9174f96fb.camel@debian.org> Subject: Re: [PATCH] ld: add --package-metadata From: Luca Boccassi To: Nick Clifton , binutils@sourceware.org Date: Tue, 24 May 2022 19:38:23 +0100 In-Reply-To: <7cd459a6-ac46-e9c2-14d1-16d78118bb92@redhat.com> References: <20220515191846.114257-1-luca.boccassi@gmail.com> <7cd459a6-ac46-e9c2-14d1-16d78118bb92@redhat.com> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-6/xsLbdwJeJkcF8M+6Db" User-Agent: Evolution 3.38.3-1+plugin MIME-Version: 1.0 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 May 2022 18:38:29 -0000 --=-6/xsLbdwJeJkcF8M+6Db Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2022-05-24 at 17:23 +0100, Nick Clifton wrote: > Hi Luca, >=20 > =C2=A0=C2=A0=C2=A0First of all a couple of questions about the need for t= he patch itself. >=20 > =C2=A0=C2=A0=C2=A0Firstly - why modify the linker itself ? Couldn't the = same effect be > =C2=A0=C2=A0=C2=A0achieved by using the assembler to create an object fil= e and then > =C2=A0=C2=A0=C2=A0including that in the link along with the other objects= ? So we are already using an intermediary artifact, and it worked well to quickly get a workable PoC together, and then hammer it in shape to deploy it distro-wide in Fedora and CBL-Mariner. Now that we are in production phase, and we'de like for the spec to expand (and take over the world!), we want to address an important shortcoming that affects distro builders. Dealing with intermediate artifacts means dealing with paths. And in practice, that gets really, really, really awkward and problematic. Almost all issues we've had are due to the fact that being able to reliably reference a path at build time (while not breaking reproducibility!) is _extremely_ complicated, and there's a huge tail end of packages were things are just too hopelessly convoluted. It might sound counter-intuitive as it's so trivial to put together an hello world, but in reality when you are dealing with 20k to 30k packages things get really, really weird. Coverage in F36 is not 100% of binaries mostly because of this. So with that experience in the bag, the most obvious next step is to have a first-class option, that allows a self-contained flag to be passed instead. After all the idea is similar to the build-id, and there we have a first-class option too, and it works well. > =C2=A0=C2=A0=C2=A0Secondly - why build json verification into the linker = ? It does not > =C2=A0=C2=A0=C2=A0feel like a linkery kind of thing to do, and because th= e verification > =C2=A0=C2=A0=C2=A0is dependent upon the linker being configured and built= with the feature > =C2=A0=C2=A0=C2=A0enabled, it means that programmers cannot rely upon the= ir packaging > =C2=A0=C2=A0=C2=A0notes always being checked. To me, I think that it wou= ld be better to > =C2=A0=C2=A0=C2=A0have a pre- or post- links stage where the note is chec= ked by a separate > =C2=A0=C2=A0=C2=A0tool. This is not aimed at individual programmers - an individual developer is not expected to worry about any of this when hacking on local software and such. The spec is made for distribution builders. And speaking as one, my life would be so much easier if input was validated before usage. Leaving it as an exercise for the reader means that _every single distribution builder_ would need to reimplement the exact same check, over and over and over again. Get it done once and everyone can benefit. JSON looks the same across all distros, after all! Or from another point of view: ld already validates other types of input. Linker scripts get validated. Flags get validated. Objects get validated. The type of build-id requested gets validated. So why not allow (optionally!) to validate this too? It's easy, just half a dozen lines of code, if the lib is missing nothing happens and it's just skipped. If there are any concerns I can flip the autoconf to be default- disabled, so that --enable has to be passed explicitly, together with making the dep available at build time? > =C2=A0=C2=A0=C2=A0Lastly, since this option is being used to create a not= e section, maybe > =C2=A0=C2=A0=C2=A0we ought to consider the need for a generic mechanism f= or the linker to > =C2=A0=C2=A0=C2=A0create sections - other than using linker scripts. For= example, suppose > =C2=A0=C2=A0=C2=A0that instead of --package-metadata=3D there was a= n option: > =C2=A0=C2=A0=C2=A0--add-note=3D,,,[] > =C2=A0=C2=A0=C2=A0which would basically do the same thing, but in a more = generic fashion. Well we would lose verification, and also it sounds quite a bit more complex (you'd need to pass IDs in hex format - quite a bit ugly) - if there hasn't been a need so far, not sure it's worth it? > =C2=A0=C2=A0=C2=A0Some thoughts on the patch itself: <..> > Also - can the JSON data be read from a file ? I can easily see problems > being encountered with the length of the linker command line if the JSON > data can only be specified as text. (Possibly this can be avoided by usi= ng > the @file syntax already supported by the linker). It could be a separate option I guess, if needed, but we wouldn't use it - as mentioned above, the main goal is to get rid of the intermediate file entirely. <..> >=20 > > +if { [istarget frv-*-*] || [istarget lm32-*-*] } { > > + return > > +} >=20 > Why are these architectures being skipped ? To be perfectly honest - cargo culting from another test :-) Removed now. Will send a v2 later with fixes for all the style issues and the ASAN one. Thanks for the review! --=20 Kind regards, Luca Boccassi --=-6/xsLbdwJeJkcF8M+6Db Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCSqx93EIPGOymuRKGv37813JB4FAmKNJh8ACgkQKGv37813 JB7tEhAAkuIsaUDPvJMGvkmHDAs1DhwnHyfOXTvCboVkXfK+jEO69B70FiZqki58 gj+E1UefRXMIDgorzrSzdvl35tjyzOuQgfcRs3ssmvbdy1QibiI5jIxxe1f1j69E 3WzOBhH0k8sqtu90s4NRCeBFweaoh6H5gRsjimzjFjPDm9HhbFwGc9u0qBzAIKwN oL5WHBqGMYhVTLy4TSqbbfqJiAsbwynypPq7PS26l8SrVpJAYsKzmX0QVpqcCAWo WaMpCmSYOsKKHE7Y6wmq/TkBrUyxXR2p9o8G1nIuZjqxG0MuOB2Hlqk0c0kZXWoi 53KkQlGFSTSA3V5DqOLujZN51RgUE2xSC8tOBCjZxwONheleE/7ByOPwWrnESfLg ZhBNN8JhIkwX8KQ5QAYehdOoxnxjMrxESfvFfeDy7f+nPiWgJLAlJTampGylvDs1 9S+NnrwxsLYjiPb6ARDKeQIbCV0J08dzS5vo/E+ZTAZs8z7iT1stQWKQXrGtwcrC DM87kvfiszvURgvMVse7YEcSdVH17rQ/NipkZwQHdw0JRL9w0Y0SLH4dW4H6TkW/ M+tUK3VRZzh4yf2qBuahE0yB5Ud5PoOAiZGbCCrhGVAMKhny/nzUWDctB6K0Cnyw scwLdXK5J1Arqohbo/F8Eo5BsNBK1DR2vrL/n4n00U/mk+RSfVk= =OvxB -----END PGP SIGNATURE----- --=-6/xsLbdwJeJkcF8M+6Db--