RFC: New gas directive: .attach_to_group

RFC: New gas directive: .attach_to_group

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

Hi Guys,

  I am planning on applying the attached patch, but I wondered if anyone
  had any comments or thoughts first.

  The patch adds a new gas directive: ".attach_to_group <name>" which
  can be used to attach the current section to a named group.  I need
  this functionality for the annobin plugin for gcc.  The plugin creates
  new note sections and it wants to place them into section groups with
  their associated code section.  (So the .annobin.text section is
  grouped with the .text section, the .annobin.text.hot section is
  grouped with the .text.hot section and so on).  The problem is that
  the compiler creates the .section directives for (most of) these code
  sections, and it does not add a group section to their declaration.
  So I need a way to retroactively add a section to a group.  Hence the
  new directive.

  Tested with a wide variety of different configurations and no
  problems.

  Thoughts ?

Cheers
  Nick

gas/
	* config/obj-elf (elf_pseudo_table): Add attach_to_group.
        (obj_elf_attach_to_group): New function.
        * doc/as.texi: Document the new directive.
	* NEWS: Mention the new feature.
        * testsuite/gas/elf/attach-1.s: New test.
        * testsuite/gas/elf/attach-1.d: New test driver.
        * testsuite/gas/elf/attach-2.s: New test.
        * testsuite/gas/elf/attach-2.d: New test driver.
        * testsuite/gas/elf/attach-err.s: New test.
        * testsuite/gas/elf/attach-err.d: New test driver.
        * testsuite/gas/elf/attach-err.err: New test error output.
        * testsuite/gas/elf/elf.exp: Run the new tests.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: attach-to-group.patch --]
[-- Type: text/x-patch, Size: 6484 bytes --]

diff --git a/gas/NEWS b/gas/NEWS
index 7ae58506ec..0ef33ab60e 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,4 +1,7 @@
 -*- text -*-
+* Add a .attach-to_group directive which allows a section to added to a group
+  even after the section has been created.
+
 * Add support for Intel TDX instructions.
 
 * Add support for Intel Key Locker instructions.
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index b1c99020a3..5f61204bce 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -78,9 +78,11 @@ static void obj_elf_gnu_attribute (int);
 static void obj_elf_tls_common (int);
 static void obj_elf_lcomm (int);
 static void obj_elf_struct (int);
+static void obj_elf_attach_to_group (int);
 
 static const pseudo_typeS elf_pseudo_table[] =
 {
+  {"attach_to_group", obj_elf_attach_to_group, 0},
   {"comm", obj_elf_common, 0},
   {"common", obj_elf_common, 1},
   {"ident", obj_elf_ident, 0},
@@ -1040,6 +1042,28 @@ obj_elf_section_name (void)
   return name;
 }
 
+static void
+obj_elf_attach_to_group (int dummy ATTRIBUTE_UNUSED)
+{
+  const char * gname = obj_elf_section_name ();
+
+  if (gname == NULL)
+    {
+      as_warn (_("group name not parseable"));
+      return;
+    }
+
+  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));
+      return;
+    }
+
+  elf_group_name (now_seg) = xstrdup (gname);
+  elf_section_flags (now_seg) |= SHF_GROUP;
+}
+
 void
 obj_elf_section (int push)
 {
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index b88c1f9997..c0baa94536 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4362,6 +4362,7 @@ Some machine configurations provide additional directives.
 * Altmacro::                    @code{.altmacro}
 * Ascii::                       @code{.ascii "@var{string}"}@dots{}
 * Asciz::                       @code{.asciz "@var{string}"}@dots{}
+* Attach_to_group::             @code{.attach_to_group @var{name}}
 * Balign::                      @code{.balign [@var{abs-expr}[, @var{abs-expr}]]}
 * Bundle directives::           @code{.bundle_align_mode @var{abs-expr}}, etc
 * Byte::                        @code{.byte @var{expressions}}
@@ -4663,6 +4664,13 @@ trailing zero byte) into consecutive addresses.
 @code{.asciz} is just like @code{.ascii}, but each string is followed by
 a zero byte.  The ``z'' in @samp{.asciz} stands for ``zero''.
 
