104584 - js_GC doesn't mark max number of (actual, formal) parameters

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Matthias Versen [:Matti]]

win2k build 20011011.. (CVS debug) and 1012 (CVS opt)

1. Load the URL
2. Watch mozilla crashing

win2k stack trace :
fdcdcdcd()
js_Interpret(JSContext * 0x037bfae0, long * 0x0012be10) line 2564 + 386 bytes
js_Invoke(JSContext * 0x037bfae0, unsigned int 1, unsigned int 0) line 826 + 13 
bytes
js_Interpret(JSContext * 0x037bfae0, long * 0x0012cb6c) line 2732 + 15 bytes
js_Invoke(JSContext * 0x037bfae0, unsigned int 1, unsigned int 0) line 826 + 13 
bytes
js_Interpret(JSContext * 0x037bfae0, long * 0x0012d8c8) line 2732 + 15 bytes
js_Invoke(JSContext * 0x037bfae0, unsigned int 0, unsigned int 0) line 826 + 13 
bytes
js_Interpret(JSContext * 0x037bfae0, long * 0x0012e624) line 2732 + 15 bytes
js_Invoke(JSContext * 0x037bfae0, unsigned int 1, unsigned int 2) line 826 + 13 
bytes
js_InternalInvoke(JSContext * 0x037bfae0, JSObject * 0x02a38ba8, long 59412960, 
unsigned int 0, unsigned int 1, long * 0x0012e804, long * 0x0012e74c) line 901 + 
20 bytes
JS_CallFunctionValue(JSContext * 0x037bfae0, JSObject * 0x02a38ba8, long 
59412960, unsigned int 1, long * 0x0012e804, long * 0x0012e74c) line 3387 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x03cbc6e0, void * 0x02a38ba8, 
void * 0x038a91e0, unsigned int 1, void * 0x0012e804, int * 0x0012e800, int 0) 
line 977 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x03905cc8, nsIDOMEvent 
* 0x0399bafc) line 155 + 74 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x03905d20, 
nsIDOMEvent * 0x0399bafc, nsIDOMEventTarget * 0x037bf970, unsigned int 1, 
unsigned int 7) line 1213 + 20 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x039a4028, 
nsIPresContext * 0x037eb618, nsEvent * 0x0012ef14, nsIDOMEvent * * 0x0012eecc, 
nsIDOMEventTarget * 0x037bf970, unsigned int 7, nsEventStatus * 0x0012ef3c) line 
1886 + 36 bytes
GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x037bf960, 
nsIPresContext * 0x037eb618, nsEvent * 0x0012ef14, nsIDOMEvent * * 0x0012eecc, 
unsigned int 1, nsEventStatus * 0x0012ef3c) line 624
DocumentViewerImpl::LoadComplete(DocumentViewerImpl * const 0x03ce3000, unsigned 
int 0) line 1103 + 47 bytes
nsDocShell::EndPageLoad(nsIWebProgress * 0x03664fd4, nsIChannel * 0x03982eb8, 
unsigned int 0) line 3751
nsWebShell::EndPageLoad(nsIWebProgress * 0x03664fd4, nsIChannel * 0x03982eb8, 
unsigned int 0) line 918
nsDocShell::OnStateChange(nsDocShell * const 0x036627e4, nsIWebProgress * 
0x03664fd4, nsIRequest * 0x03982eb8, int 131088, unsigned int 0) line 3672
nsDocLoaderImpl::FireOnStateChange(nsIWebProgress * 0x03664fd4, nsIRequest * 
0x03982eb8, int 131088, unsigned int 0) line 1110
nsDocLoaderImpl::doStopDocumentLoad(nsIRequest * 0x03982eb8, unsigned int 0) 
line 749
nsDocLoaderImpl::DocLoaderIsEmpty() line 647
nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x03664fc4, nsIRequest * 
0x03ddf0c8, nsISupports * 0x00000000, unsigned int 0) line 578
nsLoadGroup::RemoveRequest(nsLoadGroup * const 0x03c5e938, nsIRequest * 
0x03ddf0c8, nsISupports * 0x00000000, unsigned int 0) line 525 + 44 bytes
PresShell::RemoveDummyLayoutRequest() line 6257 + 42 bytes
PresShell::DoneRemovingReflowCommands() line 6213
PresShell::ProcessReflowCommands(int 1) line 6044
ReflowEvent::HandleEvent() line 5828
HandlePLEvent(ReflowEvent * 0x03ddf110) line 5842
PL_HandleEvent(PLEvent * 0x03ddf110) line 590 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00e627b0) line 520 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x001203d6, unsigned int 49332, unsigned int 0, 
long 15083440) line 1071 + 9 bytes
USER32! 77e02e98()
USER32! 77e030e0()
USER32! 77e05824()
nsAppShellService::Run(nsAppShellService * const 0x00f47180) line 457
main1(int 2, char * * 0x003577d0, nsISupports * 0x00000000) line 1291 + 32 bytes
main(int 2, char * * 0x003577d0) line 1619 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87d08()

Comment 1

[Erin Dalzell]

Maybe related to http://bugzilla.mozilla.org/show_bug.cgi?id=104591?? I see the
same crash!

Added myself to CC list

Comment 2

[Phil Schwartau]

Note: site relies on "DynAPI" js file: 

          http://www.scriptweb.ch/js-2.53/dynapi.js

Note: DynAPIObject.prototype.include() is a function that document.writes
included JS files in JS version 1.2:

