Last Comment Bug 425576 - Crash on login to Excite Japan Blog (exblog.jp) after updating to Firefox 2.0.0.13 [@ js_MarkGCThing]
: Crash on login to Excite Japan Blog (exblog.jp) after updating to Firefox 2.0...
Status: VERIFIED FIXED
[sg:critical?]
: crash, jp-critical, regression, testcase, topcrash, verified1.8.1.14, verified1.8.1.15
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 1.8 Branch
: All All
: P1 critical (vote)
: ---
Assigned To: Igor Bukanov
:
Mentors:
http://testingff.exblog.jp/
: 426349 428497 (view as bug list)
Depends on:
Blocks: 411025 425594 426307 CVE-2008-1380
  Show dependency treegraph
 
Reported: 2008-03-27 18:29 PDT by Kohei Yoshino [:kohei]
Modified: 2013-03-19 07:07 PDT (History)
19 users (show)
dveditz: blocking1.8.1.14+
dveditz: blocking1.8.1.15+
caillon: blocking1.8.0.next+
choller: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Apple Crash Report (23.58 KB, text/plain)
2008-03-27 20:28 PDT, Kohei Yoshino [:kohei]
no flags Details
Windows Vista Talkback Log (126.52 KB, text/plain)
2008-03-27 21:44 PDT, Kohei Yoshino [:kohei]
no flags Details
patch, v.0.1, based on Blake's and timeless comments (901 bytes, patch)
2008-03-28 02:26 PDT, John Daggett (:jtd)
igor: review+
brendan: superreview+
Details | Diff | Review
v1 (1.48 KB, patch)
2008-03-28 11:29 PDT, Igor Bukanov
no flags Details | Diff | Review
v2 (4.06 KB, patch)
2008-03-28 12:02 PDT, Igor Bukanov
mrbkap: review+
brendan: review+
dveditz: approval1.8.1.14+
dveditz: approval1.8.1.15+
Details | Diff | Review
web page complete save of excite jp crash page (21.84 KB, application/octet-stream)
2008-04-05 05:00 PDT, John Daggett (:jtd)
no flags Details

Description Kohei Yoshino [:kohei] 2008-03-27 18:29:20 PDT
Some Japanese users reported that they have encountered crashes on login to Excite Blog <http://www.exblog.jp/> after updating to Firefox 2.0.0.13.

http://forums.mozillazine.jp/viewtopic.php?t=7215
http://takublog.exblog.jp/7581761/
http://asfradio.exblog.jp/8538149/
http://sanggar.exblog.jp/6943437/
http://ddrer.exblog.jp/7736962/
http://th69.exblog.jp/7574222/
http://runtotorun.exblog.jp/7735841/

- error message says: The instruction at "0x600f0290" referenced memory at "0x000013e8". The Memory could not be "read". Click OK to terminate the application.
- both on Windows and Mac
- no problem with the "secure mode login" (with SSL)
- no problem when login from the home page, crash when login form "login" links on each blogs
Comment 1 Kohei Yoshino [:kohei] 2008-03-27 20:28:49 PDT
Created attachment 312185 [details]
Apple Crash Report

