Closed Bug 1659906 Opened 4 years ago Closed 4 years ago

test_jsctypes.js crashes in run_closure_tests on Apple Silicon

Categories

(Core :: js-ctypes, defect)

Product:
Core
Component:
js-ctypes
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
81 Branch
Tracking Flags:
Tracking Status
firefox81 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Bug 1659906 - Upgrade libffi to version 3.3.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1659906 - Fix variadic arguments on arm64 darwin ABI.
47 bytes, text/x-phabricator-request
Details | Review

(Edited because when I reported this, I was building with libffi master ; this is with in-tree libffi)

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x1256c0540)
    frame #0: 0x00000001256c0540
->  0x1256c0540: ldr    x16, #0xc
    0x1256c0544: adr    x17, #0x10
    0x1256c0548: br     x16
    0x1256c054c: .long  0x06c049d8                ; unknown opcode
Target 0: (xpcshell) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x1256c0540)
  * frame #0: 0x00000001256c0540
    frame #1: 0x0000000106c049a0 XUL`ffi_call_SYSV at sysv.S:163
    frame #2: 0x0000000106c034e4 XUL`ffi_call(cif=0x0000000117ff4100, fn=(0x00000001256c0540), rvalue=0x0000000120ee8138, avalue=<unavailable>) at ffi.c:840:13 [opt]
    frame #3: 0x00000001063e8478 XUL`js::ctypes::FunctionType::Call(cx=0x0000000117f38000, argc=<unavailable>, vp=0x000000016d75f6c8) at CTypes.cpp:7084:3 [opt]
    frame #4: 0x0000000106400294 XUL`js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [inlined] CallJSNative(cx=0x0000000117f38000, native=(XUL`js::ctypes::FunctionType::Call(JSContext*, unsigned int, JS::Value*) at CTypes.cpp:6962), reason=<unavailable>, args=0x000000016d75f680)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) at Interpreter.cpp:507:13 [opt]
    frame #5: 0x0000000106400150 XUL`js::InternalCallOrConstruct(cx=0x0000000117f38000, args=0x000000016d75f680, construct=<unavailable>, reason=<unavailable>) at Interpreter.cpp:579 [opt]
    frame #6: 0x00000001064004a0 XUL`js::Call(cx=<unavailable>, fval=<unavailable>, thisv=<unavailable>, args=0x000000016d75f680, rval=JS::MutableHandleValue @ x19, reason=<unavailable>) at Interpreter.cpp:681:8 [opt]
    frame #7: 0x0000000106494f48 XUL`js::ForwardingProxyHandler::call(this=<unavailable>, cx=0x0000000117f38000, proxy=<unavailable>, args=0x000000016d75fa20) const at Wrapper.cpp:163:10 [opt]
    frame #8: 0x0000000106487804 XUL`js::CrossCompartmentWrapper::call(this=0x000000010aca7f08, cx=0x0000000117f38000, wrapper=JS::HandleObject @ x21, args=0x000000016d75fa20) const at CrossCompartmentWrapper.cpp:239:19 [opt]
    frame #9: 0x000000010648d5d0 XUL`js::Proxy::call(cx=0x0000000117f38000, proxy=JS::HandleObject @ x21, args=0x000000016d75fa20) at Proxy.cpp:645:19 [opt]
    frame #10: 0x0000000106400140 XUL`js::InternalCallOrConstruct(cx=0x0000000117f38000, args=0x000000016d75fa20, construct=NO_CONSTRUCT, reason=Call) at Interpreter.cpp:573:14 [opt]
    frame #11: 0x00000001063fae44 XUL`Interpret(JSContext*, js::RunState&) [inlined] js::CallFromStack(cx=0x0000000117f38000, args=<unavailable>) at Interpreter.cpp:668:10 [opt]
    frame #12: 0x00000001063fae34 XUL`Interpret(cx=<unavailable>, state=0x000000016d75fe98) at Interpreter.cpp:3336 [opt]
    frame #13: 0x00000001063f44a8 XUL`js::RunScript(cx=0x0000000117f38000, state=0x000000016d75fe98) at Interpreter.cpp:468:13 [opt]
    frame #14: 0x000000010640002c XUL`js::InternalCallOrConstruct(cx=0x0000000117f38000, args=<unavailable>, construct=<unavailable>, reason=<unavailable>) at Interpreter.cpp:636:13 [opt]
    frame #15: 0x00000001068c1aa0 XUL`js::jit::DoCallFallback(cx=<unavailable>, frame=0x000000016d760320, stub=0x000000011c469bc0, argc=<unavailable>, vp=0x000000016d7602a0, res=JS::MutableHandleValue @ 0x000000016d75ff60) at BaselineIC.cpp:3018:10 [opt]
    frame #16: 0x00000070000247d0

