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

RESOLVED FIXED in mozilla27

Status

()

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

People

(Reporter: decoder, Assigned: h4writer)

Tracking

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

Trunk
mozilla27
x86_64
Linux
assertion, testcase
Points:
---

Firefox Tracking Flags

(firefox25 affected)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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);
(Reporter)

Comment 1

5 years ago
Created attachment 756297 [details]
[crash-signature] Machine-readable crash signature
(Reporter)

Comment 2

4 years ago
This affects at least beta by now. Can someone look at this?
status-firefox25: --- → affected
(Assignee)

Comment 3

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

Comment 4

4 years ago
Created attachment 821995 [details] [diff] [review]
bug877936-callargs

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+
(Assignee)

Comment 6

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
(Reporter)

Comment 8

4 years ago
Created attachment 823970 [details]
[crash-signature] Machine-readable crash signature
Attachment #756297 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.