[PATCH] Add --multifile-pointer-size <n> and --multifile-endian <l|L|b|B>

[PATCH] Add --multifile-pointer-size <n> and --multifile-endian <l|L|b|B>

[Tom de Vries]

Hi,

Consider binaries:
- hello.64 generated using -m64, with pointer size 8,
- hello.32 generated using -m32, with pointer size 4.

When trying to generate a multifile using files with different
pointer sizes, we get:
...
$ cp ../hello.64 1; cp 1 2; \
    cp ../hello.32 3; cp 3 4; \
    dwz -m 5 1 2 3 4; echo $?
dwz: Multi-file optimization not allowed for different pointer sizes \
  or endianity
0
...
and the multi-file optimization has not been applied for any file:
...
$ for f in 1 2 3 4; do \
    echo -n "$f: "; \
    readelf -S -W $f \
        | grep -c gnu_debugaltlink; \
    done
1: 0
2: 0
3: 0
4: 0
...

Add an option --multifile-pointer-size <n> / -p <n> that sets the pointer size
of the multifile, such that we have instead with say -p 8:
...
$ cp ../hello.64 1; cp 1 2; \
    cp ../hello.32 3; cp 3 4; \
    ./dwz -m 5 -p 8 1 2 3 4; echo $?
./dwz: File 3 skipped for multi-file optimization, different pointer size
./dwz: File 4 skipped for multi-file optimization, different pointer size
0
...
and:
...
$ for f in 1 2 3 4; do \
    echo -n "$f: "; \
    readelf -S -W $f \
        | grep -c gnu_debugaltlink; \
    done
1: 1
2: 1
3: 0
4: 0
...

Likewise, add --multifile-endian <l|L|b|B> / -e <l|L|b|B>.

Any comments?

Thanks,
- Tom

Add --multifile-pointer-size <n> and --multifile-endian <l|L|b|B>

2021-03-25  Tom de Vries  <tdevries@suse.de>

	* args.c (multifile_force_ptr_size, multifile_force_endian): New
	variable.
	(dwz_options, dwz_multi_file_options_help): Add
	--multifile-pointer-size and --multifile-endian entries.
	(parse_args): Handle 'p' and 'e' options.
	* args.h (multifile_force_ptr_size, multifile_force_endian): Declare.
	* dwz.c (struct file_result): Move up.  Add skip_multifile field.
	(write_multifile): Add res parameter.  Handle multifile_force_ptr_size
	and multifile_force_endian.
	(dwz): Update call to write_multifile.
	(init_file_result): Init skip_multifile field.
	(dwz_files_1): Handle res->skip_multifile.
	* dwz.1: Add --multifile-pointer-size and --multifile-endian entries.

---
 args.c | 43 +++++++++++++++++++++++++++++--
 args.h |  2 ++
 dwz.1  |  6 +++++
 dwz.c  | 91 +++++++++++++++++++++++++++++++++++++++++++++---------------------
 4 files changed, 112 insertions(+), 30 deletions(-)

diff --git a/args.c b/args.c
index cc8c717..cb58dd4 100644
--- a/args.c
+++ b/args.c
@@ -25,6 +25,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <error.h>
+#include <gelf.h>
 
 #include "args.h"
 
@@ -75,6 +76,11 @@ const char *multifile_name;
    the particular file is present.  */
 bool multifile_relative;
 
+/* Pointer size of multifile.  */
+int multifile_force_ptr_size;
+/* Endianity of multifile.  */
+int multifile_force_endian;
+
 /* True if DWARF 5 .debug_sup and DW_FORM_ref_sup4 / DW_FORM_strp_sup
    should be used instead of the GNU extensions .gnu_debugaltlink
    and DW_FORM_GNU_ref_alt / DW_FORM_GNU_strp_alt etc.  */
@@ -161,6 +167,10 @@ static struct option dwz_options[] =
   { "odr",		 no_argument,	    &odr, 1 },
   { "no-odr",		 no_argument,	    &odr, 0 },
   { "odr-mode",		 required_argument, &odr_mode_parsed, 1 },
+  { "multifile-pointer-size",
+			 required_argument, 0, 'p' },
+  { "multifile-endian",
+			 required_argument, 0, 'e' },
   { NULL,		 no_argument,	    0, 0 }
 };
 
@@ -219,7 +229,11 @@ static struct option_help dwz_multi_file_options_help[] =
     " to multifile." },
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
-    " GNU extension .debug_altlink." }
+    " GNU extension .debug_altlink." },
+  { "p", "multifile-pointer-size", "SIZE", NULL,
+    "Set pointer size of multifile, in number of bytes." },
+  { "e", "multifile-endian", "<l|L|b|B>", NULL,
+    "Set endianity of multifile." },
 };
 
 /* Describe misc command line options.  */
