public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ld: Support customized output section type
@ 2022-02-02  7:10 Fangrui Song
  2022-02-02 11:36 ` Nick Clifton
  2022-02-21 19:35 ` Mike Frysinger
  0 siblings, 2 replies; 10+ messages in thread
From: Fangrui Song @ 2022-02-02  7:10 UTC (permalink / raw)
  To: binutils, Alan Modra, Nick Clifton; +Cc: Luca Boccassi, Fangrui Song

The current output section type allows to set the ELF section type to
SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
to be specified. Some ELF section type names are supported as well,
e.g.:

    note (TYPE=SHT_NOTE) : { BYTE(8) }
    init_array (TYPE=14) : { BYTE(14) }
    fini_array (TYPE=SHT_FINI_ARRAY) : { BYTE(15) }

bfd/
    PR ld/28841
    * bfd-in2.h (struct bfd_section): Add type.
    (discarded_section): Add field.
    * elf.c (elf_fake_sections): Handle bfd_section::type.
    * section.c (BFD_FAKE_SECTION): Add field.
    * mri.c (mri_draw_tree): Update function call.

ld/
    PR ld/28841
    * ld.texi: Document new output section type.
    * ldlex.l: Add new token TYPE.
    * ldgram.y: Handle TYPE=exp.
    * ldlang.h: Add type_section to list of section types.
    * ldlang.c (lang_add_section): Handle type_section.
    (map_input_to_output_sections): Handle type_section.
    * testsuite/ld-scripts/output-section-types.t: Add tests.
    * testsuite/ld-scripts/output-section-types.d: Update.
---
 bfd/bfd-in2.h                                 |  7 ++-
 bfd/elf.c                                     |  4 +-
 bfd/section.c                                 |  4 +-
 ld/ld.texi                                    |  4 ++
 ld/ldgram.y                                   |  9 ++--
 ld/ldlang.c                                   | 49 ++++++++++++++++---
 ld/ldlang.h                                   |  4 +-
 ld/ldlex.l                                    |  1 +
 ld/mri.c                                      |  4 +-
 .../ld-scripts/output-section-types.d         | 16 +++---
 .../ld-scripts/output-section-types.t         |  5 ++
 11 files changed, 82 insertions(+), 25 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 8e815bab624..91ec68d0391 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1170,6 +1170,9 @@ typedef struct bfd_section
     This is used when support for non-contiguous memory regions is enabled.  */
  struct bfd_section *already_assigned;
 
+  /* Explicitly specified section type, if non-zero. */
+  unsigned int type;
+
 } asection;
 
 /* Relax table contains information about instructions which can
@@ -1352,8 +1355,8 @@ discarded_section (const asection *sec)
   /* symbol,                    symbol_ptr_ptr,                     */ \
      (struct bfd_symbol *) SYM, &SEC.symbol,                           \
                                                                        \
-  /* map_head, map_tail, already_assigned                           */ \
-     { NULL }, { NULL }, NULL                                          \
+  /* map_head, map_tail, already_assigned, type                     */ \
+     { NULL }, { NULL }, NULL, 0                                       \
                                                                        \
     }
 
diff --git a/bfd/elf.c b/bfd/elf.c
index 14c2c7ba734..1ebe7b425c4 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3279,7 +3279,9 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
 
   /* If the section type is unspecified, we set it based on
      asect->flags.  */
-  if ((asect->flags & SEC_GROUP) != 0)
+  if (asect->type != 0)
+    sh_type = asect->type;
+  else if ((asect->flags & SEC_GROUP) != 0)
     sh_type = SHT_GROUP;
   else
     sh_type = bfd_elf_get_default_section_type (asect->flags);
diff --git a/bfd/section.c b/bfd/section.c
index 899438a1c5e..2de7dbf661a 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -737,8 +737,8 @@ CODE_FRAGMENT
 .  {* symbol,                    symbol_ptr_ptr,                     *}	\
 .     (struct bfd_symbol *) SYM, &SEC.symbol,				\
 .									\
-.  {* map_head, map_tail, already_assigned                           *}	\
-.     { NULL }, { NULL }, NULL						\
+.  {* map_head, map_tail, already_assigned, type                     *}	\
+.     { NULL }, { NULL }, NULL, 0						\
 .									\
 .    }
 .
diff --git a/ld/ld.texi b/ld/ld.texi
index fc75e9b3625..e3a1e02ed25 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -5490,6 +5490,10 @@ These type names are supported for backward compatibility, and are
 rarely used.  They all have the same effect: the section should be
 marked as not allocatable, so that no memory is allocated for the
 section when the program is run.
+@item TYPE = @var{type}
+Set the section type to the integer @var{type}. For the ELF output
+file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
+@var{type}.
 @end table
 
 @kindex NOLOAD
diff --git a/ld/ldgram.y b/ld/ldgram.y
index 11c2f219c05..f5e3a834a26 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -47,6 +47,7 @@
 #endif
 
 static enum section_type sectype;
+static etree_type *sectype_value;
 static lang_memory_region_type *region;
 
 static bool ldgram_had_keep = false;
@@ -139,6 +140,7 @@ static int error_index;
 %token LD_FEATURE
 %token NOLOAD DSECT COPY INFO OVERLAY
 %token READONLY
+%token TYPE
 %token DEFINED TARGET_K SEARCH_DIR MAP ENTRY
 %token <integer> NEXT
 %token SIZEOF ALIGNOF ADDR LOADADDR MAX_K MIN_K
@@ -1058,9 +1060,8 @@ section:	NAME
 			{
 			  ldlex_popstate ();
 			  ldlex_wild ();
-			  lang_enter_output_section_statement($1, $3, sectype,
-							      $5, $7, $4,
-							      $8, $6);
+			  lang_enter_output_section_statement ($1, $3, sectype,
+					sectype_value, $5, $7, $4, $8, $6);
 			}
 		'{'
 		statement_list_opt
@@ -1131,7 +1132,7 @@ type:
 	|  INFO    { sectype = noalloc_section; }
 	|  OVERLAY { sectype = noalloc_section; }
 	|  READONLY { sectype = readonly_section; }
