[PATCH RESEND] elfedit: add support for editing e_flags

[PATCH RESEND] elfedit: add support for editing e_flags

[WANG Xuerui]

From: WANG Xuerui <git@xen0n.name>

Add a pair of elfedit options (--{input,output}-flags) for manipulating
files' e_flags field. This is useful in certain cases, e.g. if you know
what you are doing and have to make some object files look as if
compiled for a different architecture or ABI variant.
---

(Resending because I've overlooked sizeof(output_elf_flags). I wondered
how BYTE_PUT is so smart to figure out the sizes until I've discovered
its *local* definition *after* hitting send...)

I have to pick up the work because I was trying to make AMDGPU DCN work
on LoongArch, but that piece of code requires hard-float support while
the LoongArch Linux kernel is compiled in the soft-float ABI, leading to
link-time failures intermixing the two. Since the hard-float code in
question doesn't pass around FP arguments across the boundary between
object files, but rather only internally, it is safe to just edit the
e_flags of the hard-float objects to make their ABI look like LP64S.
Which led to me discovering the previous similar work [1] has stalled...
And that's why a bunch of LoongArch-related folks are on the CC list.

[1]: https://sourceware.org/pipermail/binutils/2022-April/120413.html

 binutils/doc/binutils.texi                  | 13 ++++-
 binutils/elfedit.c                          | 55 +++++++++++++++++++--
 binutils/testsuite/binutils-all/elfedit-7.d | 10 ++++
 binutils/testsuite/binutils-all/elfedit-8.d | 10 ++++
 binutils/testsuite/binutils-all/elfedit.exp |  2 +
 5 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/elfedit-7.d
 create mode 100644 binutils/testsuite/binutils-all/elfedit-8.d

diff --git a/binutils/doc/binutils.texi b/binutils/doc/binutils.texi
index b1982a95704..a261af48254 100644
--- a/binutils/doc/binutils.texi
+++ b/binutils/doc/binutils.texi
@@ -5297,10 +5297,12 @@ elfedit [@option{--input-mach=}@var{machine}]
         [@option{--input-type=}@var{type}]
         [@option{--input-osabi=}@var{osabi}]
         [@option{--input-abiversion=}@var{version}]
+        [@option{--input-flags=}@var{flags}]
         @option{--output-mach=}@var{machine}
         @option{--output-type=}@var{type}
         @option{--output-osabi=}@var{osabi}
         @option{--output-abiversion=}@var{version}
+        @option{--output-flags=}@var{flags}
         @option{--enable-x86-feature=}@var{feature}
         @option{--disable-x86-feature=}@var{feature}
         [@option{-v}|@option{--version}]
@@ -5325,7 +5327,7 @@ should be updated.
 The long and short forms of options, shown here as alternatives, are
 equivalent. At least one of the @option{--output-mach},
 @option{--output-type}, @option{--output-osabi},
-@option{--output-abiversion},
+@option{--output-abiversion}, @option{--output-flags},
 @option{--enable-x86-feature} and @option{--disable-x86-feature}
 options must be given.
 
@@ -5376,6 +5378,15 @@ isn't specified, it will match any ELF ABIVERSIONs.
 Change the ELF ABIVERSION in the ELF header to @var{version}.
 @var{version} must be between 0 and 255.
 
+@item --input-flags=@var{flags}
+Set the matching input ELF file @var{e_flags} value to @var{flags}.
+@var{flags} must be an unsigned 32-bit integer.  If @option{--input-flags}
+isn't specified, it will match any e_flags value.
+
+@item --output-flags=@var{flags}
+Change the @var{e_flags} field in the ELF header to @var{flags}.
+@var{version} must be an unsigned 32-bit integer.
+
 @item --enable-x86-feature=@var{feature}
 Set the @var{feature} bit in program property in @var{exec} or @var{dyn}
 ELF files with machine types of @var{i386} or @var{x86-64}.  The
diff --git a/binutils/elfedit.c b/binutils/elfedit.c
index 117e67639a0..8939198c415 100644
--- a/binutils/elfedit.c
+++ b/binutils/elfedit.c
@@ -68,6 +68,10 @@ enum elfclass
   };
 static enum elfclass input_elf_class = ELF_CLASS_UNKNOWN;
 static enum elfclass output_elf_class = ELF_CLASS_BOTH;
+static int check_elf_flags = 0;
+static unsigned int input_elf_flags = 0;
+static int write_elf_flags = 0;
+static unsigned int output_elf_flags = 0;
 
 #ifdef HAVE_MMAP
 #include <sys/mman.h>
@@ -394,7 +398,16 @@ update_elf_header (const char *file_name, FILE *file)
       return 0;
     }
 
