[PATCH v12 0/11 Add support for CRC64 generation in linker

[Ulf Samuelsson]

Copying Andreas Schwab, since he ran into the same problem...

=======================================================

Sorry about causing problems.

I was under the impression that bfd_vma was always 64-bit. That was 
obviously wrong.

 From the error message, it looks like it it is 32-bit on 32-bit hosts, 
and 64-bit on 64-bit hosts.

Checking "bfd/bfd-in2.h" shows that this is indeed the case.

==================

#if BFD_ARCH_SIZE >= 64
#define BFD64
#endif

and later:
#ifdef BFD64
typedef uint64_t bfd_vma;
...
#else /* not BFD64  */
typedef unsigned long bfd_vma;
...
#endif /* not BFD64  */

============================================

TEMPORARY WORKAROUND:

There is (according to comments in bfd/bfd-in2.h) a workaround by
configuring with "--enable-64-bit-bfd".

I do not know if that has any other side effects.

QUESTION: Can You guys, test "./configure --enable-64-bit-bfd" ?

QUESTION: What are the main drawbacks to this workaround?

============================================

PERMANENT FIX:

Building using 32-bit bfd_vma makes life a lot more difficult since a 
lot of the code uses the bfd_vma type.

The expression calculator implemented in ldexp.c stores all integers in 
a "bigint".

   struct big_int
     {
       bfd_vma integer;
       char *str;
     } bigint;

So if you do DIGEST POLY (64,0xD800000000000000ULL,x,y,z,v,w)
the expression calculator will return the wrong value for the all the 64 bit
constants larger than 0xFFFFFFFF.

You  do not get around this, except by redesigning a **lot**.

CONCLUSION: I do not see a permanent fix that allows the DIGEST feature
to work when building with a 32-bit BFD.

============================================

PERMANENT WORKAROUND.

If the user does not want to build with a 64-bit BFD
then the only reasonable solution I see is to check BFD64.

If not defined

* Disable 64-bit CRCs

* Allow 32-bit CRCs

I am going to start looking into that.

If the CRC-64 options are disabled, the testsuite tests for this feature 
must be disabled.

QUESTION: How can you disable tests based on BFD size?
============================================

TESTING THE PERMANENT WORKAROUND.

A problem is that this is hard to test, since I do not have any 32-bit 
linux installations.

You can set "--enable-64-bit-bfd=no", but then the m4 macro in 
config/bfdm4.m4
will check the size of "void *" and reset the bfd size to 64-bit.

It should be possible to extend config/bfd64.m4 with an 
"--enable-32-bit-bfd"
for testing purposes. I do not know if anyone ever tried to build 32-bit
on a 64-bit machine.

QUESTION: Does anyone think it is useful to build a 32-bit BFD on a 
64-bit machine?

Best Regards

Ulf Samuelsson

On 2023-03-08 10:31, Martin Liška wrote:
> On 3/7/23 19:08, Ulf Samuelsson via Binutils wrote:
>> On 2023-03-07 14:59, Nick Clifton wrote:
>>> Hi Ulf,
>>>
>>>> Patchset XII
>>>> testsuite fixed to skip some tests.
>>> Thanks.  I have now applied your patch set.
>>>
>>> I made some small changes, mostly code formatting and correcting
>>> some of the error messages.
>>>
>>> Thank you very much for persisting with this patch contribution.
>>>
>> The Buildbot reports errors though!
>>
>> Best Regards
>>
>> Ulf Samuelsson
>>
>>
>>> Cheers
>>>    Nick
>>>
>>>
> Hi.
>
> I think this patch set caused the following build failure:
>
> [  173s] gcc -DHAVE_CONFIG_H -I. -I../../ld  -I. -I../../ld -I../bfd -I../../ld/../bfd -I../../ld/../include  -fomit-frame-pointer -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type  -Wno-error     -DLOCALEDIR="\"/usr/share/locale\""  -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -Werror -DELF_LIST_OPTIONS=true -DELF_SHLIB_LIST_OPTIONS=false -DELF_PLT_UNWIND_LIST_OPTIONS=false   -fomit-frame-pointer -O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type  -Wno-error     -MT ldcrc64.o -MD -MP -MF .deps/ldcrc64.Tpo -c -o ldcrc64.o ../../ld/ldcrc64.c
> [  173s] ../../ld/ldcrc64.c:109:1: error: conflicting types for 'init_crc64_tab'; have 'uint64_t *(algorithm_desc_t *)' {aka 'long long unsigned int *(algorithm_desc_t *)'}
> [  173s]   109 | init_crc64_tab (algorithm_desc_t * dsc)
> [  173s]       | ^~~~~~~~~~~~~~
> [  173s] In file included from ../../ld/ldcrc64.c:33:
> [  173s] ../../ld/lddigest.h:166:17: note: previous declaration of 'init_crc64_tab' with type 'bfd_vma *(algorithm_desc_t *)' {aka 'long unsigned int *(algorithm_desc_t *)'}
> [  173s]   166 | extern bfd_vma *init_crc64_tab (algorithm_desc_t * dsc);
> [  173s]       |                 ^~~~~~~~~~~~~~
> [  173s] ../../ld/ldcrc64.c:140:10: error: conflicting types for 'calc_crc64'; have 'uint64_t(algorithm_desc_t *, const unsigned char *, size_t)' {aka 'long long unsigned int(algorithm_desc_t *, const unsigned char *, unsigned int)'}
> [  173s]   140 | uint64_t calc_crc64
> [  173s]       |          ^~~~~~~~~~
> [  173s] ../../ld/lddigest.h:167:16: note: previous declaration of 'calc_crc64' with type 'bfd_vma(algorithm_desc_t *, const unsigned char *, size_t)' {aka 'long unsigned int(algorithm_desc_t *, const unsigned char *, unsigned int)'}
> [  173s]   167 | extern bfd_vma calc_crc64
> [  173s]       |                ^~~~~~~~~~
> [  173s] make[4]: *** [Makefile:1604: ldcrc64.o] Error 1
> [  173s] make[4]: Leaving directory '/home/abuild/rpmbuild/BUILD/binutils-2.40.50/build-dir/ld'
>
> that happens on i586-linux-gnu host if I configure with --target=avr
>
> Can you please take a look?
> Thanks,
> Martin