User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011215 Firefox/3.0b3pre Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011215 Firefox/3.0b3pre Firefox crashes when I open 'view page info' on a non-blank page. Reproducible: Always Steps to Reproduce: 1. Open http://www.google.com 2. Right click on the page. 3. Select 'View Page Info'. Actual Results: Firefox crashes.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008011205 Minefield/3.0b3pre ID:2008011205 I see this too. Works: 20080111_1445_firefox-3.0b3pre.en-US.win32 Crash: 20080111_1522_firefox-3.0b3pre.en-US.win32 Checkins to module PhoenixTinderbox between 2008-01-11 14:45 and 2008-01-11 15:21 : http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1200091500&maxdate=1200093719 bp-7585dc8f-c11b-11dc-b1c8-001a4bd43e5c bp-88cf1b14-c11a-11dc-a27b-001a4bd43e5c bp-3a374d6e-c11a-11dc-9003-001a4bd43ed6

From the range, I don't think this crash could be caused by bug 408301 since that bug was backed out for the 2008011205 build I'm on, and I still see the crashing. That leaves one of two javascript bugs the more likely to have caused this, so I'm punting this over to the JS Engine component.

That range is missing a thinko I fixed in the next checkin outside that range -- try again with a build that picked up the next change. It'd be more obvious if I had a stack to go from, but none of the crash report links work for me.

Jeff, as I said in comment 1 and 2, I see the crash with the 2008011205 build. It looks like your follow-up thinko was landed at 2008-01-11 16:12 which was hours before today's daily build (2008011205) was done. And yes, the crash-stats seem to be taking their sweet time ;-)

I started a new build 5 minutes ago. I'll post back if the crash remains. Regarding a stack trace, I did build with --enable-debug but got not debug symbols. Am I missing anything?

Crash report is there now: bp-88cf1b14-c11a-11dc-a27b-001a4bd43e5c Frame Signature Source 0 js_ValueToNumber 1 js_ValueToECMAInt32 2 js_IntToCString 3 js_IntToCString 4 js_Interpret 5 js_Invoke 6 js_InternalInvoke 7 JS_CallFunctionValue 8 nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) mozilla/dom/src/base/nsJSEnvironment.cpp:1967 9 nsJSEventListener::HandleEvent(nsIDOMEvent*) mozilla/dom/src/events/nsJSEventListener.cpp:235 10 nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsISupports*, unsigned int) mozilla/content/events/src/nsEventListenerManager.cpp:1081 11 nsEventListenerManager::HandleEvent(nsPresContext*, nsEvent*, nsIDOMEvent**, nsISupports*, unsigned int, nsEventStatus*) mozilla/content/events/src/nsEventListenerManager.cpp:1183 12 nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, unsigned int) mozilla/content/events/src/nsEventDispatcher.cpp:206 13 nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor&, unsigned int, nsDispatchingCallback*) mozilla/content/events/src/nsEventDispatcher.cpp:264 14 nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, nsEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*) mozilla/content/events/src/nsEventDispatcher.cpp:479 15 DocumentViewerImpl::LoadComplete(unsigned int) mozilla/layout/base/nsDocumentViewer.cpp:979 16 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, unsigned int) mozilla/docshell/base/nsDocShell.cpp:5028 17 nsWebShell::EndPageLoad(nsIWebProgress*, nsIChannel*, unsigned int) mozilla/docshell/base/nsWebShell.cpp:1014 18 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int) mozilla/docshell/base/nsDocShell.cpp:4928

Waldo, this is a dup, or yours for the taking (could be a latent bug exposed by your checkin?). /be

