public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add -p native and -e native
@ 2021-04-09  9:24 Tom de Vries
  2021-04-09  9:42 ` Mark Wielaard
  0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2021-04-09  9:24 UTC (permalink / raw)
  To: dwz, jakub, mark; +Cc: Michael Matz

Hi,

Add option parameter native to options -p and -e.

We determine native as the result of:
- -p: sizeof (void *)
- -e: __BYTE_ORDER__
when compiling using CC without CFLAGS, such that if we build dwz with -m32 on
x86_64 like so:
...
$ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
...
and we have:
...
$ file ./dwz
dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
...
we still have:
...
$ ./dwz -?
  ...
  -p, --multifile-pointer-size <SIZE|auto|native>
                              Set pointer size of multifile, in number of bytes.
                              Native pointer size is 8.
                              Default value: auto.
...

Any comments?

Thanks,
- Tom

Add -p native and -e native

2021-04-09  Tom de Vries  <tdevries@suse.de>

	* Makefile (args.o): Add pattern rule.
	(native): New target.
	(clean): Update.
	* args.c (XSTR, STR, NATIVE_ENDIAN_STRING): New macro.
	(dwz_multi_file_options_help, usage): Mention -p native and -e native.
	(parse_args): Handle -p native and -e native.
	* dwz.1 (-p, -e): Mention native.
	* native.c: New file.

---
 Makefile | 11 ++++++++++-
 args.c   | 43 ++++++++++++++++++++++++++++++++++++++-----
 dwz.1    | 10 ++++++----
 native.c | 28 ++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index c23c36f..63fdb56 100644
--- a/Makefile
+++ b/Makefile
@@ -15,13 +15,22 @@ mandir = $(datarootdir)/man
 OBJECTS = args.o dwz.o hashtab.o sha1.o dwarfnames.o
 dwz: $(OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ -lelf
+args.o: native
+args.o: %.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< \
+	  -DNATIVE_ENDIAN=$(shell ./native e) \
+	  -DNATIVE_POINTER_SIZE=$(shell ./native p)
+%.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
 install: dwz
 	install -D dwz $(DESTDIR)$(bindir)/dwz
 	install -D -m 644 $(srcdir)/dwz.1 $(DESTDIR)$(mandir)/man1/dwz.1
 clean:
 	rm -f $(OBJECTS) *~ core* dwz $(TEST_EXECS) $(DWZ_TEST_SOURCES) \
-	  dwz.log dwz.sum
+	  dwz.log dwz.sum native
 	rm -Rf testsuite-bin tmp.*
+native: native.c
+	$(CC) -o $@ $<
 
 PWD:=$(shell pwd -P)
 
diff --git a/args.c b/args.c
index d44e632..4a51f8d 100644
--- a/args.c
+++ b/args.c
@@ -30,6 +30,9 @@
 
 #include "args.h"
 
+#define XSTR(s) STR(s)
+#define STR(s) #s
+
 #if DEVEL
 int tracing;
 int ignore_size;
@@ -217,6 +220,14 @@ static struct option_help dwz_single_file_options_help[] =
     "Place the output in OUTFILE." }
 };
 
+#if NATIVE_ENDIAN == __ORDER_LITTLE_ENDIAN__
+#define NATIVE_ENDIAN_STRING little
+#elif NATIVE_ENDIAN == __ORDER_BIG_ENDIAN__
+#define NATIVE_ENDIAN_STRING big
+#else
+#define NATIVE_ENDIAN_STRING not available
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -233,10 +244,12 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto>", "auto",
-    "Set pointer size of multifile, in number of bytes." },
-  { "e", "multifile-endian", "<l|b|auto>", "auto",
-    "Set endianity of multifile." },
+  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+     "Set pointer size of multifile, in number of bytes."
+    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "." },
+  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    "Set endianity of multifile."
+    " Native endianity is " XSTR (NATIVE_ENDIAN_STRING) "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel" }
 };
@@ -381,7 +394,7 @@ usage (int failing)
   FILE *stream = failing ? stderr : stdout;
   const char *header_lines[] = {
     "dwz [common options] [-h] [-m COMMONFILE] [-M NAME | -r] [-5]",
-    "    [-p <SIZE|auto>] [-e <l|b|auto>] [-j N] [FILES]",
+    "    [-p <SIZE|auto|native>] [-e <l|b|auto|native>] [-j N] [FILES]",
     "dwz [common options] -o OUTFILE FILE",
     "dwz [ -v | -? ]"
   };
@@ -646,6 +659,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      multifile_force_ptr_size = NATIVE_POINTER_SIZE;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -658,6 +676,21 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_endian = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      switch (NATIVE_ENDIAN)
+		{
+		case __ORDER_LITTLE_ENDIAN__:
+		  multifile_force_endian = ELFDATA2LSB;
+		  break;
+		case __ORDER_BIG_ENDIAN__:
+		  multifile_force_endian = ELFDATA2MSB;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine native endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])
diff --git a/dwz.1 b/dwz.1
index 6fec6ed..1cff329 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,13 +77,15 @@ 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|auto>
+.B \-p N \-\-multifile-pointer-size <N|auto|native>
 Specify the pointer size of the multifile, in bytes.  If auto, use the
-pointer size of the files, provided they match.
+pointer size of the files, provided they match.  If native, use native pointer
+size, as specified in the help message.
 .TP
-.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto>
+.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto|native>
 Specify the endianity of the multifile.  If auto, use the endianity of
-the files, provided they match.
+the files, provided they match.  If native, use native endianity, as specified
+in the help message.
 .TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.
diff --git a/native.c b/native.c
new file mode 100644
index 0000000..605502c
--- /dev/null
+++ b/native.c
@@ -0,0 +1,28 @@
+#include <string.h>
+#include <stdio.h>
+
+int
+main (int argc, char *argv[])
+{
+  const char *s;
+
+  if (argc != 2)
+    return 1;
+  s = argv[1];
+  if (strlen (s) != 1)
+    return 1;
+
+  switch (s[0])
+    {
+    case 'e':
+      printf ("%d\n", __BYTE_ORDER__);
+      break;
+    case 'p':
+      printf ("%zd\n", sizeof (void *));
+      break;
+    default:
+      return 1;
+    }
+
+  return 0;
+}

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09  9:24 [PATCH] Add -p native and -e native Tom de Vries
@ 2021-04-09  9:42 ` Mark Wielaard
  2021-04-09 12:48   ` Tom de Vries
  2021-04-09 13:03   ` Michael Matz
  0 siblings, 2 replies; 13+ messages in thread
From: Mark Wielaard @ 2021-04-09  9:42 UTC (permalink / raw)
  To: Tom de Vries; +Cc: dwz, jakub, Michael Matz

On Fri, Apr 09, 2021 at 11:24:41AM +0200, Tom de Vries wrote:
> Add option parameter native to options -p and -e.
> 
> We determine native as the result of:
> - -p: sizeof (void *)
> - -e: __BYTE_ORDER__
> when compiling using CC without CFLAGS, such that if we build dwz with -m32 on
> x86_64 like so:
> ...
> $ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
> ...
> and we have:
> ...
> $ file ./dwz
> dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
> ...
> we still have:
> ...
> $ ./dwz -?
>   ...
>   -p, --multifile-pointer-size <SIZE|auto|native>
>                               Set pointer size of multifile, in number of bytes.
>                               Native pointer size is 8.
>                               Default value: auto.
> ...
> 
> Any comments?

Except for this narrow multilib case, doesn't this actually make it
impossible to do a cross-arch build? I don't think this should be a
compile time option, unless it can derived from the target
architecture that the dwz binary is build for.

Cheers,

