Fix assertion botched by function using a previously mentioned name and therefore claiming its placeholder or declared definition; the function must have sane end vs. begin source coordinates for error reporting purposes (640075, r=njn).
authorBrendan Eich <brendan@mozilla.org>
Tue, 08 Mar 2011 23:51:27 -0800
changeset 64260 5f49ed96bfbffeb26c9acb913efb016bf492ab7c
parent 64259 b16be37906fed4c7e11de6c42cbab6e23345fcd9
child 64261 cc7311c09b562e75ba360172dea1d623a2a59759
push idunknown
push userunknown
push dateunknown
reviewersnjn
bugs640075
milestone2.0b13pre
Fix assertion botched by function using a previously mentioned name and therefore claiming its placeholder or declared definition; the function must have sane end vs. begin source coordinates for error reporting purposes (640075, r=njn).
js/src/jsparse.cpp
js/src/jsscan.cpp
js/src/tests/js1_8_5/regress/jstests.list
js/src/tests/js1_8_5/regress/regress-640075.js
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -3098,16 +3098,23 @@ Parser::functionDef(JSAtom *funAtom, Fun
             ale = tc->lexdeps.rawLookup(funAtom, hep);
             if (ale) {
                 JSDefinition *fn = ALE_DEFN(ale);
 
                 JS_ASSERT(fn->pn_defn);
                 fn->pn_type = TOK_FUNCTION;
                 fn->pn_arity = PN_FUNC;
                 fn->pn_pos.begin = pn->pn_pos.begin;
+
+                /*
+                 * Set fn->pn_pos.end too, in case of error before we parse the
+                 * closing brace.  See bug 640075.
+                 */
+                fn->pn_pos.end = pn->pn_pos.end;
+
                 fn->pn_body = NULL;
                 fn->pn_cookie.makeFree();
 
                 tc->lexdeps.rawRemove(tc->parser, ale, hep);
                 RecycleTree(pn, tc);
                 pn = fn;
             }
 
--- a/js/src/jsscan.cpp
+++ b/js/src/jsscan.cpp
@@ -394,19 +394,17 @@ TokenStream::TokenBuf::findEOL()
 }
 
 bool
 TokenStream::reportCompileErrorNumberVA(JSParseNode *pn, uintN flags, uintN errorNumber,
                                         va_list ap)
 {
     JSErrorReport report;
     char *message;
-    size_t linelength;
     jschar *linechars;
-    const jschar *linelimit;
     char *linebytes;
     bool warning;
     JSBool ok;
     const TokenPos *tp;
     uintN i;
     JSErrorReporter onError;
 
     if (JSREPORT_IS_STRICT(flags) && !cx->hasStrictOption())
@@ -436,49 +434,49 @@ TokenStream::reportCompileErrorNumberVA(
 
     report.filename = filename;
 
     tp = pn ? &pn->pn_pos : &currentToken().pos;
     report.lineno = tp->begin.lineno;
 
     /*
      * Given a token, T, that we want to complain about: if T's (starting)
-     * lineno doesn't match TokenStream's lineno, that means we've scanned
-     * past the line that T starts on, which makes it hard to print some or
-     * all of T's (starting) line for context.  So we don't even try, leaving
-     * report.linebuf and friends zeroed.  This means that any error involving
-     * a multi-line token (eg. an unterminated multi-line string literal)
-     * won't have a context printed.
+     * lineno doesn't match TokenStream's lineno, that means we've scanned past
+     * the line that T starts on, which makes it hard to print some or all of
+     * T's (starting) line for context.
+     *
+     * So we don't even try, leaving report.linebuf and friends zeroed.  This
+     * means that any error involving a multi-line token (eg. an unterminated
+     * multi-line string literal) won't have a context printed.
      */
-    if (report.lineno != lineno)
-        goto report;
-
-    linelimit = userbuf.findEOL();
-    linelength = linelimit - linebase;
+    if (report.lineno == lineno) {
+        size_t linelength = userbuf.findEOL() - linebase;
 
-    linechars = (jschar *)cx->malloc((linelength + 1) * sizeof(jschar));
-    if (!linechars) {
-        warning = false;
-        goto out;
+        linechars = (jschar *)cx->malloc((linelength + 1) * sizeof(jschar));
+        if (!linechars) {
+            warning = false;
+            goto out;
+        }
+        memcpy(linechars, linebase, linelength * sizeof(jschar));
+        linechars[linelength] = 0;
+        linebytes = js_DeflateString(cx, linechars, linelength);
+        if (!linebytes) {
+            warning = false;
+            goto out;
+        }
+
+        /* Unicode and char versions of the offending source line, without final \n */
+        report.linebuf = linebytes;
+        report.uclinebuf = linechars;
+
+        /* The lineno check above means we should only see single-line tokens here. */
+        JS_ASSERT(tp->begin.lineno == tp->end.lineno);
+        report.tokenptr = report.linebuf + tp->begin.index;
+        report.uctokenptr = report.uclinebuf + tp->begin.index;
     }
-    memcpy(linechars, linebase, linelength * sizeof(jschar));
-    linechars[linelength] = 0;
-    linebytes = js_DeflateString(cx, linechars, linelength);
-    if (!linebytes) {
-        warning = false;
-        goto out;
-    }
-    /* Unicode and char versions of the offending source line, without final \n */
-    report.linebuf = linebytes;
-    report.uclinebuf = linechars;
-
-    /* The lineno check above means we should only see single-line tokens here. */
-    JS_ASSERT(tp->begin.lineno == tp->end.lineno);
-    report.tokenptr = report.linebuf + tp->begin.index;
-    report.uctokenptr = report.uclinebuf + tp->begin.index;
 
     /*
      * If there's a runtime exception type associated with this error
      * number, set that as the pending exception.  For errors occuring at
      * compile time, this is very likely to be a JSEXN_SYNTAXERR.
      *
      * If an exception is thrown but not caught, the JSREPORT_EXCEPTION
      * flag will be set in report.flags.  Proper behavior for an error
@@ -486,17 +484,16 @@ TokenStream::reportCompileErrorNumberVA(
      * compilation errors.  The exception will remain pending, and so long
      * as the non-top-level "load", "eval", or "compile" native function
      * returns false, the top-level reporter will eventually receive the
      * uncaught exception report.
      *
      * XXX it'd probably be best if there was only one call to this
      * function, but there seem to be two error reporter call points.
      */
-  report:
     onError = cx->errorReporter;
 
     /*
      * Try to raise an exception only if there isn't one already set --
      * otherwise the exception will describe the last compile-time error,
      * which is likely spurious.
      */
     if (!(flags & TSF_ERROR)) {
--- a/js/src/tests/js1_8_5/regress/jstests.list
+++ b/js/src/tests/js1_8_5/regress/jstests.list
@@ -87,8 +87,9 @@ script regress-626436.js
 fails-if(xulRuntime.shell) script regress-633741.js
 script regress-634210-1.js
 script regress-634210-2.js
 script regress-634210-3.js
 script regress-634210-4.js
 script regress-635195.js
 script regress-636394.js
 script regress-636364.js
+script regress-640075.js
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8_5/regress/regress-640075.js
@@ -0,0 +1,15 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+"use strict";
+try {
+    eval("(function() { eval(); function eval() {} })");
+    assertEq(0, 1);
+} catch (e) {
+    assertEq(e.name, "SyntaxError");
+}
+
+reportCompare(0, 0, "ok");