Closed Bug 877936 Opened 6 years ago Closed 6 years ago

Assertion failure: CountArgSlots(script, fun) < SNAPSHOT_MAX_NARGS, at ion/Snapshots.cpp:301

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla27
Tracking Status
firefox25 --- affected

People

(Reporter: decoder, Assigned: h4writer)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files, 1 obsolete file)

The following testcase asserts on mozilla-central revision f66d956d188e (run with --ion-eager):


rex = RegExp("()()()()()()()()()()(z)?(y)");
a = ["sub"];
a[230] = '' + "a"
f = Function.apply(null, a);
"xyz".replace(rex, f);
This affects at least beta by now. Can someone look at this?
There is a limit SNAPSHOT_MAX_NARGS and we check for this. Upon deciding if we want to compile a script we check that the fun->nargs are lower than this limit. 
Now the assert checks if CountArgSlots is lower than that limit, but this adds "this" value, "scopechain" value, "return" value and sometimes argumentsobject value. So there is a decrepancy of 3-4. Or we need to lower the check in Ion.cpp, before compiling. Or we need to only check fun->nargs, instead of CountArgSlots().
This off course depends on the fact if this limit has a reason or is arbitrarely taken.

Looking...
1) Increases the limit +4 (for this, return value, argumentsobj ...)
2) Checks for fun args and limits us too (Since we fill/padd args with undefined, this is also a way to go over the limit)
3) Add checks during inlining
Assignee: general → hv1989
Attachment #821995 - Flags: review?(dvander)
Comment on attachment 821995 [details] [diff] [review]
bug877936-callargs

Review of attachment 821995 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/Snapshots.cpp
@@ +300,5 @@
>  
>  void
>  SnapshotWriter::startFrame(JSFunction *fun, JSScript *script, jsbytecode *pc, uint32_t exprStack)
>  {
> +    JS_ASSERT(CountArgSlots(script, fun) < SNAPSHOT_MAX_NARGS + 4);

nit: comment explaining the magic 4
Attachment #821995 - Flags: review?(dvander) → review+
We already missed beta, though there is no security issue by not uplifting. So I'll leave that.
I can uplift for aurora (very easy patch), but I don't think it is needed...

@dvander: thanks for quick review
https://hg.mozilla.org/integration/mozilla-inbound/rev/86650bc9c33d
https://hg.mozilla.org/mozilla-central/rev/86650bc9c33d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Attachment #756297 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.