Re: [PATCH] ld: Allow section groups to be resolved as part of a relocatable link

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Alan Modra <amodra@gmail.com> [2017-06-05 10:23:25 +0930]:

> On Thu, May 25, 2017 at 03:24:32PM +0100, Andrew Burgess wrote:
> > I've updated the patch to remove all trace (I believe) of the INHIBIT
> > side of things.  The only _new_ code is a couple of asserts inside bfd
> > that if we're _not_ resolving, then we _must_ be doing a relocatable
> > link.
> > 
> > Would you be happy with this version?
> 
> Yes, looks OK to commit.

Thanks for all the reviews on this, I appreciate you help.

Below is the part of the original patch that was removed (allows
inhibiting of group allocation).  As you pointed out this created
invalid ELFs, so I'm NOT proposing this patch for inclusion (so no
review needed), but I thought I'd post this just in case its ever of
use to anyone.

Again, thanks for all your help,

Andrew

---

ld: Add support for preventing group allocation

Adds new command line flag --inhibit-group-allocation and a linker
script command INHIBIT_GROUP_ALLOCATION, these have the effect of
preventing section groups from being resolved in a final link.

Using this flag will result in an ELF that does not comply with the gABI
specification of an ELF, the specification clearly says that a section
of type SHT_GROUP can only appear in a relocatable ELF.

ld/ChangeLog:

	* ld.h (enum group_allocation): New enum.
	(struct args_type): Change type of group_allocation field.
	* ldgram.y: Add support for FORCE_GROUP_ALLOCATION and
	INHIBIT_GROUP_ALLOCATION.
	* ldlex.h: Likewise.
	* ldlex.l: Likewise.
	* lexsup.c: Likewise.
	* ldlang.c (unique_section_p): Check resolve_section_groups flag
	not the relaxable link flag.
	(lang_add_section): Discard section groups when we're resolving
	groups.  Clear the SEC_LINK_ONCE flag if we're resolving section
	groups.
	* ldmain.c (main): Initialise resolve_section_groups flag in
	link_info based on command line flags.
	* testsuite/ld-elf/group13.d: New file.
	* testsuite/ld-elf/group13.ld: New file.
	* testsuite/ld-elf/group14.d: New file.
	* ld.texinfo (Options): Document --inhibit-group-allocation.
	(Miscellaneous Commands): Document INHIBIT_GROUP_ALLOCATION.
	* NEWS: Document new feature.
---
 ld/ChangeLog                   | 23 +++++++++++++++++++++++
 ld/NEWS                        |  5 +++++
 ld/ld.h                        | 16 ++++++++++++----
 ld/ld.texinfo                  | 25 +++++++++++++++++++++++++
 ld/ldgram.y                    |  5 ++++-
 ld/ldlex.h                     |  1 +
 ld/ldlex.l                     |  1 +
 ld/ldmain.c                    |  5 +++--
 ld/lexsup.c                    |  8 +++++++-
 ld/testsuite/ld-elf/group13.d  | 10 ++++++++++
 ld/testsuite/ld-elf/group13.ld | 14 ++++++++++++++
 ld/testsuite/ld-elf/group14.d  | 10 ++++++++++
 12 files changed, 115 insertions(+), 8 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/group13.d
 create mode 100644 ld/testsuite/ld-elf/group13.ld
 create mode 100644 ld/testsuite/ld-elf/group14.d

diff --git a/ld/NEWS b/ld/NEWS
index 98055b5f4e..aa894f186e 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -20,6 +20,11 @@
   option --force-group-allocation or by placing FORCE_GROUP_ALLOCATION into the
   linker script.
 
+* Resolution of section groups can be prevented in a final link using the
+  --inhibit-group-allocation command line flag, or the linker script command
+  INHIBIT_GROUP_ALLOCATION.  Using these will result in section groups in a fully
+  linked ELF, which is non-standard compliant (and therefore an invalid ELF).
+
 Changes in 2.28:
 
 * The EXCLUDE_FILE linker script construct can now be applied outside of the
diff --git a/ld/ld.h b/ld/ld.h
index 162e156f08..46b4792625 100644
--- a/ld/ld.h
+++ b/ld/ld.h
@@ -127,6 +127,13 @@ enum dynamic_list_enum
   dynamic_list
 };
 
