797495 - Crash [@ js::frontend::Parser::setStrictMode]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 13fd49ef7786 (run with --ion-eager):


options("strict_mode");
var f = Function("for(w in\\");

Comment 1

[Christian Holler (:decoder)]

Crash trace:

==60146== Invalid read of size 4
==60146==    at 0x83A4F62: js::frontend::Parser::setStrictMode(bool) (Parser.cpp:1773)
==60146==    by 0x83A5545: js::frontend::Parser::processDirectives(js::frontend::ParseNode*) (Parser.cpp:1887)
==60146==    by 0x83A5669: js::frontend::Parser::statements(bool*) (Parser.cpp:1912)
==60146==    by 0x83A13DB: js::frontend::Parser::functionBody(js::frontend::Parser::FunctionBodyType) (Parser.cpp:717)
==60146==    by 0x835D2E8: js::frontend::CompileFunctionBody(JSContext*, JS::Handle<JSFunction*>, JS::CompileOptions, js::AutoNameVector const&, unsigned short const*, unsigned int) (BytecodeCompiler.cpp:310)
==60146==    by 0x8128071: js::Function(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1404)
==60146==    by 0x8181C06: js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:370)
==60146==    by 0x818B3BE: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:367)
==60146==    by 0x819A020: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2461)
==60146==    by 0x818AF00: js::RunScript(JSContext*, JS::Handle<JSScript*>, js::StackFrame*) (jsinterp.cpp:306)
==60146==    by 0x818BE4B: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) (jsinterp.cpp:509)
==60146==    by 0x818C0C5: js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) (jsinterp.cpp:547)
==60146==  Address 0x0 is not stack'd, malloc'd or (recently) free'd


The --ion-eager from comment 0 is not required.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   104399:3ea27df2919a
user:        Nicholas Nethercote
date:        Tue Sep 04 18:09:31 2012 -0700
summary:     Bug 787866 (part 3) - Remove SharedContext::fun_.  r=luke.

This iteration took 56.841 seconds to run.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Loosen overly tight assertion in Parser::setStrictMode().

Luke, in bug 787866 it appears that I forgot to ask you to review the 3rd
patch, and then I landed without noticing this.  Sorry!

Before that patch landed, SharedContext had both |fun_| and |funbox_|;  the
patch effectively merged them, since they basically were equivalent.

Except they weren't equivalent in one case.  CompileFunctionBody() -- which
is used for functions created with Function() -- set |fun_| but *didn't* set
|funbox_|.  This was weird because it meant that |inFunction()| wasn't true
for a function defined by Function()!  That really doesn't seem right, and
so the fact that my patch changed it seems like a good thing.

But it leads to this assertion failure, which comes from the flipped
behaviour of inFunction() in setStrictMode() for functions created with
Function().  It's an easy fix -- I just loosened the assertion to allow for
the fact that a function created with Function() doesn't have a parent
context.

(Please feel free to go back and review the 3rd patch in bug 787866.
Additional changes have landed on top of it, but I'll do my best to apply
any suggestions you have.  Sorry again that I messed this one up.)

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 667790 [details] [diff] [review]
Loosen overly tight assertion in Parser::setStrictMode().

No worries; 'twas a righteous patch, present goofiness notwithstanding.

Comment 5

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5215c9e77553

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5215c9e77553

Comment 7

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug797495.js.