* [PATCH 0/1] gas: add new command line option --no-group-check @ 2023-07-21 15:14 Tan Yuan 2023-07-21 22:55 ` Alan Modra 2023-07-22 0:32 ` Song Fangrui 0 siblings, 2 replies; 9+ messages in thread From: Tan Yuan @ 2023-07-21 15:14 UTC (permalink / raw) To: binutils, amodra, jbeulich; +Cc: Tan Yuan 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. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-21 15:14 [PATCH 0/1] gas: add new command line option --no-group-check Tan Yuan @ 2023-07-21 22:55 ` Alan Modra 2023-07-22 3:47 ` Tan Yuan 2023-07-22 0:32 ` Song Fangrui 1 sibling, 1 reply; 9+ messages in thread From: Alan Modra @ 2023-07-21 22:55 UTC (permalink / raw) To: Tan Yuan; +Cc: binutils, jbeulich On Fri, Jul 21, 2023 at 11:14:20PM +0800, 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. Is this alternative patch sufficient for the kernel usage? * config/obj-elf.c (obj_elf_attach_to_group): Don't warn if group name matches current group for section. diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c index 753a929fb14..dc05b35ee99 100644 --- a/gas/config/obj-elf.c +++ b/gas/config/obj-elf.c @@ -1088,8 +1088,9 @@ obj_elf_attach_to_group (int dummy ATTRIBUTE_UNUSED) if (elf_group_name (now_seg)) { - as_warn (_("section %s already has a group (%s)"), - bfd_section_name (now_seg), elf_group_name (now_seg)); + if (strcmp (elf_group_name (now_seg), gname) != 0) + as_warn (_("section %s already has a group (%s)"), + bfd_section_name (now_seg), elf_group_name (now_seg)); return; } -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-21 22:55 ` Alan Modra @ 2023-07-22 3:47 ` Tan Yuan 2023-07-22 3:56 ` Xi Ruoyao 0 siblings, 1 reply; 9+ messages in thread From: Tan Yuan @ 2023-07-22 3:47 UTC (permalink / raw) To: amodra; +Cc: binutils, jbeulich, falcon Hi, Thanks for your reply. > On Fri, Jul 21, 2023 at 11:14:20PM +0800, 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. > > Is this alternative patch sufficient for the kernel usage? > > * config/obj-elf.c (obj_elf_attach_to_group): Don't warn if > group name matches current group for section. > > diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c > index 753a929fb14..dc05b35ee99 100644 > --- a/gas/config/obj-elf.c > +++ b/gas/config/obj-elf.c > @@ -1088,8 +1088,9 @@ obj_elf_attach_to_group (int dummy ATTRIBUTE_UNUSED) > > if (elf_group_name (now_seg)) > { > - as_warn (_("section %s already has a group (%s)"), > - bfd_section_name (now_seg), elf_group_name (now_seg)); > + if (strcmp (elf_group_name (now_seg), gname) != 0) > + as_warn (_("section %s already has a group (%s)"), > + bfd_section_name (now_seg), elf_group_name (now_seg)); > return; > } The alternative patch enabling attachment to the same group feasible while keeping the changes minimal. The problem is, when macro which contains ".attach_to_group" expanded multiple times within a function, we cannot make the group name the same. We hope to modify code like this in the kernel: + #define ___PASTE(a,b) a##b + #define __PASTE(a,b) ___PASTE(a,b) + #define __UNIQUE_ID_GROUP __PASTE(__PASTE(__COUNTER__, _), __LINE__) #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ - ".pushsection __ex_table, \"a\"\n" \ + ".attach_to_group " __stringify(__UNIQUE_ID_GROUP) "\n" \ + ".pushsection __ex_table, \"a?\"\n" \ "…" \ ".popsection\n" void example_func() { asm(__ASM_EXTABLE_RAW(insn, fixup, type, data)); asm(__ASM_EXTABLE_RAW(insn, fixup, type, data)); } This leads to the problem that when the macro is expanded in different locations of a function, the group name becomes different. Below, we have provided a detailed explanation as to why it is not feasible to utilize the same group name. We want to find a way to obtain a function-level unique string as the group name. The term "function-level unique" means that in different functions, the macro expands as a different string, and within the same function, it results in the same string. The obvious approach is to use the function name as the function-level unique string. However, "__func__" is not a macro; instead, it is defined as "static const char func[]". We can only obtain "func" during compilation, but not during preprocessing. Thus, I haven't found a way to make something like ".attach_to_group func" work. Or is there any way to get the name of the section and use it as the group name? Alternatively, we have considered using "ifdef" to avoid multiple expansions of ".attach_to_group,". It can be expanded in a file multiple times, and it is supposed to be expanded in a function once, which still requires a function-level unique string as a marker. Therefore, the best solution I can think of at the moment is to add an option to disable the warning for duplicate attach to group operations. I would greatly appreciate any other advice. Thanks Tan Yuan > > > > -- > Alan Modra > Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-22 3:47 ` Tan Yuan @ 2023-07-22 3:56 ` Xi Ruoyao 2023-07-23 9:31 ` Tan Yuan 0 siblings, 1 reply; 9+ messages in thread From: Xi Ruoyao @ 2023-07-22 3:56 UTC (permalink / raw) To: Tan Yuan, amodra; +Cc: binutils, jbeulich, falcon On Sat, 2023-07-22 at 11:47 +0800, Tan Yuan wrote: > > We hope to modify code like this in the kernel: > + #define ___PASTE(a,b) a##b > + #define __PASTE(a,b) ___PASTE(a,b) > + #define __UNIQUE_ID_GROUP __PASTE(__PASTE(__COUNTER__, _), > __LINE__) > > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > - ".pushsection __ex_table, \"a\"\n" \ > + ".attach_to_group " __stringify(__UNIQUE_ID_GROUP) "\n" \ > + ".pushsection __ex_table, \"a?\"\n" \ > "…" \ > ".popsection\n" Note that even if this Binutils patch is accepted, it will only show up in Binutils-2.42 and later. So the kernel maintainers will reject this change, as you cannot tell everyone "use Binutils >= 2.42 to build the kernel". I'd suggest to try figuring out a better solution. -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-22 3:56 ` Xi Ruoyao @ 2023-07-23 9:31 ` Tan Yuan 0 siblings, 0 replies; 9+ messages in thread From: Tan Yuan @ 2023-07-23 9:31 UTC (permalink / raw) To: xry111; +Cc: falcon, binutils Hi, > On Sat, 2023-07-22 at 11:47 +0800, Tan Yuan wrote: > > > > We hope to modify code like this in the kernel: > > + #define ___PASTE(a,b) a##b > > + #define __PASTE(a,b) ___PASTE(a,b) > > + #define __UNIQUE_ID_GROUP __PASTE(__PASTE(__COUNTER__, _), > > __LINE__) > > > > #define __ASM_EXTABLE_RAW(insn, fixup, type, data) \ > > - ".pushsection __ex_table, \"a\"\n" \ > > + ".attach_to_group " __stringify(__UNIQUE_ID_GROUP) "\n" \ > > + ".pushsection __ex_table, \"a?\"\n" \ > > "…" \ > > ".popsection\n" > > Note that even if this Binutils patch is accepted, it will only show up > in Binutils-2.42 and later. So the kernel maintainers will reject this > change, as you cannot tell everyone "use Binutils >= 2.42 to build the > kernel". > > I'd suggest to try figuring out a better solution. > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University Thanks your suggestion and I really need some new idea to solve this problems. This method is the best I can come up with so far. However, I don’t think "Binutils >= 2.42" is a problem, as what I am adding to kernel is an optional feature, user can choose to enable it. And it seems that we can check version of Binutils in the code[1]. [1]: https://stackoverflow.com/a/51008423 Tan Yuan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-21 15:14 [PATCH 0/1] gas: add new command line option --no-group-check Tan Yuan 2023-07-21 22:55 ` Alan Modra @ 2023-07-22 0:32 ` Song Fangrui 1 sibling, 0 replies; 9+ messages in thread From: Song Fangrui @ 2023-07-22 0:32 UTC (permalink / raw) To: Tan Yuan; +Cc: binutils, amodra, jbeulich On Fri, Jul 21, 2023 at 8:14 AM Tan Yuan <tanyuan@tinylab.org> 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. 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. > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <DS7PR12MB5765A326273E125586D742F7CB3CA@DS7PR12MB5765.namprd12.prod.outlook.com>]
* Re: [PATCH 0/1] gas: add new command line option --no-group-check [not found] <DS7PR12MB5765A326273E125586D742F7CB3CA@DS7PR12MB5765.namprd12.prod.outlook.com> @ 2023-07-23 9:10 ` Tan Yuan 2023-07-23 17:15 ` Fangrui Song 2023-07-24 9:28 ` Tan Yuan 1 sibling, 1 reply; 9+ messages in thread From: Tan Yuan @ 2023-07-23 9:10 UTC (permalink / raw) To: i; +Cc: amodra, falcon, binutils Hi, Thanks for your reply and your advice inspired me a lot. On Sat, 22 Jul 2023 at 00:32 Song Fangrui <i@maskray.me> wrote: > On Fri, Jul 21, 2023 at 8:14 AM Tan Yuan <tanyuan@tinylab.org> 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 > > 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 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check 2023-07-23 9:10 ` Tan Yuan @ 2023-07-23 17:15 ` Fangrui Song 0 siblings, 0 replies; 9+ messages in thread From: Fangrui Song @ 2023-07-23 17:15 UTC (permalink / raw) To: Tan Yuan; +Cc: amodra, falcon, binutils On Sun, Jul 23, 2023 at 2:10 AM Tan Yuan <tanyuan@tinylab.org> wrote: > > Hi, > > Thanks for your reply and your advice inspired me a lot. > > On Sat, 22 Jul 2023 at 00:32 Song Fangrui <i@maskray.me> wrote: > > On Fri, Jul 21, 2023 at 8:14 AM Tan Yuan <tanyuan@tinylab.org> 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 Tan, is the following your intended use case? cat > a.s <<eof .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. > > > 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 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/1] gas: add new command line option --no-group-check [not found] <DS7PR12MB5765A326273E125586D742F7CB3CA@DS7PR12MB5765.namprd12.prod.outlook.com> 2023-07-23 9:10 ` Tan Yuan @ 2023-07-24 9:28 ` Tan Yuan 1 sibling, 0 replies; 9+ messages in thread From: Tan Yuan @ 2023-07-24 9:28 UTC (permalink / raw) To: i; +Cc: amodra, falcon, binutils On Mon, Jul 24, 2023 at 1:15 AM Fangrui Song <i@maskray.me> wrote: > On Sun, Jul 23, 2023 at 2:10 AM Tan Yuan <tanyuan@tinylab.org> wrote: > > > > Hi, > > > > Thanks for your reply and your advice inspired me a lot. > > > > On Sat, 22 Jul 2023 at 00:32 Song Fangrui <i@maskray.me> wrote: > > > On Fri, Jul 21, 2023 at 8:14 AM Tan Yuan <tanyuan@tinylab.org> 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 <<eof > .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 > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-07-24 9:28 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-21 15:14 [PATCH 0/1] gas: add new command line option --no-group-check Tan Yuan 2023-07-21 22:55 ` Alan Modra 2023-07-22 3:47 ` Tan Yuan 2023-07-22 3:56 ` Xi Ruoyao 2023-07-23 9:31 ` Tan Yuan 2023-07-22 0:32 ` Song Fangrui [not found] <DS7PR12MB5765A326273E125586D742F7CB3CA@DS7PR12MB5765.namprd12.prod.outlook.com> 2023-07-23 9:10 ` Tan Yuan 2023-07-23 17:15 ` Fangrui Song 2023-07-24 9:28 ` Tan Yuan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).