-	;
+	|  TYPE '=' exp { sectype = type_section; sectype_value = $3; }
 
 atype:
 		'(' type ')'
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 5dd3df12a0f..a0a3c09f9a9 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1891,8 +1891,8 @@ lang_insert_orphan (asection *s,
     address = exp_intop (0);
 
   os_tail = (lang_output_section_statement_type **) lang_os_list.tail;
-  os = lang_enter_output_section_statement (secname, address, normal_section,
-					    NULL, NULL, NULL, constraint, 0);
+  os = lang_enter_output_section_statement (
+      secname, address, normal_section, 0, NULL, NULL, NULL, constraint, 0);
 
   if (add_child == NULL)
     add_child = &os->children;
@@ -2635,6 +2635,7 @@ lang_add_section (lang_statement_list_type *ptr,
     case normal_section:
     case overlay_section:
     case first_overlay_section:
+    case type_section:
       break;
     case noalloc_section:
       flags &= ~SEC_ALLOC;
@@ -4209,6 +4210,7 @@ map_input_to_output_sections
     {
       lang_output_section_statement_type *tos;
       flagword flags;
+      unsigned int type = 0;
 
       switch (s->header.type)
 	{
@@ -4261,6 +4263,37 @@ map_input_to_output_sections
 	    case noalloc_section:
 	      flags = SEC_HAS_CONTENTS;
 	      break;
+	    case type_section:
+	      if (os->sectype_value->type.node_class == etree_name
+		  && os->sectype_value->type.node_code == NAME)
+		{
+		  const char *name = os->sectype_value->name.name;
+		  if (strcmp (name, "SHT_PROGBITS") == 0)
+		    type = SHT_PROGBITS;
+		  else if (strcmp (name, "SHT_NOTE") == 0)
+		    type = SHT_NOTE;
+		  else if (strcmp (name, "SHT_NOBITS") == 0)
+		    type = SHT_NOBITS;
+		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
+		    type = SHT_INIT_ARRAY;
+		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
+		    type = SHT_FINI_ARRAY;
+		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
+		    type = SHT_PREINIT_ARRAY;
+		  else
+		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
+			   os->name);
+		}
+	     else
+	       {
+		 exp_fold_tree_no_dot (os->sectype_value);
+		 if (expld.result.valid_p)
+		   type = expld.result.value;
+		 else
+		   einfo (_ ("%F%P: invalid type for output section `%s'\n"),
+			  os->name);
+	       }
+	      break;
 	    case readonly_section:
 	      flags |= SEC_READONLY;
 	      break;
@@ -4276,6 +4309,7 @@ map_input_to_output_sections
 	    init_os (os, flags | SEC_READONLY);
 	  else
 	    os->bfd_section->flags |= flags;
+	  os->bfd_section->type = type;
 	  break;
 	case lang_input_section_enum:
 	  break;
@@ -7564,6 +7598,7 @@ lang_output_section_statement_type *
 lang_enter_output_section_statement (const char *output_section_statement_name,
 				     etree_type *address_exp,
 				     enum section_type sectype,
+				     etree_type *sectype_value,
 				     etree_type *align,
 				     etree_type *subalign,
 				     etree_type *ebase,
@@ -7581,10 +7616,12 @@ lang_enter_output_section_statement (const char *output_section_statement_name,
       os->addr_tree = address_exp;
     }
   os->sectype = sectype;
-  if (sectype != noload_section)
-    os->flags = SEC_NO_FLAGS;
-  else
+  if (sectype == type_section)
+    os->sectype_value = sectype_value;
+  else if (sectype == noload_section)
     os->flags = SEC_NEVER_LOAD;
+  else
+    os->flags = SEC_NO_FLAGS;
   os->block_value = 1;
 
   /* Make next things chain into subchain of this.  */
@@ -8901,7 +8938,7 @@ lang_enter_overlay_section (const char *name)
   etree_type *size;
 
   lang_enter_output_section_statement (name, overlay_vma, overlay_section,
-				       0, overlay_subalign, 0, 0, 0);
+				       0, 0, overlay_subalign, 0, 0, 0);
 
   /* If this is the first section, then base the VMA of future
      sections on this one.  This will work correctly even if `.' is
diff --git a/ld/ldlang.h b/ld/ldlang.h
index 0d057c9bee9..20b953eeac6 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -122,6 +122,7 @@ enum section_type
   overlay_section,
   noload_section,
   noalloc_section,
+  type_section,
   readonly_section
 };
 
@@ -166,6 +167,7 @@ typedef struct lang_output_section_statement_struct
   int constraint;
   flagword flags;
   enum section_type sectype;
+  etree_type *sectype_value;
   unsigned int processed_vma : 1;
   unsigned int processed_lma : 1;
   unsigned int all_input_readonly : 1;
@@ -545,7 +547,7 @@ extern void lang_add_output
   (const char *, int from_script);
 extern lang_output_section_statement_type *lang_enter_output_section_statement
   (const char *, etree_type *, enum section_type, etree_type *, etree_type *,
-   etree_type *, int, int);
+   etree_type *, etree_type *, int, int);
 extern void lang_final
   (void);
 extern void lang_relax_sections
diff --git a/ld/ldlex.l b/ld/ldlex.l
index 78db16e3a48..c38b46b9336 100644
--- a/ld/ldlex.l
+++ b/ld/ldlex.l
@@ -323,6 +323,7 @@ V_IDENTIFIER [*?.$_a-zA-Z\[\]\-\!\^\\]([*?.$_a-zA-Z0-9\[\]\-\!\^\\]|::)*
 <EXPRESSION>"DSECT"			{ RTOKEN(DSECT); }
 <EXPRESSION>"COPY"			{ RTOKEN(COPY); }
 <EXPRESSION>"INFO"			{ RTOKEN(INFO); }
+<EXPRESSION>"TYPE"			{ RTOKEN(TYPE); }
 <SCRIPT,EXPRESSION>"ONLY_IF_RO"		{ RTOKEN(ONLY_IF_RO); }
 <SCRIPT,EXPRESSION>"ONLY_IF_RW"		{ RTOKEN(ONLY_IF_RW); }
 <SCRIPT,EXPRESSION>"SPECIAL"		{ RTOKEN(SPECIAL); }
diff --git a/ld/mri.c b/ld/mri.c
index b428ab0d0bf..5749870ef1e 100644
--- a/ld/mri.c
+++ b/ld/mri.c
@@ -210,8 +210,8 @@ mri_draw_tree (void)
 	    base = p->vma ? p->vma : exp_nameop (NAME, ".");
 
 	  lang_enter_output_section_statement (p->name, base,
-					       p->ok_to_load ? normal_section : noload_section,
-					       align, subalign, NULL, 0, 0);
+	    p->ok_to_load ? normal_section : noload_section, 0,
+	    align, subalign, NULL, 0, 0);
 	  base = 0;
 	  tmp = (struct wildcard_list *) xmalloc (sizeof *tmp);
 	  tmp->next = NULL;
diff --git a/ld/testsuite/ld-scripts/output-section-types.d b/ld/testsuite/ld-scripts/output-section-types.d
index ab124fa4dd7..56b88ce6342 100644
--- a/ld/testsuite/ld-scripts/output-section-types.d
+++ b/ld/testsuite/ld-scripts/output-section-types.d
@@ -1,13 +1,15 @@
 #ld: -Toutput-section-types.t
 #source: align2a.s
-#objdump: -h
+#readelf: -S --wide
 #target: [is_elf_format]
 
 #...
-  . \.rom.*
-[ 	]+ALLOC, READONLY
-  . \.ro.*
-[ 	]+CONTENTS, ALLOC, LOAD, READONLY, DATA
-  . \.over.*
-[ 	]+CONTENTS, READONLY
+.* .rom          +NOBITS        +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +[0-9] +0 +[1248]
+.* .ro           +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +[0-9] +0 +[1248]
+.* .over         +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00  + +[0-9] +0 +[1248]
+.* progbits      +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +[0-9] +0 +[1248]
+.* note          +NOTE          +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +[0-9] +0 +[1248]
+.* init_array    +INIT_ARRAY    +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +[0-9] +0 +[1248]
+.* fini_array    +FINI_ARRAY    +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +[0-9] +0 +[1248]
+.* preinit_array +PREINIT_ARRAY +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +[0-9] +0 +[1248]
 #pass
diff --git a/ld/testsuite/ld-scripts/output-section-types.t b/ld/testsuite/ld-scripts/output-section-types.t
index d8fdfda1a03..834e5869d79 100644
--- a/ld/testsuite/ld-scripts/output-section-types.t
+++ b/ld/testsuite/ld-scripts/output-section-types.t
@@ -2,6 +2,11 @@ SECTIONS {
   .rom  (NOLOAD)   : { LONG(1234); }
   .ro   (READONLY) : { LONG(5678); }
   .over (OVERLAY)  : { LONG(0123); }
+  progbits (TYPE=SHT_PROGBITS) : { BYTE(1) }
+  note (TYPE=SHT_NOTE) : { BYTE(8) }
+  init_array (TYPE=14) : { BYTE(14) }
+  fini_array (TYPE=SHT_FINI_ARRAY) : { BYTE(15) }
+  preinit_array (TYPE=SHT_PREINIT_ARRAY) : { BYTE(16) }
   /DISCARD/        : { *(*) }
 
 }
-- 
2.35.0.rc2.247.g8bbb082509-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-02  7:10 [PATCH] ld: Support customized output section type Fangrui Song
@ 2022-02-02 11:36 ` Nick Clifton
  2022-02-02 11:46   ` Luca Boccassi
  2022-02-02 18:32   ` Fangrui Song
  2022-02-21 19:35 ` Mike Frysinger
  1 sibling, 2 replies; 10+ messages in thread
From: Nick Clifton @ 2022-02-02 11:36 UTC (permalink / raw)
  To: Fangrui Song, binutils, Alan Modra; +Cc: Luca Boccassi

Hi Fangrui,

> The current output section type allows to set the ELF section type to
> SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
> to be specified. Some ELF section type names are supported as well,

Thanks for the patch submission.  I am regression testing it at the moment
but in the meantime a couple of things stood out for me:


> +@item TYPE = @var{type}
> +Set the section type to the integer @var{type}. For the ELF output
> +file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
> +@var{type}.

Rather than having users guess, it would probably be best to list which
type names are supported.

Also it is probably worth documenting that it is the user's fault if they
set the section type to something which has special semantics (eg SHT_GROUP)
but then they do not also arrange for whatever necessary support that feature
needs.  Something like: "it is the user's responsibility to ensure that
any special requirements of the section type are met".



> +	    case type_section:
> +	      if (os->sectype_value->type.node_class == etree_name
> +		  && os->sectype_value->type.node_code == NAME)
> +		{
> +		  const char *name = os->sectype_value->name.name;
> +		  if (strcmp (name, "SHT_PROGBITS") == 0)
> +		    type = SHT_PROGBITS;
> +		  else if (strcmp (name, "SHT_NOTE") == 0)
> +		    type = SHT_NOTE;
> +		  else if (strcmp (name, "SHT_NOBITS") == 0)
> +		    type = SHT_NOBITS;
> +		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
> +		    type = SHT_INIT_ARRAY;
> +		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
> +		    type = SHT_FINI_ARRAY;
> +		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
> +		    type = SHT_PREINIT_ARRAY;
> +		  else
> +		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
> +			   os->name);

It might be worth adding SHT_STRTAB to this list, as I can imagine some
weird sceanario where someone would want it.


Also - given that this is a new feature, there really ought to be an entry
for it in the ld/NEWS file.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-02 11:36 ` Nick Clifton
@ 2022-02-02 11:46   ` Luca Boccassi
  2022-02-02 18:32   ` Fangrui Song
  1 sibling, 0 replies; 10+ messages in thread
From: Luca Boccassi @ 2022-02-02 11:46 UTC (permalink / raw)
  To: Nick Clifton, Fangrui Song, binutils, Alan Modra

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

On Wed, 2022-02-02 at 11:36 +0000, Nick Clifton wrote:
> Hi Fangrui,
> 
> > The current output section type allows to set the ELF section type to
> > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
> > to be specified. Some ELF section type names are supported as well,
> 
> Thanks for the patch submission.  I am regression testing it at the moment
> but in the meantime a couple of things stood out for me:
> 
> 
> > +@item TYPE = @var{type}
> > +Set the section type to the integer @var{type}. For the ELF output
> > +file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
> > +@var{type}.
> 
> Rather than having users guess, it would probably be best to list which
> type names are supported.
> 
> Also it is probably worth documenting that it is the user's fault if they
> set the section type to something which has special semantics (eg SHT_GROUP)
> but then they do not also arrange for whatever necessary support that feature
> needs.  Something like: "it is the user's responsibility to ensure that
> any special requirements of the section type are met".
> 
> 
> 
> > +	    case type_section:
> > +	      if (os->sectype_value->type.node_class == etree_name
> > +		  && os->sectype_value->type.node_code == NAME)
> > +		{
> > +		  const char *name = os->sectype_value->name.name;
> > +		  if (strcmp (name, "SHT_PROGBITS") == 0)
> > +		    type = SHT_PROGBITS;
> > +		  else if (strcmp (name, "SHT_NOTE") == 0)
> > +		    type = SHT_NOTE;
> > +		  else if (strcmp (name, "SHT_NOBITS") == 0)
> > +		    type = SHT_NOBITS;
> > +		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
> > +		    type = SHT_INIT_ARRAY;
> > +		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
> > +		    type = SHT_FINI_ARRAY;
> > +		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
> > +		    type = SHT_PREINIT_ARRAY;
> > +		  else
> > +		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
> > +			   os->name);
> 
> It might be worth adding SHT_STRTAB to this list, as I can imagine some
> weird sceanario where someone would want it.
> 
> 
> Also - given that this is a new feature, there really ought to be an entry
> for it in the ld/NEWS file.

Hi,

Does this patch allow specifying both this new attribute and the
READONLY attribute? It is required for our use case, and cannot be used
otherwise.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-02 11:36 ` Nick Clifton
  2022-02-02 11:46   ` Luca Boccassi
@ 2022-02-02 18:32   ` Fangrui Song
  2022-02-03 18:36     ` Luca Boccassi
  1 sibling, 1 reply; 10+ messages in thread
From: Fangrui Song @ 2022-02-02 18:32 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Fangrui Song, binutils, Alan Modra, Luca Boccassi

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

On 2022-02-02, Nick Clifton via Binutils wrote:
>Hi Fangrui,
>
>>The current output section type allows to set the ELF section type to
>>SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
>>to be specified. Some ELF section type names are supported as well,
>
>Thanks for the patch submission.  I am regression testing it at the moment
>but in the meantime a couple of things stood out for me:
>
>
>>+@item TYPE = @var{type}
>>+Set the section type to the integer @var{type}. For the ELF output
>>+file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
>>+@var{type}.
>
>Rather than having users guess, it would probably be best to list which
>type names are supported.
>
>Also it is probably worth documenting that it is the user's fault if they
>set the section type to something which has special semantics (eg SHT_GROUP)
>but then they do not also arrange for whatever necessary support that feature
>needs.  Something like: "it is the user's responsibility to ensure that
>any special requirements of the section type are met".

Thanks for the quick review! Adopted the wording and the change below.
The new patch is in the attachment.


For SHT_GROUP, I think it is useful to support SHT_GROUP as well. I
actually did an experiment last night but SHT_GROUP led to an internal
error. There may be some issues that need to be fixed to use the
SHT_GROUP feature.

>
>
>>+	    case type_section:
>>+	      if (os->sectype_value->type.node_class == etree_name
>>+		  && os->sectype_value->type.node_code == NAME)
>>+		{
>>+		  const char *name = os->sectype_value->name.name;
>>+		  if (strcmp (name, "SHT_PROGBITS") == 0)
>>+		    type = SHT_PROGBITS;
>>+		  else if (strcmp (name, "SHT_NOTE") == 0)
>>+		    type = SHT_NOTE;
>>+		  else if (strcmp (name, "SHT_NOBITS") == 0)
>>+		    type = SHT_NOBITS;
>>+		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
>>+		    type = SHT_INIT_ARRAY;
>>+		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
>>+		    type = SHT_FINI_ARRAY;
>>+		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
>>+		    type = SHT_PREINIT_ARRAY;
>>+		  else
>>+		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
>>+			   os->name);
>
>It might be worth adding SHT_STRTAB to this list, as I can imagine some
>weird sceanario where someone would want it.

Added.

>
>Also - given that this is a new feature, there really ought to be an entry
>for it in the ld/NEWS file.

Added. The NEW entry is for 2.39, but feel free to port it to 2.38 if
you think appropriate:)

>Cheers
>  Nick
>

[-- Attachment #2: 0001-ld-Support-customized-output-section-type.patch --]
[-- Type: text/x-diff, Size: 14753 bytes --]

From 857ebfa9b7f905d747b689360be11d7f197b8ba5 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Wed, 2 Feb 2022 10:25:45 -0800
Subject: [PATCH] ld: Support customized output section type

The current output section type allows to set the ELF section type to
SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
to be specified. Some ELF section type names are supported as well,
e.g.:

    note (TYPE=SHT_NOTE) : { BYTE(8) }
    init_array ( TYPE=14 ) : { QUAD(14) }
    fini_array (TYPE = SHT_FINI_ARRAY) : { QUAD(15) }

It is the user's responsibility to ensure that any special requirements
of the section type are met.

Currently some section names are specially handled, e.g. a ".note"
prefixed output section gets the type SHT_NOTE even if it contains no
input section. This behavior may not be removed for backward
compatibility, but it is recommended that new usage sets the type
explicitly to cater to the ELF spirit that attributes are identified as
well known integers instead of magic names.

bfd/
    PR ld/28841
    * bfd-in2.h (struct bfd_section): Add type.
    (discarded_section): Add field.
    * elf.c (elf_fake_sections): Handle bfd_section::type.
    * section.c (BFD_FAKE_SECTION): Add field.
    * mri.c (mri_draw_tree): Update function call.

ld/
    PR ld/28841
    * ld.texi: Document new output section type.
    * ldlex.l: Add new token TYPE.
    * ldgram.y: Handle TYPE=exp.
    * ldlang.h: Add type_section to list of section types.
    * ldlang.c (lang_add_section): Handle type_section.
    (map_input_to_output_sections): Handle type_section.
    * testsuite/ld-scripts/output-section-types.t: Add tests.
    * testsuite/ld-scripts/output-section-types.d: Update.
---
 bfd/bfd-in2.h                                 |  7 ++-
 bfd/elf.c                                     |  4 +-
 bfd/section.c                                 |  4 +-
 ld/NEWS                                       |  3 ++
 ld/ld.texi                                    |  7 +++
 ld/ldgram.y                                   |  9 ++--
 ld/ldlang.c                                   | 51 ++++++++++++++++---
 ld/ldlang.h                                   |  4 +-
 ld/ldlex.l                                    |  1 +
 ld/mri.c                                      |  4 +-
 .../ld-scripts/output-section-types.d         | 17 ++++---
 .../ld-scripts/output-section-types.t         |  6 +++
 12 files changed, 92 insertions(+), 25 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 8e815bab624..91ec68d0391 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1170,6 +1170,9 @@ typedef struct bfd_section
     This is used when support for non-contiguous memory regions is enabled.  */
  struct bfd_section *already_assigned;
 
+  /* Explicitly specified section type, if non-zero. */
+  unsigned int type;
+
 } asection;
 
 /* Relax table contains information about instructions which can
@@ -1352,8 +1355,8 @@ discarded_section (const asection *sec)
   /* symbol,                    symbol_ptr_ptr,                     */ \
      (struct bfd_symbol *) SYM, &SEC.symbol,                           \
                                                                        \
-  /* map_head, map_tail, already_assigned                           */ \
-     { NULL }, { NULL }, NULL                                          \
+  /* map_head, map_tail, already_assigned, type                     */ \
+     { NULL }, { NULL }, NULL, 0                                       \
                                                                        \
     }
 
diff --git a/bfd/elf.c b/bfd/elf.c
index 14c2c7ba734..1ebe7b425c4 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3279,7 +3279,9 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
 
   /* If the section type is unspecified, we set it based on
      asect->flags.  */
-  if ((asect->flags & SEC_GROUP) != 0)
+  if (asect->type != 0)
+    sh_type = asect->type;
+  else if ((asect->flags & SEC_GROUP) != 0)
     sh_type = SHT_GROUP;
   else
     sh_type = bfd_elf_get_default_section_type (asect->flags);
diff --git a/bfd/section.c b/bfd/section.c
index 899438a1c5e..2de7dbf661a 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -737,8 +737,8 @@ CODE_FRAGMENT
 .  {* symbol,                    symbol_ptr_ptr,                     *}	\
 .     (struct bfd_symbol *) SYM, &SEC.symbol,				\
 .									\
-.  {* map_head, map_tail, already_assigned                           *}	\
-.     { NULL }, { NULL }, NULL						\
+.  {* map_head, map_tail, already_assigned, type                     *}	\
+.     { NULL }, { NULL }, NULL, 0						\
 .									\
 .    }
 .
