1480550 - add ctypes support for aarch64 windows

Status: RESOLVED FIXED

(Core :: js-ctypes, enhancement, P1)

Description

[Nathan Froyd [:froydnj]]

This is not the greatest place for this bug, but the JS engine is the bit that uses libffi, so...

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: add ctypes support for aarch64 windows

I believe this is the patch we need for ctypes support, and it's better to have
it in the tree than floating around on our hard drives.

Comment 2

[Tom Tromey :tromey]

Comment on attachment 9016735 [details] [diff] [review]
add ctypes support for aarch64 windows

Review of attachment 9016735 [details] [diff] [review]:
-----------------------------------------------------------------

Please be sure to send this upstream:
https://github.com/libffi/libffi

::: js/src/ctypes/libffi/src/aarch64/ffi.c
@@ +230,5 @@
>  }
>  
> +// XXX The Win64 and the SYSV ABI are very close, differing only in their
> +// calling of varargs functions.  Since we don't care about calling varargs
> +// functions in our use of libffi (or maybe libffi doesn't even support

libffi does support calling varargs functions now.  See `ffi_prep_cif_var`.

Comment 3

[Nathan Froyd [:froydnj]]

(In reply to Tom Tromey :tromey from comment #2)
> Please be sure to send this upstream:
> https://github.com/libffi/libffi

I would like to, but upstream is so radically different from what's here, I'd have to more-or-less start over for the assembly parts (although I could reuse quite a bit of upstream's already-written assembly).  I'm not sure I'm up to writing the proper varargs support for the Win64 ABI.

> ::: js/src/ctypes/libffi/src/aarch64/ffi.c
> @@ +230,5 @@
> >  }
> >  
> > +// XXX The Win64 and the SYSV ABI are very close, differing only in their
> > +// calling of varargs functions.  Since we don't care about calling varargs
> > +// functions in our use of libffi (or maybe libffi doesn't even support
> 
> libffi does support calling varargs functions now.  See `ffi_prep_cif_var`.

Ahah, thank you!

Comment 4

[(Away)]

Comment on attachment 9016735 [details] [diff] [review]
add ctypes support for aarch64 windows

Review of attachment 9016735 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ctypes/libffi/src/aarch64/ffi.c
@@ +25,5 @@
>  #include <ffi_common.h>
>  
>  #include <stdlib.h>
>  
> +#if defined(_M_ARM64)

The two hunks in this file should use the same ifdef. I think I'd lean towards _WIN32 between the two.

@@ +230,5 @@
>  }
>  
> +// XXX The Win64 and the SYSV ABI are very close, differing only in their
> +// calling of varargs functions.  Since we don't care about calling varargs
> +// functions in our use of libffi (or maybe libffi doesn't even support

I'm assuming you'll update the comment.

Comment 5

[Tom Tromey :tromey]

(In reply to Nathan Froyd [:froydnj] from comment #3)
> (In reply to Tom Tromey :tromey from comment #2)
> > Please be sure to send this upstream:
> > https://github.com/libffi/libffi
> 
> I would like to, but upstream is so radically different from what's here,
> I'd have to more-or-less start over for the assembly parts (although I could
> reuse quite a bit of upstream's already-written assembly).  I'm not sure I'm
> up to writing the proper varargs support for the Win64 ABI.

There's a PR now: https://github.com/libffi/libffi/pull/455
I'm not really competent to review it though.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94c9a52a4f1d
add ctypes support for aarch64 windows; r=dmajor

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/94c9a52a4f1d