Confirmed on Windows XP and Mac OS X v10.4.11. Here's the Apple crash report. Talkback was not invoked.
Comment 2 John Daggett (:jtd) 2008-03-27 20:59:48 PDT
(In reply to comment #0)
> Some Japanese users reported that they have encountered crashes on login to
> Excite Blog <http://www.exblog.jp/> after updating to Firefox 2.0.0.13.

Yoshino-san, what are the steps to reproduce this?


Comment 3 Kohei Yoshino [:kohei] 2008-03-27 21:14:23 PDT
1. input username/password on http://www.exblog.jp/
2. wait a few seconds after successful login
3. crash

I didn't see the error message in my comment 0.
Comment 4 Kohei Yoshino [:kohei] 2008-03-27 21:44:03 PDT
Created attachment 312197 [details]
Windows Vista Talkback Log

The crash didn't happen every time.

On Windows Vista, Talkback was invoked instead of the system's crash reporter.
Comment 5 Kohei Yoshino [:kohei] 2008-03-27 21:57:15 PDT
TB43179753
Comment 6 timeless 2008-03-27 22:00:32 PDT
Incident ID: 43179753
Stack Signature	js_MarkGCThing 5dfb3e16
Product ID	Firefox2
Build ID	2008031114
Trigger Time	2008-03-27 21:40:54.0
Platform	Win32
Operating System	Windows NT 6.0 build 6000
Module	js3250.dll + (0001f93d)
URL visited	http://www.exblog.jp
User Comments	
Since Last Crash	100 sec
Total Uptime	283 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8-Release/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 2447
Stack Trace 	
js_MarkGCThing  [mozilla/js/src/jsgc.c, line 2447]
JS_GC  [mozilla/js/src/jsapi.c, line 1879]
nsAppStartup::Run  [mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 152]
main  [mozilla/browser/app/nsBrowserApp.cpp, line 61]
kernel32.dll + 0x43833 (0x76d93833)
ntdll.dll + 0x3a9bd (0x76fba9bd)
Comment 7 John Daggett (:jtd) 2008-03-27 22:14:25 PDT
Used latest 1.8 trunk build, no crash:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.14pre) Gecko/20080327 BonEcho/2.0.0.14pre

Steps used:

1. go to http://www.exblog.jp/
2. using the textboxes at the top left corner, enter:
   user: firefoxcrash
   pw: password
3. unclick checkbox to select non-SSL login
4. click on the orange login button
5. wait several seconds

> no problem when login from the home page, crash when login form "login" links
on each blogs

????

If the login process is not on the main Excite page, please note better steps, thanks.
Comment 8 Kohei Yoshino [:kohei] 2008-03-27 22:36:38 PDT
Crash confirmed with the 1.8 trunk:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.14pre) Gecko/20080327 BonEcho/2.0.0.14pre

TB43180349, same as comment 6.
 
>> no problem when login from the home page, crash when login form "login" links
> on each blogs
> 
> ????
> 
> If the login process is not on the main Excite page, please note better steps,
> thanks.

The crash happens on the home page. Sorry for confusing.
Comment 9 John Daggett (:jtd) 2008-03-27 22:51:03 PDT
OK, this seems to work consistently:

Steps
1. go to http://testingff.exblog.jp/
2. click on the login [ ログイン ] link in the upper left corner
3. using the textboxes at the top left corner, enter:
   user [ エキサイトID ]: firefoxcrash
   pw [ パスワード ]: password
4. leave the SSL checkbox enabled
5. click on the orange sign in button

Result: returns to initial blog page but crashes after a few seconds

Same stack trace as other reports:

#0  0x2302e978 in js_MarkGCThing ()
#1  0x2302f348 in js_GC ()
#2  0x23003c74 in JS_GC ()
#3  0x005c0868 in nsJSContext::Notify ()
#4  0x2c04bb10 in nsTimerImpl::Fire ()
#5  0x2c04bc1c in handleTimerEvent ()
#6  0x2c047aa0 in PL_HandleEvent ()
#7  0x2c0479c4 in PL_ProcessPendingEvents ()
#8  0x907df2ec in __CFRunLoopDoSources0 ()
#9  0x907de81c in __CFRunLoopRun ()
#10 0x907de29c in CFRunLoopRunSpecific ()
#11 0x932abb20 in RunCurrentEventLoopInMode ()
#12 0x932ab1b4 in ReceiveNextEventCommon ()
#13 0x932f0348 in AcquireNextEventInMode ()
#14 0x932f0138 in RunApplicationEventLoop ()
#15 0x00298d2c in nsAppShell::Run ()
#16 0x00336a48 in nsAppStartup::Run ()
#17 0x00012a14 in XRE_main ()
#18 0x0000d768 in start ()
Comment 10 Samuel Sidler (old account; do not CC) 2008-03-27 23:55:55 PDT
See also, bug 425594.
Comment 11 John Daggett (:jtd) 2008-03-27 23:58:08 PDT
Narrowed down the regression range:

