Bug 182157

Summary:

[YarrJIT][ARM] We need to save r8 as it is the initial start register

Product:

WebKit

Reporter:

Guillaume Emont <guijemont>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dominik.infuehr, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Other

OS:

Linux

Attachments:

Description	Flags
Patch	none
Patch	none

Guillaume Emont

Reported 2018-01-25 16:28:13 PST

We still need to keep on saving r6 as it is used by the MacroAssembler, which we use (we get crashes in some situations otherwise). This issue was discovered because stress/regress-174044.js crashes on a raspberry pi 2 when compiled in -O2.

Attachments
Patch (1.84 KB, patch) 2018-01-25 16:32 PST, Guillaume Emont	no flags	Details Formatted Diff Diff
Patch (2.01 KB, patch) 2018-02-13 13:34 PST, Guillaume Emont	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Guillaume Emont

Comment 1 2018-01-25 16:32:25 PST

Created attachment 332334 [details] Patch Patch fixing the issue.

Saam Barati

Comment 2 2018-01-28 23:56:09 PST

Comment on attachment 332334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332334&action=review > Source/JavaScriptCore/ChangeLog:8 > + We still need to keep on saving r6 as it is used by the MacroAssembler you mean r8, not r6?

Guillaume Emont

Comment 3 2018-01-29 07:42:32 PST

Comment on attachment 332334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332334&action=review >> Source/JavaScriptCore/ChangeLog:8 >> + We still need to keep on saving r6 as it is used by the MacroAssembler > > you mean r8, not r6? Sorry, I meant that in addition to saving $r8, as the title suggest, we can't stop saving $r6, as I initially wrongly deducted in a first version of the patch that was creating weird crashes :). My initial wrong deduction was because the initial register used to be $r6 before #182157.

Saam Barati

Comment 4 2018-01-29 11:02:39 PST

Comment on attachment 332334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332334&action=review >>> Source/JavaScriptCore/ChangeLog:8 >>> + We still need to keep on saving r6 as it is used by the MacroAssembler >> >> you mean r8, not r6? > > Sorry, I meant that in addition to saving $r8, as the title suggest, we can't stop saving $r6, as I initially wrongly deducted in a first version of the patch that was creating weird crashes :). My initial wrong deduction was because the initial register used to be $r6 before #182157. Probably worth rewording this to reflect what the patch is actually doing.

Guillaume Emont

Comment 5 2018-02-13 13:34:25 PST

Created attachment 333719 [details] Patch New patch with a more explicit ChangeLog message. Sorry it took me a while to get to it.

WebKit Commit Bot

Comment 6 2018-02-13 14:43:20 PST

Comment on attachment 333719 [details] Patch Clearing flags on attachment: 333719 Committed r228436: <https://trac.webkit.org/changeset/228436>

WebKit Commit Bot

Comment 7 2018-02-13 14:43:22 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 8 2018-02-13 14:44:18 PST

<rdar://problem/37514115>

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