Mark

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09  9:42 ` Mark Wielaard
@ 2021-04-09 12:48   ` Tom de Vries
  2021-04-09 13:03   ` Michael Matz
  1 sibling, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2021-04-09 12:48 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: dwz, jakub, Michael Matz

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

On 4/9/21 11:42 AM, Mark Wielaard wrote:
> On Fri, Apr 09, 2021 at 11:24:41AM +0200, Tom de Vries wrote:
>> Add option parameter native to options -p and -e.
>>
>> We determine native as the result of:
>> - -p: sizeof (void *)
>> - -e: __BYTE_ORDER__
>> when compiling using CC without CFLAGS, such that if we build dwz with -m32 on
>> x86_64 like so:
>> ...
>> $ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
>> ...
>> and we have:
>> ...
>> $ file ./dwz
>> dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
>> ...
>> we still have:
>> ...
>> $ ./dwz -?
>>   ...
>>   -p, --multifile-pointer-size <SIZE|auto|native>
>>                               Set pointer size of multifile, in number of bytes.
>>                               Native pointer size is 8.
>>                               Default value: auto.
>> ...
>>
>> Any comments?
> 
> Except for this narrow multilib case, doesn't this actually make it
> impossible to do a cross-arch build?

I think so, yes.  If say we'd have a cross compiler targeting arm but
running on x86_64, then the ./native exec would be for arm but we'd try
to run it on x86_64.

> I don't think this should be a
> compile time option, unless it can derived from the target
> architecture that the dwz binary is build for.

I have an updated patch that does that, AFAIU.

I just wonder whether using the term native for this functionality will
cause confusion.  Perhaps 'self' is a better way of describing this?

And now I also wonder about whether there are architectures where using
__SIZEOF_POINTER__ will be different than what is generated as dwarf
pointer size.  If so, in the previous setup I could have used CC -g,
readelf and grep to find out what is actually used and pass that as a
define to args.c

Perhaps it's a mistake to try to assign any functionality to 'native'
when crosscompiling.

Thanks,
- Tom

[-- Attachment #2: 0001-Add-p-native-and-e-native.patch --]
[-- Type: text/x-patch, Size: 4714 bytes --]

Add -p native and -e native

Add option parameter native to options -p and -e.

We determine native as the result of:
- -p: __SIZEOF_POINTER__
- -e: __BYTE_ORDER__
in dwz.

2021-04-09  Tom de Vries  <tdevries@suse.de>

	* args.c (XSTR, STR, NATIVE_ENDIAN_STRING): New macro.
	(dwz_multi_file_options_help, usage): Mention -p native and -e native.
	(parse_args): Handle -p native and -e native.
	* dwz.1 (-p, -e): Mention native.

---
 args.c | 42 +++++++++++++++++++++++++++++++++++++-----
 dwz.1  | 10 ++++++----
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/args.c b/args.c
index 23bf1d4..5a664cf 100644
--- a/args.c
+++ b/args.c
@@ -32,6 +32,8 @@
 #include "args.h"
 
 #define IMPLIES(A, B) (!((A) && !(B)))
+#define XSTR(s) STR(s)
+#define STR(s) #s
 
 #if DEVEL
 int tracing;
@@ -220,6 +222,14 @@ static struct option_help dwz_single_file_options_help[] =
     "Place the output in OUTFILE." }
 };
 
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define NATIVE_ENDIAN_STRING little
+#elif NATIVE_ENDIAN == __ORDER_BIG_ENDIAN__
+#define NATIVE_ENDIAN_STRING big
+#else
+#define NATIVE_ENDIAN_STRING not available
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -236,10 +246,12 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto>", "auto",
-    "Set pointer size of multifile, in number of bytes." },
-  { "e", "multifile-endian", "<l|b|auto>", "auto",
-    "Set endianity of multifile." },
+  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+     "Set pointer size of multifile, in number of bytes."
+    " Native pointer size is " XSTR (__SIZEOF_POINTER__) "." },
+  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    "Set endianity of multifile."
+    " Native endianity is " XSTR (NATIVE_ENDIAN_STRING) "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel." }
 };
@@ -385,7 +397,7 @@ usage (int failing)
   FILE *stream = failing ? stderr : stdout;
   const char *header_lines[] = {
     "dwz [common options] [-h] [-m COMMONFILE] [-M NAME | -r] [-5]",
-    "    [-p <SIZE|auto>] [-e <l|b|auto>] [-j N] [FILES]",
+    "    [-p <SIZE|auto|native>] [-e <l|b|auto|native>] [-j N] [FILES]",
     "dwz [common options] -o OUTFILE FILE",
     "dwz [ -v | -? ]"
   };
@@ -650,6 +662,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      multifile_force_ptr_size = __SIZEOF_POINTER__;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -662,6 +679,21 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_endian = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      switch (__BYTE_ORDER__)
+		{
+		case __ORDER_LITTLE_ENDIAN__:
+		  multifile_force_endian = ELFDATA2LSB;
+		  break;
+		case __ORDER_BIG_ENDIAN__:
+		  multifile_force_endian = ELFDATA2MSB;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine native endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])
diff --git a/dwz.1 b/dwz.1
index 6fec6ed..1cff329 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,13 +77,15 @@ 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|auto>
+.B \-p N \-\-multifile-pointer-size <N|auto|native>
 Specify the pointer size of the multifile, in bytes.  If auto, use the
-pointer size of the files, provided they match.
+pointer size of the files, provided they match.  If native, use native pointer
+size, as specified in the help message.
 .TP
-.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto>
+.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto|native>
 Specify the endianity of the multifile.  If auto, use the endianity of
-the files, provided they match.
+the files, provided they match.  If native, use native endianity, as specified
+in the help message.
 .TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09  9:42 ` Mark Wielaard
  2021-04-09 12:48   ` Tom de Vries
@ 2021-04-09 13:03   ` Michael Matz
  2021-04-09 15:58     ` Tom de Vries
  1 sibling, 1 reply; 13+ messages in thread
From: Michael Matz @ 2021-04-09 13:03 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Tom de Vries, dwz, jakub

Hello,

On Fri, 9 Apr 2021, Mark Wielaard wrote:

> > We determine native as the result of:
> > - -p: sizeof (void *)
> > - -e: __BYTE_ORDER__
> > when compiling using CC without CFLAGS, such that if we build dwz with -m32 on
> > x86_64 like so:
> > ...
> > $ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
> > ...
> > and we have:
> > ...
> > $ file ./dwz
> > dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
> > ...
> > we still have:
> > ...
> > $ ./dwz -?
> >   ...
> >   -p, --multifile-pointer-size <SIZE|auto|native>
> >                               Set pointer size of multifile, in number of bytes.
> >                               Native pointer size is 8.
> >                               Default value: auto.
> > ...
> > 
> > Any comments?
> 
> Except for this narrow multilib case, doesn't this actually make it 
> impossible to do a cross-arch build?

For cross the term "native" doesn't make sense, so it would seem valid to 
simply not support that setting with a cross (not multilib) dwz.  I.e. if 
./native can't be executed assume cross-ness and don't support -p native.

> I don't think this should be a compile time option, unless it can 
> derived from the target architecture that the dwz binary is build for.

If it can be derived from the target architecture it can't be called 
native.  If you mean a canadian cross setting (i.e. where the dwz binary 
itself is built on host to be able to run on target and produces binaries 
for target): do we really want to support such build outside the core 
toolchain?


Ciao,
Michael.

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09 13:03   ` Michael Matz
@ 2021-04-09 15:58     ` Tom de Vries
  2021-04-12 12:33       ` Michael Matz
  2021-04-12 20:14       ` [PATCH] " Mark Wielaard
  0 siblings, 2 replies; 13+ messages in thread
From: Tom de Vries @ 2021-04-09 15:58 UTC (permalink / raw)
  To: Michael Matz, Mark Wielaard; +Cc: dwz, jakub

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

On 4/9/21 3:03 PM, Michael Matz wrote:
> Hello,
> 
> On Fri, 9 Apr 2021, Mark Wielaard wrote:
> 
>>> We determine native as the result of:
>>> - -p: sizeof (void *)
>>> - -e: __BYTE_ORDER__
>>> when compiling using CC without CFLAGS, such that if we build dwz with -m32 on
>>> x86_64 like so:
>>> ...
>>> $ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
>>> ...
>>> and we have:
>>> ...
>>> $ file ./dwz
>>> dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
>>> ...
>>> we still have:
>>> ...
>>> $ ./dwz -?
>>>   ...
>>>   -p, --multifile-pointer-size <SIZE|auto|native>
>>>                               Set pointer size of multifile, in number of bytes.
>>>                               Native pointer size is 8.
>>>                               Default value: auto.
>>> ...
>>>
>>> Any comments?
>>
>> Except for this narrow multilib case, doesn't this actually make it 
>> impossible to do a cross-arch build?
> 
> For cross the term "native" doesn't make sense, so it would seem valid to 
> simply not support that setting with a cross (not multilib) dwz.  I.e. if 
> ./native can't be executed assume cross-ness and don't support -p native.
> 

I've tried yet another variant.  Instead of trying to generate an
executable and execute it, we generate an object and test properties
using readelf.

This should no longer have the cross-build problem.

WDYT?

Thanks,
- Tom


[-- Attachment #2: 0001-Add-p-native-and-e-native.patch --]
[-- Type: text/x-patch, Size: 6571 bytes --]

Add -p native and -e native

Add option parameter native to options -p and -e.

We determine native as the result of readelf output on an object generated
by using CC without CFLAGS, such that if we build dwz with -m32 on
x86_64 like so:
...
$ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
...
and we have:
...
$ file ./dwz
dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
...
we still have:
...
$ ./dwz -?
  ...
  -p, --multifile-pointer-size <SIZE|auto|native>
                              Set pointer size of multifile, in number of bytes.
                              Native pointer size is 8.
                              Default value: auto.
...

2021-04-09  Tom de Vries  <tdevries@suse.de>

	* Makefile (args.o): Add pattern rule.
	(native.o): New target.
	(clean): Update.
	* util.h (XSTR, STR): New macro.
	* args.c (NATIVE_ENDIAN_STRING): New macro.
	(dwz_multi_file_options_help, usage): Mention -p native and -e native.
	(parse_args): Handle -p native and -e native.
	* dwz.1 (-p, -e): Mention native.
	* native.c: New file.

---
 Makefile | 17 ++++++++++++++++-
 args.c   | 41 ++++++++++++++++++++++++++++++++++++-----
 dwz.1    | 10 ++++++----
 native.c |  5 +++++
 util.h   |  3 +++
 5 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 02da6c4..5f0b2f0 100644
--- a/Makefile
+++ b/Makefile
@@ -20,13 +20,28 @@ OBJECTS = args.o dwz.o hashtab.o sha1.o dwarfnames.o
 LIBS=-lelf
 dwz: $(OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
+args.o: native.o
+NATIVE_ENDIAN=$(shell readelf -h native.o \
+	| grep Data \
+	| sed 's/.*, //;s/ endian//')
+NATIVE_POINTER_SIZE=$(shell readelf -wi native.o \
+	| grep "Pointer Size:" \
+	| sed 's/.*: *//')
+args.o: %.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< \
+	  -DNATIVE_ENDIAN=$(NATIVE_ENDIAN) \
+	  -DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
+%.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
 install: dwz
 	install -D dwz $(DESTDIR)$(bindir)/dwz
 	install -D -m 644 $(srcdir)/dwz.1 $(DESTDIR)$(mandir)/man1/dwz.1
 clean:
 	rm -f $(OBJECTS) *~ core* dwz $(TEST_EXECS) $(DWZ_TEST_OBJECTS) \
-	  dwz.log dwz.sum
+	  dwz.log dwz.sum native.o
 	rm -Rf testsuite-bin tmp.*
+native.o: native.c
+	$(CC) -o $@ $< -c -g
 
 PWD:=$(shell pwd -P)
 
diff --git a/args.c b/args.c
index cb11f25..e374c26 100644
--- a/args.c
+++ b/args.c
@@ -220,6 +220,15 @@ static struct option_help dwz_single_file_options_help[] =
     "Place the output in OUTFILE." }
 };
 
+#if NATIVE_ENDIAN == little
+#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
+#elif NATIVE_ENDIAN == big
+#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
+#else
+#define NATIVE_ENDIAN not available
+#define NATIVE_ENDIAN_VAL 0
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -236,10 +245,12 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto>", "auto",
-    "Set pointer size of multifile, in number of bytes." },
-  { "e", "multifile-endian", "<l|b|auto>", "auto",
-    "Set endianity of multifile." },
+  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+     "Set pointer size of multifile, in number of bytes."
+    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "." },
+  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    "Set endianity of multifile."
+    " Native endianity is " XSTR (NATIVE_ENDIAN) "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel." }
 };
@@ -385,7 +396,7 @@ usage (int failing)
   FILE *stream = failing ? stderr : stdout;
   const char *header_lines[] = {
     "dwz [common options] [-h] [-m COMMONFILE] [-M NAME | -r] [-5]",
-    "    [-p <SIZE|auto>] [-e <l|b|auto>] [-j N] [FILES]",
+    "    [-p <SIZE|auto|native>] [-e <l|b|auto|native>] [-j N] [FILES]",
     "dwz [common options] -o OUTFILE FILE",
     "dwz [ -v | -? ]"
   };
@@ -650,6 +661,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      multifile_force_ptr_size = NATIVE_POINTER_SIZE;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -662,6 +678,21 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_endian = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      switch (NATIVE_ENDIAN_VAL)
+		{
+		case __ORDER_LITTLE_ENDIAN__:
+		  multifile_force_endian = ELFDATA2LSB;
+		  break;
+		case __ORDER_BIG_ENDIAN__:
+		  multifile_force_endian = ELFDATA2MSB;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine native endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])
diff --git a/dwz.1 b/dwz.1
index 6fec6ed..1cff329 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,13 +77,15 @@ 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|auto>
+.B \-p N \-\-multifile-pointer-size <N|auto|native>
 Specify the pointer size of the multifile, in bytes.  If auto, use the
-pointer size of the files, provided they match.
+pointer size of the files, provided they match.  If native, use native pointer
+size, as specified in the help message.
 .TP
-.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto>
+.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto|native>
 Specify the endianity of the multifile.  If auto, use the endianity of
-the files, provided they match.
+the files, provided they match.  If native, use native endianity, as specified
+in the help message.
 .TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.
diff --git a/native.c b/native.c
new file mode 100644
index 0000000..398ec67
--- /dev/null
+++ b/native.c
@@ -0,0 +1,5 @@
+int
+main (void)
+{
+  return 0;
+}
diff --git a/util.h b/util.h
index 7caac06..d542942 100644
--- a/util.h
+++ b/util.h
@@ -25,6 +25,9 @@
 #define MAX(A, B) ((A) > (B) ? (A) : (B))
 #define MIN(A, B) ((A) < (B) ? (A) : (B))
 
+#define XSTR(s) STR(s)
+#define STR(s) #s
+
 #ifndef USE_GNUC
 #ifdef __GNUC__
 #define USE_GNUC 1

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09 15:58     ` Tom de Vries
@ 2021-04-12 12:33       ` Michael Matz
  2021-04-12 15:11         ` Tom de Vries
  2021-04-12 20:14       ` [PATCH] " Mark Wielaard
  1 sibling, 1 reply; 13+ messages in thread
From: Michael Matz @ 2021-04-12 12:33 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Mark Wielaard, dwz, jakub

Hello,

On Fri, 9 Apr 2021, Tom de Vries wrote:

> >> Except for this narrow multilib case, doesn't this actually make it 
> >> impossible to do a cross-arch build?
> > 
> > For cross the term "native" doesn't make sense, so it would seem valid to 
> > simply not support that setting with a cross (not multilib) dwz.  I.e. if 
> > ./native can't be executed assume cross-ness and don't support -p native.
> 
> I've tried yet another variant.  Instead of trying to generate an 
> executable and execute it, we generate an object and test properties 
> using readelf.
> 
> This should no longer have the cross-build problem.
> 
> WDYT?

I think using readelf should work here.  But the patch has problems:

+#if NATIVE_ENDIAN == little
+#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
+#elif NATIVE_ENDIAN == big
+#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
+#else
+#define NATIVE_ENDIAN not available
+#define NATIVE_ENDIAN_VAL 0
+#endif

The preprocessor works different than you seem to assume.  Unknown tokens 
are replaced with 0, so what you've written is the same as:

+#if NATIVE_ENDIAN == 0
+#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
+#elif NATIVE_ENDIAN == 0

(except if you have defined 'little' and 'big' somewhere, but I can't see 
that?)  Also __ORDER_LITTLE_ENDIAN__ is defined by GCC since 4.6, you 
might want to check defined-ness of it before using it.  And you're 
setting NATIVE_ENDIAN_VAL to little endian in both if arms.


Ciao,
Michael.

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

* Re: [PATCH] Add -p native and -e native
  2021-04-12 12:33       ` Michael Matz
