Bug 160438 – REGRESSION (r203990): JSC Debug test stress/arity-check-ftl-throw.js failing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

Running stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool
stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool: ASSERTION FAILED: exec == vm.topCallFrame || exec == exec->lexicalGlobalObject()->globalExec() || exec == exec->vmEntryGlobalObject()->globalExec()
stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool: /Volumes/Data/slave/yosemite-debug/build/Source/JavaScriptCore/runtime/Error.cpp(144) : bool JSC::addErrorInfoAndGetBytecodeOffset(JSC::ExecState *, JSC::VM &, JSC::JSObject *, bool, CallFrame *&, unsigned int *)
stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool: test_script_1195: line 2: 70136 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --jitMemoryReservationSize\=50000 --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 arity-check-ftl-throw-more-args.js )
stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool
Running stress/arity-check-ftl-throw.js.default
Running stress/arity-check-ftl-throw.js.always-trigger-copy-phase
stress/arity-check-ftl-throw.js.default: test_script_1196: line 2: 70150 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.default: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.default
Running stress/arity-check-ftl-throw.js.no-llint
Running stress/arity-check-ftl-throw.js.no-cjit-validate-phases
Running stress/arity-check-ftl-throw.js.dfg-eager
Running stress/arity-check-ftl-throw.js.dfg-eager-no-cjit-validate
Running stress/arity-check-ftl-throw.js.dfg-maximal-flush-validate-no-cjit
Running stress/arity-check-ftl-throw.js.no-ftl
Running stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler
Running stress/arity-check-ftl-throw.js.ftl-no-cjit-no-put-stack-validate
stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler: test_script_1204: line 2: 70226 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --validateGraph\=true --useSamplingProfiler\=true --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler
Running stress/arity-check-ftl-throw.js.ftl-no-cjit-no-inline-validate
Running stress/arity-check-ftl-throw.js.ftl-eager
stress/arity-check-ftl-throw.js.ftl-no-cjit-no-put-stack-validate: test_script_1205: line 2: 70234 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --validateGraph\=true --usePutStackSinking\=false --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-no-cjit-no-put-stack-validate: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-no-cjit-no-put-stack-validate
Running stress/arity-check-ftl-throw.js.ftl-eager-no-cjit
stress/arity-check-ftl-throw.js.ftl-no-cjit-no-inline-validate: test_script_1206: line 2: 70248 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --validateGraph\=true --maximumInliningDepth\=1 --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-no-cjit-no-inline-validate: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-no-cjit-no-inline-validate
Running stress/arity-check-ftl-throw.js.ftl-no-cjit-small-pool
stress/arity-check-ftl-throw.js.ftl-eager: test_script_1207: line 2: 70257 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true --thresholdForJITAfterWarmUp\=10 --thresholdForJITSoon\=10 --thresholdForOptimizeAfterWarmUp\=20 --thresholdForOptimizeAfterLongWarmUp\=20 --thresholdForOptimizeSoon\=20 --thresholdForFTLOptimizeAfterWarmUp\=20 --thresholdForFTLOptimizeSoon\=20 --maximumEvalCacheableSourceLength\=150000 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-eager: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-eager
Running stress/array-concat-spread-object.js.default
stress/arity-check-ftl-throw.js.ftl-eager-no-cjit: test_script_1208: line 2: 70270 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --validateGraph\=true --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 --thresholdForJITAfterWarmUp\=10 --thresholdForJITSoon\=10 --thresholdForOptimizeAfterWarmUp\=20 --thresholdForOptimizeAfterLongWarmUp\=20 --thresholdForOptimizeSoon\=20 --thresholdForFTLOptimizeAfterWarmUp\=20 --thresholdForFTLOptimizeSoon\=20 --maximumEvalCacheableSourceLength\=150000 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-eager-no-cjit: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-eager-no-cjit
Running stress/array-concat-spread-object.js.always-trigger-copy-phase
stress/arity-check-ftl-throw.js.ftl-no-cjit-small-pool: test_script_1209: line 2: 70283 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --maxPerThreadStackUsage\=1572864 --jitMemoryReservationSize\=50000 --useFTLJIT\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 arity-check-ftl-throw.js )
stress/arity-check-ftl-throw.js.ftl-no-cjit-small-pool: ERROR: Unexpected exit code: 139
FAIL: stress/arity-check-ftl-throw.js.ftl-no-cjit-small-pool

