174515 – AirLowerAfterRegAlloc may incorrectly use a callee save that’s live as a scratch register

Bug 174515 - AirLowerAfterRegAlloc may incorrectly use a callee save that’s live as a scratch register

Summary: AirLowerAfterRegAlloc may incorrectly use a callee save that’s live as a scra...

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Saam Barati

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2017-07-14 11:15 PDT by Saam Barati
Modified:	2017-07-18 15:07 PDT (History)
CC List:	13 users (show)

See Also:

Attachments
IR dump (29.09 KB, patch) 2017-07-17 12:28 PDT, Saam Barati	no flags	Details \| Formatted Diff \| Diff
WIP (5.12 KB, patch) 2017-07-17 14:23 PDT, Saam Barati	no flags	Details \| Formatted Diff \| Diff
patch (7.97 KB, patch) 2017-07-18 11:51 PDT, Saam Barati	fpizlo: review+	Details \| Formatted Diff \| Diff
patch for landing (7.97 KB, patch) 2017-07-18 14:27 PDT, Saam Barati	saam: commit-queue-	Details \| Formatted Diff \| Diff
patch for landing (7.97 KB, patch) 2017-07-18 14:30 PDT, Saam Barati	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Saam Barati 2017-07-14 11:15:00 PDT

Termination Signal: Segmentation fault: 11
Termination Reason: Namespace SIGNAL, Code 0xb
Terminating Process: exc handler [0]
Triggered by Thread:  0

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   ???                           	0x000000013b72f30c 0 + 5292356364
1   JavaScriptCore                	0x0000000104d1bad4 0x1045e0000 + 7584468
2   JavaScriptCore                	0x0000000104d14f90 0x1045e0000 + 7557008
3   JavaScriptCore                	0x0000000104bfab18 0x1045e0000 + 6400792
4   JavaScriptCore                	0x0000000104bd1ab0 0x1045e0000 + 6232752
5   JavaScriptCore                	0x00000001048c5eec 0x1045e0000 + 3038956
6   jsc                           	0x0000000103e69530 0x103e64000 + 21808
7   jsc                           	0x0000000103e68840 0x103e64000 + 18496
8   libdyld.dylib                 	0x0000000185c96a14 0x185c95000 + 6676

Thread 0 crashed with ARM Thread State (64-bit):
    x0: 0xae81e0addb1644d1   x1: 0x0000000000000000   x2: 0x00000001049cf9c8   x3: 0x0000000000000001
    x4: 0x0000000000000002   x5: 0x000000000000003e   x6: 0x00000001046e3078   x7: 0x0000000000000003
    x8: 0x0000000000000007   x9: 0x0000000001001800  x10: 0x00000001049e2cec  x11: 0x0000000000002710
   x12: 0x0000000106809200  x13: 0x0000000105379560  x14: 0x00000001069880a0  x15: 0x000000010697c8b0
   x16: 0x0000000000000000  x17: 0x0000000000001936  x18: 0x0000000000000000  x19: 0x0000000000000001
   x20: 0x0000000000001936  x21: 0x0000000106953ea0  x22: 0x0000000000000144  x23: 0x000000010531c0e8
   x24: 0x0000000106993d20  x25: 0x0000000106800000  x26: 0x0000000106aec008  x27: 0xffff000000000000
   x28: 0xffff000000000002   fp: 0x000000016bf9aa90   lr: 0x000000013b72f4b0
    sp: 0x000000016bf9a9e0   pc: 0x000000013b72f30c cpsr: 0x80000000

Comment 1 Saam Barati 2017-07-17 12:25:16 PDT

The bug looks to be in Air::lowerAfterRegAlloc. The IR is sound before this, but after this, this phase clobbers a register it shouldn't.

Comment 2 Saam Barati 2017-07-17 12:28:00 PDT

Created attachment 315693 [details]
IR dump

look at defs of r19. It looks like lowering of ColdCCall introduced the bug.

Comment 3 Radar WebKit Bug Importer 2017-07-17 13:09:49 PDT

<rdar://problem/33358092>

Comment 4 Saam Barati 2017-07-17 14:23:52 PDT

Created attachment 315712 [details]
WIP

trying to write a test.

Comment 5 Saam Barati 2017-07-18 11:51:32 PDT

Created attachment 315810 [details]
patch

Comment 6 Build Bot 2017-07-18 11:54:02 PDT

Attachment 315810 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/testb3.cpp:15631:  Extra space before )  [whitespace/parens] [2]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:15631:  Missing space inside { }.  [whitespace/braces] [5]
ERROR: Source/JavaScriptCore/b3/testb3.cpp:15638:  Consider using CHECK_EQ instead of CHECK(a == b)  [readability/check] [2]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 Mark Lam 2017-07-18 13:28:53 PDT

Comment on attachment 315810 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315810&action=review

> Source/JavaScriptCore/ChangeLog:3
> +        AirLowerAfterRegAlloc may incorrectly use a callee save thatâs live as a scratch register

non-ascii character.

Comment 8 Saam Barati 2017-07-18 14:27:27 PDT

Created attachment 315839 [details]
patch for landing

Comment 9 Saam Barati 2017-07-18 14:30:22 PDT

Created attachment 315840 [details]
patch for landing

Comment 10 WebKit Commit Bot 2017-07-18 15:07:46 PDT

Comment on attachment 315840 [details]
patch for landing

Clearing flags on attachment: 315840

Committed r219633: <http://trac.webkit.org/changeset/219633>

Comment 11 WebKit Commit Bot 2017-07-18 15:07:48 PDT

All reviewed patches have been landed.  Closing bug.