Does not occur in:

Firefox 2.0.0.12
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; ja-JP-mac; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.13pre) Gecko/20080215 BonEcho/2.0.0.13pre
Build time: 15-Feb-2008 05:35

But does occur in:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.13pre) Gecko/20080216 BonEcho/2.0.0.13pre
Build time: 16-Feb-2008 08:15

Checkins to mozilla/js during that time:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=mozilla%2Fjs&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-15&maxdate=2008-02-17&cvsroot=%2Fcvsroot

Possible regression caused by patch for bug 411025?



Comment 12 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-03-28 00:39:26 PDT
It's possible. I commented in bug 411025 comment 24 (and 25) about a couple of problems that could cause crashes like this. However, I'd expect to see an error in the error console for either of those problems to occur and when I try to test locally, I don't see any errors in the error console.
Comment 13 John Daggett (:jtd) 2008-03-28 00:44:22 PDT
Not sure this is at all related, but don't we need a JS_POP_TEMP_ROOT to match the JS_PUSH_TEMP_ROOT_FUNCTION before the return here:

http://mxr.mozilla.org/mozilla1.8/source/js/src/jsapi.c#4116

The JS_PUSH_TEMP_ROOT_FUNCTION pushes on a stack-allocated value, so if we somehow "escape" this function that we'll be pointing to random poo on the stack, hence the crash later in jsgc.c at line 2947:

  JSTempValueRooter tvr;

  /* From this point the control must flow through the label out. */
  JS_PUSH_TEMP_ROOT_FUNCTION(cx, fun, &tvr);

But I really don't know this code so I could be way off base here...
Comment 14 timeless 2008-03-28 00:48:14 PDT
bug 411025 comment 25. how timely. good indepedent spot
Comment 15 John Daggett (:jtd) 2008-03-28 00:52:08 PDT
Great, looks like timeless has set up a potential patch for this attached to bug 411025.


Comment 16 John Daggett (:jtd) 2008-03-28 02:26:28 PDT
Created attachment 312233 [details] [diff] [review]
patch, v.0.1, based on Blake's and timeless comments

Simple patch based on Blake's suggestion and the patch timeless set up on 411025.  This patch is a simple delta on the MOZILLA_1_8_BRANCH code.

Fixes the testcase originally reported.

Will try to set up a simpler JS testcase for this to make it easier to verify later.
Comment 17 Reed Loden [:reed] (use needinfo?) 2008-03-28 02:34:41 PDT
Comment on attachment 312233 [details] [diff] [review]
patch, v.0.1, based on Blake's and timeless comments

Patches must have all required review(s) before requesting approval.
Comment 18 Mike Schroepfer 2008-03-28 07:55:09 PDT
Can we get a js testsuite test added for this?
Comment 19 Bob Clary [:bc:] 2008-03-28 08:36:07 PDT
I can't see all the characters on the page, and can't login to get the crashing page. Can someone who can reproduce the crash, save page complete, zip it up and attach here?
Comment 20 Brendan Eich [:brendan] 2008-03-28 09:44:48 PDT
Comment on attachment 312233 [details] [diff] [review]
patch, v.0.1, based on Blake's and timeless comments

No sr required for js/src.

