[PATCH 0/1] gas: add new command line option --no-group-check

[PATCH 0/1] gas: add new command line option --no-group-check

[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

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Alan Modra]

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

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Song Fangrui]

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
>

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Tan Yuan]

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

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Xi Ruoyao]

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

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Tan Yuan]

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

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Tan Yuan]

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
> > > >
> 
> 

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Fangrui Song]

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
> > >

Re: [PATCH 0/1] gas: add new command line option --no-group-check

[Tan Yuan]

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
> >