RFC: GAS: Add option to generate unused section symbols

RFC: GAS: Add option to generate unused section symbols

[Nick Clifton]

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

Hi Guys,

  With the 2.36 release of the binutils we stopped generating section
  symbols if they were not needed by relocations.  This helps to reduce
  object file size and enhances compatibility with the LLVM compiler.
  But I have received some bug reports about this feature, most notably
  from kernel maintainers who have found that the new behaviour breaks
  the recordmcount tool.  So I am proposing the attached patch as a
  solution.

  The patch adds an option to the assembler: --keep-unused-section-symbols=[yes|no]
  which is pretty much self describing.  The patch itself is fairly
  simple, although there is one area that might provoke comment:  Since
  the xvec structure in a BFD is accessed via a const pointer, its
  contents cannot be changed.  This makes sense most of the time, but
  poses a problem for this patch since it wants to change the
  keep_unused_section_symbols field.  So I decided that the simplest way
  to resolve this would be to allocate a new xvec structure, fill it in,
  and then change the pointer in the bfd.  This might be a bit wasteful
  of memory, but I am not expecting this feature to be used very often
  and I thought that it was better to preserve the const pointer
  semantics.

  Any comments ?

Cheers
  Nick

bfd/ChangeLog
2022-03-04  Nick Clifton  <nickc@redhat.com>

	* targets.c (bfd_set_keep_unused_section_symbols): New function.
	* bfd-in2.h: Rgenerate.

gas/ChangeLog
2022-03-04  Nick Clifton  <nickc@redhat.com>

	* as.c (keep_unused_sec_syms): New local variable.
        (parse_args): Add --keep-unused-section-symbols=[yes|no].
        (show_usage): Mention the new option.
        (main): Call bfd_set_keep_unused_section_symbols if requested.
        * doc/as.texi: Document the new option.
        * NEWS: Mention the new feature.
        * testsuite/gas/elf/unused-section-symbols.s: New source file.
        * testsuite/gas/elf/unused-section-symbols.d: New test.
        * testsuite/gas/elf/used-section-symbols.d: New test.
        * testsuite/gas/elf/elf.exp: Run the new tests.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: unused-sec-syms.patch --]
[-- Type: text/x-patch, Size: 8282 bytes --]

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index db41e7eb7fe..f4727f0e08b 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7856,6 +7856,24 @@ bfd_keep_unused_section_symbols (const bfd *abfd)
   return abfd->xvec->keep_unused_section_symbols;
 }
 
+static inline bool
+bfd_set_keep_unused_section_symbols (bfd *abfd, bool val)
+{
+  if (abfd == NULL || abfd->xvec == NULL)
+    return false;
+  if (abfd->xvec->keep_unused_section_symbols == val)
+    return true;
+  /* The xvec structure is deliberately set to const, so we have
+     to work around that here.  Since using this function should
+     be rare, this solution is better than making the default
+     structures modifiable.  */
+  struct bfd_target * new_vec = bfd_alloc (abfd, sizeof * new_vec);
+  memcpy (new_vec, abfd->xvec, sizeof * new_vec);
+  new_vec->keep_unused_section_symbols = val;
+  abfd->xvec = new_vec;
+  return true;
+}
+
 bool bfd_set_default_target (const char *name);
 
 const bfd_target *bfd_find_target (const char *target_name, bfd *abfd);
diff --git a/bfd/targets.c b/bfd/targets.c
index 18fec45f02a..9783f0474a2 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -666,6 +666,24 @@ to find an alternative output format that is suitable.
 .  return abfd->xvec->keep_unused_section_symbols;
 .}
 .
+.static inline bool
+.bfd_set_keep_unused_section_symbols (bfd *abfd, bool val)
+.{
+.  if (abfd == NULL || abfd->xvec == NULL)
+.    return false;
+.  if (abfd->xvec->keep_unused_section_symbols == val)
+.    return true;
+.  {* The xvec structure is deliberately set to const, so we have
+.     to work around that here.  Since using this function should
+.     be rare, this solution is better than making the default
+.     structures modifiable.  *}
+.  struct bfd_target * new_vec = bfd_alloc (abfd, sizeof * new_vec);
+.  memcpy (new_vec, abfd->xvec, sizeof * new_vec);
+.  new_vec->keep_unused_section_symbols = val;
+.  abfd->xvec = new_vec;
+.  return true;
+.}
+.
 */
 
 /* All known xvecs (even those that don't compile on all systems).
diff --git a/gas/NEWS b/gas/NEWS
index 924a73780c0..334cc3f2c9c 100644
--- a/gas/NEWS
+++ b/gas/NEWS
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* Add --kep-unused-section-symbols=[yes|no] option to the assembler.
+
 Changes in 2.38:
 
 * Add support for AArch64 system registers that were missing in previous
diff --git a/gas/as.c b/gas/as.c
index f320bac89b5..8732c78f59e 100644
--- a/gas/as.c
+++ b/gas/as.c
@@ -139,6 +139,8 @@ static long start_time;
 
 static int flag_macro_alternate;
 
+/* Tri-state variable.  -1: uninitialised, 0: false, 1: true.  */
+static int keep_unused_sec_syms = -1;
 \f
 #ifdef USE_EMULATIONS
 #define EMULATION_ENVIRON "AS_EMULATION"
