Closed Bug 23346 Opened 25 years ago Closed 25 years ago

'call()' inside a function invokes Function.prototype.call

Categories

(Core :: JavaScript Engine, defect, P1)

Product:
Core
Component:
JavaScript Engine
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: martin.honnen, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(16 files)

first cut at fix (jsfun.c needs more love)
83.47 KB, patch
Details | Diff | Splinter Review
proposed fix
88.01 KB, patch
Details | Diff | Splinter Review
proposed fix, ignoring whitespace changes (diff -wu)
36.77 KB, patch
Details | Diff | Splinter Review
Linux debug test results
7.33 KB, text/html
Details
updated patch for jsparse.c to fix assertbotch
6.21 KB, patch
Details | Diff | Splinter Review
replacement patch for jsobj.c
1.75 KB, patch
Details | Diff | Splinter Review
Linux debug results after new patches
4.93 KB, text/html
Details
new, complete diffs against top of trunk
121.93 KB, patch
Details | Diff | Splinter Review
Results after latest patch, 5 failures.
4.54 KB, text/html
Details
proposed fix, unified diffs against 2-3-2000 midnight trunk
323.32 KB, patch
Details | Diff | Splinter Review
diff -wu format for code reviewers
109.33 KB, patch
Details | Diff | Splinter Review
a stack wherne it asserts
3.96 KB, text/plain
Details
duh, fixed broken lookup/unlock-prop logic
18.51 KB, patch
Details | Diff | Splinter Review
new jsemit.c patch, fixes broken lookup/unlock-prop logic
18.51 KB, patch
Details | Diff | Splinter Review
a few nits picked, latest cvs diff -u patch
323.45 KB, patch
Details | Diff | Splinter Review
same as last diff -u, just diff -wu for reviewers' benefit
109.63 KB, patch
Details | Diff | Splinter Review
When defining a function named call and calling that inside another function not the function named call is called but Function.prototype.call which thereby recursively calls the calling function. Brendan says it's a bug. Here is a client side example <HTML> <HEAD> <STYLE> </STYLE> <SCRIPT> function f () { alert('f called'); if (confirm ('Call call?')) call(); } function call () { alert('call called'); } f() </SCRIPT> </HEAD> <BODY> </BODY> </HTML> as I don't have access to pure C engine currently. But using confirm at least allows to prevent infinite recursion in the bug demo.
Assignee: mccabe → brendan
Reassigning to Brendan. The issue here is not 'call' per se, but that unqualified references within a function body pick up names from Function.prototype.
Status: NEW → ASSIGNED
Target Milestone: M13
This should be fixed before JS1.5 freezes. /be
Priority: P3 → P1
P1 for JS1.5 deadline fix. /be
Whiteboard: [JS15]
Keywords: js1.5
Whiteboard: [JS15]
Removing [JS15] from Status Summary, putting js1.5 in new Keyword field :-)
OS: Windows 98 → other
Hardware: PC → All
OS: other → All
added testcase ecma_3/ExecutionContexts/regress-23346.js
Target Milestone: M13 → M14
JS1.5 is slipping past M13, sez rginda. /be
rginda, can you try my patch? It works on the top of trunk as of right now. I could use all the testsuite magic you can throw at it. Thanks, /be
Attached patch proposed fix — — Splinter Review
Better summary. /be
Summary: calling function named "call" inside a function calls Function.prototype.call → 'call()' inside a function invokes Function.prototype.call
Looking for an r= tomorrow. /be
Trying the patch out now.
Things look bad, 11 new failures. Attaching the linux debug test results. (The only failure without the patch is the one for this bug.)
Rob, can you revert jsparse.c and apply the latest attachment? Thanks, /be
Hoping to get a r= by tomorrow. Shaver? Bueller? /be
I applied the patch and compiled js/src only (it doesn't look like any apis changed). xpcshell runs, but mozilla asserts while compiling navigator.js with the attached stack. It is blowing the assert in _js_LookupProperty's JS_LOCK_OBJ(cx, obj) of JS_ASSERT(scope->count > 0 && scope->count <= 4) with a count of 5. Are you out of balance somewhere? or did you do something that makes it need a higher count? in this debug scope stack thingy?
Should be all better now (unbalanced locking shows up in debug builds with the object's scope, *(JSScope*)obj->map, with a count > 4 and file and line all stuck at the same offending JS_LOCK_OBJ or js_LookupProperty call -- the one that lacks a matching JS_UNLOCK_OBJ (or OBJ_DROP_PROPERTY after looking up a property successfully). /be
Jband, I think this patch (with the jsemit.c fix) will also make your backtrace stuff work. Now any function activation that doesn't have a call object can get one via js_GetCallObject safely (because it must not contain a with statement, a debugger statement, a direct call to eval, an assignment or mutation of the arguments variable, or other things that make functions heavyweight). /be /be
This last attachment is what I hope to check in later today, tree gods willing. So testers and reviewers, please have at it. /be
brendan, I looked at all this. I understand less than I'd like. It looks ok to me. The 'debugger' statement handler for my stack dumper does not fail on XUL JS now. The cases where we'd be calling to make a new call object at an arbitrary time from within a debugger are harder to test. I tested making a new call object for a function that does not have one (but only in the non-shared case) and that seems to work. Is it really OK to override the existing fp->varobj with the new callobj? Also, did you mean to whack the comment in the function_props declaration and the block of code in fun_getProperty?
>I looked at all this. I understand less than I'd like. It looks ok to me. The >'debugger' statement handler for my stack dumper does not fail on XUL JS now. Great! Thanks. >The cases where we'd be calling to make a new call object at an arbitrary >time from within a debugger are harder to test. These changes ensure that only a lightweight function might lack a call object at an arbitrary time during function activation, when the debugger API would make one. At that time, it's ok to make one because the scope chain and varobj are set to the function's parent, but no variables can be created by the function dynamically (the only way that could happen is via eval, but direct eval calls make the calling function heavyweight). >I tested making a new call >object for a function that does not have one (but only in the non-shared case) >and that seems to work. Is it really OK to override the existing fp->varobj >with the new callobj? Yes, see above. fp->varobj won't be used by bytecode executing in a lightweight by definition. Any bytecode generated that would use varobj and want it to be the call object was generated by jsemit.c code that flags the function as heavy (therefore, it gets a call object when invoked). Am I making sense yet? >Also, did you mean to whack the comment in the function_props declaration and >the block of code in fun_getProperty? Yes, I meant to whack those things. Now that function objects never proxy for call objects, there is no need to make 'arguments' readonly via a magic setter. Also, there's no need to convert 'arity' to __arity__'s slot, etc. No more __ name mangling to avoid collisions along the scope chain, because either the fun is lightweight and there is no call object, or it's heavy, and the Call instance has a fine, read-write 'arguments' property. r=jband@netscape.com, where's shaver? Bueller? /be
This new patch cleans up the regressions.
Shaver's in NYC at LWE. I'm checking in, r=jband@netscape.com. Thanks to John and Rob. Someone stop me fast if there's a prob. /be
I say go for it.
Fix checked in. If there are any follow-on bugs, please mail me about them at meer.net. Thanks. /be
Closing. /be
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Marking Verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: