Re: [PATCH v2] objcopy: Add elf header e_flags option in objcopy.

[WANG Xuerui <i.swmail@xen0n.name>]

Hi,

On 4/15/22 17:41, liuzhensong wrote:
> Specify flags when copying binary to elf file.
> This flags will be written to the e_flags of the elf header.
>
> usage:
>    objcopy -I binary -O target --alt-elf-eflags=val bin_file  elf_file
>
> binutils/
>    * objcopy.c
>    * doc/binutils.texi:Add the new objcopy option.
> ---
>   binutils/doc/binutils.texi |  7 +++++++
>   binutils/objcopy.c         | 16 ++++++++++++++++
>   2 files changed, 23 insertions(+)
>
> diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
> index 2c234c682aa..b36aad05c98 100644
> --- a/binutils/doc/binutils.texi
> +++ b/binutils/doc/binutils.texi
> @@ -1302,6 +1302,7 @@ objcopy [@option{-F} @var{bfdname}|@option{--target=}@var{bfdname}]
>           [@option{--weaken-symbols=}@var{filename}]
>           [@option{--add-symbol} @var{name}=[@var{section}:]@var{value}[,@var{flags}]]
>           [@option{--alt-machine-code=}@var{index}]
> +        [@option{--alt-elf-eflags=}@var{value}]
>           [@option{--prefix-symbols=}@var{string}]
>           [@option{--prefix-sections=}@var{string}]
>           [@option{--prefix-alloc-sections=}@var{string}]
> @@ -1942,6 +1943,12 @@ being used.  For ELF based architectures if the @var{index}
>   alternative does not exist then the value is treated as an absolute
>   number to be stored in the e_machine field of the ELF header.
>   
> +@item --alt-elf-eflags=@var{value}
> +If the output file has alternate ELF header e_flags, use the @var{vaule}

Typo; "value".

> +instead of the default one.  This is useful in case objcopy a binary to
> +an ELF to specify the e_flags. The value is treated as an absolute
> +number to be stored in the e_flags field of the ELF header.

Actually the whole description seems inaccurate (which field is the 
alternate to e_flags? what's e_flags's "default" value?), and the 
English is broken.

By reading the implementation it's clear the value specified here would 
get into any output file that is ELF. So no need to call it "alt"; a 
name like "--elf-eflags" would be enough.

Something like this would be better IMO:

"Meaningful only for ELF output.  Use @var{value} as e_flags of the 
output.  This is useful for changing the e_flags of an existing binary.  
The @var{value} is treated as an absolute number to be stored in the 
e_flags field of the ELF header."

> +
>   @item --writable-text
>   Mark the output text as writable.  This option isn't meaningful for all
>   object file formats.
> diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> index 6fb31c8cac7..032caf565d1 100644
> --- a/binutils/objcopy.c
> +++ b/binutils/objcopy.c
> @@ -303,6 +303,10 @@ enum long_section_name_handling
>      This is also the only behaviour for 'strip'.  */
>   static enum long_section_name_handling long_section_names = KEEP;
>   
> +/* Use alternative elf header e_flags?  */
> +static bool alt_elf_eflags_set = false;
> +static unsigned int alt_elf_eflags = 0;
> +
>   /* 150 isn't special; it's just an arbitrary non-ASCII char value.  */
>   enum command_line_switch
>   {
> @@ -310,6 +314,7 @@ enum command_line_switch
>     OPTION_ADD_GNU_DEBUGLINK,
>     OPTION_ADD_SYMBOL,
>     OPTION_ALT_MACH_CODE,
> +  OPTION_ALT_ELF_EFLAGS,
>     OPTION_CHANGE_ADDRESSES,
>     OPTION_CHANGE_LEADING_CHAR,
>     OPTION_CHANGE_SECTION_ADDRESS,
> @@ -427,6 +432,7 @@ static struct option copy_options[] =
>     {"adjust-vma", required_argument, 0, OPTION_CHANGE_ADDRESSES},
>     {"adjust-warnings", no_argument, 0, OPTION_CHANGE_WARNINGS},
>     {"alt-machine-code", required_argument, 0, OPTION_ALT_MACH_CODE},
> +  {"alt-elf-eflags", required_argument, 0, OPTION_ALT_ELF_EFLAGS},
>     {"binary-architecture", required_argument, 0, 'B'},
>     {"byte", required_argument, 0, 'b'},
>     {"change-addresses", required_argument, 0, OPTION_CHANGE_ADDRESSES},
> @@ -660,6 +666,7 @@ copy_usage (FILE *stream, int exit_status)
>        --weaken-symbols <file>       -W for all symbols listed in <file>\n\
>        --add-symbol <name>=[<section>:]<value>[,<flags>]  Add a symbol\n\
>        --alt-machine-code <index>    Use the target's <index>'th alternative machine\n\
> +     --alt-elf-eflags=<value>      Use the alternative elf header e_flags\n\
>        --writable-text               Mark the output text as writable\n\
>        --readonly-text               Make the output text write protected\n\
>        --pure                        Mark the output file as demand paged\n\
> @@ -3496,6 +3503,10 @@ copy_object (bfd *ibfd, bfd *obfd, const bfd_arch_info_type *input_arch)
>   	}
>       }
>   
> +  if (bfd_get_flavour (obfd) == bfd_target_elf_flavour
> +      && alt_elf_eflags_set)
> +    elf_elfheader (obfd)->e_flags = alt_elf_eflags;
> +
>     return true;
>   }
>   
> @@ -5863,6 +5874,11 @@ copy_main (int argc, char *argv[])
>   	    fatal (_("verilog data width must be at least 1 byte"));
>   	  break;
>   
> +	case OPTION_ALT_ELF_EFLAGS:
> +	  alt_elf_eflags_set = true;
> +	  alt_elf_eflags = parse_vma (optarg, "--alt-elf-eflags");
> +	  break;
> +
>   	case 0:
>   	  /* We've been given a long option.  */
>   	  break;