+enum group_allocation
+{
+  group_allocation_unset = 0,
+  group_allocation_force,
+  group_allocation_inhibit
+};
+
 typedef struct
 {
   /* 1 => assign space to common symbols even if `relocatable_output'.  */
@@ -172,10 +179,11 @@ typedef struct
   /* If set, display the target memory usage (per memory region).  */
   bfd_boolean print_memory_usage;
 
-  /* Should we force section groups to be resolved?  Controlled with
-     --force-group-allocation on the command line or FORCE_GROUP_ALLOCATION
-     in the linker script.  */
-  bfd_boolean force_group_allocation;
+  /* How are sections within groups handled.  Controlled with
+     --force-group-allocation and --inhibit-group-allocation on the command
+     line, or FORCE_GROUP_ALLOCATION and INHIBIT_GROUP_ALLOCATION in the
+     linker script.  */
+  enum group_allocation group_allocation;
 
   /* Big or little endian as set on command line.  */
   enum endian_enum endian;
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 790b52f025..7a05319a99 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -1496,6 +1496,19 @@
 command @code{FORCE_GROUP_ALLOCATION} has the same
 effect. @xref{Miscellaneous Commands}.
 
+@cindex group allocation in linker script
+@cindex section groups
+@cindex COMDAT
+@kindex --inhibit-group-allocation
+@item --inhibit-group-allocation
+This option prevents the linker from resolving sections groups,
+sections that are part of a group remain group members in the output
+file and the section groups are retained.  This is the default
+behaviour for a relocatable link (@samp{-r}) but this option can be
+used to change the behaviour of a final link.  The script command
+@code{INHIBIT_GROUP_ALLOCATION} has the same
+effect. @xref{Miscellaneous Commands}.
+
 @cindex symbols, from command line
 @kindex --defsym=@var{symbol}=@var{exp}
 @item --defsym=@var{symbol}=@var{expression}
@@ -3749,6 +3762,18 @@
 and to delete the section groups even if a relocatable output file is
 specified (@samp{-r}).
 
+@item INHIBIT_GROUP_ALLOCATION
+@kindex INHIBIT_GROUP_ALLOCATION
+@cindex group allocation in linker script
+@cindex section groups
+@cindex COMDAT
+This command has the same effect as the
+@samp{--inhibit-group-allocation} command-line option: @command{ld}
+will preserve section groups in the output file even if a final link
+is performed.  Section group members will not be placed like normal
+input sections but will instead be left as unique sections in the
+output file.
+
 @item INSERT [ AFTER | BEFORE ] @var{output_section}
 @kindex INSERT
 @cindex insert user script into default script
diff --git a/ld/ldgram.y b/ld/ldgram.y
index 4c1efdc2ed..7ceff2434e 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -132,6 +132,7 @@ static int error_index;
 %token '{' '}'
 %token SIZEOF_HEADERS OUTPUT_FORMAT FORCE_COMMON_ALLOCATION OUTPUT_ARCH
 %token INHIBIT_COMMON_ALLOCATION FORCE_GROUP_ALLOCATION
+%token INHIBIT_GROUP_ALLOCATION
 %token SEGMENT_START
 %token INCLUDE
 %token MEMORY
@@ -337,9 +338,11 @@ ifile_p1:
 	|	FORCE_COMMON_ALLOCATION
 		{ command_line.force_common_definition = TRUE ; }
 	|	FORCE_GROUP_ALLOCATION
-		{ command_line.force_group_allocation = TRUE ; }
+		{ command_line.group_allocation = group_allocation_force ; }
 	|	INHIBIT_COMMON_ALLOCATION
 		{ command_line.inhibit_common_definition = TRUE ; }
+	|	INHIBIT_GROUP_ALLOCATION
+		{command_line.group_allocation = group_allocation_inhibit ; }
 	|	INPUT '(' input_list ')'
 	|	GROUP
 		  { lang_enter_group (); }
diff --git a/ld/ldlex.h b/ld/ldlex.h
index 5aa7f6bc3e..4f8bf70e77 100644
--- a/ld/ldlex.h
+++ b/ld/ldlex.h
@@ -147,6 +147,7 @@ enum option_values
   OPTION_REQUIRE_DEFINED_SYMBOL,
   OPTION_ORPHAN_HANDLING,
   OPTION_FORCE_GROUP_ALLOCATION,
+  OPTION_INHIBIT_GROUP_ALLOCATION,
 };
 
 /* The initial parser states.  */
diff --git a/ld/ldlex.l b/ld/ldlex.l
index ba618ecc27..7cc8665047 100644
--- a/ld/ldlex.l
+++ b/ld/ldlex.l
@@ -276,6 +276,7 @@ V_IDENTIFIER [*?.$_a-zA-Z\[\]\-\!\^\\]([*?.$_a-zA-Z0-9\[\]\-\!\^\\]|::)*
 <BOTH,SCRIPT>"FORCE_COMMON_ALLOCATION"	{ RTOKEN(FORCE_COMMON_ALLOCATION);}
 <BOTH,SCRIPT>"FORCE_GROUP_ALLOCATION"	{ RTOKEN(FORCE_GROUP_ALLOCATION);}
 <BOTH,SCRIPT>"INHIBIT_COMMON_ALLOCATION" { RTOKEN(INHIBIT_COMMON_ALLOCATION);}
