Bug 28630 - [mingw] bfd/coffgen.c triggers "cast to pointer from integer of different size" [-Werror=int-to-pointer-cast]
Summary: [mingw] bfd/coffgen.c triggers "cast to pointer from integer of different siz...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-26 16:00 UTC by Pekka Seppänen
Modified: 2021-12-01 11:31 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pekka Seppänen 2021-11-26 16:00:50 UTC
Hi.

The following compile error happens when __INTPTR_TYPE__ is not long int (e.g. x86_64-linux-gnu and x86_64-w64-mingw32-gcc).

/usr/local/gcc-aarch64-build/src/binutils/bfd/coffgen.c: In function 'coff_print_symbol':
/usr/local/gcc-aarch64-build/src/binutils/bfd/coffgen.c:2221:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 2221 |         (char *) auxp->u.auxent.x_file.x_n.x_n.x_offset);
      |         ^

binutils/bfd/coffgen.c:
2218                   if (auxp->u.auxent.x_file.x_ftype)
2219                     fprintf (file, "ftype %d fname \"%s\"",
2220                              auxp->u.auxent.x_file.x_ftype,
2221                              (char *) auxp->u.auxent.x_file.x_n.x_n.x_offset);

A crude fix would be to do `(char *) (bfd_hostptr_t)' and it solves this issue.  However I wonder if that simply causes more troubles down the road since in this particual example (mingw32) that involves narrowing and thus the stored pointer might become invalid if it any of the MSBs are (silently) discarded.
Comment 1 Clément Chigot 2021-11-29 10:24:16 UTC
Hi, 

> A crude fix would be to do `(char *) (bfd_hostptr_t)' and it solves this issue.  > However I wonder if that simply causes more troubles down the road since in this > particual example (mingw32) that involves narrowing and thus the stored pointer > might become invalid if it any of the MSBs are (silently) discarded.

I'm not sure I correctly understand the last part. 
If I'm not mistaking, x_offset is already a pointer (but stored as a long) according to https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/coffgen.c;h=5474f6c24d8d35d513ebb0fe36912aae236d649a;hb=HEAD#l865, even natively on mingw32. 
Thus casting it to (bfd_hostptr_t) looks good to me.

And if it's casting a pointer to a (char *) which is problematic when how are you retrieving a string from a pointer in this case ?
Comment 2 Pekka Seppänen 2021-11-29 11:49:56 UTC
In this case for x86_64-w64-mingw32-gcc __SIZEOF_POINTER__ is 8 while __SIZEOF_LONG__ is only 4.  Since all store locations use an explicit cast this goes undetected.

E.g. struct internal_syment uses bfd_hostptr_t (instead of long) for _n_offset which appears to also hold either a pointer or an index.

So, maybe a better alternative would be to change x_offset (and x_zeroes) from long to bfd_hostptr_t, too, but I do not know how much havoc this creates for code that might expect x_offset being long.
Comment 3 Clément Chigot 2021-11-29 14:07:40 UTC
> So, maybe a better alternative would be to change x_offset (and x_zeroes) from 
> long to bfd_hostptr_t, too, but I do not know how much havoc this creates for
> code that might expect x_offset being long.

I've tried quickly it's working fine for AIX and doesn't require any other changes. I've also tried some COFF targets in cross compilation and there doesn't seem to have any problems too. I'm not sure it's fully relevant though as it's cross compilation.  

However, as you mentioned 'internal_syment' is already using 'bfd_hostptr_t' so it looks fine to me. But it would need more tests directly on these COFF targets, which I don't have access or someone who know deeply bfd to see if it's a possible fix.
Comment 4 Sourceware Commits 2021-12-01 11:30:45 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=92fc129e2b0066706ee8971d311f2c507ce38d4b

commit 92fc129e2b0066706ee8971d311f2c507ce38d4b
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Dec 1 11:29:34 2021 +0000

    Fix the fields in the x_n union inside the the x_file structure so that pointers can be stored.
    
            PR 28630
            * coff/internal.h (x_n): Use bfd_hostptr_t for the fields in this
            structure.
Comment 5 Nick Clifton 2021-12-01 11:31:48 UTC
I agree - since internal_auxent is an internal structure it should be safe to change the fields to use the bfd_hostptr_t type.  I have checked in a patch to do this.