Crash [@ js::ion::BaselineScript::icEntryFromPCOffset] or Assertion failure: hasBaselineScript(), at ion/BaselineInspector.h

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
x86_64
Mac OS X
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Created attachment 735451 [details]
debug and opt stacks

(function() {
    for (var x = 0; x < 9; x++) {
        for (var y = 0; y < 9; y++) {}
    }
})()

with the patch in bug 804676 comment 10, it asserts js debug shell on m-c changeset 061b9318815b with --no-baseline --no-jm and crashes js opt shell at js::ion::BaselineScript::icEntryFromPCOffset

(bhackett requested for a new bug per patch regressor of bug 804676)
(Reporter)

Comment 1

5 years ago
Highly likely to be fixed in http://hg.mozilla.org/projects/ionmonkey/rev/79f78c194329. Testcase not needed to be landed because the regressing patch is still under testing and never made it into mozilla-central.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED

Comment 2

5 years ago
> Testcase not needed to be landed because the regressing patch ... never made it 
> into mozilla-central.

I don't think that's a good reason.  "Our existing regression tests would have caught the bug" would be.  Did the patch in question pass our existing regression tests?
(Reporter)

Comment 3

5 years ago
(In reply to Jesse Ruderman from comment #2)
> > Testcase not needed to be landed because the regressing patch ... never made it 
> > into mozilla-central.
> 
> I don't think that's a good reason.  "Our existing regression tests would
> have caught the bug" would be.  Did the patch in question pass our existing
> regression tests?

Not yet presumably run. bug 804676 comment 7 mentions a run that has still some gaps. The patch is also still not yet reviewed.

Moreover, this testcase is very simple (two empty for loops, one nested in the other, within an anonymous executed function), and the fuzzers actually triggered this really easily, so we are extremely likely to catch this again.
(In reply to Jesse Ruderman from comment #2)
> I don't think that's a good reason.  "Our existing regression tests would
> have caught the bug" would be.  Did the patch in question pass our existing
> regression tests?

No, it didn't.  I just tested jit-tests with --ion before pushing, and there were a ton of failures with --no-baseline.  imo it should not be possible to turn on Ion without also turning on Baseline --- they use the same JIT infrastructure and the former needs the latter to be performant.  Running --ion --no-baseline causes crashes that no user will ever see, whereas other combinations like --ion-eager are just better at surfacing issues that users may run into.
You need to log in before you can comment on or make changes to this bug.