@ 2021-04-12 15:11         ` Tom de Vries
  2021-04-12 19:53           ` [committed] " Tom de Vries
  0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2021-04-12 15:11 UTC (permalink / raw)
  To: Michael Matz; +Cc: Mark Wielaard, dwz, jakub

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

On 4/12/21 2:33 PM, Michael Matz wrote:
> Hello,
> 
> On Fri, 9 Apr 2021, Tom de Vries wrote:
> 
>>>> Except for this narrow multilib case, doesn't this actually make it 
>>>> impossible to do a cross-arch build?
>>>
>>> For cross the term "native" doesn't make sense, so it would seem valid to 
>>> simply not support that setting with a cross (not multilib) dwz.  I.e. if 
>>> ./native can't be executed assume cross-ness and don't support -p native.
>>
>> I've tried yet another variant.  Instead of trying to generate an 
>> executable and execute it, we generate an object and test properties 
>> using readelf.
>>
>> This should no longer have the cross-build problem.
>>
>> WDYT?
> 
> I think using readelf should work here.  But the patch has problems:
> 
> +#if NATIVE_ENDIAN == little
> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
> +#elif NATIVE_ENDIAN == big
> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
> +#else
> +#define NATIVE_ENDIAN not available
> +#define NATIVE_ENDIAN_VAL 0
> +#endif
> 
> The preprocessor works different than you seem to assume.  Unknown tokens 
> are replaced with 0, so what you've written is the same as:
> 
> +#if NATIVE_ENDIAN == 0
> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
> +#elif NATIVE_ENDIAN == 0
> 
> (except if you have defined 'little' and 'big' somewhere, but I can't see 
> that?)

