226078 - Crash on www.gddkia.gov.pl

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[The Tester]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031030
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031030

This page crashes browser!

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Emmanuel Bourg]

WFM, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031030

Comment 2

[Jon Henry]

Confirmed, this crashes for me in the 20031117 Firebird nightly and Mozilla 1.5
on Win2000. Note that the steps to reproduce are not 100% right it seems...when
that site finishes loading, it redirects to a page with a single link. Clicking
on that link will then load a page which crashes Mozilla. The URL that crashes
every time for me is http://www.gddkia.gov.pl/zima_html/info.htm. I don't have a
talkback ID since recent builds do not have talkback it seems.

Changed URL, original was http://www.gddkia.gov.pl/zima_html/info.htm

Comment 3

[Jon Henry]

Original URL was actually http://www.gddkia.gov.pl/zima_html/utr_tab.htm, I
apologize for the bug spam.

Comment 4

[Olivier Cahagne]

DrWatson shows it possibly crashes in js_Invoke (probably not useful) for FB
20031116 Win2k.

Comment 5

[Boris Zbarsky [:bzbarsky]]

JavaScript strict warning: 
http://www.gddkia.gov.pl/zima_html/utils.js line 263: deprecated with statement
usage

Assertion failure: JSVAL_IS_OBJECT(rval), at
/home/bzbarsky/mozilla/xlib/mozilla/js/src/jsinterp.c:1577

Program received signal SIGABRT, Aborted.

Over to JS engine.

Comment 6

[Hermann Schwab]

TB25753006K
Talkback produced by Mozilla 1.5 on Win98SE
also crashing: 1.4.1, 1.6a, latest trunk, 
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6b) Gecko/20031117

Comment 7

[Boris Zbarsky [:bzbarsky]]

Uh-huh.  

Comment 8

[Hermann Schwab]

TB25753594Y
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031028

Comment 9

[Phil Schwartau]

Attachment: Hermann's stack traces

Comment 10

[Phil Schwartau]

Is this a JS Engine bug, or a bad call from DOM Events code?

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Confirming Linux Mozilla/Firebird recent trunk
crash is caused by function

function SetLangHead(l){
  with(p.head.document){
    for(var i=0 in TytH[l])
      if(getElementById("TxtH"+i)!=undefined)
        getElementById("TxtH"+i).innerHTML=TytH[l][i]
    if(getElementById("TxtHM"))
      getElementById("TxtHM").innerHTML=tyts[l][p.PageIdx]
  }
}

from utils.js.

Gecko crashes on  "if(getElementById("TxtH"+i)!=undefined)" part.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Any JS that crashes the JS engine would be a bug in the JS engine code...

Comment 13

[Brendan Eich [:brendan]]

Re: comment 12: we've had bugs where a bad (dangling) JSContext* was passed into
the JS engine, reproducibly, due to content tickling a bug in dialog code.  JS
crashes the JS engine != JS engine bug.

However, in this case, the testcase does demonstrate a big bad JS engine bug!
;-) Taking for 1.6b.

/be

Comment 14

[Brendan Eich [:brendan]]

Here's a non-reduced JS shell testcase:

function SetLangHead(l){
  with(p.head.document){
    for(var i in TytH[l])
      if(getElementById("TxtH"+i)!=undefined)
        getElementById("TxtH"+i).innerHTML=TytH[l][i]
    if(getElementById("TxtHM"))
      getElementById("TxtHM").innerHTML=tyts[l][p.PageIdx]
  }
}
TytH=[0,1,2,3];
p={head:{document:{getElementById: function (id){print(uneval(this), id); return
undefined;}}}};
SetLangHead(1);

If you remove the bogus initializer for var i, so the for/in loop begins "for
(var i in ...)...", then all is well.

/be

Comment 15

[Boris Zbarsky [:bzbarsky]]

> If you remove the bogus initializer for var i

I assume that was supposed to be |for (var i = 0 in .. .)| in the code you
pasted?

Point taken about bad JSContexts, but I guess I don't think of that case as "JS
crashes JS engine" but "DOM glue crashes JS engine"...  I suppose that's not a
useful distinction without staring at the code...

Comment 16

[Brendan Eich [:brendan]]

Duh, yeah: here is a reduced JS shell testcase.  The salient badness is the =0
bit after 'for(var i' and before ' in x)':

function SetLangHead(l){
  with(p){
    for(var i=0 in x)
      if(getElementById("TxtH"+i)!=undefined)
        print('huh');
  }
}
x=[0,1,2,3];
p={getElementById: function (id){print(uneval(this), id); return undefined;}};
SetLangHead(1);

Fun, eh?  Old bug, fix coming in a few minutes, assuming no Comdex mozilla booth
traffic uptick.

/be

Comment 17

[Brendan Eich [:brendan]]

Can someone evangelize that site to get rid of the bogus =0?

/be

Comment 18

[Brendan Eich [:brendan]]

Attachment: proposed fix

I took this opportunity to expand ancient tab evil in jsopcode.c.

The jsopcode.c fix helps us decompile for (var i in o) properly, for both the
heavyweight (wrapped in with) and lightweight function cases (i is a local var
in both cases, but we must look it up by name in the with-wrapped heavyweight
case).

The jsparse.c change is the main fix.  Ancient bug!

/be

Comment 19

[Mike Shaver (:shaver emeritus)]

Comment on attachment 135858 [details] [diff] [review]
proposed fix

Sure -- run the testsuite?

Comment 20

[Hermann Schwab]

requesting blocking 1.4.2 as 1.4.1 was also crashing.

Comment 21

[Brendan Eich [:brendan]]

Fixed (testsuite passed, but it needs some additions to cover all the
lightweight vs. heavyweight vs. for (var i in o) vs. for (var i = 0 in o) cases).

mkaply is driving 1.4.2 -- I'll let him approve the bug, then develop a branch
patch.

/be

Comment 22

[Mike Kaply [:mkaply]]

I'd like this for 1.4.2

Comment 23

[Bob Clary [:bc] (inactive)]

Checking in regress-226078.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-226078.js,v  <--  regress-226078.js
initial revision: 1.1
done

Comment 24

[Bob Clary [:bc] (inactive)]

verified fixed.