+@node Attach_to_group
+@section @code{.attach_to_group @var{name}}
+Attaches the current section to the named group.  This is like declaring
+the section with the @code{G} attribute, but can be done after the section
+has been created.  Note if the group section  does not exist at the point that
+this directive is used then it will be created.
+
 @node Balign
 @section @code{.balign[wl] [@var{abs-expr}[, @var{abs-expr}[, @var{abs-expr}]]]}
 
@@ -6663,7 +6671,9 @@ a few exceptions to this rule however.  Processor and application specific
 flags can be added to an already defined section.  The @code{.interp},
 @code{.strtab} and @code{.symtab} sections can have the allocate flag
 (@code{a}) set after they are initially defined, and the @code{.note-GNU-stack}
-section may have the executable (@code{x}) flag added.
+section may have the executable (@code{x}) flag added.  Also note that the
+@code{.attach_to_group} directive can be used to add a section to a group even
+if the section was not originally declared to be part of that group.
 
 The optional @var{type} argument may contain one of the following constants:
 
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index 135ade24ec..5ce9691b38 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -139,6 +139,10 @@ if { [is_elf_format] } then {
     run_dump_test "group1b"
     run_dump_test "group2"
     run_dump_test "group3"
+
+    run_dump_test "attach-1"
+    run_dump_test "attach-err"
+    
     switch -glob $target_triplet {
 	hppa64*-*-hpux* { }
 	riscv*-*-* { }
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-1.s	2020-09-29 14:30:06.242727518 +0100
@@ -0,0 +1,11 @@
+	.text
+	.nop
+	
+	.section foo, "G", %progbits , foo.group
+	.word 0
+
+	.text
+	/* This is the intended use of the .attach_to_group directive.
+	   It attaches a previously defined section (.text) to a
+	   previously defined group (foo.group).  */
+	.attach_to_group foo.group
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-1.d	2020-09-29 15:53:50.914680109 +0100
@@ -0,0 +1,11 @@
+#readelf: --section-groups
+#name: Attaching a section to a group
+#source: attach-1.s
+
+#...
+group section \[    1\] `\.group' \[foo\.group\] contains . sections:
+   \[Index\]    Name
+   \[    .\]   [P|\.text]
+   \[    .\]   foo
+#pass
+
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-2.d	2020-09-29 14:56:01.546024741 +0100
@@ -0,0 +1,11 @@
+#readelf: --section-groups
+#name: Attaching a section to a non-existant group
+#source: attach-2.s
+
+#...
+group section \[    1\] `\.group' \[foo\.group\] contains 2 sections:
+   \[Index\]    Name
+   \[    .\]   bar
+   \[    .\]   foo
+#pass
+
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-2.s	2020-09-29 14:53:25.041006132 +0100
@@ -0,0 +1,9 @@
+	.section bar
+	.nop
+	.attach_to_group foo.group
+	
+	.section foo, "G", %note , foo.group
+	.word 0
+
+	.section bar
+	.nop
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-err.d	2020-09-29 14:48:14.501953361 +0100
@@ -0,0 +1,3 @@
+#name: Errors generated by .attach_to_group
+#source: attach-err.s
+#error_output: attach-err.err
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-err.err	2020-09-29 15:54:35.415401125 +0100
@@ -0,0 +1,4 @@
+.*: Assembler messages:
+.*:4: Warning: section [P|\.text] already has a group \(does\.not\.exist\)
+.*:5: Error: missing name
+.*:5: Warning: group name not parseable
--- /dev/null	2020-09-29 09:19:37.528268715 +0100
+++ current/gas/testsuite/gas/elf/attach-err.s	2020-09-29 14:45:34.540949111 +0100
@@ -0,0 +1,5 @@
+
+	/* Test the error messages that should be generated.  */
+	.attach_to_group does.not.exist  /* This is OK, the group does not have to exist.  */
+	.attach_to_group foo.group  /* Already attached.  */
+	.attach_to_group  /* Missing group name.  */

Re: RFC: New gas directive: .attach_to_group

[Fangrui Song]

On 2020-09-29, Nick Clifton via Binutils wrote:
>Hi Guys,
>
>  I am planning on applying the attached patch, but I wondered if anyone
>  had any comments or thoughts first.
>
>  The patch adds a new gas directive: ".attach_to_group <name>" which
>  can be used to attach the current section to a named group.  I need
>  this functionality for the annobin plugin for gcc.  The plugin creates
>  new note sections and it wants to place them into section groups with
>  their associated code section.  (So the .annobin.text section is
>  grouped with the .text section, the .annobin.text.hot section is
>  grouped with the .text.hot section and so on).  The problem is that
>  the compiler creates the .section directives for (most of) these code
>  sections, and it does not add a group section to their declaration.
>  So I need a way to retroactively add a section to a group.  Hence the
>  new directive.
>
>  Tested with a wide variety of different configurations and no
>  problems.
>
>  Thoughts ?

Hi Nick,

Is this directive an extension to the existing feature: ?

   .section .text.foo,"axG",@progbits,group
   
   .section .rodata.foo,"a?"

If the section group intention of .annobin.text is so that .annobin.text
can be discarded if the associated .text is discarded (--gc-sections),
an alternative is SHF_LINK_ORDER

   HJ has done much work on this.
   There are a few additions:
   https://sourceware.org/pipermail/binutils/2020-August/112732.html

Re: RFC: New gas directive: .attach_to_group

[Nick Clifton]

Hi Fangrui, 
> Is this directive an extension to the existing feature: ?

Yes.

>   .section .text.foo,"axG",@progbits,group
>     .section .rodata.foo,"a?"

The idea being that new directive allows the section group to be
declared at a later date, after the section has been declared.  This
is important because two different code generators are involved.  GCC
is generating the section declarations and annobin is generating the
section groups.

> If the section group intention of .annobin.text is so that .annobin.text
> can be discarded if the associated .text is discarded (--gc-sections),

It is.

> an alternative is SHF_LINK_ORDER

Hmm, interesting.  I shall investigate and see if I can adapt annobin to
use this method.  (Which presumably will work with LLVM's assembler as well
as gas).

Cheers
  Nick

PS. I see that PR 26253 has not been addressed, sorry about that.  I will
try to have a look at it later this week.

[PATCH] elf: Also check linked-to section for SHT_NOTE section

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]

On Wed, Sep 30, 2020 at 3:56 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Fangrui, Hi H.J.,
>
> > If the section group intention of .annobin.text is so that .annobin.text
> > can be discarded if the associated .text is discarded (--gc-sections),
> > an alternative is SHF_LINK_ORDER
>
>   Sorry to bother you with this, but I am having some troubles
>   getting to grip with using SHF_LINK_ORDER to eliminate unused
>   sections.  I create a small test file to compare using groups
>   and link_orders but it does not work as I would expect:
>
>   % cat eliminate.s
>         .text
>         .nop
>
>         .section .unused1, "ax", %progbits
>         .nop
>
>         .section .unused2, "axG", %progbits, unused_group
>         .nop
>
>         .section .gnu.note.text, "", %note
>         .word 1
>
>         .section .gnu.note1, "o", %note, .unused1
>         .word 2
>
>         .section .gnu.note2, "G", %note, unused_group
>         .word 3
>
>   % as eliminate.s -o elminate.o
>   % ld -e0 --gc-sections --print-gc-sections elminate.o
>   ld: removing unused section '.group' in file 'eliminate.o'
>   ld: removing unused section '.text' in file 'eliminate.o'
>   ld: removing unused section '.unused1' in file 'eliminate.o'
>   ld: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
>   ld: removing unused section '.gnu.note2[unused_group]' in file 'eliminate.o'
>   ld: a.out: sh_link of section `.gnu.note1' points to discarded section `.unused1' of `eliminate.o'
>   ld/: final link failed: bad value
>
>   I thought that the sh_link from .gnu.note1 to .unused1 would mean that it
>   should be discarded when .unused1 is discarded.  But instead I get an
>   error.
>
>   I am probably doing something silly, but please could you point me in the
>   right direction ?
>

Please try this patch.

-- 
H.J.

[-- Attachment #2: 0001-elf-Also-check-linked-to-section-for-SHT_NOTE-sectio.patch --]
[-- Type: application/x-patch, Size: 2576 bytes --]

Re: [PATCH] elf: Also check linked-to section for SHT_NOTE section

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

Hi H.J.

  Thanks for taking an interest in this.

> Please try this patch.
  It did not appear to make a difference. :-(

  I am using an extended test case (attached) but the results are the
  same as before:

  % as eliminate.s -o eliminate.o
  % ld -e 0 --gc-sections --print-gc-sections eliminate.o 
  ld: removing unused section '.group' in file 'eliminate.o'
  ld: removing unused section '.group' in file 'eliminate.o'
  ld: removing unused section '.text' in file 'eliminate.o'
  ld: removing unused section '.unused1' in file 'eliminate.o'
  ld: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
  ld: removing unused section '.unused3[unused_group2]' in file 'eliminate.o'
  ld: removing unused section '.gnu.note2[unused_group]' in file 'eliminate.o'
  ld: removing unused section '.gnu.note3[unused_group2]' in file 'eliminate.o'
  ld: a.out: sh_link of section `.gnu.note1' points to discarded section `.unused1' of `eliminate.o'
  ld: final link failed: bad value

Cheers
  Nick

[-- Attachment #2: eliminate.s --]
[-- Type: text/plain, Size: 401 bytes --]

	.text
	.nop
	
	.section .unused1, "ax", %progbits
	.nop
	
	.section .unused2, "axG", %progbits, unused_group
	.nop

	.section .unused3, "axG", %progbits, unused_group2
	.nop
	
	.section .gnu.note.text, "", %note
	.word 1

	.section .gnu.note1, "o", %note, .unused1
	.word 2

	.section .gnu.note2, "G", %note, unused_group
	.word 3

	.section .gnu.note3, "oG", %note, .unused3, unused_group2
	.word 4

Re: [PATCH] elf: Also check linked-to section for SHT_NOTE section

[H.J. Lu]

On Wed, Sep 30, 2020 at 7:01 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi H.J.
>
>   Thanks for taking an interest in this.
>
> > Please try this patch.
>   It did not appear to make a difference. :-(
>
>   I am using an extended test case (attached) but the results are the
>   same as before:
>
>   % as eliminate.s -o eliminate.o
>   % ld -e 0 --gc-sections --print-gc-sections eliminate.o
>   ld: removing unused section '.group' in file 'eliminate.o'
>   ld: removing unused section '.group' in file 'eliminate.o'
>   ld: removing unused section '.text' in file 'eliminate.o'
>   ld: removing unused section '.unused1' in file 'eliminate.o'
>   ld: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
>   ld: removing unused section '.unused3[unused_group2]' in file 'eliminate.o'
>   ld: removing unused section '.gnu.note2[unused_group]' in file 'eliminate.o'
>   ld: removing unused section '.gnu.note3[unused_group2]' in file 'eliminate.o'
>   ld: a.out: sh_link of section `.gnu.note1' points to discarded section `.unused1' of `eliminate.o'
>   ld: final link failed: bad value
>
> Cheers
>   Nick

Works for me:

[hjl@gnu-cfl-2 ld]$ gcc -c eliminate.s
[hjl@gnu-cfl-2 ld]$ ./ld-new -e 0 --gc-sections --print-gc-sections eliminate.o
./ld-new: removing unused section '.group' in file 'eliminate.o'
./ld-new: removing unused section '.group' in file 'eliminate.o'
./ld-new: removing unused section '.text' in file 'eliminate.o'
./ld-new: removing unused section '.unused1' in file 'eliminate.o'
./ld-new: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
./ld-new: removing unused section '.unused3[unused_group2]' in file
'eliminate.o'
./ld-new: removing unused section '.gnu.note1' in file 'eliminate.o'
./ld-new: removing unused section '.gnu.note2[unused_group]' in file
'eliminate.o'
./ld-new: removing unused section '.gnu.note3[unused_group2]' in file
'eliminate.o'
[hjl@gnu-cfl-2 ld]$


-- 
H.J.

Re: [PATCH] elf: Also check linked-to section for SHT_NOTE section

[Fangrui Song]

On Wed, Sep 30, 2020 at 7:03 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Sep 30, 2020 at 7:01 AM Nick Clifton <nickc@redhat.com> wrote:
> >
> > Hi H.J.
> >
> >   Thanks for taking an interest in this.
> >
> > > Please try this patch.
> >   It did not appear to make a difference. :-(
> >
> >   I am using an extended test case (attached) but the results are the
> >   same as before:
> >
> >   % as eliminate.s -o eliminate.o
> >   % ld -e 0 --gc-sections --print-gc-sections eliminate.o
> >   ld: removing unused section '.group' in file 'eliminate.o'
> >   ld: removing unused section '.group' in file 'eliminate.o'
> >   ld: removing unused section '.text' in file 'eliminate.o'
> >   ld: removing unused section '.unused1' in file 'eliminate.o'
> >   ld: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
> >   ld: removing unused section '.unused3[unused_group2]' in file 'eliminate.o'
> >   ld: removing unused section '.gnu.note2[unused_group]' in file 'eliminate.o'
> >   ld: removing unused section '.gnu.note3[unused_group2]' in file 'eliminate.o'
> >   ld: a.out: sh_link of section `.gnu.note1' points to discarded section `.unused1' of `eliminate.o'
> >   ld: final link failed: bad value
> >
> > Cheers
> >   Nick
>
> Works for me:
>
> [hjl@gnu-cfl-2 ld]$ gcc -c eliminate.s
> [hjl@gnu-cfl-2 ld]$ ./ld-new -e 0 --gc-sections --print-gc-sections eliminate.o
> ./ld-new: removing unused section '.group' in file 'eliminate.o'
> ./ld-new: removing unused section '.group' in file 'eliminate.o'
> ./ld-new: removing unused section '.text' in file 'eliminate.o'
> ./ld-new: removing unused section '.unused1' in file 'eliminate.o'
> ./ld-new: removing unused section '.unused2[unused_group]' in file 'eliminate.o'
> ./ld-new: removing unused section '.unused3[unused_group2]' in file
> 'eliminate.o'
> ./ld-new: removing unused section '.gnu.note1' in file 'eliminate.o'
> ./ld-new: removing unused section '.gnu.note2[unused_group]' in file
> 'eliminate.o'
> ./ld-new: removing unused section '.gnu.note3[unused_group2]' in file
> 'eliminate.o'
> [hjl@gnu-cfl-2 ld]$
>
>
> --
> H.J.

Thanks HJ for the patch. The --gc-sections semantics I expect are
written down here:
https://sourceware.org/pipermail/binutils/2020-February/109851.html

(If SHF_GNU_RETAIN
(https://sourceware.org/pipermail/binutils/2020-September/113534.html)
is accepted, a SHF_GNU_RETAIN section will be added to the GC root
list.
I forwarded the progress to a few toolchain folks on LLVM side and
encouraged them to reply if they still have concerns.)