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

VERIFIED WONTFIX

Status

Tamarin
Virtual Machine
P3
normal
VERIFIED WONTFIX
9 years ago
9 years ago

People

(Reporter: Brent Baker, Assigned: Brent Baker)

Tracking

unspecified
flash10.1
ARM
Windows Mobile 6 Professional
Bug Flags:
in-testsuite +
flashplayer-qrb +
flashplayer-triage +

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
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)
Flags: in-testsuite+
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
(Assignee)

Updated

9 years ago
Summary: Stack-overflow on winmo when running sunspider/s3d-cude.as with the interpreter → Injection: Stack-overflow on winmo when running sunspider/s3d-cude.as with the interpreter

Comment 1

9 years ago
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.

Updated

9 years ago
No longer blocks: 456054

Comment 2

9 years ago
Is this still reproducible?
Flags: flashplayer-triage+

Updated

9 years ago
Flags: flashplayer-triage?
(Assignee)

Comment 3

9 years ago
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
(Assignee)

Updated

9 years ago
Flags: flashplayer-triage? → flashplayer-triage+
(Assignee)

Comment 4

9 years ago
Reproduced using build 2106

Updated

9 years ago
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Flags: flashplayer-qrb? → flashplayer-qrb+
Priority: -- → P3
Target Milestone: --- → flash10.1

Comment 5

9 years ago
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

9 years ago
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

9 years ago
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.
Assignee: lhansen → brbaker
(Assignee)

Comment 8

9 years ago
Created attachment 389960 [details] [diff] [review]
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.
Attachment #389960 - Flags: review?(dschaffe)

Comment 9

9 years ago
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?
Attachment #389960 - Flags: review?(dschaffe) → review+
(Assignee)

Comment 10

9 years ago
--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
(Assignee)

Comment 11

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

2195:75c8c5e8554f
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Resolution: FIXED → WONTFIX

Updated

9 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.