+<BOTH,SCRIPT>"INHIBIT_GROUP_ALLOCATION" { RTOKEN(INHIBIT_GROUP_ALLOCATION);}
 <BOTH,SCRIPT>"SECTIONS"			{ RTOKEN(SECTIONS);}
 <BOTH,SCRIPT>"INSERT"			{ RTOKEN(INSERT_K);}
 <BOTH,SCRIPT>"AFTER"			{ RTOKEN(AFTER);}
diff --git a/ld/ldmain.c b/ld/ldmain.c
index ee5ab1166a..ee534abe30 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -386,8 +386,9 @@ main (int argc, char **argv)
       info_msg ("\n==================================================\n");
     }
 
-  if (command_line.force_group_allocation
-      || !bfd_link_relocatable (&link_info))
+  if (command_line.group_allocation == group_allocation_force
+      || (command_line.group_allocation == group_allocation_unset
+	  && !bfd_link_relocatable (&link_info)))
     link_info.resolve_section_groups = TRUE;
   else
     link_info.resolve_section_groups = FALSE;
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 95c7e599b9..880b5dee49 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -115,6 +115,9 @@ static const struct ld_option ld_options[] =
   { {"force-group-allocation", no_argument, NULL,
      OPTION_FORCE_GROUP_ALLOCATION},
     '\0', NULL, N_("Force group members out of groups"), TWO_DASHES },
+  { {"inhibit-group-allocation", no_argument, NULL,
+     OPTION_INHIBIT_GROUP_ALLOCATION},
+    '\0', NULL, N_("Prevent section groups from being resolved"), TWO_DASHES },
   { {"entry", required_argument, NULL, 'e'},
     'e', N_("ADDRESS"), N_("Set start address"), TWO_DASHES },
   { {"export-dynamic", no_argument, NULL, OPTION_EXPORT_DYNAMIC},
@@ -771,7 +774,10 @@ parse_args (unsigned argc, char **argv)
 	  command_line.force_common_definition = TRUE;
 	  break;
         case OPTION_FORCE_GROUP_ALLOCATION:
-          command_line.force_group_allocation = TRUE;
+          command_line.group_allocation = group_allocation_force;
+          break;
+        case OPTION_INHIBIT_GROUP_ALLOCATION:
+          command_line.group_allocation = group_allocation_inhibit;
           break;
 	case OPTION_DEFSYM:
 	  lex_string = optarg;
diff --git a/ld/testsuite/ld-elf/group13.d b/ld/testsuite/ld-elf/group13.d
new file mode 100644
index 0000000000..bba9b08cb0
--- /dev/null
+++ b/ld/testsuite/ld-elf/group13.d
@@ -0,0 +1,10 @@
+#source: group1a.s
+#source: group1b.s
+#ld: -T group13.ld
+#readelf: -g
+#xfail: d30v-*-* dlx-*-* i960-*-* pj*-*-*
+# generic linker targets don't comply with all symbol merging rules
+
+COMDAT group section \[ +[0-9]+\] `\.group' \[foo_group\] contains 1 sections:
+   \[Index\]    Name
+   \[ +[0-9]+\]   \.text
diff --git a/ld/testsuite/ld-elf/group13.ld b/ld/testsuite/ld-elf/group13.ld
new file mode 100644
index 0000000000..dcbbd7790c
--- /dev/null
+++ b/ld/testsuite/ld-elf/group13.ld
@@ -0,0 +1,14 @@
+INHIBIT_GROUP_ALLOCATION
+
+PHDRS
+{
+  header PT_PHDR PHDRS ;
+  image  PT_LOAD PHDRS;
+}
+
+SECTIONS
+{
+  . = 0x1000;
+  .text : { *(.text) *(.rodata.brlt) } :image :header
+  /DISCARD/ : { *(.dropme) *(.reginfo) *(.MIPS.abiflags) }
+}
diff --git a/ld/testsuite/ld-elf/group14.d b/ld/testsuite/ld-elf/group14.d
new file mode 100644
index 0000000000..bce6806327
--- /dev/null
+++ b/ld/testsuite/ld-elf/group14.d
@@ -0,0 +1,10 @@
+#source: group1a.s
+#source: group1b.s
+#ld: -T group.ld --inhibit-group-allocation
+#readelf: -g
+#xfail: d30v-*-* dlx-*-* i960-*-* pj*-*-*
+# generic linker targets don't comply with all symbol merging rules
+
+COMDAT group section \[ +[0-9]+\] `\.group' \[foo_group\] contains 1 sections:
+   \[Index\]    Name
+   \[ +[0-9]+\]   \.text
-- 
2.12.2