run_FunctionType_tests also crashes with a similar signature.

(Edited because when I reported this, I was building with libffi master ; this is with in-tree libffi)

Turns out I was using a bastardized version. Using libffi 3.3 properly, the crash doesn't appear (but it does with what we have in-tree right now, which is a patched 3.1). The downside of the upgrade is that it breaks variadic support.

FTR, 3.3 also fixes:

0:00.71 FAIL run_single_abi_tests - [run_single_abi_tests : 1140] 5.5646195388104475e-14 == 0
/Users/glandium/gecko-dev/obj-aarch64-apple-darwin20.0.0/_tests/xpcshell/toolkit/components/ctypes/tests/unit/test_jsctypes.js:run_single_abi_tests:1140
/Users/glandium/gecko-dev/obj-aarch64-apple-darwin20.0.0/_tests/xpcshell/toolkit/components/ctypes/tests/unit/test_jsctypes.js:run_basic_abi_tests:1015
/Users/glandium/gecko-dev/obj-aarch64-apple-darwin20.0.0/_tests/xpcshell/toolkit/components/ctypes/tests/unit/test_jsctypes.js:run_float_tests:1644
/Users/glandium/gecko-dev/obj-aarch64-apple-darwin20.0.0/_tests/xpcshell/toolkit/components/ctypes/tests/unit/test_jsctypes.js:run_test:96
/Users/glandium/gecko-dev/testing/xpcshell/head.js:_execute_test:571

The series is split for ease of review. This part replaces the
directory entirely with the latest released version, dropping any
patches applied to our tree.

All the patches previously applied, except the one from bug 1279096,
are either irrelevant (as pertaining to changes to the upstream build
system we don't use anymore), were applied upstream, or the issue they
fixed were fixed differently upstream.

Two additional patches, sent upstream as
https://github.com/libffi/libffi/pull/579 and
https://github.com/libffi/libffi/pull/580, are needed to fix our build
with, respectively, mingw-clang and GCC.

Our build system is adjusted according to upstream's configure.ac
and configure.host.

Normal arguments that spill on the stack are packed, but not variadic
arguments. This is handled correctly for their placement already, but
code generated on the callee side with va_list expects word-size
sign-extension, so we need to fill the entire word.

Upstreamed as https://github.com/libffi/libffi/pull/577.

This is interdependent with bug 1659905.

Depends on: 1659905
Attachment #9171313 - Attachment description: Bug 1659906 - Adjust the build system to the new libffi version. → Bug 1659906 - Upgrade libffi to version 3.3.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/08a1c02d93e4
Upgrade libffi to version 3.3. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/40edcd06d482
Fix variadic arguments on arm64 darwin ABI. r=froydnj
Attachment #9171312 - Attachment is obsolete: true
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/6f6c608cb8df
Upgrade libffi to version 3.3. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/a195d2e3119c
Fix variadic arguments on arm64 darwin ABI. r=froydnj
Flags: needinfo?(mh+mozilla)

https://hg.mozilla.org/mozilla-central/rev/6f6c608cb8df
https://hg.mozilla.org/mozilla-central/rev/a195d2e3119c

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox81: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Regressions: 1660828
See Also: → 1664111
Blocks: 1687433
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: