Closed Bug 791589 Opened 7 years ago Closed 7 years ago

IonMonkey: BraidJump crash: visitApplyArgsGeneric() doesn't use ImmGCPtr for ArgumentsRectifier

Categories

(Core :: JavaScript Engine, defect, critical)

18 Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: bhearsum, Assigned: sstangl)

References

()

Details

(Keywords: crash, regression, Whiteboard: [ion:p1:fx18])

Crash Data

Attachments

(1 file)

Twice today I was playing the game in the linked URL and Firefox crashed. I've played this game tons of times in the past 6 months and never had an issue. Strangely, my crash signatures are very different:
https://crash-stats.mozilla.com/report/index/bp-49ebfb52-0b35-4efc-842f-b731e2120916
https://crash-stats.mozilla.com/report/index/bp-dd70542a-9121-4f4b-818c-cda722120916

I was running a build of http://hg.mozilla.org/mozilla-central/rev/f7c89de3ab43 at the time.

Filed under JS because one of the crashes is in JS, and IonMonkey very recently landed on Nightly.
There is only one crash of each signature.
Do you have reliable STR? In that case, does it happen in Safe Mode?
Severity: normal → critical
Crash Signature: [@ js::ion::InvokeFunction]
Keywords: crash, regression
Version: 14 Branch → 18 Branch
(In reply to Ben Hearsum [:bhearsum] from comment #0)
> Twice today I was playing the game in the linked URL and Firefox crashed.
> I've played this game tons of times in the past 6 months and never had an
> issue.
> 
> […]
> 
> Filed under JS because one of the crashes is in JS, and IonMonkey very
> recently landed on Nightly.

I would be interested if you can get a bigger backtrace (with the DOM crash) by going into about:config and enabling "profiler.enabled", this would slow down a bit the JS engine but should provide some mechanism which hopefully should help the crash reporter to provide a better backtrace.  If you don't want to get many slow down caused by the sampling profiler, you can increase the period of the sampling in "profiler.interval".
Summary: crash while playing http://www.syngames.net/game/braidjump → IonMonkey: crash while playing http://www.syngames.net/game/braidjump
I'll see if I can repro with the profiler on. Anything special I have to do to get the stack afterwards?
(In reply to Ben Hearsum [:bhearsum] from comment #3)
> I'll see if I can repro with the profiler on. Anything special I have to do
> to get the stack afterwards?

No, hopefully, the crash report should be able to walk the stack and follow the IonMonkey stack frames.  I don't think we have any instrumentation to use SPS stack to provide useful JS locations yet, but we should look into it if this works.
Ben, how easy is it for you to reproduce this? (I'm trying to crash the game on my Mac but it seems to work.)

It might be worth re-testing after bug 790464 lands.
Whiteboard: [ion:p1:fx18]
I can repro nearly at will. It takes about 30s to 2min on my Linux machine to crash. I just crashed with the profiler enabled, in fact: https://crash-stats.mozilla.com/report/index/36b21922-9e2c-4a18-9a8d-21e1d2120917
I can reproduce this with the latest nightly.
Appears to be fixed by Bug 779411 -- with its patch applied, the game runs fine locally. Leaving unresolved so Ben can verify after Bug 779411 is fixed.
(In reply to Ben Hearsum [:bhearsum] from comment #6)
> I can repro nearly at will. It takes about 30s to 2min on my Linux machine
> to crash. I just crashed with the profiler enabled, in fact:
> https://crash-stats.mozilla.com/report/index/36b21922-9e2c-4a18-9a8d-
> 21e1d2120917

Thanks for testing with the profiler, it did not work as I expected. You can restore the default parameters. I haven't been able to reproduce it too.
I'm still crashing even thought bug 790464/779411 are fixed. https://crash-stats.mozilla.com/report/index/bp-540393f9-6c70-40ee-ad75-dca412120919 is my most recent one, with a useless stack =\.
I'm not able to reproduce it locally. Are there any STR other than playing the game for a while?
Reproduced with a debug build! We're in business.
The ArgumentsRectifier is being GC'd because visitApplyArgsGeneric() uses an ImmWord() to move the address, not a markable ImmGCPtr(). We then run off into the weeds and execute a ton of bogus random instructions, then maybe crash. This is actually s-s, but it's fixed now.

Giving review to Waldo because the fix is simple, other people are suspiciously absent, and I want this in-tree asap.
Attachment #662712 - Flags: review?(jwalden+bmo)
Attachment #662712 - Flags: review?(jwalden+bmo) → review?(mrosenberg)
Comment on attachment 662712 [details] [diff] [review]
Mark ArgumentsRectifier in visitApplyArgsGeneric().

I assume that since this is in the argumentsrectifier, even though we're doing more memory access, it has 0 impact on perf.
Attachment #662712 - Flags: review?(mrosenberg) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/99a42cef003f

Thanks, Marty and Ben. Yeah, this will have no effect on performance. Verified in-browser that braidjump runs with a debug build without dying with this patch applied. There might be other bugs that aren't cropping up for me, but if there are, we can open separate bugs for them.
Summary: IonMonkey: crash while playing http://www.syngames.net/game/braidjump → IonMonkey: BraidJump crash: visitApplyArgsGeneric() doesn't use ImmGCPtr for ArgumentsRectifier
Assignee: general → sstangl
https://hg.mozilla.org/mozilla-central/rev/99a42cef003f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.