** The following JSC stress test failures have been introduced:
	stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool
	stress/arity-check-ftl-throw.js.default
	stress/arity-check-ftl-throw.js.ftl-eager
	stress/arity-check-ftl-throw.js.ftl-eager-no-cjit
	stress/arity-check-ftl-throw.js.ftl-no-cjit-no-inline-validate
	stress/arity-check-ftl-throw.js.ftl-no-cjit-no-put-stack-validate
	stress/arity-check-ftl-throw.js.ftl-no-cjit-small-pool
	stress/arity-check-ftl-throw.js.ftl-no-cjit-validate-sampling-profiler

[Ryan Haddad]

https://build.webkit.org/builders/Apple%20Yosemite%20Debug%20JSC%20%28Tests%29/builds/6592

[Ryan Haddad]

Created attachment 285063 [details]
Crashlog

Crashlog from the assertion failure during stress/arity-check-ftl-throw-more-args.js.ftl-no-cjit-small-pool

[Filip Pizlo]

I'm looking at this now.

[Filip Pizlo]

I think I have a fix.  These were all latent bugs, that were masked by the fact that our varargs stack overflow handling basically never triggered.

[Filip Pizlo]

Created attachment 285065 [details]
the patch

[Filip Pizlo]

Comment on attachment 285065 [details]
the patch

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

> Source/JavaScriptCore/interpreter/StackVisitor.cpp:47
> +        while (static_cast<void*>(m_frame.m_VMEntryFrame) == static_cast<void*>(topFrame)) {

This should be an 'if'.

[Mark Lam]

Comment on attachment 285065 [details]
the patch

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

> Source/JavaScriptCore/ChangeLog:29
> +        We mad to ShadowChicken processing, which invokes StackVisitor, when we have topCallFrame

"mad to"?

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:185
> +        NativeCallFrameTracer subTracer(&vm, exec);

This doesn't hurt, but I don't think this should have made a difference because CommonSlowPaths::interpreterThrowInCaller() will create a NativeCallFrameTracer() on the exec.

> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:199
> +        NativeCallFrameTracer subTracer(&vm, exec);

Ditto.

[Filip Pizlo]

(In reply to comment #7)
> Comment on attachment 285065 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285065&action=review
> 
> > Source/JavaScriptCore/ChangeLog:29
> > +        We mad to ShadowChicken processing, which invokes StackVisitor, when we have topCallFrame
> 
> "mad to"?
> 
> > Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:185
> > +        NativeCallFrameTracer subTracer(&vm, exec);
> 
> This doesn't hurt, but I don't think this should have made a difference
> because CommonSlowPaths::interpreterThrowInCaller() will create a
> NativeCallFrameTracer() on the exec.

It makes a huge difference.  Before we get to internreterThrowInCaller(), we do createStackOverflowError() or whatever.  That's where we were dying.

> 
> > Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:199
> > +        NativeCallFrameTracer subTracer(&vm, exec);
> 
> Ditto.

See above.

[Mark Lam]

Comment on attachment 285065 [details]
the patch

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

>>> Source/JavaScriptCore/ChangeLog:29
>>> +        We mad to ShadowChicken processing, which invokes StackVisitor, when we have topCallFrame
>> 
>> "mad to"?
> 
> It makes a huge difference.  Before we get to internreterThrowInCaller(), we do createStackOverflowError() or whatever.  That's where we were dying.

Ahh.  I see (re CommonSlowPaths.cpp).

FYI, you have a typo in the ChangeLog here.  "mad to"?

[Filip Pizlo]

(In reply to comment #9)
> Comment on attachment 285065 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285065&action=review
> 
> >>> Source/JavaScriptCore/ChangeLog:29
> >>> +        We mad to ShadowChicken processing, which invokes StackVisitor, when we have topCallFrame
> >> 
> >> "mad to"?
> > 
> > It makes a huge difference.  Before we get to internreterThrowInCaller(), we do createStackOverflowError() or whatever.  That's where we were dying.
> 
> Ahh.  I see (re CommonSlowPaths.cpp).
> 
> FYI, you have a typo in the ChangeLog here.  "mad to"?

I fixed it!  It was meant to be "may do".

[Build Bot]

Comment on attachment 285065 [details]
the patch

Attachment 285065 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1796110

New failing tests:
inspector/debugger/breakpoint-syntax-error-top-level.html

[Build Bot]

Created attachment 285068 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 285065 [details]
the patch

Attachment 285065 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1796116

New failing tests:
inspector/debugger/breakpoint-syntax-error-top-level.html

[Build Bot]

Created attachment 285069 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Filip Pizlo]

Created attachment 285070 [details]
better patch

I forgot a null check in that last one.

[Mark Lam]

Comment on attachment 285070 [details]
better patch

r=me

[Filip Pizlo]

Created attachment 285072 [details]
patch for landing

[Filip Pizlo]

Created attachment 285073 [details]
for real this time

Fix an assertion after addressing the NativeCallFrameTracer assertion feedback from Saam

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/204010