@@ -408,6 +410,10 @@ Options:\n\
   fprintf (stream, _("\
   @FILE                   read options from FILE\n"));
 
+  fprintf (stream, _("\
+  --keep-unused-section-symbols=[yes|no]\n\
+                          do/do not generate section symbols even if they are not needed\n"));
+  
   md_show_usage (stream);
 
   fputc ('\n', stream);
@@ -504,7 +510,8 @@ parse_args (int * pargc, char *** pargv)
       OPTION_COMPRESS_DEBUG,
       OPTION_NOCOMPRESS_DEBUG,
       OPTION_NO_PAD_SECTIONS,
-      OPTION_MULTIBYTE_HANDLING  /* = STD_BASE + 40 */
+      OPTION_MULTIBYTE_HANDLING,  /* = STD_BASE + 40 */
+      OPTION_KEEP_UNUSED_SECTION_SYMBOLS
     /* When you add options here, check that they do
        not collide with OPTION_MD_BASE.  See as.h.  */
     };
@@ -565,24 +572,25 @@ parse_args (int * pargc, char *** pargv)
        ports use -k to enable PIC assembly.  */
     ,{"keep-locals", no_argument, NULL, 'L'}
     ,{"keep-locals", no_argument, NULL, 'L'}
+    ,{"keep-unused-section-symbols", required_argument, NULL, OPTION_KEEP_UNUSED_SECTION_SYMBOLS}
     ,{"listing-lhs-width", required_argument, NULL, OPTION_LISTING_LHS_WIDTH}
     ,{"listing-lhs-width2", required_argument, NULL, OPTION_LISTING_LHS_WIDTH2}
     ,{"listing-rhs-width", required_argument, NULL, OPTION_LISTING_RHS_WIDTH}
     ,{"listing-cont-lines", required_argument, NULL, OPTION_LISTING_CONT_LINES}
     ,{"MD", required_argument, NULL, OPTION_DEPFILE}
     ,{"mri", no_argument, NULL, 'M'}
+    ,{"multibyte-handling", required_argument, NULL, OPTION_MULTIBYTE_HANDLING}
     ,{"nocpp", no_argument, NULL, OPTION_NOCPP}
     ,{"no-pad-sections", no_argument, NULL, OPTION_NO_PAD_SECTIONS}
     ,{"no-warn", no_argument, NULL, 'W'}
     ,{"reduce-memory-overheads", no_argument, NULL, OPTION_REDUCE_MEMORY_OVERHEADS}
     ,{"statistics", no_argument, NULL, OPTION_STATISTICS}
     ,{"strip-local-absolute", no_argument, NULL, OPTION_STRIP_LOCAL_ABSOLUTE}
+    ,{"traditional-format", no_argument, NULL, OPTION_TRADITIONAL_FORMAT}
     ,{"version", no_argument, NULL, OPTION_VERSION}
     ,{"verbose", no_argument, NULL, OPTION_VERBOSE}
     ,{"target-help", no_argument, NULL, OPTION_TARGET_HELP}
-    ,{"traditional-format", no_argument, NULL, OPTION_TRADITIONAL_FORMAT}
     ,{"warn", no_argument, NULL, OPTION_WARN}
