Re: armv7 ABI fix

2018-03-01 Thread Jonathan Gray
On Tue, Feb 27, 2018 at 10:04:15PM +0100, Mark Kettenis wrote:
> The "new" AAPCS-based ABI that we have been using on armv7 for a while
> now requires various 64-bit types to be aligned on an 8-byte boundary.
> Unfortunately we didn't realize this when we switched and didn't
> adjust the definition of _ALIGNBYTES accordingly.  The diff below
> fixes that.
> 
> However, this triggers a flag day.  It changes the CMSG ABI, which
> means that file descriptor passing breaks as soon as you boot a new
> kernel with old userland.  That sucks since many of our daemons use
> file descriptor passing, including sshd.  Can we deal with yet another
> ABI break on armv7?  I'm not sure the ports builders have recovered
> from the switch to clang yet...

ok jsg@

> 
> 
> Index: arch/arm/include/_types.h
> ===
> RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 _types.h
> --- arch/arm/include/_types.h 26 Sep 2015 16:01:00 -  1.17
> +++ arch/arm/include/_types.h 27 Feb 2018 20:54:05 -
> @@ -51,7 +51,7 @@ typedef struct label_t {
>   * This does not reflect the optimal alignment, just the possibility
>   * (within reasonable limits).
>   */
> -#define  _ALIGNBYTES (sizeof(int) - 1)
> +#define  _ALIGNBYTES (sizeof(double) - 1)
>  #define  _STACKALIGNBYTES7
>  #define  _ALIGN(p)   (((unsigned long)(p) + _ALIGNBYTES) & 
> ~_ALIGNBYTES)
>  #define  _ALIGNED_POINTER(p,t)   unsigned long)(p)) & (sizeof(t) - 
> 1)) == 0)
> 



Re: armv7 ABI fix

2018-02-27 Thread Peter Hessler
On 2018 Feb 27 (Tue) at 22:04:15 +0100 (+0100), Mark Kettenis wrote:
:The "new" AAPCS-based ABI that we have been using on armv7 for a while
:now requires various 64-bit types to be aligned on an 8-byte boundary.
:Unfortunately we didn't realize this when we switched and didn't
:adjust the definition of _ALIGNBYTES accordingly.  The diff below
:fixes that.
:
:However, this triggers a flag day.  It changes the CMSG ABI, which
:means that file descriptor passing breaks as soon as you boot a new
:kernel with old userland.  That sucks since many of our daemons use
:file descriptor passing, including sshd.  Can we deal with yet another
:ABI break on armv7?  I'm not sure the ports builders have recovered
:from the switch to clang yet...
:

That isn't the problem for armv7 ports builders, the main problem is all
of the alignment faults.  I have no problems with ABI breaks, do what
you need to.

I only use snaps for base+xenocara, so I can simply wait until a fixed
set is available.


-- 
It was a book to kill time for those who liked it better dead.



Re: armv7 ABI fix

2018-02-27 Thread Otto Moerbeek
On Tue, Feb 27, 2018 at 10:04:15PM +0100, Mark Kettenis wrote:

> The "new" AAPCS-based ABI that we have been using on armv7 for a while
> now requires various 64-bit types to be aligned on an 8-byte boundary.
> Unfortunately we didn't realize this when we switched and didn't
> adjust the definition of _ALIGNBYTES accordingly.  The diff below
> fixes that.
> 
> However, this triggers a flag day.  It changes the CMSG ABI, which
> means that file descriptor passing breaks as soon as you boot a new
> kernel with old userland.  That sucks since many of our daemons use
> file descriptor passing, including sshd.  Can we deal with yet another
> ABI break on armv7?  I'm not sure the ports builders have recovered
> from the switch to clang yet...

We need to bite the bullet here. Having _ALIGNBYTES wrong could lead to all
kinds of hard to debug problems way worse than a flag day.

-Otto

> 
> 
> Index: arch/arm/include/_types.h
> ===
> RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 _types.h
> --- arch/arm/include/_types.h 26 Sep 2015 16:01:00 -  1.17
> +++ arch/arm/include/_types.h 27 Feb 2018 20:54:05 -
> @@ -51,7 +51,7 @@ typedef struct label_t {
>   * This does not reflect the optimal alignment, just the possibility
>   * (within reasonable limits).
>   */
> -#define  _ALIGNBYTES (sizeof(int) - 1)
> +#define  _ALIGNBYTES (sizeof(double) - 1)
>  #define  _STACKALIGNBYTES7
>  #define  _ALIGN(p)   (((unsigned long)(p) + _ALIGNBYTES) & 
> ~_ALIGNBYTES)
>  #define  _ALIGNED_POINTER(p,t)   unsigned long)(p)) & (sizeof(t) - 
> 1)) == 0)



Re: armv7 ABI fix

2018-02-27 Thread Theo de Raadt
>However, this triggers a flag day.  It changes the CMSG ABI, which
>means that file descriptor passing breaks as soon as you boot a new
>kernel with old userland.  That sucks since many of our daemons use
>file descriptor passing, including sshd.  Can we deal with yet another
>ABI break on armv7?  I'm not sure the ports builders have recovered
>from the switch to clang yet...

Absolutely.  We need to -- this crutial definition is wrong.



armv7 ABI fix

2018-02-27 Thread Mark Kettenis
The "new" AAPCS-based ABI that we have been using on armv7 for a while
now requires various 64-bit types to be aligned on an 8-byte boundary.
Unfortunately we didn't realize this when we switched and didn't
adjust the definition of _ALIGNBYTES accordingly.  The diff below
fixes that.

However, this triggers a flag day.  It changes the CMSG ABI, which
means that file descriptor passing breaks as soon as you boot a new
kernel with old userland.  That sucks since many of our daemons use
file descriptor passing, including sshd.  Can we deal with yet another
ABI break on armv7?  I'm not sure the ports builders have recovered
from the switch to clang yet...


Index: arch/arm/include/_types.h
===
RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
retrieving revision 1.17
diff -u -p -r1.17 _types.h
--- arch/arm/include/_types.h   26 Sep 2015 16:01:00 -  1.17
+++ arch/arm/include/_types.h   27 Feb 2018 20:54:05 -
@@ -51,7 +51,7 @@ typedef struct label_t {
  * This does not reflect the optimal alignment, just the possibility
  * (within reasonable limits).
  */
-#define_ALIGNBYTES (sizeof(int) - 1)
+#define_ALIGNBYTES (sizeof(double) - 1)
 #define_STACKALIGNBYTES7
 #define_ALIGN(p)   (((unsigned long)(p) + _ALIGNBYTES) & 
~_ALIGNBYTES)
 #define_ALIGNED_POINTER(p,t)   unsigned long)(p)) & (sizeof(t) - 
1)) == 0)