@@ -487,7 +501,7 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
   while (1)
     {
       int option_index = -1;
-      int c = getopt_long (argc, argv, "m:o:qhl:L:M:r?v5", dwz_options,
+      int c = getopt_long (argc, argv, "m:o:qhl:L:M:r?v5p:e:", dwz_options,
 			   &option_index);
       if (c == -1)
 	break;
@@ -616,6 +630,31 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	  dwarf_5 = true;
 	  break;
 
+	case 'p':
+	  l = strtoul (optarg, &end, 0);
+	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
+	    error (1, 0, "invalid argument -l %s", optarg);
+	  multifile_force_ptr_size = l;
+	  break;
+
+	case 'e':
+	  if (strlen (optarg) != 1)
+	    error (1, 0, "invalid argument -l %s", optarg);
+	  switch (optarg[0])
+	    {
+	    case 'l':
+	    case 'L':
+	      multifile_force_endian = ELFDATA2LSB;
+		break;
+	    case 'b':
+	    case 'B':
+	      multifile_force_endian = ELFDATA2MSB;
+		break;
+	    default:
+	      error (1, 0, "invalid argument -l %s", optarg);
+	    }
+	  break;
+
 	case 'v':
 	  version ();
 	  break;
diff --git a/args.h b/args.h
index c899003..8d34483 100644
--- a/args.h
+++ b/args.h
@@ -80,6 +80,8 @@ extern enum odr_mode odr_mode;
 extern const char *multifile;
 extern const char *multifile_name;
 extern bool multifile_relative;
+extern int multifile_force_ptr_size;
+extern int multifile_force_endian;
 
 extern unsigned char multifile_mode;
 
diff --git a/dwz.1 b/dwz.1
index 19dc814..ffe0c71 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,6 +77,12 @@ the executable or shared library to the file named in the argument
 of the \fB-m\fR option.  Either \fB-M\fR or \fB-r\fR
 option can be specified, but not both.
 .TP
+.B \-p N \-\-multifile-pointer-size N
+Specify the pointer size of the multifile, in bytes.
+.TP
+.B \-p <l|L|B|b> \-\-multifile-endian <l|L|B|b>
+Specify the endianity of the multifile.
+.TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.
 .TP
diff --git a/dwz.c b/dwz.c
index b865a92..77833e1 100644
--- a/dwz.c
+++ b/dwz.c
@@ -15039,10 +15039,27 @@ clear_p2_field (void)
 }
 #endif
 
+/* Helper structure for hardlink discovery.  */
+struct file_result
+{
+  /* -3: Uninitialized.
+     -2: Already processed under different name.
+     -1: Ignore.
+      0: Processed, changed.
+      1: Processed, unchanged.  */
+  int res;
+  dev_t dev;
+  ino_t ino;
+  nlink_t nlink;
+  size_t hardlink_to;
+  unsigned int die_count;
+  bool skip_multifile;
+};
+
 /* Collect potentially shareable DIEs, strings and .debug_macro
    opcode sequences into temporary .debug_* files.  */
 static int
-write_multifile (DSO *dso)
+write_multifile (DSO *dso, struct file_result *res)
 {
   dw_cu_ref cu;
   bool any_cus = false;
@@ -15058,18 +15075,50 @@ write_multifile (DSO *dso)
   if (multi_ehdr.e_ident[0] == '\0')
     multi_ehdr = dso->ehdr;
 
-  if ((multi_ptr_size && ptr_size != multi_ptr_size)
-      || (multi_endian
-	  && multi_endian != (do_read_32 == buf_read_ule32
-			      ? ELFDATA2LSB : ELFDATA2MSB)))
+  if (multifile_force_ptr_size && ptr_size != multifile_force_ptr_size)
+    {
+      error (0, 0, "File %s skipped for multi-file optimization, different"
+	     " pointer size", dso->filename);
+      res->skip_multifile = true;
+      return 1;
+    }
+  else if (multi_ptr_size == 0)
+    multi_ptr_size = ptr_size;
+  else if (ptr_size != multi_ptr_size)
+    {
+      error (0, 0, "Multi-file optimization not allowed for different"
+	     " pointer sizes");
+      multifile = NULL;
+      return 1;
+    }
+  else
+    {
+      /* Same ptr_size.  */
+    }
+
+  int endianity = (do_read_32 == buf_read_ule32
+		   ? ELFDATA2LSB
+		   : ELFDATA2MSB);
+  if (multifile_force_endian && endianity != multifile_force_endian)
+    {
+      error (0, 0, "File %s skipped for multi-file optimization, different"
+	     " endianity", dso->filename);
+      res->skip_multifile = true;
+      return 1;
+    }
+  else if (multi_endian == 0)
+    multi_endian = endianity;
+  else if (multi_endian != endianity)
     {
       error (0, 0, "Multi-file optimization not allowed for different"
-		   " pointer sizes or endianity");
+	     " endianity");
       multifile = NULL;
       return 1;
     }
-  multi_ptr_size = ptr_size;
-  multi_endian = do_read_32 == buf_read_ule32 ? ELFDATA2LSB : ELFDATA2MSB;
+  else
+    {
+      /* Same endianity.  */
+    }
 
 #if DEVEL
   clear_p2_field ();
@@ -15256,22 +15305,6 @@ remove_empty_pus (void)
   return 0;
 }
 