-    ,{"multibyte-handling", required_argument, NULL, OPTION_MULTIBYTE_HANDLING}
   };
 
   /* Construct the option lists from the standard list and the target
@@ -1117,6 +1125,16 @@ This program has absolutely no warranty.\n"));
 
 	case OPTION_HASH_TABLE_SIZE:
 	  break;
+
+	case OPTION_KEEP_UNUSED_SECTION_SYMBOLS:
+	  if (strcasecmp (optarg, "no") == 0)
+	    keep_unused_sec_syms = false;
+	  else if (strcasecmp (optarg, "yes") == 0)
+	    keep_unused_sec_syms = true;
+	  else
+	    as_fatal (_("Invalid --keep-unused-section-sybmols option: `%s'"),
+		      optarg);
+	  break;
 	}
     }
 
@@ -1376,6 +1394,10 @@ main (int argc, char ** argv)
   output_file_create (out_file_name);
   gas_assert (stdoutput != 0);
 
+  if (keep_unused_sec_syms != -1
+      && ! bfd_set_keep_unused_section_symbols (stdoutput, keep_unused_sec_syms))
+    as_fatal (_("unable to set unused section symbol behaviour"));
+  
   dot_symbol_init ();
 
 #ifdef tc_init_after_args
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 55b3156f1fa..a52df2c8f34 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -246,6 +246,7 @@ gcc(1), ld(1), and the Info entries for @file{binutils} and @file{ld}.
  [@b{--elf-stt-common=[no|yes]}]
  [@b{--generate-missing-build-notes=[no|yes]}]
  [@b{--multibyte-handling=[allow|warn|warn-sym-only]}]
+ [@b{--keep-unused-section-symbols=[yes|no]}]
  [@b{--target-help}] [@var{target-options}]
  [@b{--}|@var{files} @dots{}]
 @c
@@ -847,6 +848,11 @@ This option is accepted but has no effect on the @value{TARGET} family.
 Issue warnings when difference tables altered for long displacements.
 @end ifset
 
+@item --keep-unused-section-symbols=@var{yes|no}
+Instruct the assembler to generate or discard unused section symbols.  The
+default for most targets is to omit section symbols that are not referenced by
+relocations, but this option can be used to ensure a certain behaviour.
+
 @item -L
 @itemx --keep-locals
 Keep (in the symbol table) local symbols.  These symbols start with
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index aacecb79daf..cc3dd8e046a 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -338,5 +338,7 @@ if { [is_elf_format] } then {
 
     run_dump_test "bignums"
     run_dump_test "section-symbol-redef"
+    run_dump_test "unused-section-symbols"
+    run_dump_test "used-section-symbols"
     run_dump_test "pr27228"
 }
--- /dev/null	2022-03-04 07:47:47.418599136 +0000
+++ current/gas/testsuite/gas/elf/unused-section-symbols.s	2022-03-04 13:33:46.885508007 +0000
@@ -0,0 +1,5 @@
+	.section .text
+	.dc.a	label
+	.section .data
+label:
+	.word 2
--- /dev/null	2022-03-04 07:47:47.418599136 +0000
+++ current/gas/testsuite/gas/elf/unused-section-symbols.d	2022-03-04 13:44:46.845885157 +0000
@@ -0,0 +1,9 @@
+#as: --keep-unused-section-symbols=yes
+#objdump: --syms
+
+.*/unused-section-symbols.o:.*
+#...
+.*.text.*
+#...
+.*.data.*
+#pass
--- /dev/null	2022-03-04 07:47:47.418599136 +0000
+++ current/gas/testsuite/gas/elf/used-section-symbols.d	2022-03-04 13:47:23.791784996 +0000
@@ -0,0 +1,11 @@
+#source: unused-section-symbols.s
+#as: --keep-unused-section-symbols=no
+#objdump: --syms
+
+#failif
+.*used-section-symbols.o:.*
+#...
+.*.text.*
+#...
+
+

Re: RFC: GAS: Add option to generate unused section symbols

[H.J. Lu]

On Fri, Mar 4, 2022 at 7:54 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>
> Hi Guys,
>
>   With the 2.36 release of the binutils we stopped generating section
>   symbols if they were not needed by relocations.  This helps to reduce
>   object file size and enhances compatibility with the LLVM compiler.
>   But I have received some bug reports about this feature, most notably
>   from kernel maintainers who have found that the new behaviour breaks
>   the recordmcount tool.  So I am proposing the attached patch as a

I ran into this problem in the Linux kernel build.   I believe the proper
solution is to change the Linux kernel not to include the .o files which
are empty and unused.  It isn't so hard to do.

>   solution.
>
>   The patch adds an option to the assembler: --keep-unused-section-symbols=[yes|no]
>   which is pretty much self describing.  The patch itself is fairly
>   simple, although there is one area that might provoke comment:  Since
>   the xvec structure in a BFD is accessed via a const pointer, its
>   contents cannot be changed.  This makes sense most of the time, but
>   poses a problem for this patch since it wants to change the
>   keep_unused_section_symbols field.  So I decided that the simplest way
>   to resolve this would be to allocate a new xvec structure, fill it in,
>   and then change the pointer in the bfd.  This might be a bit wasteful
>   of memory, but I am not expecting this feature to be used very often
>   and I thought that it was better to preserve the const pointer
>   semantics.
>
>   Any comments ?
>
> Cheers
>   Nick


-- 
H.J.

Re: RFC: GAS: Add option to generate unused section symbols

[Michael Matz]

Hello,

On Fri, 4 Mar 2022, Nick Clifton via Binutils wrote:

>   With the 2.36 release of the binutils we stopped generating section
>   symbols if they were not needed by relocations.  This helps to reduce
>   object file size and enhances compatibility with the LLVM compiler.
>   But I have received some bug reports about this feature, most notably
>   from kernel maintainers who have found that the new behaviour breaks
>   the recordmcount tool.

This problem was fixed in the kernel (and backported by distros to their 
kernels if binutils was updated); it's fairly simplistic changes.

I don't see the need for returning (optionally) to the old behaviour 
again.  I mean, at the time, when 2.36 came out, sure, the patch might 
have made sense, but now?


Ciao,
Michael.

Re: RFC: GAS: Add option to generate unused section symbols

[Nick Clifton]

Hi Guys,

On 3/4/22 16:19, Michael Matz wrote:
> This problem was fixed in the kernel (and backported by distros to their
> kernels if binutils was updated); it's fairly simplistic changes.
> 
> I don't see the need for returning (optionally) to the old behaviour
> again.  I mean, at the time, when 2.36 came out, sure, the patch might
> have made sense, but now?


Great - I was actually hoping that this would be the response.  But I was
not sure how the kernel folks felt about the issue, so I thought that it
would be worth generating a patch just in case.  (Besides it was fun to
write).  I am quite happy to set it aside however.

Cheers
   Nick