Re: [PATCH] Add -p native and -e native

[Tom de Vries <tdevries@suse.de>]

[-- 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])