diff --git a/ld/NEWS b/ld/NEWS
index dbb402d1f8a..a498abaf0f9 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* TYPE=<type> is now supported in an output section description to set the
+  section type value.
+
 Changes in 2.38:
 
 * Add -z pack-relative-relocs/-z no pack-relative-relocs to x86 ELF
diff --git a/ld/ld.texi b/ld/ld.texi
index fc75e9b3625..83c3ae8e6fd 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -5490,6 +5490,13 @@ These type names are supported for backward compatibility, and are
 rarely used.  They all have the same effect: the section should be
 marked as not allocatable, so that no memory is allocated for the
 section when the program is run.
+@item TYPE = @var{type}
+Set the section type to the integer @var{type}. When generating an ELF
+output file, type names @code{SHT_PROGBITS}, @code{SHT_STRTAB},
+@code{SHT_NOTE}, @code{SHT_INIT_ARRAY}, @code{SHT_FINI_ARRAY}, and
+@code{SHT_PREINIT_ARRAY} are also allowed for @var{type}.  It is the
+user's responsibility to ensure that any special requirements of the
+section type are met.
 @end table
 
 @kindex NOLOAD
diff --git a/ld/ldgram.y b/ld/ldgram.y
index 11c2f219c05..f5e3a834a26 100644
--- a/ld/ldgram.y
+++ b/ld/ldgram.y
@@ -47,6 +47,7 @@
 #endif
 
 static enum section_type sectype;