Oops, thanks for pointing that out.

Fixed by no longer passing -DNATIVE_ENDIAN=little, but instead
-DNATIVE_ENDIAN_VAL=ELFDATA2LSB.

> Also __ORDER_LITTLE_ENDIAN__ is defined by GCC since 4.6, you 
> might want to check defined-ness of it before using it.

Fixed by using -DNATIVE_ENDIAN_VAL=ELFDATA2LSB, __ORDER_LITTLE_ENDIAN__
is no longer used.

> And you're 
> setting NATIVE_ENDIAN_VAL to little endian in both if arms.

Oops again, fixed now.

Thanks,
- Tom



[-- Attachment #2: 0001-Add-p-native-and-e-native.patch --]
[-- Type: text/x-patch, Size: 6958 bytes --]

Add -p native and -e native

Add option parameter native to options -p and -e.

We determine native as the result of readelf output on an object generated
by using CC without CFLAGS, such that if we build dwz with -m32 on
x86_64 like so:
...
$ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
...
and we have:
...
$ file ./dwz
dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
...
we still have:
...
$ ./dwz -?
  ...
  -p, --multifile-pointer-size <SIZE|auto|native>
                              Set pointer size of multifile, in number of bytes.
                              Native pointer size is 8.
                              Default value: auto.
...

2021-04-12  Tom de Vries  <tdevries@suse.de>

	* Makefile (args.o): Add pattern rule.
	(native.o): New target.
	(clean): Update.
	* util.h (XSTR, STR): New macro.
	* args.c (NATIVE_ENDIAN_STRING): New macro.
	(dwz_multi_file_options_help, usage): Mention -p native and -e native.
	(parse_args): Handle -p native and -e native.
	* dwz.1 (-p, -e): Mention native.
	* native.c: New file.

---
 Makefile | 20 +++++++++++++++++++-
 args.c   | 38 +++++++++++++++++++++++++++++++++-----
 dwz.1    | 10 ++++++----
 native.c |  5 +++++
 util.h   |  3 +++
 5 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 95fc80b..23c1ebb 100644
--- a/Makefile
+++ b/Makefile
@@ -20,13 +20,31 @@ OBJECTS = args.o dwz.o hashtab.o pool.o sha1.o dwarfnames.o
 LIBS=-lelf
 dwz: $(OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
+args.o: native.o
+args.o: %.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< \
+	  -DNATIVE_ENDIAN_VAL=$(NATIVE_ENDIAN_VAL) \
+	  -DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
+NATIVE_ENDIAN=$(shell readelf -h native.o \
+	| grep Data \
+	| sed 's/.*, //;s/ endian//')
+NATIVE_ENDIAN_LITTLE=$(findstring $(NATIVE_ENDIAN),$(findstring little,$(NATIVE_ENDIAN)))
+NATIVE_ENDIAN_BIG=$(findstring $(NATIVE_ENDIAN),$(findstring big,$(NATIVE_ENDIAN)))
+NATIVE_ENDIAN_VAL=$(if $(NATIVE_ENDIAN_LITTLE),ELFDATA2LSB,$(if $(NATIVE_ENDIAN_BIG),ELFDATA2MSB,ELFDATANONE))
+NATIVE_POINTER_SIZE=$(shell readelf -wi native.o \
+	| grep "Pointer Size:" \
+	| sed 's/.*: *//')
+%.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
 install: dwz
 	install -D dwz $(DESTDIR)$(bindir)/dwz
 	install -D -m 644 $(srcdir)/dwz.1 $(DESTDIR)$(mandir)/man1/dwz.1
 clean:
 	rm -f $(OBJECTS) *~ core* dwz $(TEST_EXECS) $(DWZ_TEST_OBJECTS) \
-	  dwz.log dwz.sum
+	  dwz.log dwz.sum native.o
 	rm -Rf testsuite-bin tmp.*
+native.o: native.c
+	$(CC) -o $@ $< -c -g
 
 PWD:=$(shell pwd -P)
 
diff --git a/args.c b/args.c
index cb11f25..e93c24d 100644
--- a/args.c
+++ b/args.c
@@ -220,6 +220,14 @@ static struct option_help dwz_single_file_options_help[] =
     "Place the output in OUTFILE." }
 };
 
+#if NATIVE_ENDIAN_VAL == ELFDATA2MSB
+#define NATIVE_ENDIAN "big"
+#elif NATIVE_ENDIAN_VAL == ELFDATA2LSB
+#define NATIVE_ENDIAN "little"
+#else
+#define NATIVE_ENDIAN "not available"
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -236,10 +244,12 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto>", "auto",
-    "Set pointer size of multifile, in number of bytes." },
-  { "e", "multifile-endian", "<l|b|auto>", "auto",
-    "Set endianity of multifile." },
+  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+     "Set pointer size of multifile, in number of bytes."
+    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "." },
+  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    "Set endianity of multifile."
+    " Native endianity is " NATIVE_ENDIAN "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel." }
 };
@@ -385,7 +395,7 @@ usage (int failing)
   FILE *stream = failing ? stderr : stdout;
   const char *header_lines[] = {
     "dwz [common options] [-h] [-m COMMONFILE] [-M NAME | -r] [-5]",
-    "    [-p <SIZE|auto>] [-e <l|b|auto>] [-j N] [FILES]",
+    "    [-p <SIZE|auto|native>] [-e <l|b|auto|native>] [-j N] [FILES]",
     "dwz [common options] -o OUTFILE FILE",
     "dwz [ -v | -? ]"
   };
@@ -650,6 +660,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      multifile_force_ptr_size = NATIVE_POINTER_SIZE;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -662,6 +677,19 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_endian = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      switch (NATIVE_ENDIAN_VAL)
+		{
+		case ELFDATA2MSB:
+		case ELFDATA2LSB:
+		  multifile_force_endian = NATIVE_ENDIAN_VAL;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine native endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])
diff --git a/dwz.1 b/dwz.1
index 6fec6ed..1cff329 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,13 +77,15 @@ 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|auto>
+.B \-p N \-\-multifile-pointer-size <N|auto|native>
 Specify the pointer size of the multifile, in bytes.  If auto, use the
-pointer size of the files, provided they match.
+pointer size of the files, provided they match.  If native, use native pointer
+size, as specified in the help message.
 .TP
-.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto>
+.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto|native>
 Specify the endianity of the multifile.  If auto, use the endianity of
-the files, provided they match.
+the files, provided they match.  If native, use native endianity, as specified
+in the help message.
 .TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.
diff --git a/native.c b/native.c
new file mode 100644
index 0000000..398ec67
--- /dev/null
+++ b/native.c
@@ -0,0 +1,5 @@
+int
+main (void)
+{
+  return 0;
+}
diff --git a/util.h b/util.h
index 7caac06..d542942 100644
--- a/util.h
+++ b/util.h
@@ -25,6 +25,9 @@
 #define MAX(A, B) ((A) > (B) ? (A) : (B))
 #define MIN(A, B) ((A) < (B) ? (A) : (B))
 
+#define XSTR(s) STR(s)
+#define STR(s) #s
+
 #ifndef USE_GNUC
 #ifdef __GNUC__
 #define USE_GNUC 1

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

