arguments.callee broken for joined function objects

RESOLVED FIXED

Status

()

Core
JavaScript Engine
P3
normal
RESOLVED FIXED
18 years ago
16 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({js1.5})

Trunk
js1.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(6 attachments)

(Assignee)

Description

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

/be
(Assignee)

Comment 1

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

/be
Keywords: js1.5
(Assignee)

Comment 2

18 years ago
Created attachment 6952 [details] [diff] [review]
proposed fix
(Assignee)

Comment 3

18 years ago
Adding tvollmer@hyperwave.com -- Till, can you try the attached patch and see 
whether it helps your multi-threaded function prototype object problems?  
Thanks,

/be
(Assignee)

Comment 4

18 years ago
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
Assignee: rogerl → brendan
(Assignee)

Comment 5

18 years ago
Created attachment 7255 [details] [diff] [review]
proposed further fix for argv-copying js_Invoke hard case
(Assignee)

Comment 6

18 years ago
Adding jband, my best code-buddy lately.

/be
Status: NEW → ASSIGNED
(Assignee)

Comment 7

18 years ago
Created attachment 7256 [details] [diff] [review]
my hasty JS_ASSERT was the shits; better jsinterp.c patch
(Assignee)

Comment 8

18 years ago
Created attachment 7257 [details] [diff] [review]
good grief!  another old, very rare bug in this hard case
(Assignee)

Comment 9

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

/be
(Assignee)

Comment 10

18 years ago
Created attachment 7258 [details] [diff] [review]
add assertion to last patch's change from previous patch
(Assignee)

Comment 11

18 years ago
Created attachment 7262 [details] [diff] [review]
Ok, this is it!  the final patch, thanks to jband
(Assignee)

Comment 12

18 years ago
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
(Assignee)

Comment 13

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

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.