-/* Helper structure for hardlink discovery.  */
-struct file_result
-{
-  /* -3: Uninitialized.
-     -2: Already processed under different name.
-     -1: Ignore.
-      0: Processed, changed.
-      1: Processed, unchanged.  */
-  int res;
-  dev_t dev;
-  ino_t ino;
-  nlink_t nlink;
-  size_t hardlink_to;
-  unsigned int die_count;
-};
-
 /* Handle compression of a single file FILE.  If OUTFILE is
    non-NULL, the result will be stored into that file, otherwise
    the result will be written into a temporary file that is renamed
@@ -15380,7 +15413,7 @@ dwz (const char *file, const char *outfile, struct file_result *res)
 				    + debug_sections[DEBUG_TYPES].new_size));
 
 	  if (multifile && !fi_multifile && !low_mem)
-	    write_multifile (dso);
+	    write_multifile (dso, res);
 
 	  cleanup ();
 	  if (outfile != NULL)
@@ -15519,7 +15552,7 @@ dwz (const char *file, const char *outfile, struct file_result *res)
 	  debug_sections[DEBUG_GNU_PUBTYPES].new_size = 0;
 
 	  if (multifile && !fi_multifile && !low_mem)
-	    write_multifile (dso);
+	    write_multifile (dso, res);
 
 	  bool save_to_temp = save_temps && multifile && multifile_mode == 0;
 	  cleanup ();
@@ -16244,6 +16277,7 @@ init_file_result (struct file_result *res)
 {
   res->die_count = 0;
   res->res = -3;
+  res->skip_multifile = false;
 }
 
 /* Dwarf-compress FILE.  If OUTFILE, write to result to OUTFILE, otherwise
@@ -16420,7 +16454,7 @@ dwz_files_1 (int nr_files, char *files[], bool hardlink,
       thisret = dwz_with_low_mem (file, NULL, res, &low_mem_p);
       if (thisret == 1)
 	ret = 1;
-      else if (!low_mem_p && resa[i].res >= 0)
+      else if (!low_mem_p && !res->skip_multifile && resa[i].res >= 0)
 	successcount++;
     }
 
@@ -16463,7 +16497,8 @@ dwz_files_1 (int nr_files, char *files[], bool hardlink,
 	  multifile_mode = MULTIFILE_MODE_FI;
 	  /* Don't process again files that couldn't
 	     be processed successfully.  Also skip hard links.  */
-	  if (resa[i].res == -1 || resa[i].res == -2)
+	  if (resa[i].res == -1 || resa[i].res == -2
+	      || resa[i].skip_multifile)
 	    continue;
 	  for (cu = alt_first_cu; cu; cu = cu->cu_next)
 	    alt_clear_dups (cu->cu_die);

Re: [PATCH] Add --multifile-pointer-size <n> and --multifile-endian <l|L|b|B>

[Mark Wielaard]

Hi Tom,

On Thu, Mar 25, 2021 at 04:23:35PM +0100, Tom de Vries wrote:
> Consider binaries:
> - hello.64 generated using -m64, with pointer size 8,
> - hello.32 generated using -m32, with pointer size 4.
> 
> When trying to generate a multifile using files with different
> pointer sizes, we get:
> ...
> $ cp ../hello.64 1; cp 1 2; \
>     cp ../hello.32 3; cp 3 4; \
>     dwz -m 5 1 2 3 4; echo $?
> dwz: Multi-file optimization not allowed for different pointer sizes \
>   or endianity
> 0
> ...
> and the multi-file optimization has not been applied for any file:
> ...
> $ for f in 1 2 3 4; do \
>     echo -n "$f: "; \
>     readelf -S -W $f \
>         | grep -c gnu_debugaltlink; \
>     done
> 1: 0
> 2: 0
> 3: 0
> 4: 0
> ...
> 
> Add an option --multifile-pointer-size <n> / -p <n> that sets the pointer size
> of the multifile, such that we have instead with say -p 8:
> ...
> $ cp ../hello.64 1; cp 1 2; \
>     cp ../hello.32 3; cp 3 4; \
>     ./dwz -m 5 -p 8 1 2 3 4; echo $?
> ./dwz: File 3 skipped for multi-file optimization, different pointer size
> ./dwz: File 4 skipped for multi-file optimization, different pointer size
> 0
> ...
> and:
> ...
> $ for f in 1 2 3 4; do \
>     echo -n "$f: "; \
>     readelf -S -W $f \
>         | grep -c gnu_debugaltlink; \
>     done
> 1: 1
> 2: 1
> 3: 0
> 4: 0
> ...
> 
> Likewise, add --multifile-endian <l|L|b|B> / -e <l|L|b|B>.
> 
> Any comments?

Would it make sense to default to the ptrsize/endianity of the native
arch we are running on? Instead of the ptrsize/endianity of the first
file seen?

Cheers,

Mark