* [committed] Add -p native and -e native
  2021-04-12 15:11         ` Tom de Vries
@ 2021-04-12 19:53           ` Tom de Vries
  0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2021-04-12 19:53 UTC (permalink / raw)
  To: Michael Matz; +Cc: Mark Wielaard, dwz, jakub

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

On 4/12/21 5:11 PM, Tom de Vries wrote:
> On 4/12/21 2:33 PM, Michael Matz wrote:
>> Hello,
>>
>> On Fri, 9 Apr 2021, Tom de Vries wrote:
>>
>>>>> Except for this narrow multilib case, doesn't this actually make it 
>>>>> impossible to do a cross-arch build?
>>>>
>>>> For cross the term "native" doesn't make sense, so it would seem valid to 
>>>> simply not support that setting with a cross (not multilib) dwz.  I.e. if 
>>>> ./native can't be executed assume cross-ness and don't support -p native.
>>>
>>> I've tried yet another variant.  Instead of trying to generate an 
>>> executable and execute it, we generate an object and test properties 
>>> using readelf.
>>>
>>> This should no longer have the cross-build problem.
>>>
>>> WDYT?
>>
>> I think using readelf should work here.  But the patch has problems:
>>
>> +#if NATIVE_ENDIAN == little
>> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
>> +#elif NATIVE_ENDIAN == big
>> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
>> +#else
>> +#define NATIVE_ENDIAN not available
>> +#define NATIVE_ENDIAN_VAL 0
>> +#endif
>>
>> The preprocessor works different than you seem to assume.  Unknown tokens 
>> are replaced with 0, so what you've written is the same as:
>>
>> +#if NATIVE_ENDIAN == 0
>> +#define NATIVE_ENDIAN_VAL __ORDER_LITTLE_ENDIAN__
>> +#elif NATIVE_ENDIAN == 0
>>
>> (except if you have defined 'little' and 'big' somewhere, but I can't see 
>> that?)
> 
> Oops, thanks for pointing that out.
> 
> Fixed by no longer passing -DNATIVE_ENDIAN=little, but instead
> -DNATIVE_ENDIAN_VAL=ELFDATA2LSB.
> 
>> Also __ORDER_LITTLE_ENDIAN__ is defined by GCC since 4.6, you 
>> might want to check defined-ness of it before using it.
> 
> Fixed by using -DNATIVE_ENDIAN_VAL=ELFDATA2LSB, __ORDER_LITTLE_ENDIAN__
> is no longer used.
> 
>> And you're 
>> setting NATIVE_ENDIAN_VAL to little endian in both if arms.
> 
> Oops again, fixed now.
> 

Hmm, I seem to have forgotten to run the tests, so there was still an
issue that args-for-test.o needed the same CFLAGS update as args.o.

Fixed in attached patch, committed.

Thanks,
- Tom

[-- Attachment #2: 0001-Add-p-native-and-e-native.patch --]
[-- Type: text/x-patch, Size: 7497 bytes --]

Add -p native and -e native

Add option parameter native to options -p and -e.

We determine native as the result of readelf output on an object generated
by using CC without CFLAGS, such that if we build dwz with -m32 on
x86_64 like so:
...
$ make CFLAGS="-m32 -O2 -g" LDFLAGS=-m32
...
and we have:
...
$ file ./dwz
dwz: ELF 32-bit LSB executable, Intel 80386 <SNIP>
...
we still have:
...
$ ./dwz -?
  ...
  -p, --multifile-pointer-size <SIZE|auto|native>
                              Set pointer size of multifile, in number of bytes.
                              Native pointer size is 8.
                              Default value: auto.
...

2021-04-12  Tom de Vries  <tdevries@suse.de>

	* Makefile (args.o): Add dependency on native.o.
	(CFLAGS_FOR_SOURCE, NATIVE_ENDIAN, NATIVE_ENDIAN_LITTLE)
	(NATIVE_ENDIAN_BIG, NATIVE_ENDIAN_VAL): New var.
	(%.o: %.c): Add pattern rule.
	(clean): Update.
	(native.o): Add pattern rule.
	* util.h (XSTR, STR): New macro.
	* args.c (NATIVE_ENDIAN): New macro.
	(dwz_multi_file_options_help, usage): Mention -p native and -e native.
	(parse_args): Handle -p native and -e native.
	* dwz.1 (-p, -e): Mention native.
	* native.c: New file.

---
 Makefile | 25 +++++++++++++++++++++++--
 args.c   | 38 +++++++++++++++++++++++++++++++++-----
 dwz.1    | 10 ++++++----
 native.c |  5 +++++
 util.h   |  3 +++
 5 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 95fc80b..9394ef4 100644
--- a/Makefile
+++ b/Makefile
@@ -20,13 +20,30 @@ OBJECTS = args.o dwz.o hashtab.o pool.o sha1.o dwarfnames.o
 LIBS=-lelf
 dwz: $(OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
+args.o: native.o
+args.o: CFLAGS_FOR_SOURCE = \
+	-DNATIVE_ENDIAN_VAL=$(NATIVE_ENDIAN_VAL) \
+	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
+NATIVE_ENDIAN=$(shell readelf -h native.o \
+	| grep Data \
+	| sed 's/.*, //;s/ endian//')
+NATIVE_ENDIAN_LITTLE=$(findstring $(NATIVE_ENDIAN),$(findstring little,$(NATIVE_ENDIAN)))
+NATIVE_ENDIAN_BIG=$(findstring $(NATIVE_ENDIAN),$(findstring big,$(NATIVE_ENDIAN)))
+NATIVE_ENDIAN_VAL=$(if $(NATIVE_ENDIAN_LITTLE),ELFDATA2LSB,$(if $(NATIVE_ENDIAN_BIG),ELFDATA2MSB,ELFDATANONE))
+NATIVE_POINTER_SIZE=$(shell readelf -wi native.o \
+	| grep "Pointer Size:" \
+	| sed 's/.*: *//')
+%.o: %.c
+	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< $(CFLAGS_FOR_SOURCE)
 install: dwz
 	install -D dwz $(DESTDIR)$(bindir)/dwz
 	install -D -m 644 $(srcdir)/dwz.1 $(DESTDIR)$(mandir)/man1/dwz.1
 clean:
 	rm -f $(OBJECTS) *~ core* dwz $(TEST_EXECS) $(DWZ_TEST_OBJECTS) \
-	  dwz.log dwz.sum
+	  dwz.log dwz.sum native.o
 	rm -Rf testsuite-bin tmp.*
+native.o: native.c
+	$(CC) -o $@ $< -c -g
 
 PWD:=$(shell pwd -P)
 
@@ -59,13 +76,17 @@ DWZ_TEST_OBJECTS := $(patsubst %.o,%-for-test.o,$(OBJECTS))
 dwz-for-test: $(DWZ_TEST_OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
 	rm -f $(DWZ_TEST_OBJECTS)
+args-for-test.o: CFLAGS_FOR_SOURCE = \
+	-DNATIVE_ENDIAN_VAL=$(NATIVE_ENDIAN_VAL) \
+	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
 $(DWZ_TEST_OBJECTS): %-for-test.o : %.c
 	$(CC) $< -o $@ -c \
 	  -DUSE_GNUC=0 -DDEVEL \
 	  -O2 -g \
 	  $(CFLAGS_COMMON) \
 	  -DDWZ_VERSION='"for-test"' \
-	  $(CFLAGS_COPYRIGHT)
+	  $(CFLAGS_COPYRIGHT) \
+	  $(CFLAGS_FOR_SOURCE)
 
 min:
 	$(CC) $(TEST_SRC)/min.c $(TEST_SRC)/min-2.c -o $@ -g
diff --git a/args.c b/args.c
index cb11f25..e93c24d 100644
--- a/args.c
+++ b/args.c
@@ -220,6 +220,14 @@ static struct option_help dwz_single_file_options_help[] =
     "Place the output in OUTFILE." }
 };
 
+#if NATIVE_ENDIAN_VAL == ELFDATA2MSB
+#define NATIVE_ENDIAN "big"
+#elif NATIVE_ENDIAN_VAL == ELFDATA2LSB
+#define NATIVE_ENDIAN "little"
+#else
+#define NATIVE_ENDIAN "not available"
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -236,10 +244,12 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto>", "auto",
-    "Set pointer size of multifile, in number of bytes." },
-  { "e", "multifile-endian", "<l|b|auto>", "auto",
-    "Set endianity of multifile." },
+  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+     "Set pointer size of multifile, in number of bytes."
+    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "." },
+  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    "Set endianity of multifile."
+    " Native endianity is " NATIVE_ENDIAN "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel." }
 };