DynAPIObject.prototype.include = function(src,pth) {
 src=src.split('.');
 if (src[src.length-1] == 'js') src.length -= 1;
 var path=pth||this.librarypath||'';
 if (path.substr(path.length-1) != "/") path += "/";
 var pckg=src[0];
 var grp=src[1];
 var file=src[2];
 if (file=='*') {
  if (this.packages[pckg]) group=this.packages[pckg].libs[grp];
  if (group) for (var i=0;i<group.length;i++) document.write('<script 
language="Javascript1.2" 
src="'+path+pckg+'/'+grp+'/'+group[i]+'.js"><\/script>');
		else alert('include()\n\nThe following package could not be 
loaded:\n'+src+'\n\nmake sure you specified the correct path.');
	} else document.write('<script language="Javascript1.2" 
src="'+path+src.join('/')+'.js"><\/script>');
}

Comment 3

[Phil Schwartau]

Here's where it crashes in js_Interpret:


case JSOP_SETPROP:
   /* Pop the right-hand side into rval for OBJ_SET_PROPERTY. */
   rval = FETCH_OPND(-1);

   /* Get an immediate atom naming the property. */
   atom = GET_ATOM(cx, script, pc);
   id   = (jsid)atom;
   PROPERTY_OP(-2, CACHED_SET(OBJ_SET_PROPERTY(cx, obj, id, &rval))); <---CRASH
   sp--;
   STORE_OPND(-1, rval);
   break;

Comment 4

[Phil Schwartau]

Note: to crash, it's sufficient to load the menu frame at this site:

          http://www.scriptweb.ch/menu.php3


Here is another important file:

          http://www.scriptweb.ch/js-2.53/dynapi/api/dynlayer.js


Note: the DynAPI JS files are apparently templates available at

          http://dynapi.sourceforge.net/dynapi/

See 

bug 54458  Evangelism - DYNAPI 
bug 57456  Evangelism - Contact HTML Editor/Tool vendors to update thei 
bug 67147  Evangelismberculo.com - Javascript-powered menu doesn't function 

Comment 5

[Phil Schwartau]

From what I can see, the www.scriptweb.ch site is using an updated
DynAPI template that recognizes Netscape 6, so I don't think there
are Evangelism issues here. 

Reassigning to DOM Level 0 - perhaps the DynAPI code is doing something
bad in reading/writing DOM properties. That's my guess, at any rate;
I don't think this is a JS Engine bug - 

cc'ing jband in case I'm off base here.

Comment 6

[Phil Schwartau]

Attachment: menu.php3 with one line commented out; will load in Mozilla

Comment 7

[Phil Schwartau]

Perhaps the above attachment will help in debugging; at least it will
load in Mozilla. Here is the one line I commented out:

arMenu = new Array();
arMenu[0] = new Array("Verband","templ.php3?pid=11", ETC.

DynAPI.onLoad = function() {
        menu = new l8MenuBar(arMenu);
        
        DynAPI.document.addChild(menu);
        ///////////////////////////////////////menu.init();
}

Comment 8

[Phil Schwartau]

I crash on Linux, too. OS : Win --> All

Comment 9

[John Bandhauer]

This is a JS Engine bug. The problem is that we are apparently not rooting slots 
of arguments that are declared but not passed in.

The script that was running is in 
http://www.scriptweb.ch/js-2.53/dynapi/event/listeners.js

(goto http://www.scriptweb.ch/js-2.53/dynapi/event/ and save that file if you 
want)

It was running line 45 of the function at line 42.

At that place we are setting the 'target' property on a plain JSObject. The obj 
is a reasonable pointer that points at garbage and it crashes when that is 
dereferenced.

The problem there is demonstrated by the (jsshell) test case:

function foo(e) {
    if(!e) 
        e = {};
    print(e);
    gc();
    print(e);
}

foo();

This case will crash at the second "print(e);".

Comment 10

[Brendan Eich [:brendan]]

Attachment: proposed fix (shades of a recent jsfun.c patch)

Comment 11

[Brendan Eich [:brendan]]

Cc: list, please give fast r= and sr=.

/be

Comment 12

[John Bandhauer]

Comment on attachment 53620 [details] [diff] [review]
proposed fix (shades of a recent jsfun.c patch)

r/sr=jband.
Looks right and worksforme.

I guess I could have taken the next step and just written the patch myself. Thanks.

Comment 13

[Mike Shaver (:shaver emeritus)]

Comment on attachment 53620 [details] [diff] [review]
proposed fix (shades of a recent jsfun.c patch)

r=shaver.

Comment 14

[Brendan Eich [:brendan]]

bug 97921 was the one whose fix used JS_MAX(fp->argc, fp->fun->nargs) to bound
the number of roots at fp->argv.  Shaver, you sr'd that with jband, can you do
the double-sr= duty here too?  Thanks.

/be

Comment 15

[Brendan Eich [:brendan]]

Fix is in.  Thanks (sorry, shaver -- missed your comment somehow!).

/be

Comment 16

[Phil Schwartau]

jband's test added to JS testsuite:

           mozilla/js/tests/ecma_3/Function/regress-104584.js


Test crashed in optimized and debug JS shells before the fix; 
now passes on WinNT, Linux, and Mac9.1.

Marking Verified Fixed -

Comment 17

[Phil Schwartau]

Also verified with trunk binaries 20011016xx on WinNT, Linux, Mac OS9.1.
On all three platforms, the URL loads without crashing now.