Closed Bug 1003161 Opened 6 years ago Closed 6 years ago

Assertion failure: !script()->formalIsAliased(i), at c:\work\mozilla\builds\nightly\mozilla\js\src\vm/Stack-inl.h:133

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: cbook, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, regression)

Attachments

(3 files)

Attached file stack
Steps to reproduce on Win7 Debug Build

Load http://www.techtudo.com.br/tudo-sobre/s/album-virtual-da-copa-do-mundo-2014.html 
-> Assertion failure: !script()->formalIsAliased(i), at c:\work\mozilla\builds\nightly\mozilla\js\src\vm/Stack-inl.h:133

will try to if i get a testcase etc
Flags: needinfo?(jdemooij)
I can reproduce this with m-c/m-i debug builds on OS X.

Bisecting points to bug 995336. Brian can you take a look? Thanks!
Blocks: 995336
Flags: needinfo?(jdemooij) → needinfo?(bhackett1024)
Attached file testcase
should crash some seconds after load (at least here on win7 debug builds)
(In reply to Carsten Book [:Tomcat] from comment #2)
> Created attachment 8415216 [details]
> testcase
> 
> should crash some seconds after load (at least here on win7 debug builds)

at least still crash locally when using this testcase (not from bugzilla attachment somehow)
guessing sec-high on the grounds that the JS team is usually serious when they assert something. Feel free to correct after analysis.
Keywords: regression, sec-high
Attached patch patchSplinter Review
Bug 995336 forgot to incorporate the check that scripts we use lazy arguments with do not have any aliased formals, so that arguments[i] will always access the canonical location of the arguments on the stack.
Assignee: nobody → bhackett1024
Attachment #8415684 - Flags: review?(jdemooij)
Flags: needinfo?(bhackett1024)
This assertion should only be a semantics problem --- when it fails the arguments[i] will be accessing a non-canonical location for the argument, but it should still be a copy of the argument's original value.
Group: core-security
Keywords: sec-high
(In reply to Daniel Veditz [:dveditz] from comment #4)
> guessing sec-high on the grounds that the JS team is usually serious when
> they assert something. Feel free to correct after analysis.

We assert anything and everything we possibly can.  Consequences range from none to everything.  It's not possible to rate just given there's an assertion.
Comment on attachment 8415684 [details] [diff] [review]
patch

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

Good find.

::: js/src/jit/IonAnalysis.cpp
@@ +2403,5 @@
>  
> +    /*
> +     * Always construct arguments objects when in debug mode and for generator
> +     * scripts (generators can be suspended when speculation fails).
> +     *

Nit: please convert this to a // comment.

@@ +2418,5 @@
> +     *
> +     * New accesses on 'arguments' can occur through 'eval' or the debugger
> +     * statement. In the former case, we will dynamically detect the use and
> +     * mark the arguments optimization as having failed.
> +     */

Same here.

@@ +2502,5 @@
> +    /*
> +     * If a script explicitly accesses the contents of 'arguments', and has
> +     * formals which may be stored as part of a call object, don't use lazy
> +     * arguments. The compiler can then assume that accesses through
> +     * arguments[i] will be on unaliased variables.

Same here.
Attachment #8415684 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/6173eafc6e42
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.