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