From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from bg4.exmail.qq.com (bg4.exmail.qq.com [43.154.221.58]) by sourceware.org (Postfix) with ESMTPS id 62AE53858C60 for ; Mon, 24 Jul 2023 09:28:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 62AE53858C60 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tinylab.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tinylab.org X-QQ-mid: bizesmtp71t1690190883tmn3n9ic Received: from tanyuan-Surface-Book-2.localdom ( [61.141.78.189]) by bizesmtp.qq.com (ESMTP) with id ; Mon, 24 Jul 2023 17:28:02 +0800 (CST) X-QQ-SSF: 01200000000000403000000A0000000 X-QQ-FEAT: E05nFjkNFLzPXn0MJF9D2tuH6w5V6r8PyuNnyUI3pmBEO8uFYHDVUIk2kpX+3 5PJqcMv0m/2nf8nQJFzMmWalf2EYpr77o1fyon2XyT3L8dtN/qmJhYKAS2Gsp/CKriPEJGm RC8MjR6kXldJ+XoWRzU09zwwFGiW/8rQuNFMAhZbSdd0EaMgvYdgonyqsgEucvMr7n/DZyu gg2GDvh0bQNDvTNByXwuHSl1iDrAT/LncxtttUCr59ffKjrO1+d7yBwCSwMv48muQR1uEAv a/t6u84fbiJfoOduv6BA9s07Qf9d5Mn+rdv37vBTwdSkgnBCV7Mq4RpQjvAJsWaw1ivQF8j dKSG7Cldl4/ydueikcFkBlAYJtGqFaI8wmU7ONsjMW54Y1rj3hZNWjedcTutg== X-QQ-GoodBg: 0 X-BIZMAIL-ID: 12972901185715509684 From: Tan Yuan To: i@maskray.me Cc: amodra@gmail.com, falcon@tinylab.org, binutils@sourceware.org Subject: Re: [PATCH 0/1] gas: add new command line option --no-group-check Date: Mon, 24 Jul 2023 17:28:02 +0800 Message-Id: <20230724092802.110-1-tanyuan@tinylab.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrgz:qybglogicsvrgz5a-1 X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: On Mon, Jul 24, 2023 at 1:15 AM Fangrui Song wrote: > On Sun, Jul 23, 2023 at 2:10 AM Tan Yuan wrote: > > > > Hi, > > > > Thanks for your reply and your advice inspired me a lot. > > > > On Sat, 22 Jul 2023 at 00:32 Song Fangrui wrote: > > > On Fri, Jul 21, 2023 at 8:14 AM Tan Yuan wrote: > > > > > > > > Hi, list > > > > > > > > This patch introduces a new option that allows suppressing the > > > > warning when attaching a group to a section that already belongs to a group. > > > > > > > > The kernel extensively uses the ".pushsection" directive, which > > > > poses issues with proper reference building and garbage > > > > collection. To address this problem, the kernel uses "KEEP" in the > > > > linker script to prevent certain sections from being garbage > > > > collected. Consequently, some sections that should have been > > > > garbage collected are retained. For instance, if the section > > > > pusher is being garbage collected, the pushed sections are also > > > > supposed to be collected, but the linker script retains them. > > > > > > > > To resolve this, we can utilize ".attach_to_group" alongside > > > > ".pushsection," which enables the section pusher and the pushed > > > > sections to be grouped together, making them either eligible for > > > > garbage collection or kept together. > > > > > > > > Currently, the kernel encapsulates the use of ".pushsection" > > > > within macros, leading to thousands of instances where these > > > > macros are expanded. Rather than adding ".attach_to_group" to a > > > > thousand functions, we can incorporate it into this macro. > > > > However, some functions expand this macro multiple times, > > > > resulting in multiple attachments to the group for those > > > > functions. Unfortunately, there is no alternative method to ensure > > > > a single expansion of ".attach_to_group" As a solution, we propose adding an option to address this issue. > > > > > > > > Thanks. > > > > > > I am curious whether the "kernel" in your message refers to the Linux kernel. > > > .attach_to_group is a new directive from Oct 2020 and the Linux > > > kernel doesn't use it. > > > > The Linux kernel uses ".pushsection" and forcefully "KEEP" them. It > > unnecessarily retains some pushed sections, increasing the size of > > vmlinux. I came up with an idea that we can group section pushers and > > pushed sections and make them garbage collect together, as if there's > > no section pusher, there won't be a need for pushed sections anymore. > > > > > If you have plans to use .attach_to_group in the upstream kernel, > > > I'd like to hear more as LLVM integrated assembler doesn't support > > > .attach_to_group and I think it would be difficult to add the support. > > > Some use cases of .attach_to_group (which doesn't intend to support > > > GRP_COMDAT groups) can be replaced with SHF_LINK_ORDER [1] (with > > > good support in newer binutils; I am unsure whether it's 2.35/2.36 > > > that it looks stable): > > > > > > [1]: > > > https://maskray.me/blog/2021-01-31-metadata-sections-comdat-and-shf- > > > link-order I shall revise the article, but it basically covers most > > > things I can think of. > > > > I had the pleasure of reading your article and was pleasantly > > surprised to discover the functionality of SHF_LINK_ORDER. After some > > experiments, the SHF_LINK_ORDER methods results same with the “.attach_to_group” methods. > > > > However, when it comes to .s file, SHF_LINK_ORDER methods cannot > > replace with ".attach_to_group" methods. It's seems to be hard to find > > a symbol name to refer when assembly macro expand. > > > > For example, if we modify the kernel code as follows, adding a label > > before the .pushsection to make the pushed section refer to the section pusher. > > > > diff --git a/arch/riscv/include/asm/asm-extable.h b/arch/riscv/include/asm/asm-extable.h > > index 14be0673f5b5..1ade4f8fd79e 100644 > > --- a/arch/riscv/include/asm/asm-extable.h > > +++ b/arch/riscv/include/asm/asm-extable.h > > @@ -7,10 +7,13 @@ > > #define EX_TYPE_BPF 2 > > #define EX_TYPE_UACCESS_ERR_ZERO 3 > > > > +#define ___PASTE(a,b) a##b > > +#define __PASTE(a,b) ___PASTE(a,b) > > + > > #ifdef __ASSEMBLY__ > > > > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > > - .pushsection __ex_table, "a"; \ > > + __PASTE(__extable,__LINE__): .pushsection __ex_table, "ao",__PASTE(__extable,__LINE__); \ > > .balign 4; \ > > .long ((insn) - .); \ > > .long ((fixup) - .); \ > > > > __ASM_EXTABLE_RAW will expand to linux/arch/riscv/lib/uaccess.s: > > > > .macro _asm_extable, insn, fixup > > __extable30: .pushsection __ex_table, "ao",__extable30; .balign 4; .long ((\insn) - .); .long ((\fixup) - .); .short (1); .short (0); .popsection; > > .endm > > > > The C macro expands to an assembly macro, which, in turn, is expanded > > multiple times, leading to error of multiple definition of symbols. > > This situation is similar to receiving a warning when attempting to > > attach to a group multiple times. > > > > And it seems that there is no way to set symbol only in section scope, > > and there is no fixed symbol in sections pusher that pushed section can refer to. > > Is there any possible solution to such a situation? > > > > At last, thank you once again for suggesting the SHF_LINK_ORDER method. > > > > Tan Yuan Hi, We just found an alternative way to solve this problem. As the perfect functionality of SHF_LINK_ORDER, it can replace .attach_to_group completely in our working patch to Linux kernel. Very grateful for your advice. > Hi Tan, is the following your intended use case? > > cat > a.s < .macro _asm_extable, insn, fixup > .pushsection __ex_table, "a" > .attach_to_group .text > .byte 0 > .popsection > .endm > > .globl __asm_copy_to_user > __asm_copy_to_user: > 100: > _asm_extable 100b, 10f > addi a1, a1, 1 > _asm_extable 100b, 10f > addi a0, a0, 1 > 10: > eof > riscv64-linux-gnu-gcc -c a.s > > % readelf -g a.o > > group section [ 1] `.group' [.text] contains 2 sections: > [Index] Name > [ 5] __ex_table > [ 6] __ex_table > > There are two __ex_table sections in the section group. This is a bit wasteful due to excess section headers, but the linker will combine them. > > If you switch to SHF_LINK_ORDER, you can use the section symbol .text: > .pushsection __ex_table, "ao", @progbits, .text This is well supported in gas from 2.35 onwards and LLVM integrated assembler. It is a lot complicated, as -ffunction-sections should be enable so the .text become something else like .text.function_name. But it won’t be a problem anymore. We use some macro magic to solve it. I just check GCC 7.3 with binutils 2.30 works well with our SHF_LINK_ORDER patch when compiling kernel. By the way, do you know when LLVM integrated assembler supports SHF_LINK_ORDER? > > > > Tan Yuan (1): gas: add new command line option --no-group-check > > > > > > > > gas/as.c | 10 +++++++++- gas/as.h | 3 +++ > > > > gas/config/obj-elf.c | 2 +- gas/doc/as.texi | 4 ++++ 4 files > > > > changed, 17 insertions(+), 2 deletions(-) > > > > > > > > -- 2.41.0 > > > > > >