From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) by sourceware.org (Postfix) with ESMTPS id DCC683851C1B for ; Tue, 29 Sep 2020 21:37:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DCC683851C1B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=mittosystems.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jozef.l@mittosystems.com Received: by mail-wr1-x442.google.com with SMTP id z1so7083098wrt.3 for ; Tue, 29 Sep 2020 14:37:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mittosystems.com; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=sbVK5WPX8C8KBjxkazMG8ChS5xaaw++xYtrAMbMMu60=; b=N8mW/FWPyzH0hlGDHKFenlZkMEwaZQIUeHSZ2q+qCfo9dJP2AKyTPjviRnNH9Q497E jVaNdzxj+g1ISS+uaLdrA7Z3GVrBRVGZvnKIT1P7XJaYhuKQL2RLne6jfHRDEaDkVK49 DxaGB0BieMtUy5J6OMsmJ/8CFFf2NpehvB9NXT8cQoxlhyk9f6sW+yhr6IplMEs4p7HP FBYoG7jwfkek1yH4ldd4zl31sWH7ALiMcRZZFPmic2icIMRXfhPXJKDeoQi6hC0VSw5M zRtg4rHRLO98fXcvTu+kzbD3LM33gVOT8WvzXCEJX3OO1ZWftN+b6B8ivYcRabEaDZnN T6tg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=sbVK5WPX8C8KBjxkazMG8ChS5xaaw++xYtrAMbMMu60=; b=BfJn7hs/ff9lb+Lo95vbMi6oZbXo+1ERHu/ywcb9l7+Yy+wR9YkyJrBIkuPemETcNv fBPZ3kzClOoTSeDjA6Ny1Fft/ABkQYLxfM8Cr3j7rR9JS0ycG67wFChNFaFdneh6mMJ1 JynGOUHfHt6yQEJafALY0U3rJdNb2yNnaNkfQlCzyzNrk6E5T4OPzuLtxtbfHOJi8WJq nkDoj6M8trTjwQfjkWDUwkbrZRRE2Y6a7Imn3zgPPc70i817xHCcHOUnShaan2OS8YgN 7jh1aPBdEVPZXOvT5VQJoxRLX/W+h5prVv3Dxj+WMTpfaHtIjruK/I4CFUxnJTlaKoAD o8og== X-Gm-Message-State: AOAM532IXIPu0asmMwzxQRPYfpKLTEbx/TEFFX15ewcD2aehweRtrK4S RlFbinjLxfGeMyv6TSWy7rcUnA== X-Google-Smtp-Source: ABdhPJyMCbpgm2DkHeYDfoX6y6Le65DFbGlTJWOzQpP8cdQME/04qxF4sPn2W1yw9V+gsoh2of7mtg== X-Received: by 2002:adf:cf0b:: with SMTP id o11mr6236641wrj.94.1601415466465; Tue, 29 Sep 2020 14:37:46 -0700 (PDT) Received: from jozef-acer-manjaro ([2a01:4b00:87fd:900:5e1d:5c99:56da:76e8]) by smtp.gmail.com with ESMTPSA id l126sm7088500wmf.39.2020.09.29.14.37.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Sep 2020 14:37:43 -0700 (PDT) Date: Tue, 29 Sep 2020 22:37:41 +0100 From: Jozef Lawrynowicz To: Fangrui Song Cc: binutils@sourceware.org, gnu-gabi@sourceware.org Subject: Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag Message-ID: <20200929213741.jqegh62d7jne5uyo@jozef-acer-manjaro> Mail-Followup-To: Fangrui Song , binutils@sourceware.org, gnu-gabi@sourceware.org References: <20200928132613.btkqaoomv4fdnupn@jozef-acer-manjaro> <20200929044353.vx26ypc2oioqbmfb@gmail.com> <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro> <20200929193806.m6u6ra6oijqkstfo@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200929193806.m6u6ra6oijqkstfo@gmail.com> X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gnu-gabi@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gnu-gabi mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Sep 2020 21:37:50 -0000 On Tue, Sep 29, 2020 at 12:38:06PM -0700, Fangrui Song wrote: > On 2020-09-29, Jozef Lawrynowicz wrote: > > There are more logic holes in your other email that I didn't > > respond to, but I've already spent a tonne of time trying to address > > your points (most of which you ignored, so why did I bother?), so I > > think I'm done debating this. > > If other people back up your suggestion, and address my concerns, then > > we can re-open this. > > Hi Jozef, apologies if I did not respond to your previous replies in > time or if I acted as if I "blocked" your change. I maintain LLVM's > linker LLD and am also a contributor to its binary utilities > (llvm-readelf, llvm-objdump, ...). If GCC makes use of this flag for __attribute__((used)), > LLD and LLVM's binary utilities do need to support it. > So I want to participate in the design before we regret :) > > I have studied linker GC quite a lot (contributed patches in LLD > and coordinated on the feature parity among the 3 linkers > (e.g. https://sourceware.org/pipermail/binutils/2020-August/112732.html) > > LLD and GNU ld, LLVM's integrated assembler and GNU as have achieved consensus on many aspects > on SHF_LINK_ORDER and 'unique' linkage. Credits to H.J. for all the binutils side work. > ) > > I have asked a couple of toolchain folks. Many do share the same feeling > with me: an additional GC feature is useful but whether it should be > this exact form of a new flag we don't know. Hi Fangrui, Thank you for providing some more information to backup your points, I will address those below. > > Your patch also deprives an OS-specific flag from ELFOSABI_FREEBSD:) > I am not a committer of FreeBSD and I am surely not authoritative on > whether this is acceptable. But, FreeBSD does use the aforementioned > LLVM tools and I don't want to dirty the code base by adding > if (osabi == ELFOSABI_FREEBSD) do something. I don't have any insight into the ELFOSABI_FREEBSD change, H.J. added that part, I assume it is very closely aligned with the "pure" GNU OSABI, so any changes to one should be kept in parity with the other. However, that is only my assumption. > > We have 8 bits in SHF_MASKOS: not too few, but also not too many. We > should be prudent with new flags, especially if we already have a > facility for section retaining. There is a difference ".reloc requires a > known live section" which might cause friction but it has the advantage that it is > time-tested and has good toolchain support. Yes, this is something I considered, but there are 6 bits remaining to be used, with SHF_GNU_RETAIN there will be 5 bits remaining. Given how little movement there has been within the SHF_MAKOS bits for the past 20 years, I don't think we need to be too protective of the bits. They are there to be used after all, and IMO SHF_GNU_RETAIN is a very well-defined use case to use one of those bits. For the .reloc mechanism, in every object file with a retained section you will need to have an empty input section which is guaranteed to be kept by the linker. How is the compiler to know what that section is? > > > Opinions from others: > > * Peter Smith mentioned that Arm's proprietary toolchain does something > similar to SHF_GNU_RETAIN > (https://www.keil.com/support/man/docs/armcc/armcc_chr1359124983230.htm) > "I put __attribute__((used)) on my function and yet the tools removed > it anyway." > > * James Henderson mentioned whether we can > "create a new section type listing section indexes that should be > retained" > As a variant, we could use a SHT_GROUP with a new GRP_* bit. > Anchor: [group] (see below) Using groups was discussed on the GNU gABI thread, but I don't really see the benefit of the added complication. Doesn't handling new section group types still require significant coordination between all the different compilers, assemblers and linkers? Any attempt to reference a section which does not exist in the object file, and is not guaranteed to be present in the linked output file, is a non-starter in my opinion. We give users the freedom to do what they want with linker scripts as ELF doesn't mandate any section must be linked into the final executable. I think it's better to consolidate the requirement to retain within the section that requires retaining, instead of spreading the requirement to retain between ELF constructs which have unrelated purposes, and need to be aligned between different stages of the build process. > > > Do we want to make life easier for ourselves, or easier for our users? > > > > I get that ABI changes can be a bit disruptive, but this new flag in particular really isn't complicated anyway. > > On 2020-09-29, Jozef Lawrynowicz wrote: > > On Mon, Sep 28, 2020 at 09:43:53PM -0700, Fangrui Song wrote: > > > On 2020-09-28, Jozef Lawrynowicz wrote: > > > > The attached patch is version 2 of the SHF_GNU_RETAIN patch that was > > > > previously discussed here: > > > > https://sourceware.org/pipermail/binutils/2020-September/113406.html > > > > > > > > The following changes have been made: > > > > - Removed the .retain directive > > > > - The assembler will create different input sections for sections with > > > > the same name but SHF_GNU_RETAIN set/unset (thanks to H.J. for that). > > > > This means the linker will be able to do a better job garbage > > > > collecting input sections, as the "retain" attribute applied to a > > > > symbol declaration in the source code will not cause other parts of > > > > the program that are not required, but are in the same section, to be > > > > unnecessarily retained. > > > > - Added GNU OSABI handling (also thanks to H.J.). > > > > > > My point from https://sourceware.org/pipermail/binutils/2020-September/113466.html stands. > > > Section flags are a bit cumbersome. If the following > > > > > > // a.h > > > __attribute__((section("sec"))) > > > inline void bar() { ... } > > > // a.c > > > #include "a.h" > > > __attribute__((section("sec"), retain)) > > > void foo() { > > > } > > > > > > compiles to > > > > > > .section sec,"a",@progbits > > > ... > > > .section sec,"aR",@progbits > > > ... > > > > > > You will get a gas error for changing section flags > > > (https://sourceware.org/pipermail/binutils/2020-February/109945.html) > > > > > > > There is no error in this case. > > The original patch handled it, and it has been further improved in v2. > > There's even a testcase testing exactly the functionality you say is > > broken. > > > > gas/testsuite/gas/elf/section22.s: > > .section .bss,"aw" > > ... > > .section .bss,"awR",%nobits > > > > I already gave a detailed response to your reloc proposal: > > https://sourceware.org/pipermail/binutils/2020-September/113450.html > > Letting the following assembly creates two .bss sections adds an exception to > "gas: error for section type, attr, or entsize change" > (https://sourceware.org/pipermail/binutils/2020-February/109945.html) > for which I have achieved consensus with Alan Modra and some LLVM > toolchain folks. That is why I hope we could re-consider this design > choice. > > .section .bss,"aw" > .section .bss,"awR",%nobits I agree that this could be considered slightly inelegant, but much less so than having .reloc directives in arbitrary empty sections. Once someone understands what the "R" flag does, it should be clear why we want to create a separate input section for it. If we really want, we can force the compiler to create unique section names for declarations which have the "retain" attribute applied. I assume most of the time, users will want this fine section granularity. I would prefer not to go that route and leave it to the users to specify unique sections if they want. > > See above for [group]: it is actually nice to use a group here, because > the quadruple > (section name, group name if "G", linked-to section if "o", unique id of > "unique") is the section key. The following creates a section different > from the default .bss: > > .section .bss,"awG",%nobits,gc_root I don't think it is really very important whether input sections with the same name are unique or not within an object file. ELF doesn't care about this. You can have sections with the same name and same flags in different object files and the linker can just deal with it. It's the same situation if you have two sections from the same object file which have the same name. Within an object file, you'll end up with all the non-SHF_GNU_RETAIN sections with the same name being merged together by the assembler, and all those with SHF_GNU_RETAIN being merged. I don't think there is any benefit to keep SHF_GNU_RETAIN sections with the same name as separate input sections in the object file. Thanks, Jozef > > > Why don't you first answer my questions about why using relocs is a > > really bad idea, is a hack, doesn't conceptually make any sense, and > > will confuse users? > > > > You never actually gave a reason that using relocs is better than > > SHF_GNU_RETAIN, you seem to be pushing this alternative implementation just > > because it is an alternative implementation. I don't count your > > description of section flags as "cumbersome" as a reason we should > > shelve this proposal. > > I hope my previous paragraphs have answered this point. > > > > .reloc is really convenience in this case. You can add as many .reloc > > > directives as you like, each contributing one R_*_NONE to the object > > > file but zero cost to the linked image. > > > > Are we really counting a section flag as adding "cost" to a linked > > image? Your method requires a "benign zero-sized section" (suggested in > > your previous email), doesn't this also add cost? What about the cost of > > having nonsensical relocs in object files? > > > > If we are using the term "cost" loosely, and to further our own point, > > then SHF_GNU_RETAIN actually has negative cost to the linked image. You > > can have different input sections with the same name, but with different > > states for SHF_GNU_RETAIN (set and unset), and so garbage collection > > will operate with finer granularity, removing more unneeded parts of the > > program compared to your reloc mechanism, which would keep the entire > > section containing the symbol referenced by the reloc. > > > > Thanks, > > Jozef