/be
Comment 21 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-28 09:52:41 PDT
Do we need this on 1.9?  I thought the problem was from an error in the 1.8.1 patch.  Can people reproduce it on trunk?
Comment 22 Igor Bukanov 2008-03-28 10:01:37 PDT
(In reply to comment #21)
> Do we need this on 1.9? 

No, this the problem was introduced when I ported trunk's patch into the 1.8 branch.
Comment 23 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-03-28 10:05:57 PDT
It would be good to also address bug 411025 comment 24 at the same time with this patch...
Comment 24 Igor Bukanov 2008-03-28 11:22:53 PDT
Comment on attachment 312233 [details] [diff] [review]
patch, v.0.1, based on Blake's and timeless comments

It is a must to fix the issue that Blake has found here as well.
Comment 25 Igor Bukanov 2008-03-28 11:29:32 PDT
Created attachment 312323 [details] [diff] [review]
v1

This is the previous patch plus the wrong goto fix.
Comment 26 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-03-28 11:30:18 PDT
Comment on attachment 312323 [details] [diff] [review]
v1

Thanks.
Comment 27 Igor Bukanov 2008-03-28 12:02:17 PDT
Created attachment 312333 [details] [diff] [review]
v2

When porting the trunk patch to the branch I not only made the goto/returns bugs, bit I also completely forgot that on 1.8 JSFunction is GCX_PRIVATE and has no associated marking/tracing code. So the patch not only introduced the bug but was completely useless to protect against GC hazards!

The new patch replaces that JS_PUSH_TEMP_FUNCTION insanity by JS_PUSH_TEMP_OBJECT and makes sure that the result of the function is rooted via GCX_OBJECT newborn.
Comment 28 Brendan Eich [:brendan] 2008-03-28 12:25:08 PDT
Comment on attachment 312333 [details] [diff] [review]
v2

Many apologies for the bad review. The 1.8 branch is fading from memory and I made the mistake of ass-u-ming things fresh in mind from the trunk.

/be
Comment 29 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-03-28 12:32:30 PDT
Comment on attachment 312333 [details] [diff] [review]
v2

That makes a lot more sense.
Comment 30 Daniel Veditz [:dveditz] 2008-03-31 14:13:20 PDT
Comment on attachment 312333 [details] [diff] [review]
v2

approved for 1.8.1.14, a=dveditz
Comment 31 John Daggett (:jtd) 2008-03-31 22:29:06 PDT
Tried creating a simple testcase by saving the complete page but the bug doesn't occur with just the saved page unfortunately.  This is related to the use of eval's in the js, correct?  Seems like there should be a much, much simpler way of causing the bad codepath to get executed.

Comment 32 Gen Kanai [:gen] 2008-04-01 02:48:47 PDT
Excite Japan has updated their website to 'fix' this problem, so the need for a quick turn-around on 2.0.0.14 is lessened.

http://blog.excite.co.jp/staff/6962026/

That said, we'll be extra careful and test with Excite Blog to ensure that the changes that Excite Japan made today won't be reflected in the updated branch.
Comment 33 Daniel Veditz [:dveditz] 2008-04-02 00:04:39 PDT
(In reply to comment #32)
> Excite Japan has updated their website to 'fix' this problem, so the need for a
> quick turn-around on 2.0.0.14 is lessened.

This is a new, extremely high-frequency topcrash (especially when you count the new js_GC and js_GetGCThingFlags crashes as the same thing), and almost none of the submitted comments mention Excite. I have not noticed a change in the incoming rates for these crashes after Excite fixed their site--in fact the rate seems to have gone up a little due to the usual increase in the number of mid-week users, totally swamping any relief from Excite Japan. We still need to fix this.

Igor: please land on the MOZILLA_1_8_BRANCH as soon as possible so we can test whether this does seem to help.
Comment 34 Igor Bukanov 2008-04-02 00:33:57 PDT
I checked in the patch from the comment 27 to MOZILLA_1_8_BRANCH:

Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.214.2.42; previous revision: 3.214.2.41
done
Checking in jscntxt.h;
/cvsroot/mozilla/js/src/jscntxt.h,v  <--  jscntxt.h
new revision: 3.80.4.24; previous revision: 3.80.4.23
done
Checking in jsprvtd.h;
/cvsroot/mozilla/js/src/jsprvtd.h,v  <--  jsprvtd.h
new revision: 3.17.20.7; previous revision: 3.17.20.6
done
Comment 35 Igor Bukanov 2008-04-02 00:35:43 PDT
Question: should I mark the bug as fixed1.8.1.14 or fixed1.8.1.15 given that the fix is landed on MOZILLA_1_8_BRANCH?
Comment 36 Samuel Sidler (old account; do not CC) 2008-04-02 08:30:40 PDT
(In reply to comment #35)
> Question: should I mark the bug as fixed1.8.1.14 or fixed1.8.1.15 given that
> the fix is landed on MOZILLA_1_8_BRANCH?

fixed1.8.1.15. 1.8.1.14 will come off a relbranch.

Igor, is there any way to test this programmatically?
Comment 37 Bob Clary [:bc:] 2008-04-02 11:04:27 PDT
fyi, been trying the urls found in talkback and scanning top sites and haven't yet come up with a reproducible test case.
Comment 38 Kohei Yoshino [:kohei] 2008-04-03 00:30:48 PDT
I couldn't create a crash testcase, but the Excite Japan team told us that they only fixed their JavaScript redirection like this:

<html>
<head>
    <script type="text/javascript">
    <!--
    document.location.href= "http://www.exblog.jp/";
    //-->  
    </script>
</head>
<body>
</body>
</html>

Fixed:

<html>
<head>
    <script type="text/javascript">
    <!--
    window.onload = function(){
            window.location.replace("http://www.exblog.jp");
            return;
    }
    //-->  
    </script>
</head>
<body>
</body>
</html>
Comment 39 Igor Bukanov 2008-04-03 00:54:45 PDT
(In reply to comment #38)
> I couldn't create a crash testcase, but the Excite Japan team told us that they
> only fixed their JavaScript redirection like this:
> 
> <html>
> <head>
>     <script type="text/javascript">
>     <!--
>     document.location.href= "http://www.exblog.jp/";
>     //-->  
>     </script>
> </head>
> <body>
> </body>
> </html>

This sample code does not execute the code with the bug in JS_CompileUCFunctionForPrincipals from js/src/jsapi.c.
Comment 40 John Daggett (:jtd) 2008-04-03 01:21:13 PDT
(In reply to comment #39)
> This sample code does not execute the code with the bug in
> JS_CompileUCFunctionForPrincipals from js/src/jsapi.c.

Just to be clear, before Excite JP updated their code I could reproduce this problem using the steps noted in comment 9.  After building with the attached patch v.0.1 (the change in JS_CompileUCFunctionForPrincipals) the crash no longer occurred.

I have the saved webpage at home but with the saved page the crash does not occur.  I'll attach it later tonight, in case that might help.
Comment 41 Daniel Veditz [:dveditz] 2008-04-03 19:38:34 PDT
Comment on attachment 312333 [details] [diff] [review]
v2

approved for 1.8.1.14, a=dveditz for release-drivers.

Please land on the GECKO181_20080311_RELBRANCH for this re-release.
Comment 42 Igor Bukanov 2008-04-04 00:47:07 PDT
I checked in the patch from the comment 27 to GECKO181_20080311_RELBRANCH:

Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.214.2.41.4.1; previous revision: 3.214.2.41
done
Checking in jscntxt.h;
/cvsroot/mozilla/js/src/jscntxt.h,v  <--  jscntxt.h
new revision: 3.80.4.23.4.1; previous revision: 3.80.4.23
done
Checking in jsprvtd.h;
/cvsroot/mozilla/js/src/jsprvtd.h,v  <--  jsprvtd.h
new revision: 3.17.20.6.4.1; previous revision: 3.17.20.6
done
Comment 43 Bob Clary [:bc:] 2008-04-04 23:59:25 PDT
we|i have no idea how to reproduce or create a test for this. 
Comment 44 Samuel Sidler (old account; do not CC) 2008-04-05 02:38:34 PDT
Igor is still trying to find a way to reproduce and create a test for this. Let's give him a bit before we minus this.
Comment 45 John Daggett (:jtd) 2008-04-05 05:00:13 PDT
Created attachment 313799 [details]
web page complete save of excite jp crash page

saved version of page that caused crash, before Excite JP made changes to prevent the crash.  Does *not* crash with saved version but maybe there are clues in here as to why it crashed.
Comment 49 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-04-11 14:11:46 PDT
*** Bug 428497 has been marked as a duplicate of this bug. ***
Comment 50 Matthias Versen [:Matti] 2008-04-18 14:41:41 PDT
*** Bug 426349 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.