+static etree_type *sectype_value;
 static lang_memory_region_type *region;
 
 static bool ldgram_had_keep = false;
@@ -139,6 +140,7 @@ static int error_index;
 %token LD_FEATURE
 %token NOLOAD DSECT COPY INFO OVERLAY
 %token READONLY
+%token TYPE
 %token DEFINED TARGET_K SEARCH_DIR MAP ENTRY
 %token <integer> NEXT
 %token SIZEOF ALIGNOF ADDR LOADADDR MAX_K MIN_K
@@ -1058,9 +1060,8 @@ section:	NAME
 			{
 			  ldlex_popstate ();
 			  ldlex_wild ();
-			  lang_enter_output_section_statement($1, $3, sectype,
-							      $5, $7, $4,
-							      $8, $6);
+			  lang_enter_output_section_statement ($1, $3, sectype,
+					sectype_value, $5, $7, $4, $8, $6);
 			}
 		'{'
 		statement_list_opt
@@ -1131,7 +1132,7 @@ type:
 	|  INFO    { sectype = noalloc_section; }
 	|  OVERLAY { sectype = noalloc_section; }
 	|  READONLY { sectype = readonly_section; }
-	;
+	|  TYPE '=' exp { sectype = type_section; sectype_value = $3; }
 
 atype:
 		'(' type ')'
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 5dd3df12a0f..62ca0c9d059 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1891,8 +1891,8 @@ lang_insert_orphan (asection *s,
     address = exp_intop (0);
 
   os_tail = (lang_output_section_statement_type **) lang_os_list.tail;
-  os = lang_enter_output_section_statement (secname, address, normal_section,
-					    NULL, NULL, NULL, constraint, 0);
+  os = lang_enter_output_section_statement (
+      secname, address, normal_section, 0, NULL, NULL, NULL, constraint, 0);
 
   if (add_child == NULL)
     add_child = &os->children;
@@ -2635,6 +2635,7 @@ lang_add_section (lang_statement_list_type *ptr,
     case normal_section:
     case overlay_section:
     case first_overlay_section:
+    case type_section:
       break;
     case noalloc_section:
       flags &= ~SEC_ALLOC;
@@ -4209,6 +4210,7 @@ map_input_to_output_sections
     {
       lang_output_section_statement_type *tos;
       flagword flags;
+      unsigned int type = 0;
 
       switch (s->header.type)
 	{
@@ -4261,6 +4263,39 @@ map_input_to_output_sections
 	    case noalloc_section:
 	      flags = SEC_HAS_CONTENTS;
 	      break;
+	    case type_section:
+	      if (os->sectype_value->type.node_class == etree_name
+		  && os->sectype_value->type.node_code == NAME)
+		{
+		  const char *name = os->sectype_value->name.name;
+		  if (strcmp (name, "SHT_PROGBITS") == 0)
+		    type = SHT_PROGBITS;
+		  else if (strcmp (name, "SHT_STRTAB") == 0)
+		    type = SHT_STRTAB;
+		  else if (strcmp (name, "SHT_NOTE") == 0)
+		    type = SHT_NOTE;
+		  else if (strcmp (name, "SHT_NOBITS") == 0)
+		    type = SHT_NOBITS;
+		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
+		    type = SHT_INIT_ARRAY;
+		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
+		    type = SHT_FINI_ARRAY;
+		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
+		    type = SHT_PREINIT_ARRAY;
+		  else
+		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
+			   os->name);
+		}
+	     else
+	       {
+		 exp_fold_tree_no_dot (os->sectype_value);
+		 if (expld.result.valid_p)
+		   type = expld.result.value;
+		 else
+		   einfo (_ ("%F%P: invalid type for output section `%s'\n"),
+			  os->name);
+	       }
+	      break;
 	    case readonly_section:
 	      flags |= SEC_READONLY;
 	      break;
@@ -4276,6 +4311,7 @@ map_input_to_output_sections
 	    init_os (os, flags | SEC_READONLY);
 	  else
 	    os->bfd_section->flags |= flags;
+	  os->bfd_section->type = type;
 	  break;
 	case lang_input_section_enum:
 	  break;
@@ -7564,6 +7600,7 @@ lang_output_section_statement_type *
 lang_enter_output_section_statement (const char *output_section_statement_name,
 				     etree_type *address_exp,
 				     enum section_type sectype,
+				     etree_type *sectype_value,
 				     etree_type *align,
 				     etree_type *subalign,
 				     etree_type *ebase,
@@ -7581,10 +7618,12 @@ lang_enter_output_section_statement (const char *output_section_statement_name,
       os->addr_tree = address_exp;
     }
   os->sectype = sectype;
-  if (sectype != noload_section)
-    os->flags = SEC_NO_FLAGS;
-  else
+  if (sectype == type_section)
+    os->sectype_value = sectype_value;
+  else if (sectype == noload_section)
     os->flags = SEC_NEVER_LOAD;
+  else
+    os->flags = SEC_NO_FLAGS;
   os->block_value = 1;
 
   /* Make next things chain into subchain of this.  */
@@ -8901,7 +8940,7 @@ lang_enter_overlay_section (const char *name)
   etree_type *size;
 
   lang_enter_output_section_statement (name, overlay_vma, overlay_section,
-				       0, overlay_subalign, 0, 0, 0);
+				       0, 0, overlay_subalign, 0, 0, 0);
 
   /* If this is the first section, then base the VMA of future
      sections on this one.  This will work correctly even if `.' is
diff --git a/ld/ldlang.h b/ld/ldlang.h
index 0d057c9bee9..20b953eeac6 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -122,6 +122,7 @@ enum section_type
   overlay_section,
   noload_section,
   noalloc_section,
+  type_section,
   readonly_section
 };
 
@@ -166,6 +167,7 @@ typedef struct lang_output_section_statement_struct
   int constraint;
   flagword flags;
   enum section_type sectype;
+  etree_type *sectype_value;
   unsigned int processed_vma : 1;
   unsigned int processed_lma : 1;
   unsigned int all_input_readonly : 1;
@@ -545,7 +547,7 @@ extern void lang_add_output
   (const char *, int from_script);
 extern lang_output_section_statement_type *lang_enter_output_section_statement
   (const char *, etree_type *, enum section_type, etree_type *, etree_type *,
-   etree_type *, int, int);
+   etree_type *, etree_type *, int, int);
 extern void lang_final
   (void);
 extern void lang_relax_sections
diff --git a/ld/ldlex.l b/ld/ldlex.l
index 78db16e3a48..c38b46b9336 100644
--- a/ld/ldlex.l
+++ b/ld/ldlex.l
@@ -323,6 +323,7 @@ V_IDENTIFIER [*?.$_a-zA-Z\[\]\-\!\^\\]([*?.$_a-zA-Z0-9\[\]\-\!\^\\]|::)*
 <EXPRESSION>"DSECT"			{ RTOKEN(DSECT); }
 <EXPRESSION>"COPY"			{ RTOKEN(COPY); }
 <EXPRESSION>"INFO"			{ RTOKEN(INFO); }
+<EXPRESSION>"TYPE"			{ RTOKEN(TYPE); }
 <SCRIPT,EXPRESSION>"ONLY_IF_RO"		{ RTOKEN(ONLY_IF_RO); }
 <SCRIPT,EXPRESSION>"ONLY_IF_RW"		{ RTOKEN(ONLY_IF_RW); }
 <SCRIPT,EXPRESSION>"SPECIAL"		{ RTOKEN(SPECIAL); }
diff --git a/ld/mri.c b/ld/mri.c
index b428ab0d0bf..5749870ef1e 100644
--- a/ld/mri.c
+++ b/ld/mri.c
@@ -210,8 +210,8 @@ mri_draw_tree (void)
 	    base = p->vma ? p->vma : exp_nameop (NAME, ".");
 
 	  lang_enter_output_section_statement (p->name, base,
-					       p->ok_to_load ? normal_section : noload_section,
-					       align, subalign, NULL, 0, 0);
+	    p->ok_to_load ? normal_section : noload_section, 0,
+	    align, subalign, NULL, 0, 0);
 	  base = 0;
 	  tmp = (struct wildcard_list *) xmalloc (sizeof *tmp);
 	  tmp->next = NULL;
diff --git a/ld/testsuite/ld-scripts/output-section-types.d b/ld/testsuite/ld-scripts/output-section-types.d
index ab124fa4dd7..214b1cf6e58 100644
--- a/ld/testsuite/ld-scripts/output-section-types.d
+++ b/ld/testsuite/ld-scripts/output-section-types.d
@@ -1,13 +1,16 @@
 #ld: -Toutput-section-types.t
 #source: align2a.s
-#objdump: -h
+#readelf: -S --wide
 #target: [is_elf_format]
 
 #...
-  . \.rom.*
-[ 	]+ALLOC, READONLY
-  . \.ro.*
-[ 	]+CONTENTS, ALLOC, LOAD, READONLY, DATA
-  . \.over.*
-[ 	]+CONTENTS, READONLY
+.* .rom          +NOBITS        +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +0 +0 +[1248]
+.* .ro           +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +0 +0 +[1248]
+.* .over         +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00  + +0 +0 +[1248]
+.* progbits      +PROGBITS      +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +0 +0 +[1248]
+.* strtab        +STRTAB        +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +0 +0 +[1248]
+.* note          +NOTE          +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +00 +A +0 +0 +[1248]
+.* init_array    +INIT_ARRAY    +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +0 +0 +[1248]
+.* fini_array    +FINI_ARRAY    +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +0 +0 +[1248]
+.* preinit_array +PREINIT_ARRAY +[0-9a-f]+ +[0-9a-f]+ +[0-9a-f]+ +0[48] +A +0 +0 +[1248]
 #pass
diff --git a/ld/testsuite/ld-scripts/output-section-types.t b/ld/testsuite/ld-scripts/output-section-types.t
index d8fdfda1a03..4fd498c9ba0 100644
--- a/ld/testsuite/ld-scripts/output-section-types.t
+++ b/ld/testsuite/ld-scripts/output-section-types.t
@@ -2,6 +2,12 @@ SECTIONS {
   .rom  (NOLOAD)   : { LONG(1234); }
   .ro   (READONLY) : { LONG(5678); }
   .over (OVERLAY)  : { LONG(0123); }
+  progbits (TYPE=SHT_PROGBITS) : { BYTE(1) }
+  strtab (TYPE = SHT_STRTAB) : { BYTE(0) }
+  note (TYPE =SHT_NOTE) : { BYTE(8) }
+  init_array (TYPE= 14) : { QUAD(14) }
+  fini_array ( TYPE=SHT_FINI_ARRAY) : { QUAD(15) }
+  preinit_array (TYPE=SHT_PREINIT_ARRAY ) : { QUAD(16) }
   /DISCARD/        : { *(*) }
 
 }
-- 
2.35.0.rc2.247.g8bbb082509-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-02 18:32   ` Fangrui Song
@ 2022-02-03 18:36     ` Luca Boccassi
  2022-02-03 19:46       ` Fangrui Song
  0 siblings, 1 reply; 10+ messages in thread
From: Luca Boccassi @ 2022-02-03 18:36 UTC (permalink / raw)
  To: Fangrui Song, Nick Clifton; +Cc: binutils, Alan Modra

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

On Wed, 2022-02-02 at 10:32 -0800, Fangrui Song wrote:
> On 2022-02-02, Nick Clifton via Binutils wrote:
> > Hi Fangrui,
> > 
> > > The current output section type allows to set the ELF section type to
> > > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
> > > to be specified. Some ELF section type names are supported as well,
> > 
> > Thanks for the patch submission.  I am regression testing it at the moment
> > but in the meantime a couple of things stood out for me:
> > 
> > 
> > > +@item TYPE = @var{type}
> > > +Set the section type to the integer @var{type}. For the ELF output
> > > +file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
> > > +@var{type}.
> > 
> > Rather than having users guess, it would probably be best to list which
> > type names are supported.
> > 
> > Also it is probably worth documenting that it is the user's fault if they
> > set the section type to something which has special semantics (eg SHT_GROUP)
> > but then they do not also arrange for whatever necessary support that feature
> > needs.  Something like: "it is the user's responsibility to ensure that
> > any special requirements of the section type are met".
> 
> Thanks for the quick review! Adopted the wording and the change below.
> The new patch is in the attachment.
> 
> 
> For SHT_GROUP, I think it is useful to support SHT_GROUP as well. I
> actually did an experiment last night but SHT_GROUP led to an internal
> error. There may be some issues that need to be fixed to use the
> SHT_GROUP feature.
> 
> > 
> > 
> > > +	    case type_section:
> > > +	      if (os->sectype_value->type.node_class == etree_name
> > > +		  && os->sectype_value->type.node_code == NAME)
> > > +		{
> > > +		  const char *name = os->sectype_value->name.name;
> > > +		  if (strcmp (name, "SHT_PROGBITS") == 0)
> > > +		    type = SHT_PROGBITS;
> > > +		  else if (strcmp (name, "SHT_NOTE") == 0)
> > > +		    type = SHT_NOTE;
> > > +		  else if (strcmp (name, "SHT_NOBITS") == 0)
> > > +		    type = SHT_NOBITS;
> > > +		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
> > > +		    type = SHT_INIT_ARRAY;
> > > +		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
> > > +		    type = SHT_FINI_ARRAY;
> > > +		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
> > > +		    type = SHT_PREINIT_ARRAY;
> > > +		  else
> > > +		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
> > > +			   os->name);
> > 
> > It might be worth adding SHT_STRTAB to this list, as I can imagine some
> > weird sceanario where someone would want it.
> 
> Added.
> 
> > 
> > Also - given that this is a new feature, there really ought to be an entry
> > for it in the ld/NEWS file.
> 
> Added. The NEW entry is for 2.39, but feel free to port it to 2.38 if
> you think appropriate:)

Hi,

I tested this patch, it doesn't seem to allow combining multiple
attributes. Tried both in the same brackets and separately, eg:

.note.foo (READONLY) (TYPE=SHT_NOTE)

.note.foo (READONLY TYPE=SHT_NOTE)

Could you please send a new revision that fixes this (no opinion on the
syntax), so that we can use it? Thanks!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-03 18:36     ` Luca Boccassi
@ 2022-02-03 19:46       ` Fangrui Song
  2022-02-03 20:04         ` Luca Boccassi
  0 siblings, 1 reply; 10+ messages in thread
From: Fangrui Song @ 2022-02-03 19:46 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: Nick Clifton, binutils, Alan Modra

On 2022-02-03, Luca Boccassi wrote:
>On Wed, 2022-02-02 at 10:32 -0800, Fangrui Song wrote:
>> On 2022-02-02, Nick Clifton via Binutils wrote:
>> > Hi Fangrui,
>> >
>> > > The current output section type allows to set the ELF section type to
>> > > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary section value
>> > > to be specified. Some ELF section type names are supported as well,
>> >
>> > Thanks for the patch submission.  I am regression testing it at the moment
>> > but in the meantime a couple of things stood out for me:
>> >
>> >
>> > > +@item TYPE = @var{type}
>> > > +Set the section type to the integer @var{type}. For the ELF output
>> > > +file, some type names (e.g. @code{SHT_NOTE}) are also allowed for
>> > > +@var{type}.
>> >
>> > Rather than having users guess, it would probably be best to list which
>> > type names are supported.
>> >
>> > Also it is probably worth documenting that it is the user's fault if they
>> > set the section type to something which has special semantics (eg SHT_GROUP)
>> > but then they do not also arrange for whatever necessary support that feature
>> > needs.  Something like: "it is the user's responsibility to ensure that
>> > any special requirements of the section type are met".
>>
>> Thanks for the quick review! Adopted the wording and the change below.
>> The new patch is in the attachment.
>>
>>
>> For SHT_GROUP, I think it is useful to support SHT_GROUP as well. I
>> actually did an experiment last night but SHT_GROUP led to an internal
>> error. There may be some issues that need to be fixed to use the
>> SHT_GROUP feature.
>>
>> >
>> >
>> > > +	    case type_section:
>> > > +	      if (os->sectype_value->type.node_class == etree_name
>> > > +		  && os->sectype_value->type.node_code == NAME)
>> > > +		{
>> > > +		  const char *name = os->sectype_value->name.name;
>> > > +		  if (strcmp (name, "SHT_PROGBITS") == 0)
>> > > +		    type = SHT_PROGBITS;
>> > > +		  else if (strcmp (name, "SHT_NOTE") == 0)
>> > > +		    type = SHT_NOTE;
>> > > +		  else if (strcmp (name, "SHT_NOBITS") == 0)
>> > > +		    type = SHT_NOBITS;
>> > > +		  else if (strcmp (name, "SHT_INIT_ARRAY") == 0)
>> > > +		    type = SHT_INIT_ARRAY;
>> > > +		  else if (strcmp (name, "SHT_FINI_ARRAY") == 0)
>> > > +		    type = SHT_FINI_ARRAY;
>> > > +		  else if (strcmp (name, "SHT_PREINIT_ARRAY") == 0)
>> > > +		    type = SHT_PREINIT_ARRAY;
>> > > +		  else
>> > > +		    einfo (_ ("%F%P: invalid type for output section `%s'\n"),
>> > > +			   os->name);
>> >
>> > It might be worth adding SHT_STRTAB to this list, as I can imagine some
>> > weird sceanario where someone would want it.
>>
>> Added.
>>
>> >
>> > Also - given that this is a new feature, there really ought to be an entry
>> > for it in the ld/NEWS file.
>>
>> Added. The NEW entry is for 2.39, but feel free to port it to 2.38 if
>> you think appropriate:)
>
>Hi,
>
>I tested this patch, it doesn't seem to allow combining multiple
>attributes. Tried both in the same brackets and separately, eg:
>
>.note.foo (READONLY) (TYPE=SHT_NOTE)
>
>.note.foo (READONLY TYPE=SHT_NOTE)
>
>Could you please send a new revision that fixes this (no opinion on the
>syntax), so that we can use it? Thanks!
>

It doesn't, as I am not sure the combination is useful.

---

I raised my concern twice when READONLY was added:

* https://sourceware.org/pipermail/binutils/2021-May/116579.html
* https://sourceware.org/pipermail/binutils/2021-July/117492.html

See also Michael Matz's https://sourceware.org/pipermail/binutils/2022-February/119598.html

"IOW: I think the introduction of READONLY was ill-advised :-/  But, ...  well :)"

In another place, you said

> As I have already shown, it is necessary. The note is read/write
> otherwise:
> 
> [ 3] .note.package NOTE 00000000000002e8 000002e8
> 
> 0000000000000030  0000000000000000  WA       0     0     4
> and that breaks processing the note from core files. It needs to be
> read/only, like the build-id, and hence the required change in bfd to
> make it possible to do so.

I believe none of Alan, Matz, and I can reproduce what you have seen.
(If Alan noticed it, he should have fixed it in https://sourceware.org/bugzilla/show_bug.cgi?id=26378#c11)
Can you give more instructions how your .note.package got the SHF_WRITE
flag? It looks like a separate ld bug, if your ld included the above change.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-03 19:46       ` Fangrui Song
@ 2022-02-03 20:04         ` Luca Boccassi
  2022-02-03 20:48           ` Fangrui Song
  2022-02-04 16:05           ` Michael Matz
  0 siblings, 2 replies; 10+ messages in thread
From: Luca Boccassi @ 2022-02-03 20:04 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Nick Clifton, binutils, Alan Modra

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

On Thu, 2022-02-03 at 11:46 -0800, Fangrui Song wrote:
> On 2022-02-03, Luca Boccassi wrote:
> > On Wed, 2022-02-02 at 10:32 -0800, Fangrui Song wrote:
> > > On 2022-02-02, Nick Clifton via Binutils wrote:
> > > > Hi Fangrui,
> > > > 
> > > > > The current output section type allows to set the ELF section
> > > > > type to
> > > > > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary
> > > > > section value
> > > > > to be specified. Some ELF section type names are supported as
> > > > > well,
> > > > 
> > > > Thanks for the patch submission.  I am regression testing it at
> > > > the moment
> > > > but in the meantime a couple of things stood out for me:
> > > > 
> > > > 
> > > > > +@item TYPE = @var{type}
> > > > > +Set the section type to the integer @var{type}. For the ELF
> > > > > output
> > > > > +file, some type names (e.g. @code{SHT_NOTE}) are also
> > > > > allowed for
> > > > > +@var{type}.
> > > > 
> > > > Rather than having users guess, it would probably be best to
> > > > list which
> > > > type names are supported.
> > > > 
> > > > Also it is probably worth documenting that it is the user's
> > > > fault if they
> > > > set the section type to something which has special semantics
> > > > (eg SHT_GROUP)
> > > > but then they do not also arrange for whatever necessary
> > > > support that feature
> > > > needs.  Something like: "it is the user's responsibility to
> > > > ensure that
> > > > any special requirements of the section type are met".
> > > 
> > > Thanks for the quick review! Adopted the wording and the change
> > > below.
> > > The new patch is in the attachment.
> > > 
> > > 
> > > For SHT_GROUP, I think it is useful to support SHT_GROUP as well.
> > > I
> > > actually did an experiment last night but SHT_GROUP led to an
> > > internal
> > > error. There may be some issues that need to be fixed to use the
> > > SHT_GROUP feature.
> > > 
> > > > 
> > > > 
> > > > > +           case type_section:
> > > > > +             if (os->sectype_value->type.node_class ==
> > > > > etree_name
> > > > > +                 && os->sectype_value->type.node_code ==
> > > > > NAME)
> > > > > +               {
> > > > > +                 const char *name = os->sectype_value-
> > > > > >name.name;
> > > > > +                 if (strcmp (name, "SHT_PROGBITS") == 0)
> > > > > +                   type = SHT_PROGBITS;
> > > > > +                 else if (strcmp (name, "SHT_NOTE") == 0)
> > > > > +                   type = SHT_NOTE;
> > > > > +                 else if (strcmp (name, "SHT_NOBITS") == 0)
> > > > > +                   type = SHT_NOBITS;
> > > > > +                 else if (strcmp (name, "SHT_INIT_ARRAY") ==
> > > > > 0)
> > > > > +                   type = SHT_INIT_ARRAY;
> > > > > +                 else if (strcmp (name, "SHT_FINI_ARRAY") ==
> > > > > 0)
> > > > > +                   type = SHT_FINI_ARRAY;
> > > > > +                 else if (strcmp (name, "SHT_PREINIT_ARRAY")
> > > > > == 0)
> > > > > +                   type = SHT_PREINIT_ARRAY;
> > > > > +                 else
> > > > > +                   einfo (_ ("%F%P: invalid type for output
> > > > > section `%s'\n"),
> > > > > +                          os->name);
> > > > 
> > > > It might be worth adding SHT_STRTAB to this list, as I can
> > > > imagine some
> > > > weird sceanario where someone would want it.
> > > 
> > > Added.
> > > 
> > > > 
> > > > Also - given that this is a new feature, there really ought to
> > > > be an entry
> > > > for it in the ld/NEWS file.
> > > 
> > > Added. The NEW entry is for 2.39, but feel free to port it to
> > > 2.38 if
> > > you think appropriate:)
> > 
> > Hi,
> > 
> > I tested this patch, it doesn't seem to allow combining multiple
> > attributes. Tried both in the same brackets and separately, eg:
> > 
> > .note.foo (READONLY) (TYPE=SHT_NOTE)
> > 
> > .note.foo (READONLY TYPE=SHT_NOTE)
> > 
> > Could you please send a new revision that fixes this (no opinion on
> > the
> > syntax), so that we can use it? Thanks!
> > 
> 
> It doesn't, as I am not sure the combination is useful.

I understand you don't like it, but as I've already explained again,
and again, and again, it is needed, and frankly I'm getting exhausted.
If you don't want to update your patch that's fine, it's no problem at
all, I'll send a follow-up myself if it gets merged.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-03 20:04         ` Luca Boccassi
@ 2022-02-03 20:48           ` Fangrui Song
  2022-02-04 16:05           ` Michael Matz
  1 sibling, 0 replies; 10+ messages in thread
From: Fangrui Song @ 2022-02-03 20:48 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: Nick Clifton, binutils, Alan Modra

On 2022-02-03, Luca Boccassi wrote:
>On Thu, 2022-02-03 at 11:46 -0800, Fangrui Song wrote:
>> On 2022-02-03, Luca Boccassi wrote:
>> > On Wed, 2022-02-02 at 10:32 -0800, Fangrui Song wrote:
>> > > On 2022-02-02, Nick Clifton via Binutils wrote:
>> > > > Hi Fangrui,
>> > > >
>> > > > > The current output section type allows to set the ELF section
>> > > > > type to
>> > > > > SHT_PROGBITS or SHT_NOLOAD. This patch allows an arbitrary
>> > > > > section value
>> > > > > to be specified. Some ELF section type names are supported as
>> > > > > well,
>> > > >
>> > > > Thanks for the patch submission.  I am regression testing it at
>> > > > the moment
>> > > > but in the meantime a couple of things stood out for me:
>> > > >
>> > > >
>> > > > > +@item TYPE = @var{type}
>> > > > > +Set the section type to the integer @var{type}. For the ELF
>> > > > > output
>> > > > > +file, some type names (e.g. @code{SHT_NOTE}) are also
>> > > > > allowed for
>> > > > > +@var{type}.
>> > > >
>> > > > Rather than having users guess, it would probably be best to
>> > > > list which
>> > > > type names are supported.
>> > > >
>> > > > Also it is probably worth documenting that it is the user's
>> > > > fault if they
>> > > > set the section type to something which has special semantics
>> > > > (eg SHT_GROUP)
>> > > > but then they do not also arrange for whatever necessary
>> > > > support that feature
>> > > > needs.  Something like: "it is the user's responsibility to
>> > > > ensure that
>> > > > any special requirements of the section type are met".
>> > >
>> > > Thanks for the quick review! Adopted the wording and the change
>> > > below.
>> > > The new patch is in the attachment.
>> > >
>> > >
>> > > For SHT_GROUP, I think it is useful to support SHT_GROUP as well.
>> > > I
>> > > actually did an experiment last night but SHT_GROUP led to an
>> > > internal
>> > > error. There may be some issues that need to be fixed to use the
>> > > SHT_GROUP feature.
>> > >
>> > > >
>> > > >
>> > > > > +           case type_section:
>> > > > > +             if (os->sectype_value->type.node_class ==
>> > > > > etree_name
>> > > > > +                 && os->sectype_value->type.node_code ==
>> > > > > NAME)
>> > > > > +               {
>> > > > > +                 const char *name = os->sectype_value-
>> > > > > >name.name;
>> > > > > +                 if (strcmp (name, "SHT_PROGBITS") == 0)
>> > > > > +                   type = SHT_PROGBITS;
>> > > > > +                 else if (strcmp (name, "SHT_NOTE") == 0)
>> > > > > +                   type = SHT_NOTE;
>> > > > > +                 else if (strcmp (name, "SHT_NOBITS") == 0)
>> > > > > +                   type = SHT_NOBITS;
>> > > > > +                 else if (strcmp (name, "SHT_INIT_ARRAY") ==
>> > > > > 0)
>> > > > > +                   type = SHT_INIT_ARRAY;
>> > > > > +                 else if (strcmp (name, "SHT_FINI_ARRAY") ==
>> > > > > 0)
>> > > > > +                   type = SHT_FINI_ARRAY;
>> > > > > +                 else if (strcmp (name, "SHT_PREINIT_ARRAY")
>> > > > > == 0)
>> > > > > +                   type = SHT_PREINIT_ARRAY;
>> > > > > +                 else
>> > > > > +                   einfo (_ ("%F%P: invalid type for output
>> > > > > section `%s'\n"),
>> > > > > +                          os->name);
>> > > >
>> > > > It might be worth adding SHT_STRTAB to this list, as I can
>> > > > imagine some
>> > > > weird sceanario where someone would want it.
>> > >
>> > > Added.
>> > >
>> > > >
>> > > > Also - given that this is a new feature, there really ought to
>> > > > be an entry
>> > > > for it in the ld/NEWS file.
>> > >
>> > > Added. The NEW entry is for 2.39, but feel free to port it to
>> > > 2.38 if
>> > > you think appropriate:)
>> >
>> > Hi,
>> >
>> > I tested this patch, it doesn't seem to allow combining multiple
>> > attributes. Tried both in the same brackets and separately, eg:
>> >
>> > .note.foo (READONLY) (TYPE=SHT_NOTE)
>> >
>> > .note.foo (READONLY TYPE=SHT_NOTE)
>> >
>> > Could you please send a new revision that fixes this (no opinion on
>> > the
>> > syntax), so that we can use it? Thanks!
>> >
>>
>> It doesn't, as I am not sure the combination is useful.
>
>I understand you don't like it, but as I've already explained again,
>and again, and again, it is needed, and frankly I'm getting exhausted.
>If you don't want to update your patch that's fine, it's no problem at
>all, I'll send a follow-up myself if it gets merged.

What I repeatedly requested is how you got SHF_WRITE .note.package but I
never never get an example. You may understand how I got exausted after
my two questions in 2021 were ignored and the recent
https://sourceware.org/pipermail/binutils/2022-January/119537.html got a
reply "it is used" with no detail.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-03 20:04         ` Luca Boccassi
  2022-02-03 20:48           ` Fangrui Song
@ 2022-02-04 16:05           ` Michael Matz
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Matz @ 2022-02-04 16:05 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: Fangrui Song, binutils

Hello Luca,

On Thu, 3 Feb 2022, Luca Boccassi wrote:

> I understand you don't like it, but as I've already explained again, and 
> again, and again, it is needed, and frankly I'm getting exhausted.

But it's not, you are wrong on that account.  It once was necessary until 
the problem (of generating writable sectons by default) was fixed.  There 
was only a very short time window when (READONLY) was supported _and_ 
necessary.  The bug (PR26378) got fixed in January 2021 (so 2.36 should 
have it), and since then we have:

% cat x.c
int main()
{
  return 0;
}
% cat metadata.ld
SECTIONS
{
    .note.foobar : ALIGN(4) {
        BYTE(0x42)
    }
}
INSERT AFTER .note.gnu.build-id;
% gcc x.c -Wl,-T,metadata.ld
% readelf -SW a.out | grep note
  [ 2] .note.gnu.property NOTE            0000000000400338 000338 000020 
00   A  0   0  8
  [ 3] .note.gnu.build-id NOTE            0000000000400358 000358 000024 
00   A  0   0  4
  [ 4] .note.foobar      NOTE            000000000040037c 00037c 000001 00   
A  0   0  4
  [ 5] .note.ABI-tag     NOTE            0000000000400380 000380 000020 00   
A  0   0  4
% ld -v
GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.37.20211112-3

As you can see, no "(READONLY)" but still a read-only .note.foobar 
section.  (Ignore the fact that my contents of .note.foobar isn't a 
correct SHT_NOTE content)

That's what I meant with ill-advised: a feature was added to work-around 
a bug, instead of the bug being fixed.  The bug now is fixed and the 
feature is useless but needs to be kept for backward comp, except maybe 
it could be removed again if the few users could be convinced to just not 
use it anymore; so could you be convinced? :)


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ld: Support customized output section type
  2022-02-02  7:10 [PATCH] ld: Support customized output section type Fangrui Song
  2022-02-02 11:36 ` Nick Clifton
@ 2022-02-21 19:35 ` Mike Frysinger
  1 sibling, 0 replies; 10+ messages in thread
From: Mike Frysinger @ 2022-02-21 19:35 UTC (permalink / raw)
  To: Fangrui Song; +Cc: binutils, Alan Modra, Nick Clifton, Luca Boccassi

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

On 01 Feb 2022 23:10, Fangrui Song via Binutils wrote:
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h

it looks like you've edited this file instead of generating.  so when
generating it from its sources, it breaks.  you can verify with:
touch bfd/*.h
make headers

looks like you forgot to update the docs in section.c to include |type|.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-02-21 19:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02  7:10 [PATCH] ld: Support customized output section type Fangrui Song
2022-02-02 11:36 ` Nick Clifton
2022-02-02 11:46   ` Luca Boccassi
2022-02-02 18:32   ` Fangrui Song
2022-02-03 18:36     ` Luca Boccassi
2022-02-03 19:46       ` Fangrui Song
2022-02-03 20:04         ` Luca Boccassi
2022-02-03 20:48           ` Fangrui Song
2022-02-04 16:05           ` Michael Matz
2022-02-21 19:35 ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).