496942 - Injection: Stack-overflow on winmo when running sunspider/s3d-cude.as with the interpreter

Status: VERIFIED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect, P3)

Description

[Brent Baker]

The stack overflow does not happen in hybrid or jit mode. 

All version of the s3d-cube produce the stack-overflow (as3/as3vector/default).

This injection happened in changeset 1793:45a3367b83b4
http://hg.mozilla.org/tamarin-redux/rev/45a3367b83b4
Fix 456054 - TC uses fixed 512K stack for all platforms except windows (r+edwsmith)

Comment 1

[Lars T Hansen]

I see that patch increased the stack headroom on Windows Mobile from 16KB to 32KB (to correspond with other embedded platforms).  That was deliberate.  However, since the stack on Windows Mobile is supposedly tiny (64KB is a number that's thrown around a lot) the 32KB headroom may be too aggressive as it leaves only 32KB of stack.  Yet 16KB of headroom is a little too tight for comfort - or at least it was, until we throttled the GC recursion.

The "right" fix here is not obviously to revert to 16KB headroom "because it's been that way" but to consider whether we should move to a stack-in-heap strategy instead, to allow us to use larger stacks.

Comment 2

[Dan Smith]

Is this still reproducible?

Comment 3

[Brent Baker]

This is reproducible using the HTC Fuze device when running the test with the interpreter, jit and hybrid modes are ok.

cd test/performance
HYBRID MODE
$ ../../build/buildbot/slaves/all/tools/ceremoteshell.exe sunspider/s3d-cube.abc
metric time 7000

JIT MODE
$ ../../build/buildbot/slaves/all/tools/ceremoteshell.exe -Ojit sunspider/s3d-cube.abc
metric time 7000

INTERP MODE
$ ../../build/buildbot/slaves/all/tools/ceremoteshell.exe -Dinterp sunspider/s3d-cube.abc
Error: Error #1023

Comment 4

[Brent Baker]

Reproduced using build 2106

Comment 5

[Lars T Hansen]

Fact of life if you ask me, see my earlier comments.

We need a higher-level decision about whether to implement a stack-in-heap strategy for the avmshell.  If not then this is a clear WONTFIX.  If so, we need a different bug to track that work item.

Comment 6

[Edwin Smith]

I don't think the cost of implementing stack-in-heap in the shell is justified, just for this bug.  suggest we mark this WONTFIX and exclude s3d-cube from winmo performance testing (only in -Dinterp)

Comment 7

[Trevor Baker]

Won't fix.  reassigning to brent to deal with the skip/expectedfailure/deep6 of the test for interp.  Please mark resolved/wontfix once your finished.

Comment 8

[Brent Baker]

Attachment: Exclude .*/s3d-cube.as when running interp perf on mobile

Needed to pass any vmargs that are being used when setting the --config with *-mobile so that we can specifically skip the test when the test is being run in interp mode.

Comment 9

[Dan Schaffer]

Comment on attachment 389960 [details] [diff] [review]
Exclude .*/s3d-cube.as when running interp perf on mobile

in acceptance test suite the --vmargs are added to the config string.  in performance this does not happen?

Comment 10

[Brent Baker]

--vmargs are only added if the --config string is calculated, the handful of performance runs that use the *-mobile are passing in the --config string so that bypasses the code that would have added the --vmargs

Comment 11

[Brent Baker]

Pushed patch to skip .*/s3d-cube.as when running interp perf on mobile

2195:75c8c5e8554f