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