microblaze: fix build error on 32-bit hosts

Message ID 20231007220105.818599-1-mark@klomp.org
State New
Headers
Series microblaze: fix build error on 32-bit hosts |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_check--master-arm fail Patch failed to apply

Commit Message

Mark Wielaard Oct. 7, 2023, 10:01 p.m. UTC
  commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
introduced a build error on 32-bit hosts:

elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
      |                                                   ~~^                    ~~~~~~~~~~~~~~
      |                                                     |                        |
      |                                                     long unsigned int        bfd_vma {aka unsigned int}
      |                                                   %x
elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
      |                                                   |                            |
      |                                                   long unsigned int            bfd_vma {aka unsigned int}
      |                                                 %x

Fix by explicitly casting the r_addend to long.
---
 bfd/elf32-microblaze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Michael Eager Oct. 7, 2023, 10:53 p.m. UTC | #1
On 10/7/23 15:01, Mark Wielaard wrote:
> commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
> introduced a build error on 32-bit hosts:
> 
> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
>   1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
>        |                                                   ~~^                    ~~~~~~~~~~~~~~
>        |                                                     |                        |
>        |                                                     long unsigned int        bfd_vma {aka unsigned int}
>        |                                                   %x
> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
>   2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
>        |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
>        |                                                   |                            |
>        |                                                   long unsigned int            bfd_vma {aka unsigned int}
>        |                                                 %x
> 
> Fix by explicitly casting the r_addend to long.
> ---
>   bfd/elf32-microblaze.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
> index a8ced43c08a..2c584f91a4e 100644
> --- a/bfd/elf32-microblaze.c
> +++ b/bfd/elf32-microblaze.c
> @@ -1986,7 +1986,7 @@ microblaze_elf_relax_section (bfd *abfd,
>   		/* Validate the in-band val.  */
>   		val = bfd_get_32 (abfd, contents + irel->r_offset);
>   		if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == R_MICROBLAZE_32_NONE) {
> -		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
> +		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, (long) irel->r_addend);
>   		}
>   		irel->r_addend -= (efix - sfix);
>   		/* Should use HOWTO.  */
> @@ -2071,7 +2071,7 @@ microblaze_elf_relax_section (bfd *abfd,
>   
>   		  val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
>   		  if (val != irelscan->r_addend) {
> -			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
> +			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, (long) irelscan->r_addend);
>   		  }
>   
>   		  irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);

I didn't see any build errors, building with GCC-12.3.1.

Which version of GCC are you using?

(Patch was reverted.)
  
Michael Eager Oct. 7, 2023, 11:07 p.m. UTC | #2
On 10/7/23 15:53, Michael Eager wrote:
> On 10/7/23 15:01, Mark Wielaard wrote:
>> commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
>> introduced a build error on 32-bit hosts:
>>
>> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
>> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of 
>> type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka 
>> ‘unsigned int’} [-Werror=format=]
>>   1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, irel->r_addend);
>>        |                                                   
>> ~~^                    ~~~~~~~~~~~~~~
>>        |                                                     
>> |                        |
>>        |                                                     long 
>> unsigned int        bfd_vma {aka unsigned int}
>>        |                                                   %x
>> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of 
>> type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka 
>> ‘unsigned int’} [-Werror=format=]
>>   2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, irelscan->r_addend);
>>        |                                                 
>> ~~^                    ~~~~~~~~~~~~~~~~~~
>>        |                                                   
>> |                            |
>>        |                                                   long 
>> unsigned int            bfd_vma {aka unsigned int}
>>        |                                                 %x
>>
>> Fix by explicitly casting the r_addend to long.
>> ---
>>   bfd/elf32-microblaze.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
>> index a8ced43c08a..2c584f91a4e 100644
>> --- a/bfd/elf32-microblaze.c
>> +++ b/bfd/elf32-microblaze.c
>> @@ -1986,7 +1986,7 @@ microblaze_elf_relax_section (bfd *abfd,
>>           /* Validate the in-band val.  */
>>           val = bfd_get_32 (abfd, contents + irel->r_offset);
>>           if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == 
>> R_MICROBLAZE_32_NONE) {
>> -            fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, irel->r_addend);
>> +            fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, (long) irel->r_addend);
>>           }
>>           irel->r_addend -= (efix - sfix);
>>           /* Should use HOWTO.  */
>> @@ -2071,7 +2071,7 @@ microblaze_elf_relax_section (bfd *abfd,
>>             val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
>>             if (val != irelscan->r_addend) {
>> -            fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, irelscan->r_addend);
>> +            fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, (long) irelscan->r_addend);
>>             }
>>             irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, 
>> sec);
> 
> I didn't see any build errors, building with GCC-12.3.1.

Building with GCC-13.2.1 on Fedora 38 -- no error messages.

> Which version of GCC are you using?
> 
> (Patch was reverted.)
>
  
Mark Wielaard Oct. 7, 2023, 11:14 p.m. UTC | #3
Hi,

On Sat, Oct 07, 2023 at 04:07:30PM -0700, Michael Eager wrote:
> 
> Building with GCC-13.2.1 on Fedora 38 -- no error messages.
> 
> >Which version of GCC are you using?

