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