Bug 206703

Summary: Fix OpenSource iphoneos arm64e build
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: JavaScriptCoreAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, keith_miller, mark.lam, msaboff, ryanhaddad, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=206620
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Jonathan Bedard
Reported 2020-01-23 14:59:27 PST
On arm64e, _STRUCT_ARM_THREAD_STATE64 is opaque. We need to handle this case so WebKit builds with arm64e.
Attachments
Patch (3.11 KB, patch)
2020-01-23 15:22 PST, Jonathan Bedard
no flags
Patch (9.18 KB, patch)
2020-01-24 11:36 PST, Jonathan Bedard
no flags
Patch (8.61 KB, patch)
2020-01-24 11:41 PST, Jonathan Bedard
no flags
Patch (9.43 KB, patch)
2020-01-24 15:54 PST, Jonathan Bedard
no flags
Patch (9.53 KB, patch)
2020-01-27 14:54 PST, Jonathan Bedard
no flags
Jonathan Bedard
Comment 1 2020-01-23 15:22:25 PST
Yusuke Suzuki
Comment 2 2020-01-23 15:32:38 PST
Comment on attachment 388599 [details] Patch I think we should not use __opaque_xxx since it is opaque. When introducing PLATFORM_REGISTERS_WITH_PROFILE feature, we defined getter / setter for stack-pointer and instruction-pointer. And instructionPointerImpl and stackPointerImpl are the implementation for these getters and setters. Let's use the macro in getters / setters directly and avoid using stackPointerImpl / instructionPointerImpl for ARM64 / ARM64E Darwin.
Mark Lam
Comment 3 2020-01-23 15:34:17 PST
Comment on attachment 388599 [details] Patch r=me
Mark Lam
Comment 4 2020-01-23 15:37:03 PST
Comment on attachment 388599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388599&action=review > Source/JavaScriptCore/runtime/MachineContext.h:98 > + return reinterpret_cast<void*&>(regs.__opaque_sp); Actually, Yusuke is right. Can you use the appropriate macros / setter / getters for this instead of accessing __opaque_sp directly? > Source/JavaScriptCore/runtime/MachineContext.h:404 > + return reinterpret_cast<void*&>(regs.__opaque_pc); Ditto.
Jonathan Bedard
Comment 5 2020-01-24 11:36:10 PST
Jonathan Bedard
Comment 6 2020-01-24 11:37:19 PST
(In reply to Mark Lam from comment #4) > Comment on attachment 388599 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=388599&action=review > > > Source/JavaScriptCore/runtime/MachineContext.h:98 > > + return reinterpret_cast<void*&>(regs.__opaque_sp); > > Actually, Yusuke is right. Can you use the appropriate macros / setter / > getters for this instead of accessing __opaque_sp directly? > > > Source/JavaScriptCore/runtime/MachineContext.h:404 > > + return reinterpret_cast<void*&>(regs.__opaque_pc); > > Ditto. Drafted a change which uses the macros, it's substantially more complicated, though. Still working on testing it everywhere, but wanted it up for discussion.
Jonathan Bedard
Comment 7 2020-01-24 11:41:20 PST
Yusuke Suzuki
Comment 8 2020-01-24 15:29:46 PST
Comment on attachment 388713 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388713&action=review r=me with nit. > Source/JavaScriptCore/runtime/MachineContext.h:60 > +#if OS(DARWIN) && !USE(PLATFORM_REGISTERS_WITH_PROFILE) && (CPU(ARM_THUMB2) || CPU(ARM) || CPU(ARM64)) > +#define USE_DARWIN_REGISTER_MACROS 1 > +#endif Let's put this in wtf/PlatformUse.h
Jonathan Bedard
Comment 9 2020-01-24 15:54:53 PST
Jonathan Bedard
Comment 10 2020-01-24 15:58:50 PST
(In reply to Jonathan Bedard from comment #9) > Created attachment 388735 [details] > Patch I intend to land this on Monday, because while it's tested, it has not been exhaustively tested on every effected configuration.
WebKit Commit Bot
Comment 11 2020-01-27 12:06:50 PST
The commit-queue encountered the following flaky tests while processing attachment 388735 [details]: editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 12 2020-01-27 12:07:28 PST
Comment on attachment 388735 [details] Patch Clearing flags on attachment: 388735 Committed r255159: <https://trac.webkit.org/changeset/255159>
WebKit Commit Bot
Comment 13 2020-01-27 12:07:29 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 14 2020-01-27 12:08:12 PST
Ryan Haddad
Comment 15 2020-01-27 13:37:28 PST
Reverted r255159 for reason: Broke the watchOS build. Committed r255169: <https://trac.webkit.org/changeset/255169>
Jonathan Bedard
Comment 16 2020-01-27 14:54:36 PST
WebKit Commit Bot
Comment 17 2020-01-27 17:30:13 PST
The commit-queue encountered the following flaky tests while processing attachment 388917 [details]: editing/spelling/spellcheck-attribute.html bug 206178 (authors: g.czajkowski@samsung.com, mark.lam@apple.com, and rniwa@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 18 2020-01-27 17:30:46 PST
Comment on attachment 388917 [details] Patch Clearing flags on attachment: 388917 Committed r255216: <https://trac.webkit.org/changeset/255216>
WebKit Commit Bot
Comment 19 2020-01-27 17:30:48 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.