Bug 168502 – REGRESSION(r212778): It made 400 tests crash on AArch64 Linux

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Csaba Osztrogonác]

before: https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4617 (r212462)
after: https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4618 (r212482)

I bisected manually, and https://trac.webkit.org/changeset/212466 is the culprit.
r212465 - https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4627
r212466 - https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release/builds/4626

[Csaba Osztrogonác]

Haven't you noticed this regression on internal iOS JSC tester bots too?

[Filip Pizlo]

Passed tests just fine on my iPad. 

Idea: what happens when you make Linux take the same path as Windows in RegisterStae.h?  Basically make it fall through to the setjmp code.

[Filip Pizlo]

Did you get a stack trace?

[Csaba Osztrogonác]

r212466 was rolled out and relanded in https://trac.webkit.org/changeset/212778
Now we have only 400 crashes on AArch64 Linux. 

(In reply to comment #2)
> Passed tests just fine on my iPad. 
> 
> Idea: what happens when you make Linux take the same path as Windows in
> RegisterStae.h?  Basically make it fall through to the setjmp code.
Will check soon.

(In reply to comment #3)
> Did you get a stack trace?
Not yet, I'm going to do a debug build in the near future.

[Csaba Osztrogonác]

(In reply to comment #2)
> Idea: what happens when you make Linux take the same path as Windows in
> RegisterStae.h?  Basically make it fall through to the setjmp code.

This idea fixed this issue. What does it mean?


(In reply to comment #3)
> Did you get a stack trace?

Unfortunately there are no crashes in debug mode.

[Csaba Osztrogonác]

(In reply to comment #5)
> (In reply to comment #2)
> > Idea: what happens when you make Linux take the same path as Windows in
> > RegisterStae.h?  Basically make it fall through to the setjmp code.
> 
> This idea fixed this issue. What does it mean?
> 
> 
> (In reply to comment #3)
> > Did you get a stack trace?
> 
> Unfortunately there are no crashes in debug mode.

AArch64 Linux platform is still broken due to this GC bug.
Any plan to fix it or should we use the setjmp codepath?

[Filip Pizlo]

(In reply to comment #5)
> (In reply to comment #2)
> > Idea: what happens when you make Linux take the same path as Windows in
> > RegisterStae.h?  Basically make it fall through to the setjmp code.
> 
> This idea fixed this issue. What does it mean?

It means that this is the fix to this issue. From what I remember setjmp is very fast on Linux so this should be fine. 

> 
> 
> (In reply to comment #3)
> > Did you get a stack trace?
> 
> Unfortunately there are no crashes in debug mode.

[Csaba Osztrogonác]

Created attachment 303550 [details]
Patch

[Filip Pizlo]

Comment on attachment 303550 [details]
Patch

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

> Source/JavaScriptCore/heap/RegisterState.h:32
> -#if !OS(WINDOWS)
> +#if !OS(WINDOWS) && !(OS(LINUX) && CPU(ARM64))

This is good.

FWIW, it's probably also a bit safer to use setjmp on all Linuxes.

I've written this kind of stuff on Linux before and always found that setjmp was the canonical Linux way of getting register state.

The only reason why we now do it differently is that Darwin's setjmp does some extra state saving that makes it just a tad bit too slow.  I don't think Linux's setjmp has that kind of problem on any architecture.  It would even be fine to change this to OS(DARWIN) rather than !OS(WINDOWS) && !OS(LINUX).

If you want to make any of those changes then lets do it in another patch.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 303550 [details]:

media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html bug 169158 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 303550 [details]:

media/modern-media-controls/tracks-support/tracks-support-click-track-in-panel.html bug 169159 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 303550 [details]
Patch

Clearing flags on attachment: 303550

Committed r213472: <http://trac.webkit.org/changeset/213472>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.