-  /* Update e_machine, e_type and EI_OSABI.  */
+  /* Skip if e_flags doesn't match. */
+  if (check_elf_flags && elf_header.e_flags != input_elf_flags)
+    {
+      error
+	(_("%s: Unmatched e_flags: 0x%lx is not 0x%x\n"),
+	 file_name, elf_header.e_flags, input_elf_flags);
+      return 0;
+    }
+
+  /* Update e_machine, e_type, OSABI, ABIVERSION and e_flags.  */
   switch (class)
     {
     default:
@@ -410,6 +423,8 @@ update_elf_header (const char *file_name, FILE *file)
 	ehdr32.e_ident[EI_OSABI] = output_elf_osabi;
       if (output_elf_abiversion != -1)
 	ehdr32.e_ident[EI_ABIVERSION] = output_elf_abiversion;
+      if (write_elf_flags)
+	BYTE_PUT (ehdr32.e_flags, output_elf_flags);
       status = fwrite (&ehdr32, sizeof (ehdr32), 1, file) == 1;
       break;
     case ELFCLASS64:
@@ -421,6 +436,8 @@ update_elf_header (const char *file_name, FILE *file)
 	ehdr64.e_ident[EI_OSABI] = output_elf_osabi;
       if (output_elf_abiversion != -1)
 	ehdr64.e_ident[EI_ABIVERSION] = output_elf_abiversion;
+      if (write_elf_flags)
+	BYTE_PUT (ehdr64.e_flags, output_elf_flags);
       status = fwrite (&ehdr64, sizeof (ehdr64), 1, file) == 1;
       break;
     }
@@ -904,6 +921,8 @@ enum command_line_switch
     OPTION_OUTPUT_OSABI,
     OPTION_INPUT_ABIVERSION,
     OPTION_OUTPUT_ABIVERSION,
+    OPTION_INPUT_FLAGS,
+    OPTION_OUTPUT_FLAGS,
 #ifdef HAVE_MMAP
     OPTION_ENABLE_X86_FEATURE,
     OPTION_DISABLE_X86_FEATURE,
@@ -920,6 +939,8 @@ static struct option options[] =
   {"output-osabi",	required_argument, 0, OPTION_OUTPUT_OSABI},
   {"input-abiversion",	required_argument, 0, OPTION_INPUT_ABIVERSION},
   {"output-abiversion",	required_argument, 0, OPTION_OUTPUT_ABIVERSION},
+  {"input-flags",	required_argument, 0, OPTION_INPUT_FLAGS},
+  {"output-flags",	required_argument, 0, OPTION_OUTPUT_FLAGS},
 #ifdef HAVE_MMAP
   {"enable-x86-feature",
 			required_argument, 0, OPTION_ENABLE_X86_FEATURE},
@@ -958,7 +979,11 @@ usage (FILE *stream, int exit_status)
   --output-osabi [%s]\n\
                               Set output OSABI\n\
   --input-abiversion [0-255]  Set input ABIVERSION\n\
-  --output-abiversion [0-255] Set output ABIVERSION\n"),
+  --output-abiversion [0-255] Set output ABIVERSION\n\
+  --input-flags [32-bit unsigned integer]\n\
+                              Set input e_flags\n\
+  --output-flags [32-bit unsigned integer]\n\
+                              Set output e_flags\n"),
 	   osabi, osabi);
 #ifdef HAVE_MMAP
   fprintf (stream, _("\
@@ -983,6 +1008,7 @@ main (int argc, char ** argv)
 {
   int c, status;
   char *end;
+  unsigned long tmp;
 
 #ifdef HAVE_LC_MESSAGES
   setlocale (LC_MESSAGES, "");
@@ -1062,6 +1088,28 @@ main (int argc, char ** argv)
 	    }
 	  break;
 
+	case OPTION_INPUT_FLAGS:
+	  check_elf_flags = 1;
+	  tmp = strtoul (optarg, &end, 0);
+	  if (*end != '\0' || tmp > 0xffffffff)
+	    {
+	      error (_("Invalid e_flags: %s\n"), optarg);
+	      return 1;
+	    }
+	  input_elf_flags = (unsigned int) tmp;
+	  break;
+
+	case OPTION_OUTPUT_FLAGS:
+	  write_elf_flags = 1;
+	  tmp = strtoul (optarg, &end, 0);
+	  if (*end != '\0' || tmp > 0xffffffff)
+	    {
+	      error (_("Invalid e_flags: %s\n"), optarg);
+	      return 1;
+	    }
+	  output_elf_flags = (unsigned int) tmp;
+	  break;
+
 #ifdef HAVE_MMAP
 	case OPTION_ENABLE_X86_FEATURE:
 	  if (elf_x86_feature (optarg, 1) < 0)
@@ -1094,7 +1142,8 @@ main (int argc, char ** argv)
 #endif
 	  && output_elf_type == -1
 	  && output_elf_osabi == -1
-	  && output_elf_abiversion == -1))
+	  && output_elf_abiversion == -1
+	  && ! write_elf_flags))
     usage (stderr, 1);
 
   status = 0;
diff --git a/binutils/testsuite/binutils-all/elfedit-7.d b/binutils/testsuite/binutils-all/elfedit-7.d
new file mode 100644
index 00000000000..8e5a8b7d019
--- /dev/null
+++ b/binutils/testsuite/binutils-all/elfedit-7.d
@@ -0,0 +1,10 @@
+#PROG: elfedit
+#elfedit: --output-flags 0xfedcba98
+#source: empty.s
+#readelf: -h
+#name: Update ELF header 7 (hexadecimal e_flags value)
+#target: *-*-linux* *-*-gnu*
+
+#...
+  Flags:[ \t]+0xfedcba98.*
+#...
diff --git a/binutils/testsuite/binutils-all/elfedit-8.d b/binutils/testsuite/binutils-all/elfedit-8.d
new file mode 100644
index 00000000000..9538f4486b8
--- /dev/null
+++ b/binutils/testsuite/binutils-all/elfedit-8.d
@@ -0,0 +1,10 @@
+#PROG: elfedit
+#elfedit: --output-flags 12345678
+#source: empty.s
+#readelf: -h
+#name: Update ELF header 8 (decimal e_flags value)
+#target: *-*-linux* *-*-gnu*
+
+#...
+  Flags:[ \t]+0xbc614e.*
+#...
diff --git a/binutils/testsuite/binutils-all/elfedit.exp b/binutils/testsuite/binutils-all/elfedit.exp
index 33f4bb05529..47e94e5a775 100644
--- a/binutils/testsuite/binutils-all/elfedit.exp
+++ b/binutils/testsuite/binutils-all/elfedit.exp
@@ -26,3 +26,5 @@ run_dump_test "elfedit-3"
 run_dump_test "elfedit-4"
 run_dump_test "elfedit-5"
 run_dump_test "elfedit-6"
+run_dump_test "elfedit-7"
+run_dump_test "elfedit-8"
-- 
2.39.1

Re: [PATCH RESEND] elfedit: add support for editing e_flags

[Jan Beulich]

On 02.03.2023 09:14, WANG Xuerui wrote:
> @@ -5376,6 +5378,15 @@ isn't specified, it will match any ELF ABIVERSIONs.
>  Change the ELF ABIVERSION in the ELF header to @var{version}.
>  @var{version} must be between 0 and 255.
>  
> +@item --input-flags=@var{flags}
> +Set the matching input ELF file @var{e_flags} value to @var{flags}.
> +@var{flags} must be an unsigned 32-bit integer.  If @option{--input-flags}
> +isn't specified, it will match any e_flags value.

This doesn't really explain what the behavior is; I had to look at the code
to see what's intended. However, ...

> +@item --output-flags=@var{flags}
> +Change the @var{e_flags} field in the ELF header to @var{flags}.
> +@var{version} must be an unsigned 32-bit integer.

... instead of this pair of options, for anything flags-ish wouldn't it be
more convenient to have a way to merely specify a flags-to-clear and a flags-
to-set value? That would imo also eliminate the need for checking that the
input object's flags match a certain value.

> @@ -394,7 +398,16 @@ update_elf_header (const char *file_name, FILE *file)
>        return 0;
>      }
>  
> -  /* Update e_machine, e_type and EI_OSABI.  */
> +  /* Skip if e_flags doesn't match. */
> +  if (check_elf_flags && elf_header.e_flags != input_elf_flags)
> +    {
> +      error
> +	(_("%s: Unmatched e_flags: 0x%lx is not 0x%x\n"),

May I suggest to use %#x (and alike) wherever suitable? It's a shorter
string literal and doesn't needlessly clutter the value of 0 with a 0x
prefix.

> @@ -1062,6 +1088,28 @@ main (int argc, char ** argv)
>  	    }
>  	  break;
>  
> +	case OPTION_INPUT_FLAGS:
> +	  check_elf_flags = 1;

Please use bool/true/false for boolean variables.

> +	  tmp = strtoul (optarg, &end, 0);
> +	  if (*end != '\0' || tmp > 0xffffffff)

I'm afraid this may trigger compiler warnings: For one because the constant
isn't flagged as unsigned. And then (for 32-bit targets) for being an
always-true comparison.

> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/elfedit-7.d
> @@ -0,0 +1,10 @@
> +#PROG: elfedit
> +#elfedit: --output-flags 0xfedcba98
> +#source: empty.s
> +#readelf: -h
> +#name: Update ELF header 7 (hexadecimal e_flags value)
> +#target: *-*-linux* *-*-gnu*

Any reason *-*-elf* isn't included here? But I guess you really want to
use [is_elf_format] anyway.

Also neither of the two tests uses the other option. If that's to remain,
I think it wants testing (I realize that doing so may require further
limiting the targets on which such a test can be easily run).

Jan

Re: [PATCH RESEND] elfedit: add support for editing e_flags

[Jan Beulich]

On 02.03.2023 09:58, Jan Beulich via Binutils wrote:
> On 02.03.2023 09:14, WANG Xuerui wrote:
>> +	  tmp = strtoul (optarg, &end, 0);
>> +	  if (*end != '\0' || tmp > 0xffffffff)
> 
> I'm afraid this may trigger compiler warnings: For one because the constant
> isn't flagged as unsigned. And then (for 32-bit targets) for being an
> always-true comparison.

Argh, always-false (but you get the point).

Jan