Bug 160124 – [JSC] Adjust SP first before performing vararg setup, fix test stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler crashing on GTK bot

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Catanzaro]

Layout test stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler is crashing on the GTK release bot:

 4235/36510 ........... 
 4235/36510 ...........
 4235/36510 ............
                         
stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler: Segmentation fault (core dumped)

 4235/36510 ............
                         
stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler: ERROR: Unexpected exit code: 139

 4235/36510 ............

Further investigation required to get a stack trace.

[Michael Catanzaro]

So, unless we have some mechanism I'm unaware of to skip JSC tests, I think we should remove this one until it can be fixed.

Note: the output is a bit different now:

stress/spread-forward-call-varargs-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler: Exception: Error: Bad assertion

 6257/40795 ............
                         
stress/spread-forward-call-varargs-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler: [e-mail]:3:24

 6257/40795 ............
                         
stress/spread-forward-call-varargs-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler: global [e-mail]:40:15

 6257/40795 ............
                         
stress/spread-forward-call-varargs-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler: ERROR: Unexpected exit code: 3

[Carlos Garcia Campos]

I doesn't always crash, though. It happens quite often, but sometimes it passes.

[Carlos Alberto Lopez Perez]

The title and comment 1 on this bug talk about stress/arity-check-ftl-throw.js
Comment 2 talks about stress/spread-forward-call-varargs-stack-overflow.js


I don't see stress/arity-check-ftl-throw.js failing often lately.

But stress/spread-forward-call-varargs-stack-overflow.js fails a lot. I skipped it on bug 169206


So.. lets keep this bug for tracking stress/arity-check-ftl-throw.js and eventually skip it, if we detect it fails too often

[Yusuke Suzuki]

Created attachment 303656 [details]
Patch

WIP: Investigating...

[Yusuke Suzuki]

I think I found the issue and this patch solves it.
But it is super ad-hoc & nasty patch.
I'll read the code more and upload the solid patch later.

Good news: at least, this uploaded patch makes GTK JSC 64bit test 0 failures!

[Carlos Garcia Campos]

(In reply to comment #5)
> I think I found the issue and this patch solves it.
> But it is super ad-hoc & nasty patch.
> I'll read the code more and upload the solid patch later.
> 
> Good news: at least, this uploaded patch makes GTK JSC 64bit test 0 failures!

Thank you very much Yusuke!

[Yusuke Suzuki]

Created attachment 303808 [details]
Patch

[Mark Lam]

Comment on attachment 303808 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:11
> +        If we do not that, OS can break the values that is stored beyond the stack

/If we do not that, OS/If we don't do that, the OS/

[Yusuke Suzuki]

Comment on attachment 303808 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/ChangeLog:11
>> +        If we do not that, OS can break the values that is stored beyond the stack
> 
> /If we do not that, OS/If we don't do that, the OS/

Fixed.

[Yusuke Suzuki]

Created attachment 303897 [details]
Patch for landing

[Yusuke Suzuki]

Committed r213631: <http://trac.webkit.org/changeset/213631>

[Yusuke Suzuki]

(In reply to comment #11)
> Committed r213631: <http://trac.webkit.org/changeset/213631>

Oops, when using Tools/Scripts/webkit-patch apply-from-bug ID, the old patch is applied...