Re: [PATCH] ld: Support customized output section type

[Nick Clifton <nickc@redhat.com>]

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