Last Comment Bug 351116 - Crash if formal parameter and inner function have the same name [@ js_DecompileFunction]
: Crash if formal parameter and inner function have the same name [@ js_Decompi...
Status: VERIFIED FIXED
[sg:critical?]
: crash, testcase, verified1.8.0.8, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.8.1
Assigned To: Brendan Eich [:brendan]
:
:
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2006-09-01 18:13 PDT by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
3 users (show)
mtschrep: blocking1.8.1+
dveditz: blocking1.8.0.8+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (2.43 KB, patch)
2006-09-01 18:34 PDT, Brendan Eich [:brendan]
no flags Details | Diff | Splinter Review
fix (5.01 KB, patch)
2006-09-01 20:22 PDT, Brendan Eich [:brendan]
no flags Details | Diff | Splinter Review
fix, v3 (5.27 KB, patch)
2006-09-02 00:01 PDT, Brendan Eich [:brendan]
mrbkap: review+
dveditz: approval1.8.0.8+
mtschrep: approval1.8.1+
Details | Diff | Splinter Review

Description Jesse Ruderman 2006-09-01 18:13:31 PDT
javascript:function (s) { function s() { } }

causes a scary crash, at least in a debug build of Firefox:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xdadadae2

Thread 0 Crashed:
0   js_DecompileFunction + 996 (jsopcode.c:3258)
1   JS_DecompileFunction + 140 (jsapi.c:4222)
2   js_fun_toString + 2008 (jsfun.c:1476)
3   fun_toString + 68 (jsfun.c:1486)
4   js_Invoke + 4912 (jsinterp.c:1350)
5   js_InternalInvoke + 444 (jsinterp.c:1448)
6   js_TryMethod + 416 (jsobj.c:4534)
7   js_DefaultValue + 112 (jsobj.c:3809)
8   js_ValueToString + 152 (jsstr.c:2681)
9   JS_ValueToString + 40 (jsapi.c:535)
10  JSValueToAString(JSContext*, long, nsAString_internal*, int*) + 116 (nsJSEnvironment.cpp:1160)
11  nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned, unsigned, nsAString_internal*, int*) + 1612 (nsJSEnvironment.cpp:1322)
12  nsJSThunk::EvaluateScript(nsIChannel*) + 2844 (nsJSProtocolHandler.cpp:255)
Comment 1 Brendan Eich [:brendan] 2006-09-01 18:27:38 PDT
Fairly old bug, good find.  Not the usual decompiler-doesn't-do-right fluff :-P.

/be
Comment 2 Brendan Eich [:brendan] 2006-09-01 18:34:30 PDT
Created attachment 236491 [details] [diff] [review]
fix
Comment 3 Brendan Eich [:brendan] 2006-09-01 18:35:26 PDT
Comment on attachment 236491 [details] [diff] [review]
fix

Not the right patch...

/be
Comment 4 Brendan Eich [:brendan] 2006-09-01 20:22:16 PDT
Created attachment 236505 [details] [diff] [review]
fix

Unlike var, function creates a new property, so we must use SPROP_IS_DUPLICATE to keep the formal parameter property around.

/be
Comment 5 Brendan Eich [:brendan] 2006-09-02 00:01:16 PDT
Created attachment 236517 [details] [diff] [review]
fix, v3

Just comment fixes, some were quite out of date.

/be
Comment 6 Brendan Eich [:brendan] 2006-09-02 13:35:32 PDT
Fixed on trunk.

/be
Comment 7 Bob Clary [:bc:] 2006-09-02 17:49:54 PDT
This only crashed for me when I used the javascript: url .
Checking in regress-351116.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-351116.js,v  <--  regress-351116.js
initial revision: 1.1
Comment 8 Bob Clary [:bc:] 2006-09-04 19:23:01 PDT
verified fixed 1.9 20060903 windows/mac*/linux
Comment 9 Mike Schroepfer 2006-09-05 11:22:36 PDT
Comment on attachment 236517 [details] [diff] [review]
fix, v3

a=schrep for drivers.
Comment 10 Brendan Eich [:brendan] 2006-09-05 12:28:56 PDT
Fixed on the 1.8 branch.

/be
Comment 11 Bob Clary [:bc:] 2006-09-06 12:53:15 PDT
verified fixed 1.8 20060906 windows/mac*/linux
Comment 12 Daniel Veditz [:dveditz] 2006-09-26 14:48:52 PDT
Comment on attachment 236517 [details] [diff] [review]
fix, v3

approved for 1.8.0 branch, a=dveditz for drivers
Comment 13 Brendan Eich [:brendan] 2006-09-26 15:49:46 PDT
Fixed on the 1.8.0 branch:

Checking in jsopcode.c;
/cvsroot/mozilla/js/src/jsopcode.c,v  <--  jsopcode.c
new revision: 3.89.2.8.2.9; previous revision: 3.89.2.8.2.8
done
Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.142.2.6.2.8; previous revision: 3.142.2.6.2.7
done

/be
Comment 14 Bob Clary [:bc:] 2006-09-27 12:37:51 PDT
verified fixed 1.8.0.8 20060927 windows/mac*/linux
Comment 15 Mike Hommey [:glandium] 2006-10-28 02:06:08 PDT
Am I right saying this is not a problem in the 1.7 branch ? Mozilla complains about a syntax error at the first parenthesis in the testcase...
Comment 16 Daniel Veditz [:dveditz] 2006-11-01 18:29:54 PST
You can get around the syntax error with

  javascript:x = function (s) { function s() { } }

That makes unfixed 1.8.0.x releases crash, but doesn't crash FF1.0.x.

Note You need to log in before you can comment on or make changes to this bug.