add ctypes support for aarch64 windows

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

(Blocks 1 bug)

unspecified
mozilla65
ARM64
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

This is not the greatest place for this bug, but the JS engine is the bit that uses libffi, so...
Component: JavaScript Engine → js-ctypes
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.
Attachment #9016735 - Flags: review?(dmajor)
Assignee: nobody → nfroyd
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`.
(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 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.
Attachment #9016735 - Flags: review?(dmajor) → review+
Priority: -- → P1
(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.
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94c9a52a4f1d
add ctypes support for aarch64 windows; r=dmajor
https://hg.mozilla.org/mozilla-central/rev/94c9a52a4f1d
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.