diff mbox

Adjust _Unwind_Word in unwind.h to version in libgcc.

Message ID mp7b4a$8q4$1@ger.gmane.org
State Committed
Headers

Commit Message

Stefan Liebler July 28, 2015, 7:31 a.m. UTC 
  Hi,

I get the following warning while building glibc on s390-32 with gcc 
option -mzarch:

In file included from unwind.c:26:0:
unwind.c: In function 'unwind_stop':
../sysdeps/s390/jmpbuf-unwind.h:37:10: error: cast to pointer from 
integer of different size [-Werror=int-to-pointer-cast]
           (void *) (_Unwind_GetCFA  (_context) \
           ^
../sysdeps/s390/jmpbuf-unwind.h:51:17: note: in definition of macro 
'_JMPBUF_UNWINDS_ADJ'
    ((uintptr_t) (_address) - (_adj) < _jmpbuf_sp (_jmpbuf) - (_adj))
                  ^
unwind.c:62:12: note: in expansion of macro '_JMPBUF_CFA_UNWINDS_ADJ'
        || ! _JMPBUF_CFA_UNWINDS_ADJ (buf->cancel_jmp_buf[0].jmp_buf, 
context,

Building on s390-32 in esa-mode or s390-64 is fine.

_Unwind_GetCFA returns an _Unwind_Word which is an unsigned
with a size of 4 bytes on s390-32 (esa-mode) and 8 bytes on s390-64.
On s390-32 (zarch-mode), _Unwind_Word has a size of 8 bytes, too.

_Unwind_Word is defined in sysdeps/generic/unwind.h as
typedef unsigned _Unwind_Word __attribute__((__mode__(__word__)));

In libgcc unwind header (<gcc-src>/libgcc/unwind-generic.h) this typedef 
has changed to
"typedef unsigned _Unwind_Word 
__attribute__((__mode__(__unwind_word__)));" in June 2008.
With this mode, _Unwind_Word has a size of 4 bytes on s390-32 (zarch-mode).
The same change applies to _Unwind_Sword.
Thus this patch updates the unwind header according to these changes.

Afterwards, the int-to-pointer-cast-warning is gone away on s390-32 
(zarch-mode) and the testsuite runs with the same test-failures as 
s390-32 (esa-mode) plus FAIL: c++-types-check. Here register_t is 
expected to has a size of 4 bytes, but it has a size of 8 bytes
due to
posix/sys/types.h:205:typedef int register_t __attribute__ ((__mode__ 
(__word__)));

The libgcc-patch for gcc 4.4 can be found here:
"[PATCH, spu, unwind] Remove attribute ((mode (word))) from unwind.h"
https://gcc.gnu.org/ml/gcc-patches/2008-06/msg00969.html

Ok to commit?

Bye
Stefan

2015-07-28  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* sysdeps/generic/unwind.h
	(_Unwind_Word): Use __mode__(__unwind_word__)
	instead of __mode__(__word__).
	(_Unwind_Sword): Likewise.

Comments

Andreas Schwab July 28, 2015, 8:05 a.m. UTC | #1 
I see three uses of _Unwind_Word as a pointer: in
sysdeps/generic/unwind-dw2.c _Unwind_GetGR and _Unwind_SetGR and in
sysdeps/generic/unwind-pe.h read_uleb128.  Is the new definition correct
for them?

Andreas.
Stefan Liebler July 28, 2015, 1:32 p.m. UTC | #2 
On 07/28/2015 10:05 AM, Andreas Schwab wrote:
> I see three uses of _Unwind_Word as a pointer: in
> sysdeps/generic/unwind-dw2.c _Unwind_GetGR and _Unwind_SetGR and in
> sysdeps/generic/unwind-pe.h read_uleb128.  Is the new definition correct
> for them?
>
> Andreas.
>

sysdeps/generic/unwind-pe.h read_uleb128:
_Unwind_Word has now a size of 4bytes on s390-32 (esa or zarch mode),
which can store a pointer, which has 4bytes on s390-32 (esa or zarch 
mode). On s390-64 both _Unwind_Word and a pointer have a size of 8 bytes.

sysdeps/generic/unwind-dw2.c _Unwind_GetGR and _Unwind_SetGR:
Here the 4 bytes on s390-32 (esa or zarch mode) is correct, too.
Without this patch, 8 bytes would be dereferenced on s390-32 
(zarch-mode), which is incorrect.
For s390-64, 8 bytes is correct.
Carlos O'Donell July 31, 2015, 1:18 p.m. UTC | #3 
On 07/28/2015 09:32 AM, Stefan Liebler wrote:
> On 07/28/2015 10:05 AM, Andreas Schwab wrote:
>> I see three uses of _Unwind_Word as a pointer: in
>> sysdeps/generic/unwind-dw2.c _Unwind_GetGR and _Unwind_SetGR and in
>> sysdeps/generic/unwind-pe.h read_uleb128.  Is the new definition correct
>> for them?
>>
>> Andreas.
>>
> 
> sysdeps/generic/unwind-pe.h read_uleb128:
> _Unwind_Word has now a size of 4bytes on s390-32 (esa or zarch mode),
> which can store a pointer, which has 4bytes on s390-32 (esa or zarch mode). On s390-64 both _Unwind_Word and a pointer have a size of 8 bytes.
> 
> sysdeps/generic/unwind-dw2.c _Unwind_GetGR and _Unwind_SetGR:
> Here the 4 bytes on s390-32 (esa or zarch mode) is correct, too.
> Without this patch, 8 bytes would be dereferenced on s390-32 (zarch-mode), which is incorrect.
> For s390-64, 8 bytes is correct.
> 

Does this mean s390 has 2 supported ABIs for 32-bit?

If so, we should adjust:

https://sourceware.org/glibc/wiki/ABIList?highlight=%28ABI%29#s390

Cheers,
Carlos.
Andreas Krebbel July 31, 2015, 1:53 p.m. UTC | #4 
On Fri, Jul 31, 2015 at 09:18:46AM -0400, Carlos O'Donell wrote:
> 
> Does this mean s390 has 2 supported ABIs for 32-bit?
> 
> If so, we should adjust:
> 
> https://sourceware.org/glibc/wiki/ABIList?highlight=%28ABI%29#s390

No, there is only one ABI for s390-32.  s390-32 ESA and s390-32 ZARCH
both are supposed to adhere to the 31 bit ABI.  The latter will use 64
bit registers only in non-ABI relevant situations.  In order to
achieve this in GCC we had to increase the register size to 64 bit
even when compiling with -m31. Unfortunately the register size was
exposed to outside GCC in some places (e.g. mode(__word__)).  That's
why something like __unwind_word__ was invented (there are others).
mode("__unwind_word__") changes only with ABI switches (-m31/-m64)
while mode("__word__") does change with -mesa/-mzarch.

What Stefan just fixed was a problem where the libgcc and Glibc unwind
code was out of sync.

Bye,

-Andreas-
Carlos O'Donell July 31, 2015, 3:49 p.m. UTC | #5 
On 07/31/2015 09:53 AM, Andreas Krebbel wrote:
> On Fri, Jul 31, 2015 at 09:18:46AM -0400, Carlos O'Donell wrote:
>>
>> Does this mean s390 has 2 supported ABIs for 32-bit?
>>
>> If so, we should adjust:
>>
>> https://sourceware.org/glibc/wiki/ABIList?highlight=%28ABI%29#s390
> 
> No, there is only one ABI for s390-32.  s390-32 ESA and s390-32 ZARCH
> both are supposed to adhere to the 31 bit ABI.  The latter will use 64
> bit registers only in non-ABI relevant situations.  In order to
> achieve this in GCC we had to increase the register size to 64 bit
> even when compiling with -m31. Unfortunately the register size was
> exposed to outside GCC in some places (e.g. mode(__word__)).  That's
> why something like __unwind_word__ was invented (there are others).
> mode("__unwind_word__") changes only with ABI switches (-m31/-m64)
> while mode("__word__") does change with -mesa/-mzarch.
> 
> What Stefan just fixed was a problem where the libgcc and Glibc unwind
> code was out of sync.

Thanks for explaining that.

Cheers,
Carlos.
Stefan Liebler Aug. 5, 2015, 10:40 a.m. UTC | #6 
ping

Ok to commit?

On 07/28/2015 09:31 AM, Stefan Liebler wrote:
> Hi,
>
> I get the following warning while building glibc on s390-32 with gcc
> option -mzarch:
>
> In file included from unwind.c:26:0:
> unwind.c: In function 'unwind_stop':
> ../sysdeps/s390/jmpbuf-unwind.h:37:10: error: cast to pointer from
> integer of different size [-Werror=int-to-pointer-cast]
>            (void *) (_Unwind_GetCFA  (_context) \
>            ^
> ../sysdeps/s390/jmpbuf-unwind.h:51:17: note: in definition of macro
> '_JMPBUF_UNWINDS_ADJ'
>     ((uintptr_t) (_address) - (_adj) < _jmpbuf_sp (_jmpbuf) - (_adj))
>                   ^
> unwind.c:62:12: note: in expansion of macro '_JMPBUF_CFA_UNWINDS_ADJ'
>         || ! _JMPBUF_CFA_UNWINDS_ADJ (buf->cancel_jmp_buf[0].jmp_buf,
> context,
>
> Building on s390-32 in esa-mode or s390-64 is fine.
>
> _Unwind_GetCFA returns an _Unwind_Word which is an unsigned
> with a size of 4 bytes on s390-32 (esa-mode) and 8 bytes on s390-64.
> On s390-32 (zarch-mode), _Unwind_Word has a size of 8 bytes, too.
>
> _Unwind_Word is defined in sysdeps/generic/unwind.h as
> typedef unsigned _Unwind_Word __attribute__((__mode__(__word__)));
>
> In libgcc unwind header (<gcc-src>/libgcc/unwind-generic.h) this typedef
> has changed to
> "typedef unsigned _Unwind_Word
> __attribute__((__mode__(__unwind_word__)));" in June 2008.
> With this mode, _Unwind_Word has a size of 4 bytes on s390-32 (zarch-mode).
> The same change applies to _Unwind_Sword.
> Thus this patch updates the unwind header according to these changes.
>
> Afterwards, the int-to-pointer-cast-warning is gone away on s390-32
> (zarch-mode) and the testsuite runs with the same test-failures as
> s390-32 (esa-mode) plus FAIL: c++-types-check. Here register_t is
> expected to has a size of 4 bytes, but it has a size of 8 bytes
> due to
> posix/sys/types.h:205:typedef int register_t __attribute__ ((__mode__
> (__word__)));
>
> The libgcc-patch for gcc 4.4 can be found here:
> "[PATCH, spu, unwind] Remove attribute ((mode (word))) from unwind.h"
> https://gcc.gnu.org/ml/gcc-patches/2008-06/msg00969.html
>
> Ok to commit?
>
> Bye
> Stefan
>
> 2015-07-28  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>      * sysdeps/generic/unwind.h
>      (_Unwind_Word): Use __mode__(__unwind_word__)
>      instead of __mode__(__word__).
>      (_Unwind_Sword): Likewise.
Andreas Schwab Aug. 5, 2015, 10:54 a.m. UTC | #7 
Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> 2015-07-28  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
> 	* sysdeps/generic/unwind.h
> 	(_Unwind_Word): Use __mode__(__unwind_word__)
> 	instead of __mode__(__word__).
> 	(_Unwind_Sword): Likewise.

Ok.

Andreas.
diff mbox

Patch

diff --git a/sysdeps/generic/unwind.h b/sysdeps/generic/unwind.h
index 41b6aec..75d3084 100644
--- a/sysdeps/generic/unwind.h
+++ b/sysdeps/generic/unwind.h
@@ -31,8 +31,8 @@  extern "C" {
 
 /* @@@ The IA-64 ABI uses uint64 throughout.  Most places this is
    inefficient for 32-bit and smaller machines.  */
-typedef unsigned _Unwind_Word __attribute__((__mode__(__word__)));
-typedef signed _Unwind_Sword __attribute__((__mode__(__word__)));
+typedef unsigned _Unwind_Word __attribute__((__mode__(__unwind_word__)));
+typedef signed _Unwind_Sword __attribute__((__mode__(__unwind_word__)));
 #if defined(__ia64__) && defined(__hpux__)
 typedef unsigned _Unwind_Ptr __attribute__((__mode__(__word__)));
 #else