Bug 13623 – Decompilation of function doesn't compile with "++(x,y)"

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jesse Ruderman]

> f = function(){ ++(x,y) };
function () 
{
  ++x, y;
}

> eval("" + f)
SyntaxError: Parse error

I think the best way to avoid this "decompilation of a function fails to compile" bug is to reject the initial function at compile time, like Firefox does.

Found by jsfunfuzz.

[Eric Seidel (no email)]

Except there is no "compile time" in JavaScriptCore.  JSC parses the source to make an abstract syntax tree and then walks the tree for execution.

[Jesse Ruderman]

"Parse time" then? :)

[Kimmo Kinnunen]

Created attachment 14702 [details]
Patch for toString()ing pre/postfix/typeof operators that contain grouping expression

The patch makes errors like these produce run-time exception similar to the other varians of this error, like "function f() { (g())++; }"

SyntaxError for eval("function() {++x, y; }") is another issue, caused by treating 'function' as function declaration and not function expression. Firefox treats anonymous functions that appear in SourceElement context as  function expressions.

[Kimmo Kinnunen]

Comment on attachment 14702 [details]
Patch for toString()ing pre/postfix/typeof operators that contain grouping expression

Seems that I got some conflict fragments in LayoutTests/ChangeLog..

[Kimmo Kinnunen]

Created attachment 14703 [details]
Patch for toString()ing pre/postfix/typeof operators that contain grouping expression. 

Same patch as above. Seems that the conflict markers in LayoutTests/ChangeLog had been committed to the repository. I removed them locally, and that's why they show up in the diff. (conflict markers not removed in this)

About the previous Firefox comment: I'm no expert in how firefox/spidermonkey works, but that's my guess why the anonymous function expression works in the SourceElement context.

[Darin Adler]

Comment on attachment 14703 [details]
Patch for toString()ing pre/postfix/typeof operators that contain grouping expression. 

r=me

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> Same patch as above. Seems that the conflict markers in LayoutTests/ChangeLog
> had been committed to the repository. I removed them locally, and that's why
> they show up in the diff. (conflict markers not removed in this)

That's my bad.  I just removed them.

[Jesse Ruderman]

(In reply to comment #0)
> > eval("" + f)
> SyntaxError: Parse error

(In reply to comment #3)
> SyntaxError for eval("function() {++x, y; }") is another issue, caused by
> treating 'function' as function declaration and not function expression.
> Firefox treats anonymous functions that appear in SourceElement context as 
> function expressions.

Oops, I forgot that I needed parentheses there.  Firefox trunk requires parentheses too (see https://bugzilla.mozilla.org/show_bug.cgi?id=376052).

[Sam Weinig]

Landed in r21473 and r21393 ?

[Kimmo Kinnunen]

(In reply to comment #9)
> Landed in r21473 and r21393 ?
> 

I don't understand the comment/question, but I believe the patch has not yet been applied (r21742). The mentioned revs are different patches (fixing different bugs)

[Kimmo Kinnunen]

The first patch is ok but has the LayoutTests/ChangeLog problem.
The second patch was a old patch for different bug. 

Sorry for the mixup. I'll upload the first patch again with correct ChangeLog..

[Kimmo Kinnunen]

Created attachment 14719 [details]
The correct patch (same as the first one, except the LayoutTests/ChangeLog)

[Sam Weinig]

Landed, for real this time, in r21761.