494208 - Crash in js3250.dll when java applet (java plugin older than 1.6.0_07) loads if IE Tab or Noscript extensions are installed [@ ClaimTitle]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Michael Kraft [:morac]]

On my work PC, Shiretoko will crash the moment a java applet starts to load if either IE Tab 1.5.20090207 and/or NoScript 1.9.2.8 are installed and enabled.  This only happens in the Shiretoko 20090517 and later versions.  Neither Firefox 3.0.0.10 nor the trunk nightlies will crash.

It makes no difference if JIT is enabled or not.  My work PC has java version 1.6.0_02 on it.  I can't update the java version on my work PC so I'll need to test my home PC to see if this is a problem with the latest version of java or not.  Seeing how I'm seeing reports of the same crash on the Crash Stats site and a number of them mention java, I'm guessing the java version may not matter.

Here's the crash dump in Shiretoko 20090521.  It's pretty useless though since it just lists js3250.dll@0x6a3f4 as the crash address.  The crash address is consistent as the crash always occurs @0x6a3f4 in the 20090521 release.  Each release has a different crash address.

http://crash-stats.mozilla.com/report/index/f15c0467-18ac-4057-ba1f-312ce2090521


Since the problem in the Shiretoko 20090517 load, the problem had to be caused by something checked in 20090516. That would be one of these:

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=May+15%2C+2009&enddate=May+17%2C+2009


I tried to find an existing bug report on this since the problem does not occur in the Trunk nightly loads and I figured it might have been fixed already, but was unable to do so.  I also tried older versions of Minefield and could not reproduce.

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Could use help with a regression range on TM, and testing with more configs.

Comment 2

[u279076]

Tried to reproduce this with Shiretoko 20090521 on Ubuntu 9.04 32-bit with No-script 1.9.2.8 using JRE 1.6.0_13.  Does not crash.  Maybe Windows only?

Comment 3

[Aakash Desai [:aakashd]]

WFM on build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.11) Gecko/2009051909 Firefox/3.0.11 (.NET CLR 3.5.30729) ID:2009051909


with IE Tab plugin and Java 1.6.0_13

Comment 4

[Tracy Walker [:tracy]]

I can't reproduce that crash on windows XP with Shiretoko nightly from 20090517 using Noscript 1.9.2.8 with Java Plug-in 1.6.0_11  Nor was I able to reproduce with IE Tab 1.5.20090207 added to the above

The older Java Plugin version is looking like the culprit.

Comment 5

[Aakash Desai [:aakashd]]

Confirmed: Crash on Java version 1.6.0_02 with the same build Id and IE Tab plugin.

Comment 6

[Michael Kraft [:morac]]

It looks like this is most likely only an issue with older versions of Java. Unfortunately I'm not an administrator on my work PC so I can't upgrade the Java version to test this out.  I can test it on my home PC later today.  It's curious though as to why this only affects Shiretoko and not Minefield.


I found a few crash reports for Shiretoko 20090520 with different java versions of java.dll, but the latest one I could find is 6.0.70.6 (possibly version 1.6.0_07):
http://crash-stats.mozilla.com/report/index/7da8d78d-a9a7-48a6-b197-2da4c2090521
http://crash-stats.mozilla.com/report/index/ce079ba6-37d9-40fa-bd83-ff8b42090520
http://crash-stats.mozilla.com/report/index/488224f8-b9bb-49db-9696-4f0252090521

6.0.30.5 (guessing version 1.6.0_03):
http://crash-stats.mozilla.com/report/index/3ef5aff2-cae9-4991-bd17-eca332090520

Here's mine with version 6.0.20.6 (java 1.6.0_02):
http://crash-stats.mozilla.com/report/index/80ccb60b-89fc-4192-aab8-545402090521


I'll need to check what the latest version of java.dll is when I get home.  It's possible this is a non-issue with JRE 1.6.0_13 and realistically every one should upgrade to that, but sometimes it's not possible, like in my case.  I've told the IT guys they should upgrade Java for security reasons, but they haven't bothered to do it.

In the mean time here are two crash addresses in case anyone wants to look them up on Crash Stats:
Shiretoko 20090520: js3250.dll@0x6a246
Shiretoko 20090521: js3250.dll@0x6a3f4

Comment 7

[Tracy Walker [:tracy]]