@@ -385,7 +395,7 @@ usage (int failing)
   FILE *stream = failing ? stderr : stdout;
   const char *header_lines[] = {
     "dwz [common options] [-h] [-m COMMONFILE] [-M NAME | -r] [-5]",
-    "    [-p <SIZE|auto>] [-e <l|b|auto>] [-j N] [FILES]",
+    "    [-p <SIZE|auto|native>] [-e <l|b|auto|native>] [-j N] [FILES]",
     "dwz [common options] -o OUTFILE FILE",
     "dwz [ -v | -? ]"
   };
@@ -650,6 +660,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      multifile_force_ptr_size = NATIVE_POINTER_SIZE;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -662,6 +677,19 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_endian = 0;
 	      break;
 	    }
+	  if (strcmp (optarg, "native") == 0)
+	    {
+	      switch (NATIVE_ENDIAN_VAL)
+		{
+		case ELFDATA2MSB:
+		case ELFDATA2LSB:
+		  multifile_force_endian = NATIVE_ENDIAN_VAL;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine native endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])
diff --git a/dwz.1 b/dwz.1
index 6fec6ed..1cff329 100644
--- a/dwz.1
+++ b/dwz.1
@@ -77,13 +77,15 @@ 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|auto>
+.B \-p N \-\-multifile-pointer-size <N|auto|native>
 Specify the pointer size of the multifile, in bytes.  If auto, use the
-pointer size of the files, provided they match.
+pointer size of the files, provided they match.  If native, use native pointer
+size, as specified in the help message.
 .TP
-.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto>
+.B \-p <l|b|auto> \-\-multifile-endian <l|b|auto|native>
 Specify the endianity of the multifile.  If auto, use the endianity of
-the files, provided they match.
+the files, provided they match.  If native, use native endianity, as specified
+in the help message.
 .TP
 .B \-q \-\-quiet
 Silence up some of the most common messages.
diff --git a/native.c b/native.c
new file mode 100644
index 0000000..398ec67
--- /dev/null
+++ b/native.c
@@ -0,0 +1,5 @@
+int
+main (void)
+{
+  return 0;
+}
diff --git a/util.h b/util.h
index 7caac06..d542942 100644
--- a/util.h
+++ b/util.h
@@ -25,6 +25,9 @@
 #define MAX(A, B) ((A) > (B) ? (A) : (B))
 #define MIN(A, B) ((A) < (B) ? (A) : (B))
 
+#define XSTR(s) STR(s)
+#define STR(s) #s
+
 #ifndef USE_GNUC
 #ifdef __GNUC__
 #define USE_GNUC 1

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

* Re: [PATCH] Add -p native and -e native
  2021-04-09 15:58     ` Tom de Vries
  2021-04-12 12:33       ` Michael Matz
@ 2021-04-12 20:14       ` Mark Wielaard
  2021-04-13  7:45         ` Tom de Vries
  1 sibling, 1 reply; 13+ messages in thread
From: Mark Wielaard @ 2021-04-12 20:14 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Michael Matz, dwz, jakub

Hi,

On Fri, Apr 09, 2021 at 05:58:34PM +0200, Tom de Vries wrote:
> >> Except for this narrow multilib case, doesn't this actually make it 
> >> impossible to do a cross-arch build?
> > 
> > For cross the term "native" doesn't make sense, so it would seem valid to 
> > simply not support that setting with a cross (not multilib) dwz.  I.e. if 
> > ./native can't be executed assume cross-ness and don't support -p native.
> 
> I've tried yet another variant.  Instead of trying to generate an
> executable and execute it, we generate an object and test properties
> using readelf.
> 
> This should no longer have the cross-build problem.
> 
> WDYT?

Can we take a step back, because I think I lost the plot, sorry.

Why are we going through all this?

So with this when something is build as 32bit it can use 64bit as
"native" pointer size. Or if something is cross compiled to
little-endian it can still report big-endian as "native",

But when does that ever make sense?  Why would one run the 32bit
binary on a 64bit system and wanting the default -p native be 64bit
instead of 32bit? Wouldn't one install and run the actual "native"
64-bit binary in that case?

Thanks,

Mark

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

* Re: [PATCH] Add -p native and -e native
  2021-04-12 20:14       ` [PATCH] " Mark Wielaard
@ 2021-04-13  7:45         ` Tom de Vries
  2021-04-13  8:33           ` Tom de Vries
  0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2021-04-13  7:45 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Michael Matz, dwz, jakub

On 4/12/21 10:14 PM, Mark Wielaard wrote:
> Hi,
> 
> On Fri, Apr 09, 2021 at 05:58:34PM +0200, Tom de Vries wrote:
>>>> Except for this narrow multilib case, doesn't this actually make it 
>>>> impossible to do a cross-arch build?
>>>
>>> For cross the term "native" doesn't make sense, so it would seem valid to 
>>> simply not support that setting with a cross (not multilib) dwz.  I.e. if 
>>> ./native can't be executed assume cross-ness and don't support -p native.
>>
>> I've tried yet another variant.  Instead of trying to generate an
>> executable and execute it, we generate an object and test properties
>> using readelf.
>>
>> This should no longer have the cross-build problem.
>>
>> WDYT?
> 
> Can we take a step back, because I think I lost the plot, sorry.
> 

Sure, of course.

> Why are we going through all this?
> 
> So with this when something is build as 32bit it can use 64bit as
> "native" pointer size. Or if something is cross compiled to
> little-endian it can still report big-endian as "native",
> 

Yes.  Native is defined by the current implementation as the default
endiannes and pointer size of code generated by the compiler used to
compile dwz.

So if you're using a compiler that by default generates 32-bit while
targeting 64-bit platform, then the resulting -p native is 32-bit.

> But when does that ever make sense?

It doesn't.

The current implementation makes sense if you use a native compiler
(i.e., generates 64-bit code for a 64-bit platform), and the same holds
in a cross-compiling scenario.

I think the assumption that was made here is that the implementation is
good enough if it gives good results for the native compiler scenario.

> Why would one run the 32bit
> binary on a 64bit system and wanting the default -p native be 64bit
> instead of 32bit? 

No idea why one would want to run the 32-bit binary on a 64bit system in
the first place.  But it's possible.

If we have an option -p/-e native, it needs to be assigned a semantics
in that case.

And in the case of using a native compiler, the semantics are accurate.

> Wouldn't one install and run the actual "native"
> 64-bit binary in that case?

Yes, that's what I would do.

Thanks,
- Tom

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

* Re: [PATCH] Add -p native and -e native
  2021-04-13  7:45         ` Tom de Vries
@ 2021-04-13  8:33           ` Tom de Vries
  2021-04-13 10:04             ` Mark Wielaard
  0 siblings, 1 reply; 13+ messages in thread
From: Tom de Vries @ 2021-04-13  8:33 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Michael Matz, dwz, jakub

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

On 4/13/21 9:45 AM, Tom de Vries wrote:
> On 4/12/21 10:14 PM, Mark Wielaard wrote:
>> Hi,
>>
>> On Fri, Apr 09, 2021 at 05:58:34PM +0200, Tom de Vries wrote:
>>>>> Except for this narrow multilib case, doesn't this actually make it 
>>>>> impossible to do a cross-arch build?
>>>>
>>>> For cross the term "native" doesn't make sense, so it would seem valid to 
>>>> simply not support that setting with a cross (not multilib) dwz.  I.e. if 
>>>> ./native can't be executed assume cross-ness and don't support -p native.
>>>
>>> I've tried yet another variant.  Instead of trying to generate an
>>> executable and execute it, we generate an object and test properties
>>> using readelf.
>>>
>>> This should no longer have the cross-build problem.
>>>
>>> WDYT?
>>
>> Can we take a step back, because I think I lost the plot, sorry.
>>
> 
> Sure, of course.
> 
>> Why are we going through all this?
>>
>> So with this when something is build as 32bit it can use 64bit as
>> "native" pointer size. Or if something is cross compiled to
>> little-endian it can still report big-endian as "native",
>>
> 
> Yes.  Native is defined by the current implementation as the default
> endiannes and pointer size of code generated by the compiler used to
> compile dwz.
> 
> So if you're using a compiler that by default generates 32-bit while
> targeting 64-bit platform, then the resulting -p native is 32-bit.
> 
>> But when does that ever make sense?
> 
> It doesn't.
> 
> The current implementation makes sense if you use a native compiler
> (i.e., generates 64-bit code for a 64-bit platform), and the same holds
> in a cross-compiling scenario.
> 
> I think the assumption that was made here is that the implementation is
> good enough if it gives good results for the native compiler scenario.
> 
>> Why would one run the 32bit
>> binary on a 64bit system and wanting the default -p native be 64bit
>> instead of 32bit? 
> 
> No idea why one would want to run the 32-bit binary on a 64bit system in
> the first place.  But it's possible.
> 
> If we have an option -p/-e native, it needs to be assigned a semantics
> in that case.
> 
> And in the case of using a native compiler, the semantics are accurate.
> 
>> Wouldn't one install and run the actual "native"
>> 64-bit binary in that case?
> 
> Yes, that's what I would do.
> 