(In reply to comment #5) > I started a new build 5 minutes ago. I'll post back if the crash remains. > Regarding a stack trace, I did build with --enable-debug but got not debug > symbols. Am I missing anything? > Sorry for taking too much to reply. it is still in my build

Yep, easily reproducible -- exact same stack for me. Setting 1.9+, though I bet this'll probably get knocked out trivially by Waldo or someone else tomorrow or monday.

this crash also happens when clicking on the "Tell me more"-link of "Larry" on a https-site (like bugzilla).

(In reply to comment #10) > this crash also happens when clicking on the "Tell me more"-link of "Larry" on > a https-site (like bugzilla). > indeed

The stack in comment 6 is mangled; js_IntToCString is a leaf function. Might want to have the Breakbag/Airpad people look into that, assuming it's not a dup. So, the problem is easy to explain, and I'm too lazy to explain twice: <Waldo> num_toLocaleString? <Waldo> no args <Waldo> no extra spaces for temporary rooting <Waldo> but it calls num_toString <Waldo> which takes an argument <Waldo> so toLocaleString accesses outside of the usable range of |vp| <Waldo> and sadly, it seems I can't jst change the minargs argument to JS_FN <Waldo> because it appears we have a JS_ASSERT(minargs <= nargs) for fast natives I'm still trying to decide what's the right thing to do for the last two lines; maybe a temp root or something.

Yes, fast-natives used to support local GC roots (guaranteed extra args, per the extra field in JSFunctionSpec). Bug 397239 revised and simplified SpiderMonkey to remove this feature in common among fast and slow natives. You must use a tvr now. /be

Comment on attachment 296911 [details] [diff] [review] Patch >+ ok = num_toString(cx, argc, roots); s/argc/0/

Comment on attachment 296911 [details] [diff] [review] Patch Sigh. Igor should take a look too, he may have a better idea. /be

I could be missing something, but given the current: static JSFunctionSpec number_methods[] = { ... JS_FN(js_toString_str, num_toString, 0,0,JSFUN_THISP_NUMBER), JS_FN(js_toLocaleString_str, num_toLocaleString, 0,0,JSFUN_THISP_NUMBER), ... }; it is safe to call num_toString from num_toLocaleString with the arguments passed to num_toLocaleString. num_toString always checks argc != 0 before accessing the optional base argument. Thus the bug must be with something else.

Um, are you looking at the most recent source? I clearly removed the argc != 0 check in revision 3.96: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/js/src/jsnum.c&rev=3.98&mark=258#245 Speaking of that, we always do have the option of just readding that check and changing the num_toString argument in num_toLocaleString from |argc| to 0, easily the simplest option for fixing this bug -- maybe that's better than this mess?

(In reply to comment #20) > Um, are you looking at the most recent source? I clearly removed the argc != 0 > check in revision 3.96: My bad, I indeed have not updated my tree. > ... we always do have the option of just readding that check and > changing the num_toString argument in num_toLocaleString from |argc| to 0, > easily the simplest option for fixing this bug... In my view that would be the best solution.

Given how hard it would be to spot bugs like this it would be better to remove the minimal arg support from fast native altogether. That would remove the need to an extra checks when calling the fast native for the price of the explicit checks in the native code.

Okay, let's go with restoring the argc check. Another thing to note: the argc check is actually necessary to restore correct behavior to toLocaleString when called with a first argument, which before my patches here would have been interpreted as a radix -- but the spec explicitly doesn't say to do this (arg reserved presumably for locale info). Too bad we didn't have any regression tests for the method... I think I like removing minargs from fast natives altogether; it only seems useful if you have situations like what happened here, except reversed. I doubt those are all that common, and it seems like we'd double-pay in branches in at least some cases where minargs < argc. Fodder for a new bug, I think; feel free to file and assign to me.

(In reply to comment #23) > Created an attachment (id=296981) [details] > Restore the argc check, s/argc/0/, test The patch should also change JS_FN declaration for num_toString in number_methods to state 0 minargs.

(In reply to comment #23) > Fodder for a new bug, I think; feel free to file and assign to me. See bug 412296.

I'll commit this later today -- no real rush beyond getting it in the next nightly.

Waldo, you had a mismatch in quotes in that test. Also, when you add a js test, please minus the in-litmus flag and plus the in-testsuite flag. Thanks!

/cvsroot/mozilla/js/tests/ecma_3/Number/15.7.4.3-01.js,v <-- 15.7.4.3-01.js new revision: 1.2; previous revision: 1.1

(In reply to comment #28) > Waldo, you had a mismatch in quotes in that test. Also, when you add a js test, > please minus the in-litmus flag and plus the in-testsuite flag. Thanks! Oops; I could have sworn I tested that before committing. As for the flags, I hadn't flipped them because after committing I was too tired to stay up to see that the commit stuck, and I figured it made more sense to get it in for the nightly than to wait any longer. Anyway, FIXED.

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008011504 Minefield/3.0b3pre. Also verified on the nightly on Win Vista Ultimate.