33391 - arguments.callee broken for joined function objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Brendan Eich [:brendan]]

See the news thread for test case and diagnosis.  Patch coming up.

/be

Comment 1

[Brendan Eich [:brendan]]

This is a backward incompatibility.  It should be fixed for js1.5.

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 3

[Brendan Eich [:brendan]]

Adding [e-mail] -- Till, can you try the attached patch and see 
whether it helps your multi-threaded function prototype object problems?  
Thanks,

/be

Comment 4

[Brendan Eich [:brendan]]

Further patch to jsinterp.c coming up -- it turns out that fp->argv may be
non-null but fp->argv[-2] is not valid, in the unlikely case that not enough
actual args were passed to a function, *and* the stack arena in which those
arguments were pushed as operands of JSOP_CALL or JSOP_NEW does not contain
enough contiguous space for the missing args and "extra" (local GC root) stack
slots needed by the function.  In that case, js_Invoke copies the args, but not
argv[-2] or argv[-1].  D'oh!

Norris, I think this accounts for the erratic fp->argv[-2] behavior you saw when
testing your first patch for http://bugzilla.mozilla.org/show_bug.cgi?id=34364
(although I don't see why qualifying the fp->argv non-null test with fp->fun
would help).  Otherwise, inspection of js/src/*.c grepping for 'argv = ' finds
no way for argv to be set such that argv[-2] would not be the callee object.

/be

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed further fix for argv-copying js_Invoke hard case

Comment 6

[Brendan Eich [:brendan]]

Adding jband, my best code-buddy lately.

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: my hasty JS_ASSERT was the shits; better jsinterp.c patch

Comment 8

[Brendan Eich [:brendan]]

Attachment: good grief! another old, very rare bug in this hard case

Comment 9

[Brendan Eich [:brendan]]

If it's guaranteed to be contiguous, why didn't I assert?  Urgh, revised final
patch coming right up.

/be

Comment 10

[Brendan Eich [:brendan]]

Attachment: add assertion to last patch's change from previous patch

Comment 11

[Brendan Eich [:brendan]]

Attachment: Ok, this is it! the final patch, thanks to jband

Comment 12

[Brendan Eich [:brendan]]

The next-to-last patch was flailing hard, trying to get back the surplus slots
it gave up in the vain hope of avoiding allocating a new arena.  The final patch
doesn't even mess with surplus measurement if the call needs more contiguous
argv space than fits in the current arena.

/be

Comment 13

[Brendan Eich [:brendan]]

Need a test-case, generated JS with functions having 1..largeN stack depth and
1..largeM parameters, called with missing actual args!

/be