FWIW, would this patch address your concerns?

It adds an option -p/-e self, and when we do:
...
$ make clean; make CFLAGS="-O2 -g -m32" LDFLAGS=-m32
...
we have:
...
$ ./dwz -?
  ...
  -p, --multifile-pointer-size <SIZE|auto|native|self>
                              Set pointer size of multifile, in number
of bytes.
                              Native pointer size is 8. Self pointer
size is 4.
                              Default value: auto.
...

Thanks,
- Tom

[-- Attachment #2: 0001-Add-p-self-and-e-self.patch --]
[-- Type: text/x-patch, Size: 4993 bytes --]

Add -p self and -e self

---
 Makefile | 23 +++++++++++++++++++----
 args.c   | 36 ++++++++++++++++++++++++++++++++----
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 9394ef4..3f5163a 100644
--- a/Makefile
+++ b/Makefile
@@ -20,10 +20,12 @@ OBJECTS = args.o dwz.o hashtab.o pool.o sha1.o dwarfnames.o
 LIBS=-lelf
 dwz: $(OBJECTS)
 	$(CC) $(LDFLAGS) -o $@ $^ $(LIBS)
-args.o: native.o
+args.o: native.o self.o
 args.o: CFLAGS_FOR_SOURCE = \
 	-DNATIVE_ENDIAN_VAL=$(NATIVE_ENDIAN_VAL) \
-	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
+	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE) \
+	-DSELF_ENDIAN_VAL=$(SELF_ENDIAN_VAL) \
+	-DSELF_POINTER_SIZE=$(SELF_POINTER_SIZE)
 NATIVE_ENDIAN=$(shell readelf -h native.o \
 	| grep Data \
 	| sed 's/.*, //;s/ endian//')
@@ -33,6 +35,15 @@ NATIVE_ENDIAN_VAL=$(if $(NATIVE_ENDIAN_LITTLE),ELFDATA2LSB,$(if $(NATIVE_ENDIAN_
 NATIVE_POINTER_SIZE=$(shell readelf -wi native.o \
 	| grep "Pointer Size:" \
 	| sed 's/.*: *//')
+SELF_ENDIAN=$(shell readelf -h self.o \
+	| grep Data \
+	| sed 's/.*, //;s/ endian//')
+SELF_ENDIAN_LITTLE=$(findstring $(SELF_ENDIAN),$(findstring little,$(SELF_ENDIAN)))
+SELF_ENDIAN_BIG=$(findstring $(SELF_ENDIAN),$(findstring big,$(SELF_ENDIAN)))
+SELF_ENDIAN_VAL=$(if $(SELF_ENDIAN_LITTLE),ELFDATA2LSB,$(if $(SELF_ENDIAN_BIG),ELFDATA2MSB,ELFDATANONE))
+SELF_POINTER_SIZE=$(shell readelf -wi self.o \
+	| grep "Pointer Size:" \
+	| sed 's/.*: *//')
 %.o: %.c
 	$(CC) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< $(CFLAGS_FOR_SOURCE)
 install: dwz
@@ -40,10 +51,12 @@ install: dwz
 	install -D -m 644 $(srcdir)/dwz.1 $(DESTDIR)$(mandir)/man1/dwz.1
 clean:
 	rm -f $(OBJECTS) *~ core* dwz $(TEST_EXECS) $(DWZ_TEST_OBJECTS) \
-	  dwz.log dwz.sum native.o
+	  dwz.log dwz.sum native.o self.o
 	rm -Rf testsuite-bin tmp.*
 native.o: native.c
 	$(CC) -o $@ $< -c -g
+self.o: native.c
+	$(CC) -o $@ $< -c $(CPPFLAGS) $(CFLAGS) -g
 
 PWD:=$(shell pwd -P)
 
@@ -78,7 +91,9 @@ dwz-for-test: $(DWZ_TEST_OBJECTS)
 	rm -f $(DWZ_TEST_OBJECTS)
 args-for-test.o: CFLAGS_FOR_SOURCE = \
 	-DNATIVE_ENDIAN_VAL=$(NATIVE_ENDIAN_VAL) \
-	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE)
+	-DNATIVE_POINTER_SIZE=$(NATIVE_POINTER_SIZE) \
+	-DSELF_ENDIAN_VAL=$(SELF_ENDIAN_VAL) \
+	-DSELF_POINTER_SIZE=$(SELF_POINTER_SIZE)
 $(DWZ_TEST_OBJECTS): %-for-test.o : %.c
 	$(CC) $< -o $@ -c \
 	  -DUSE_GNUC=0 -DDEVEL \
diff --git a/args.c b/args.c
index e93c24d..5e0404a 100644
--- a/args.c
+++ b/args.c
@@ -228,6 +228,14 @@ static struct option_help dwz_single_file_options_help[] =
 #define NATIVE_ENDIAN "not available"
 #endif
 
+#if SELF_ENDIAN_VAL == ELFDATA2MSB
+#define SELF_ENDIAN "big"
+#elif SELF_ENDIAN_VAL == ELFDATA2LSB
+#define SELF_ENDIAN "little"
+#else
+#define SELF_ENDIAN "not available"
+#endif
+
 /* Describe mult-file command line options.  */
 static struct option_help dwz_multi_file_options_help[] =
 {
@@ -244,12 +252,14 @@ static struct option_help dwz_multi_file_options_help[] =
   { "5", "dwarf-5", NULL, NULL,
     "Emit DWARF 5 standardized supplementary object files instead of"
     " GNU extension .debug_altlink." },
-  { "p", "multifile-pointer-size", "<SIZE|auto|native>", "auto",
+  { "p", "multifile-pointer-size", "<SIZE|auto|native|self>", "auto",
      "Set pointer size of multifile, in number of bytes."
-    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "." },
-  { "e", "multifile-endian", "<l|b|auto|native>", "auto",
+    " Native pointer size is " XSTR (NATIVE_POINTER_SIZE) "."
+    " Self pointer size is " XSTR (SELF_POINTER_SIZE) "."},
+  { "e", "multifile-endian", "<l|b|auto|native|self>", "auto",
     "Set endianity of multifile."
-    " Native endianity is " NATIVE_ENDIAN "." },
+    " Native endianity is " NATIVE_ENDIAN "."
+    " Self endianity is " SELF_ENDIAN "." },
   { "j", "jobs", "<n>", "number of processors / 2",
     "Process <n> files in parallel." }
 };
@@ -665,6 +675,11 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 	      multifile_force_ptr_size = NATIVE_POINTER_SIZE;
 	      break;
 	    }
+	  if (strcmp (optarg, "self") == 0)
+	    {
+	      multifile_force_ptr_size = SELF_POINTER_SIZE;
+	      break;
+	    }
 	  l = strtoul (optarg, &end, 0);
 	  if (*end != '\0' || optarg == end || (unsigned int) l != l)
 	    error (1, 0, "invalid argument -l %s", optarg);
@@ -690,6 +705,19 @@ parse_args (int argc, char *argv[], bool *hardlink, const char **outfile)
 		}
 	      break;
 	    }
+	  if (strcmp (optarg, "self") == 0)
+	    {
+	      switch (SELF_ENDIAN_VAL)
+		{
+		case ELFDATA2MSB:
+		case ELFDATA2LSB:
+		  multifile_force_endian = SELF_ENDIAN_VAL;
+		  break;
+		default:
+		  error (1, 0, "Cannot determine self endian");
+		}
+	      break;
+	    }
 	  if (strlen (optarg) != 1)
 	    error (1, 0, "invalid argument -l %s", optarg);
 	  switch (optarg[0])

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

* Re: [PATCH] Add -p native and -e native
  2021-04-13  8:33           ` Tom de Vries
@ 2021-04-13 10:04             ` Mark Wielaard
  2021-04-13 11:15               ` Tom de Vries
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Wielaard @ 2021-04-13 10:04 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Michael Matz, dwz, jakub

Hi Tom,

On Tue, Apr 13, 2021 at 10:33:16AM +0200, Tom de Vries wrote:
> On 4/13/21 9:45 AM, Tom de Vries wrote:
> > On 4/12/21 10:14 PM, Mark Wielaard wrote:
> >> Why are we going through all this?
> >>
> >> So with this when something is build as 32bit it can use 64bit as
> >> "native" pointer size. Or if something is cross compiled to
> >> little-endian it can still report big-endian as "native",
> >>
> > 
> > Yes.  Native is defined by the current implementation as the default
> > endiannes and pointer size of code generated by the compiler used to
> > compile dwz.
> > 
> > So if you're using a compiler that by default generates 32-bit while
> > targeting 64-bit platform, then the resulting -p native is 32-bit.
> > 
> >> But when does that ever make sense?
> > 
> > It doesn't.
> > 
> > The current implementation makes sense if you use a native compiler
> > (i.e., generates 64-bit code for a 64-bit platform), and the same holds
> > in a cross-compiling scenario.
> > 
> > I think the assumption that was made here is that the implementation is
> > good enough if it gives good results for the native compiler scenario.
> > 
> >> Why would one run the 32bit
> >> binary on a 64bit system and wanting the default -p native be 64bit
> >> instead of 32bit? 
> > 
> > No idea why one would want to run the 32-bit binary on a 64bit system in
> > the first place.  But it's possible.
> > 
> > If we have an option -p/-e native, it needs to be assigned a semantics
> > in that case.
> > 
> > And in the case of using a native compiler, the semantics are accurate.
> > 
> >> Wouldn't one install and run the actual "native"
> >> 64-bit binary in that case?
> > 
> > Yes, that's what I would do.

So "native" is not actually needed?

> FWIW, would this patch address your concerns?
> 
> It adds an option -p/-e self, and when we do:
> ...
> $ make clean; make CFLAGS="-O2 -g -m32" LDFLAGS=-m32
> ...
> we have:
> ...
> $ ./dwz -?
>   ...
>   -p, --multifile-pointer-size <SIZE|auto|native|self>
>                               Set pointer size of multifile, in number
> of bytes.
>                               Native pointer size is 8. Self pointer
> size is 4.
>                               Default value: auto.

So the semantics of "self" make sense to me, but the implementation doesn't.

My concerns are that "native" has semantics that nobody would use and
it introduces a complicated build rule for the "native.o" and defines
based on readelf scraping.

"self" semantics make sense to me (and I would actually call that
"native") but you don't need this complicated trick of generating an
.o file and using readelf to create defines. You can simply use sizeof
(void *) and #include <endian.h> plus __BYTE_ORDER to define the
endian and pointer sizes.

So can we just have native or self based on sizeof (void *) and
__BYTE_ORDER and get rid of this Makefile trickery?

Cheers,

Mark

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

* Re: [PATCH] Add -p native and -e native
  2021-04-13 10:04             ` Mark Wielaard
@ 2021-04-13 11:15               ` Tom de Vries
  0 siblings, 0 replies; 13+ messages in thread
From: Tom de Vries @ 2021-04-13 11:15 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Michael Matz, dwz, jakub

On 4/13/21 12:04 PM, Mark Wielaard wrote:
> Hi Tom,
> 
> On Tue, Apr 13, 2021 at 10:33:16AM +0200, Tom de Vries wrote:
>> On 4/13/21 9:45 AM, Tom de Vries wrote:
>>> On 4/12/21 10:14 PM, Mark Wielaard wrote:
>>>> Why are we going through all this?
>>>>
>>>> So with this when something is build as 32bit it can use 64bit as
>>>> "native" pointer size. Or if something is cross compiled to
>>>> little-endian it can still report big-endian as "native",
>>>>
>>>
>>> Yes.  Native is defined by the current implementation as the default
>>> endiannes and pointer size of code generated by the compiler used to
>>> compile dwz.
>>>
>>> So if you're using a compiler that by default generates 32-bit while
>>> targeting 64-bit platform, then the resulting -p native is 32-bit.
>>>
>>>> But when does that ever make sense?
>>>
>>> It doesn't.
>>>
>>> The current implementation makes sense if you use a native compiler
>>> (i.e., generates 64-bit code for a 64-bit platform), and the same holds
>>> in a cross-compiling scenario.
>>>
>>> I think the assumption that was made here is that the implementation is
>>> good enough if it gives good results for the native compiler scenario.
>>>
>>>> Why would one run the 32bit
>>>> binary on a 64bit system and wanting the default -p native be 64bit
>>>> instead of 32bit? 
>>>
>>> No idea why one would want to run the 32-bit binary on a 64bit system in
>>> the first place.  But it's possible.
>>>
>>> If we have an option -p/-e native, it needs to be assigned a semantics
>>> in that case.
>>>
>>> And in the case of using a native compiler, the semantics are accurate.
>>>
>>>> Wouldn't one install and run the actual "native"
>>>> 64-bit binary in that case?
>>>
>>> Yes, that's what I would do.
> 
> So "native" is not actually needed?
> 

That's not the conclusion I draw based on the argumentation above.

>> FWIW, would this patch address your concerns?
>>
>> It adds an option -p/-e self, and when we do:
>> ...
>> $ make clean; make CFLAGS="-O2 -g -m32" LDFLAGS=-m32
>> ...
>> we have:
>> ...
>> $ ./dwz -?
>>   ...
>>   -p, --multifile-pointer-size <SIZE|auto|native|self>
>>                               Set pointer size of multifile, in number
>> of bytes.
>>                               Native pointer size is 8. Self pointer
>> size is 4.
>>                               Default value: auto.
> 
> So the semantics of "self" make sense to me, but the implementation doesn't.
> 
> My concerns are that "native" has semantics that nobody would use

I think for the use cases that are obvious, self and native have the
same semantics and users will pick either.

For the non-obvious cases, I'm not sure what usage will look like, so
I'd say the solution there is to define options with clear semantics and
let the user figure it out.

> and
> it introduces a complicated build rule for the "native.o" and defines
> based on readelf scraping.
> 
> "self" semantics make sense to me (and I would actually call that
> "native") but you don't need this complicated trick of generating an
> .o file and using readelf to create defines. You can simply use sizeof
> (void *) and #include <endian.h> plus __BYTE_ORDER to define the
> endian and pointer sizes.
> 
> So can we just have native or self based on sizeof (void *) and
> __BYTE_ORDER and get rid of this Makefile trickery?
> 

I think the current implementation is reliable.  I'm not sure about your
proposal.

Looking at the gcc sources, I find:
...
    dw2_asm_output_data (1, DWARF2_ADDR_SIZE, "Pointer Size (in bytes)");
...
and:
...
#ifndef DWARF2_ADDR_SIZE
#define DWARF2_ADDR_SIZE ((POINTER_SIZE + BITS_PER_UNIT - 1) /
BITS_PER_UNIT)
#endif
...
and then some hardcoded overrides in some targets.

I don't see any guarantee that the pointer size as used in generated
dwarf matches sizeof (void *).

So concretely, my fear is that for some target you'd compile dwz
natively, then compile some app natively, use -p self and find that the
app not matches -p self.

Thanks,
- Tom

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

end of thread, other threads:[~2021-04-13 11:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09  9:24 [PATCH] Add -p native and -e native Tom de Vries
2021-04-09  9:42 ` Mark Wielaard
2021-04-09 12:48   ` Tom de Vries
2021-04-09 13:03   ` Michael Matz
2021-04-09 15:58     ` Tom de Vries
2021-04-12 12:33       ` Michael Matz
2021-04-12 15:11         ` Tom de Vries
2021-04-12 19:53           ` [committed] " Tom de Vries
2021-04-12 20:14       ` [PATCH] " Mark Wielaard
2021-04-13  7:45         ` Tom de Vries
2021-04-13  8:33           ` Tom de Vries
2021-04-13 10:04             ` Mark Wielaard
2021-04-13 11:15               ` Tom de Vries

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).