gcc (Debian 10.2.1-6) 10.2.1 20210110 on debian-i386.

But I don't think it depends on the gcc version.  It depends on having
an 32bit host that is configured with --enable-targets=all.

Cheers,

Mark
  
Neal Frager Oct. 9, 2023, 11:58 a.m. UTC | #4
Hi Mark, Maciej, Michael,

> 
> Building with GCC-13.2.1 on Fedora 38 -- no error messages.
> 
> >Which version of GCC are you using?

> gcc (Debian 10.2.1-6) 10.2.1 20210110 on debian-i386.

> But I don't think it depends on the gcc version.  It depends on having an 32bit host that is configured with --enable-targets=all.

I added the following configs which I was not previously using in my binutils build:
--enable-targets=all
--enable-obsolete
--enable-plugins

I have also tried with various gcc versions, and I am unable to duplicate the build issue you are reporting.

The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.

Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

Best regards,
Neal Frager
AMD
  
Mark Wielaard Oct. 9, 2023, 12:37 p.m. UTC | #5
Hi Neal,

On Mon, 2023-10-09 at 11:58 +0000, Frager, Neal wrote:
> The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.
> 
> Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

It is as the error says:

elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
      |                                                   ~~^                    ~~~~~~~~~~~~~~
      |                                                     |                        |
      |                                                     long unsigned int        bfd_vma {aka unsigned int}
      |                                                   %x
elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
      |                                                   |                            |
      |                                                   long unsigned int            bfd_vma {aka unsigned int}
      |                                                 %x


On a 32bit host bfd_vma is an unsigned int, but you are using a long
printf format.

Casting the r_addend to (long) is one way to resolve this.

Cheers,

Mark

P.S. If you have a commit access to binutils you can use a try branch
to test patches against builder.sourceware.org which contains several
32bit hosts. See https://sourceware.org/binutils/wiki/Buildbot
  
Neal Frager Oct. 9, 2023, 12:42 p.m. UTC | #6
Hi Mark,

> The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.
> 
> Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

> It is as the error says:

> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
> 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
>      |                                                   ~~^                    ~~~~~~~~~~~~~~
>      |                                                     |                        |
>      |                                                     long unsigned int        bfd_vma {aka unsigned int}
>      |                                                   %x
> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
> 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
>      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
>      |                                                   |                            |
>      |                                                   long unsigned int            bfd_vma {aka unsigned int}
>      |                                                 %x


> On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.

> Casting the r_addend to (long) is one way to resolve this.

I already solved this build issue.  I thought the issue Maciej was reporting was another one.

If this is the only build issue, then I will submit v3 which solves this problem now.

Could you please confirm that this is the only issue that needs to be fixed?

Best regards,
Neal Frager
AMD
  
Mark Wielaard Oct. 9, 2023, 12:48 p.m. UTC | #7
Hi Neal,

On Mon, 2023-10-09 at 12:42 +0000, Frager, Neal 
> > On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.
> 
> > Casting the r_addend to (long) is one way to resolve this.
> 
> I already solved this build issue.  I thought the issue Maciej was reporting was another one.
> 
> If this is the only build issue, then I will submit v3 which solves this problem now.
> 
> Could you please confirm that this is the only issue that needs to be fixed?

It is the only issue I noticed that prevented the build to complete on
a 32bit host (with --enable-target=all). I didn't try to run the
testsuite.

Cheers,

Mark
  
Neal Frager Oct. 9, 2023, 12:56 p.m. UTC | #8
Hi Mark,

> > On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.
> 
> > Casting the r_addend to (long) is one way to resolve this.
> 
> I already solved this build issue.  I thought the issue Maciej was reporting was another one.
> 
> If this is the only build issue, then I will submit v3 which solves this problem now.
> 
> Could you please confirm that this is the only issue that needs to be fixed?

> It is the only issue I noticed that prevented the build to complete on a 32bit host (with --enable-target=all). I didn't try to run the testsuite.

Thank you for confirming this.  I just submitted v3 of my patch which will fix this issue.

@Maciej, could you please test v3 of my patch to verify that it resolves the issue you reported?
https://patchwork.sourceware.org/project/binutils/patch/20231009125144.377940-1-neal.frager@amd.com/

I have also added assembler tests to the gas testsuite for the new instructions.

Thank you for your help!

Best regards,
Neal Frager
AMD
  

Patch

diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a8ced43c08a..2c584f91a4e 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -1986,7 +1986,7 @@  microblaze_elf_relax_section (bfd *abfd,
 		/* Validate the in-band val.  */
 		val = bfd_get_32 (abfd, contents + irel->r_offset);
 		if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == R_MICROBLAZE_32_NONE) {
-		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
+		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, (long) irel->r_addend);
 		}
 		irel->r_addend -= (efix - sfix);
 		/* Should use HOWTO.  */
@@ -2071,7 +2071,7 @@  microblaze_elf_relax_section (bfd *abfd,
 
 		  val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
 		  if (val != irelscan->r_addend) {
-			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
+			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, (long) irelscan->r_addend);
 		  }
 
 		  irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);