From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by sourceware.org (Postfix) with ESMTPS id 8596A3857C57; Tue, 29 Sep 2020 19:38:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8596A3857C57 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=maskray.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=emacsray@gmail.com Received: by mail-pf1-f193.google.com with SMTP id l126so5649571pfd.5; Tue, 29 Sep 2020 12:38:09 -0700 (PDT) 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:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=/IwiB03Y5nlxiRq+Owx8iFhIIfuurkpyJjUgWyZIH3o=; b=RO3GQmzpp5MQrruqSb+aAMasPyyOCJYUq7G69ae3WwYfB76Kp9ywxyR3hdR/bcywy3 fULBEyBLh6qKPS70F+V9tOOFbaBs+Ouwf/m9DWDdjrxtuqr5ix/x3o4ufR4KAAl+nlVv NAEGW2gWodh6Svrt78yTs3wP8A4/q7nc7FkYuARj5i35o38qeIhbarZ6nGM4gtCu2HnF eG5Vuab4QWyITXQSSmCHVgrV84VYLwjUqA7DLoxhpdKiTFfYQSVjWJvwtCr4Qdqj6jj7 riixHNTiFQySbzVtzr3jVIdZLtAwPgYs7/pzQCZ6EjioxYms5r+iPtsCxGlxxcjfIPhP Zomw== X-Gm-Message-State: AOAM532kYwTNgdstbuVNe2vHf8Jun/W4MsjWfjPX2l6OsTzkjR9wyOK3 yZgDS0+C6RTr9DmDHvP5f0wYIwl98JXWXA== X-Google-Smtp-Source: ABdhPJzVZWPMR7+Wl6v39VakxVcz9cOuWtFA3LftPV9kII8TbpM9OfdpntEAsq+HoSb4I5p4Z4XMtA== X-Received: by 2002:a63:8c6:: with SMTP id 189mr4577854pgi.207.1601408287665; Tue, 29 Sep 2020 12:38:07 -0700 (PDT) Received: from localhost ([2601:647:4b01:ae80::51fb]) by smtp.gmail.com with ESMTPSA id gp8sm2311819pjb.3.2020.09.29.12.38.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Sep 2020 12:38:07 -0700 (PDT) Date: Tue, 29 Sep 2020 12:38:06 -0700 From: Fangrui Song To: binutils@sourceware.org, gnu-gabi@sourceware.org Subject: Re: [PATCH v2] Support for SHF_GNU_RETAIN ELF Section Flag Message-ID: <20200929193806.m6u6ra6oijqkstfo@gmail.com> References: <20200928132613.btkqaoomv4fdnupn@jozef-acer-manjaro> <20200929044353.vx26ypc2oioqbmfb@gmail.com> <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20200929100442.o3e66jbmqpv5cajh@jozef-acer-manjaro> X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no 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 19:38:11 -0000 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. 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. 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. 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." I have personally used a .reloc to retain __attribute__((used, section(".data.PERSIST_SIG"))) static char SigAfl[] = "##SIG_AFL_PERSISTENT##"; * 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) > 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 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 >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