Bug 169436

Summary:

[JSC][Linux] Implement VMTrap in Linux ports

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

JavaScriptCore

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, commit-queue, dbates, keith_miller, mark.lam, msaboff, ossy, saam

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

168920

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	mark.lam: review+

Yusuke Suzuki

Reported 2017-03-09 13:13:59 PST

Let's do that!

Attachments
Patch (44.18 KB, patch) 2017-03-11 01:11 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (44.20 KB, patch) 2017-03-11 01:20 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (44.23 KB, patch) 2017-03-11 01:28 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.04 KB, patch) 2017-03-11 01:46 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.04 KB, patch) 2017-03-11 01:47 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.04 KB, patch) 2017-03-11 01:52 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.03 KB, patch) 2017-03-11 01:59 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.20 KB, patch) 2017-03-11 02:10 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.20 KB, patch) 2017-03-11 07:59 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (47.24 KB, patch) 2017-03-11 09:01 PST, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (48.56 KB, patch) 2017-03-13 10:49 PDT, Yusuke Suzuki	mark.lam: review+	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2017-03-11 01:11:40 PST

Created attachment 304144 [details] Patch

Yusuke Suzuki

Comment 2 2017-03-11 01:20:33 PST

Created attachment 304145 [details] Patch

Yusuke Suzuki

Comment 3 2017-03-11 01:28:29 PST

Created attachment 304146 [details] Patch

Yusuke Suzuki

Comment 4 2017-03-11 01:46:15 PST

Created attachment 304147 [details] Patch

Yusuke Suzuki

Comment 5 2017-03-11 01:47:50 PST

Created attachment 304148 [details] Patch

Yusuke Suzuki

Comment 6 2017-03-11 01:52:38 PST

Created attachment 304149 [details] Patch

Yusuke Suzuki

Comment 7 2017-03-11 01:59:17 PST

Created attachment 304150 [details] Patch

Yusuke Suzuki

Comment 8 2017-03-11 02:10:10 PST

Created attachment 304151 [details] Patch

Yusuke Suzuki

Comment 9 2017-03-11 07:59:11 PST

Created attachment 304162 [details] Patch

Yusuke Suzuki

Comment 10 2017-03-11 09:01:38 PST

Created attachment 304163 [details] Patch

Yusuke Suzuki

Comment 11 2017-03-13 10:49:38 PDT

Created attachment 304275 [details] Patch

Mark Lam

Comment 12 2017-03-13 16:25:08 PDT

Comment on attachment 304275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304275&action=review Thanks for the refactoring. It makes the code looks so much cleaner. r=me with some suggested changes. > Source/JavaScriptCore/runtime/MachineContext.h:103 > +#else // !OS(DARWIN) nit: using OS(WINDOWS) here would be more informative. > Source/JavaScriptCore/runtime/MachineContext.h:206 > +#else // !OS(DARWIN) ditto: #elif OS(WINDOWS) > Source/JavaScriptCore/runtime/MachineContext.h:307 > +#else // !OS(DARWIN) ditto: #elif OS(WINDOWS) > Source/JavaScriptCore/runtime/MachineContext.h:416 > +#else // !OS(DARWIN) ditto: #elif OS(WINDOWS) > Source/JavaScriptCore/runtime/MachineContext.h:478 > + return reinterpret_cast<void*&>((uintptr_t&) machineContext.gregs[5]); How did you come up with gregs[5]? The rest of the register values came from the mapping for regT1. On MIPS, regT1 is MIPSRegisters::v1. Does MIPSRegisters::v1 map to gregs[5]? > Source/JavaScriptCore/runtime/MachineContext.h:532 > +#else // !OS(DARWIN) ditto: #elif OS(WINDOWS)

Yusuke Suzuki

Comment 13 2017-03-14 00:31:05 PDT

Comment on attachment 304275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304275&action=review Thanks! >> Source/JavaScriptCore/runtime/MachineContext.h:103 >> +#else // !OS(DARWIN) > > nit: using OS(WINDOWS) here would be more informative. Fixed. >> Source/JavaScriptCore/runtime/MachineContext.h:206 >> +#else // !OS(DARWIN) > > ditto: #elif OS(WINDOWS) Fixed. >> Source/JavaScriptCore/runtime/MachineContext.h:307 >> +#else // !OS(DARWIN) > > ditto: #elif OS(WINDOWS) Fixed. >> Source/JavaScriptCore/runtime/MachineContext.h:416 >> +#else // !OS(DARWIN) > > ditto: #elif OS(WINDOWS) Fixed. >> Source/JavaScriptCore/runtime/MachineContext.h:478 >> + return reinterpret_cast<void*&>((uintptr_t&) machineContext.gregs[5]); > > How did you come up with gregs[5]? The rest of the register values came from the mapping for regT1. On MIPS, regT1 is MIPSRegisters::v1. Does MIPSRegisters::v1 map to gregs[5]? This is GPRInfo::argument1 register. It is mapped to MIPSRegister::a1. In MIPS, registers are indexed as, zero = 0 at = 1 v0 = 2 v1 = 3 a0 = 4 a1 = 5 This is why we specify 5 here. >> Source/JavaScriptCore/runtime/MachineContext.h:532 >> +#else // !OS(DARWIN) > > ditto: #elif OS(WINDOWS) Fixed.

Yusuke Suzuki

Comment 14 2017-03-14 00:33:16 PDT

Committed r213886: <http://trac.webkit.org/changeset/213886>

Csaba Osztrogonác

Comment 15 2017-03-14 08:31:15 PDT

(In reply to comment #14) > Committed r213886: <http://trac.webkit.org/changeset/213886> It broke the CLoop build on El Capitan: https://build.webkit.org/builders/Apple%20El%20Capitan%20LLINT%20CLoop%20%28BuildAndTest%29/builds/15032

Mark Lam

Comment 16 2017-03-14 10:02:58 PDT

(In reply to comment #15) > (In reply to comment #14) > > Committed r213886: <http://trac.webkit.org/changeset/213886> > > It broke the CLoop build on El Capitan: > https://build.webkit.org/builders/ > Apple%20El%20Capitan%20LLINT%20CLoop%20%28BuildAndTest%29/builds/15032 Speculative build fix for this landed in r213904: <http://trac.webkit.org/r213904>.

Note You need to log in before you can comment on or make changes to this bug.