From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 8F5E93857001; Wed, 30 Sep 2020 14:13:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8F5E93857001 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=matz@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id A316EAC9A; Wed, 30 Sep 2020 14:13:08 +0000 (UTC) Date: Wed, 30 Sep 2020 14:13:08 +0000 (UTC) From: Michael Matz To: Roland McGrath cc: Fangrui Song , Binutils Development , gnu-gabi@sourceware.org Subject: Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag In-Reply-To: Message-ID: References: <20200928132613.btkqaoomv4fdnupn@jozef-acer-manjaro> <20200929044353.vx26ypc2oioqbmfb@gmail.com> <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro> <20200929193806.m6u6ra6oijqkstfo@gmail.com> <20200929213741.jqegh62d7jne5uyo@jozef-acer-manjaro> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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: Wed, 30 Sep 2020 14:13:11 -0000 Hello, On Tue, 29 Sep 2020, Roland McGrath wrote: > Since group semantics mean that if any section in a group is retained the > whole group must be retained, it seems to me that making the feature a > group flag rather than a section flag makes sense. I don't think it really > has any downside. It's a little bit more structure to generate a group but > everything already knows how to do it so that should be no problem. While > it is true that multiple sections with the same name and different > attributes should already be fine, the place where multiple different > sections of the same name are usually seen today is when each different > section reusing a name is in a different group from the other sections with > the same name. pseudo-code for retaining a section in producer. SHF_GNU_RETAIN: retain_section(S): S.flag |= SHF_GNU_RETAIN GRP_RETAIN: retain_section(S): if !retain_group: retain_group = new_group(GRP_RETAIN) retain_group.add (S) pseudo-code for interpreting this in the consumer (which presumably is section based as basic building block, as groups are optional). SHF_GNU_RETAIN: in the 'output_section_p' predicate, add " || S.flag & SHF_GNU_RETAIN". GRP_RETAIN: in preparatory per-object parsing, in the existing group parser: if G.type == GRP_RETAIN: foreach S in G: S.needed = true in the 'output_section_p' predicate, add " || S.needed" That's the additional structure you mention. While it's not much down-side, what is the up-side? There should be one, not just only small down-sides. Ciao, Michael. > > On Tue, Sep 29, 2020 at 2:37 PM Jozef Lawrynowicz > wrote: > > > 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 > > > > >