If I let http://java.sun.com/applets/jdk/1.4/demo/applets/Clock/example1.html in the config mentioned in comment #4 then let it run for a while, 10-15 minutes later Firefox crashes.  However, crash reporter doesn't fire.  hmmmm?

Comment 8

[Aakash Desai [:aakashd]]

Found the regression range: any version of java before 1.6.0_07 (not including) either hangs the browser or crashes it in this case. I'm removing the qawanted keyword and adding relnote for a nomination to take care of users with older versions of java.

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

What's the offending commit in _our_ code, though -- this worked prior to 20090517 and works on the trunk now, so it seems like we can fix it ourselves too?

Comment 10

[Aakash Desai [:aakashd]]

The issue first starts in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090517 Shiretoko/3.5b5pre (.NET CLR 3.5.30729) ID:20090517065612

Comment 11

[Tony Chung [:tchung]]

(In reply to comment #9)
> What's the offending commit in _our_ code, though -- this worked prior to
> 20090517 and works on the trunk now, so it seems like we can fix it ourselves
> too?

Shaver, is the pushlog range in comment 0 for mozilla-1.9.1 not enough to tell? 

> http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?startdate=May+15%2C+2009&enddate=May+17%2C+2009

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

No, there are like 100 commits in there, many of them in the JS engine.  We need to know which of those (using tracemonkey hourlies might help) to know how to fix the issue in our side on 1.9.1.x.

Alternatively/additionally, we want to know which of the commits that's now on trunk fixed it.  That would let us work from the other side.

Comment 13

[Michael Kraft [:morac]]

I used the hourly releases from http://hourly-archive.localgho.st/hourly-archive2/mozilla-1.9.1-win32/ and narrowed down the regression range.

Good: 1242494055-20090516101415-df1677120450-firefox-3.5b5pre.en-US.win32.zip
Bad: 1242499556-20090516114556-a1319ee53a74-firefox-3.5b5pre.en-US.win32.zip


So that means the problem was caused by one of these:

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=df1677120450&tochange=a1319ee53a74

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Are there localghost hourlies for tracemonkey?  That's a pretty huge regression range still. :-/

Comment 15

[Michael Kraft [:morac]]

I don't know.  The only reason I knew about that site was because someone posted a link to it in bug 494199, which also needs local hourlies for tracemonkey.

Comment 18

[Aakash Desai [:aakashd]]

Are the hourlies still available somewhere?

Comment 19

[Tony Chung [:tchung]]

There are only http://hourly-archive.localgho.st/win32.html, but it doesnt archive back to may 17th.

I guess my question remains: we know the regression build from comment 10, we have the list of testcases in this bugs's URL and the list from bug 494977, and we have the wide regression range from comment 0.  Given that hourly builds dont trace that far back, what are other things we can do to help get you what you need?

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We could use hg bisect to get the patch which caused this. I could do this tomorrow morning (in around 5h). We have a VM ready for that from the QA storage. Al will this be working OOTB?

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can somebody please tell me where I can download the Java6u3 installer? I cannot find any website which offers this old version. Every page redirects to the latest installer. :/

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Nevermind, got it via IRC really quick: http://java.sun.com/products/archive/

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, I'm able to reproduce the crash with latest Shiretoko on Vista. Running the nightly build in parallel with WinDBG shows me the following output:

command window
==============
(a14.e0c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00c1e000 ebx=6b6cc4cc ecx=00000e0c edx=053e6560 esi=00bcc000 edi=a7a4a2a1
eip=6ca5a2c3 esp=003af6e4 ebp=00bbac00 iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010286
*** WARNING: Unable to verify checksum for C:\mozilla\bin\Shiretoko\js3250.dll
js3250!ClaimTitle+0x57773:
6ca5a2c3 39879c010000    cmp     dword ptr [edi+19Ch],eax ds:0023:a7a4a43d=????????
*** WARNING: Unable to verify checksum for C:\mozilla\bin\Shiretoko\nspr4.dll
*** WARNING: Unable to verify checksum for C:\mozilla\bin\Shiretoko\xul.dll

It makes me thinking something is wrong with ClaimTitle which is a function inside jslock.cpp. Looking at the hg log of this file there was a checkin on May the 16th: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/720ed363ce2d

This perfectly fits into the above regression range and is also listed under the first comment from Robert. That means I blame bug 491126 (Sharing object map for fast array instances) to be the cause here. CC'ing Igor and Brendan to this bug.

Removing qawanted keyword since this has been successfully processed by QA for now.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: WinDBG stack

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

No idea if we need to block here but for safety lets ask.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: locals output

And here the values of the variables.

Comment 27

[Michael Kraft [:morac]]

Bug 491126 is also in the trunk though and it doesn't look like any of those files have been changed since the fix for bug 491126 landed.  Any idea why the fix for bug 491126 would crash Shiretoko, but not Minefield?

Comment 28

[Masatoshi Kimura [:emk]]

(In reply to comment #27)
Probably because this bug is related to LiveConnect to old plug-in.
I could reproduce this using 1.6.0_13 when I unchecked "next-generation Java Plug-in" in Java Control Panel.
Minefield has removed a support for LiveConnect to old plug-in (see bug 485984).
Unfortunately it will not be acceptable to 191 branch, of course.

Comment 29

[Michael Kraft [:morac]]

I'm guessing the "fix" then is to basically add a release note telling users to upgrade to the latest version of Java or to disable the java plug-in for those of us who can't.  

I'm curious if this is also a problem with Java 5.0.  I'm guessing it would be.

Comment 30

[Masatoshi Kimura [:emk]]

Isn't backing out bug 491126 an option?

Comment 31

[Igor Bukanov]

The bug is a regression from the bug 489501 - I forgot to check for native objects in js_SetProtoOrParent. The bug 491126 just exposed that.

Comment 32

[Igor Bukanov]

Attachment: fix v1

The patch adds the missing check for a native object in js_SetProtoOrParent when making sure that obj owns the scope.

Comment 33

[Robert Sayre]

Comment on attachment 380248 [details] [diff] [review]
fix v1

This function returns JSBool, so it should return JS_FALSE on error.

Comment 34

[Igor Bukanov]

(In reply to comment #33)
> (From update of attachment 380248 [details] [diff] [review])
> This function returns JSBool, so it should return JS_FALSE on error.

Why?

Comment 35

[Robert Sayre]

(In reply to comment #34)
> (In reply to comment #33)
> > (From update of attachment 380248 [details] [diff] [review] [details])
> > This function returns JSBool, so it should return JS_FALSE on error.
> 
> Why?

because you need "!!" just to coerce it right? Wouldn't it be cleaner if it was a JSBool?

Comment 36

[Igor Bukanov]

(In reply to comment #35)
> because you need "!!" just to coerce it right? Wouldn't it be cleaner if it was
> a JSBool?

No: !! is necessary to convert JSBool into bool without warnings. The compiler does not see that JSBool value can have only 0 and 1 as values so it can not simply treat the bits stored in JSBool as bool. A good compiler warns about that.

On the other hand the implicit conversion from a boolean (especially from true/false compile-time constant) into int is trouble-free.

Comment 37

[Brendan Eich [:brendan]]

In particular, we have true and false nicely used all over jstracer.cpp and some other new-ish code, but we're forced to keep using JSBool FASTCALL return types, for ABI reasons. No warnings from return true or return false in such a function.

/be

Comment 38

[Brendan Eich [:brendan]]

Comment on attachment 380248 [details] [diff] [review]
fix v1

Construing sayrer's point narrowly misses the fact (also not visible in the patch's context) that the other two returns use JS_TRUE and JS_FALSE. It seems better to use JS_FALSE (or ok) than to mix false in one third of the returns of literal boolean value-codes.

r=me with that nit addressed.

/be

Comment 39

[Igor Bukanov]

pushed to TM with an extra change to use good old JS_FALSE in place of all too modern false: 

http://hg.mozilla.org/tracemonkey/rev/a3dcea4508e6

Comment 40

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/a3dcea4508e6

Comment 41

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f7d59bc2b3d5

Comment 42

[Michael Kraft [:morac]]

Verified Fixed:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090601 Firefox/3.5b5pre

Comment 43

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yes, looks good with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090601 Shiretoko/3.5pre

Comment 44

[Michael Lefevre]

Re-marking verified

Comment 45

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #44)
> Re-marking verified

It has not been verified on trunk yet. Which will even be hard because we don't run the old plugin anymore.

Comment 46

[Mike Beltzner [:beltzner, not reading bugmail]]

(removing relnote tag)