271716 - crash on infinite loop creating new arrays [@ JS_TypeOfValue]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

cvs from yesterday. loading the attached testcase sometimes crashes (usually it
gives me the "do you want to abort" dialog, if I say no I get it again after
some time)

stack:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1081527136 (LWP 4345)]
0x40189139 in JS_TypeOfValue (cx=0x8c85500, v=147277832) at
/home/chb/mozilla/js/src/jsapi.c:608
608                 ops = obj->map->ops;
Current language:  auto; currently c
(gdb) bt
#0  0x40189139 in JS_TypeOfValue (cx=0x8c85500, v=147277832) at
/home/chb/mozilla/js/src/jsapi.c:608
#1  0x41610c9d in nsEventReceiverSH::SetProperty (this=0x8255890,
wrapper=0xae1e108, cx=0x8c85500, obj=0x8c73a98,
    id=137200692, vp=0xbfffdbac, _retval=0xbfffdafc) at
/home/chb/mozilla/dom/src/base/nsDOMClassInfo.cpp:5212


I'll attach the full stack as an attachment.

(gdb) print obj->map
$2 = (JSObjectMap *) 0x0

(gdb) print *obj
$3 = {map = 0x0, slots = 0x8c736b9}

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: stack trace

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: testcase

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: stack trace with local variables

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

I should mention: the testcase is from
http://www.heise.de/newsticker/meldung/53644 (a german news site), which claims
this crashes each time (also opera, msie)

Comment 5

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041122
A reduced testcase crashes immediately without warning

<SCRIPT>
a = new Array();
while (1) a = new Array(a);
</SCRIPT>

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB2174006K

Comment 6

[OstGote!]

see also bug 271718, Bug 271739, and Bug 271734 (all different stack traces)

Comment 7

[Brendan Eich [:brendan]]

comment 5 dups bug 203278, please don't mix unrelated signatures.  Let's use
this bug to track the JS_TypeOfValue crash.

/be

Comment 8

[Brendan Eich [:brendan]]

*** Bug 271822 has been marked as a duplicate of this bug. ***

Comment 9

[Brendan Eich [:brendan]]

*** Bug 272136 has been marked as a duplicate of this bug. ***

Comment 10

[Brendan Eich [:brendan]]

*** Bug 272296 has been marked as a duplicate of this bug. ***

Comment 11

[Dmbtech]

I was wondering is this is at all related to bug 203278 with the infinite
objects-creating loop.  Maybe a simular algorithm to prevent this can fix this
bug? Also, i believe we should nominate the fix if there is a fix for fx 1.0.1.
That my opinion. I believe this could lead to a DOS attack.

Comment 12

[Brendan Eich [:brendan]]

Re: comment 11: please read comment 7 again.  Bug 203278 fixes all stack
overflow crashes in the mark phase of the JS GC, there's no need for a similar
anything. This bug is tracking a different crash signature from a stack overflow
that users reported seeing with the same testcase.

Anyone who has the fix for bug 203278, please retest and report your results
here -- if you cannot recreate the stack signature in attachment 167016 [details], or any
other bad outcome, then this bug was probably fixed recently, independently of
the GC mark phase stack overflow bug.

/be

Comment 13

[Brendan Eich [:brendan]]

Attachment: proposed fix for stack at attachment 167016

The interpreter has failed to protect newborn function objects, and catch
variable initial values, against recycling by the GC, should a JSClass
addProperty hook do two more object allocations, the second of which runs up
against a memory limit and nests a last ditch GC.

There are other problems in or near XBL with handling OOM, as these warnings
show:

JavaScript error: , line 0:
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsDOMClassInfo.cpp, line
1223
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsDOMClassInfo.cpp, line
1223
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsXBLProtoImpl.cpp, line
116
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed, file nsXBLProtoImpl.cpp, line
77

After these and more like them, my build eventually crashed here:

#0  0x015b4b62 in nsXBLPrototypeBinding::InitClass (this=0x97c1cd8,
    aClassName=@0x97a3a78, aContext=0x0, aScriptObject=0x0,
    aClassObject=0xfef361c8) at nsXBLPrototypeBinding.cpp:734
#1  0x015bfe86 in nsXBLProtoImpl::CompilePrototypeMembers (this=0x97a3a78,
    aBinding=0x97c1cd8) at nsXBLProtoImpl.cpp:155
#2  0x015bfc5a in nsXBLProtoImpl::InitTargetObjects (this=0x97a3a78,
    aBinding=0x97c1cd8, aContext=0x93ae0e8, aBoundElement=0x93938b8,
    aScriptObject=0xfef361c8, aTargetClassObject=0xfef362bc)
    at nsXBLProtoImpl.cpp:97

Anyone (timeless) know of a bug on file for this already?

/be

Comment 14

[timeless]

the crashes and asserts sound familiar, gmail doesn't recognize it.

just a reminder, you own bug 262948.

Comment 15

[Kevin Brosnan]

*** Bug 273871 has been marked as a duplicate of this bug. ***

Comment 16

[Brendan Eich [:brendan]]

> just a reminder, you own bug 262948.

I know my buglist -- how about you keep unnecessary nags out of unrelated bugs.
 Use email if you must, but save it for bugs that actually have priority.

/be

Comment 17

[Brendan Eich [:brendan]]

Attachment: proposed fix, v2

Slightly better, use obj2 temp to save/restore fp->scopeChain.

Any addProperty implementation that evaluates in the current frame will find a
scope chain that includes the function being defined, which is odd, but this
seems worth swallowing for fast LIFO rooting.

/be

Comment 18

[Brendan Eich [:brendan]]

Attachment: proposed fix, v3

The previous patch could leave fp->scopeChain "pushed" to protect the newborn
function object, wrongly.  This always pops before any goto out with (!ok).

While reviewing the previous patch, the general problem of scope chain popping
on error provoked me to make a testcase and file bug 273931.

/be

Comment 19

[Brendan Eich [:brendan]]

Comment on attachment 168328 [details] [diff] [review]
proposed fix, v3

This is ready for review.

Thanks for the good stack, biesi!  If anyone gets the XBL or DOM crash after
out-of-memory that I mentioned in comment 13, feel free to file a bug.	I
suspect OOM handling is a trail of tears, and I'm not ready to march down it,
although I believe timeless has gone there and seen the skulls, repeatedly.

/be

Comment 20

[Mike Shaver (:shaver emeritus)]

Comment on attachment 168328 [details] [diff] [review]
proposed fix, v3

>              * However, we could be in a Program being eval'd from inside a
>-             * with statement, so we need to distinguish variables object from
>-             * scope chain head.  Hence the two assignments to parent below.
>+             * with statement, so we need to distinguish scope chain head from
>+             * variables object.  Hence the obj2 vs. parent distinction below.
>              * First we make sure the function object we're defining has the
>              * right scope chain.  Then we define its name in fp->varobj.

Have to say, |obj2| doesn't scream "scope chain head" to me.  A savedScopeChain
local
would make this code easier to analyze outside the context of this patch,
especially
because obj2 is often object-in-which-lookup-found-property elsewhere in our
beloved monkey.  (It's no more clear for that, but history does have its
advantages.)

>           case JSOP_INITCATCHVAR:
>-            /* Pop the property's value into rval. */
>+            /* Load the property's value into rval. */
>             JS_ASSERT(sp - fp->spbase >= 2);
>-            rval = POP_OPND();
>+            rval = FETCH_OPND(-1);
> 
>             /* Get the immediate catch variable name into id. */
>             atom = GET_ATOM(cx, script, pc);
>             id   = ATOM_TO_JSID(atom);
> 
>             /* Find the object being initialized at top of stack. */
>-            lval = FETCH_OPND(-1);
>+            lval = FETCH_OPND(-2);
>             JS_ASSERT(JSVAL_IS_OBJECT(lval));
>             obj = JSVAL_TO_OBJECT(lval);
> 
>             /* Define obj[id] to contain rval and to be permanent. */
>             SAVE_SP(fp);
>             ok = OBJ_DEFINE_PROPERTY(cx, obj, id, rval, NULL, NULL,
>                                      JSPROP_PERMANENT, NULL);
>             if (!ok)
>                 goto out;
>+            sp--;
>             break;
> #endif /* JS_HAS_EXCEPTIONS */

A comment above the otherwise easy-to-miss sp-- explaining that we're now
popping
the property's rval (instead of earlier, which would leave it unprotected
during
the call out to OBJ_DEFINE_PROPERTY) would make me even happier with this
otherwise-righteous patch.

Comment 21

[Brendan Eich [:brendan]]

I heard recently, again (dbaron is my source, IIRC), that every block local adds
another stupid stack slot, at least with gcc-latest.  So I would rather not add
to the stack budget for js_Interpret, if you can live with obj2 here.  The
extent of the use of this generically-named variable is short, several dozen
lines in each case.  I could #define and #undef a local synonym, but that's
pretty ugly.

That deferred sp--; is done elsewhere without comment, but sure -- I'll comment.

/be

Comment 22

[Brendan Eich [:brendan]]

Fixed.  This might be good to fix on active branches.  Nominating.

/be

Comment 23

[Ben Fowler]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a6) Gecko/20041212
Firefox/1.0+

(Today's checkout and build despite the Gecko number)

To encourage communication amongst actual and potential talented testers of
Firefox and other browsers, it might be desirable to offer some feedback to 
Heise along the lines that despite the handicap of the umständlich Bugzilla

Es gibt jetzt eine Korrektur (oder Richtigstellung) für Firefox.

Comment 24

[Asa Dotzler [:asa]]

1.7.5 has shipped. Moving request to 1.7.6.

Comment 25

[Erik Fabert]

*** Bug 279666 has been marked as a duplicate of this bug. ***

Comment 26

[Daniel Veditz [:dveditz]]

Get it in for 1.7.6.  Is this wanted for ff1.0.1 too?

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 168328 [details] [diff] [review]
proposed fix, v3

a=1.7.6 drivers

Comment 28

[Martijn Wargers (dead)]

*** Bug 281968 has been marked as a duplicate of this bug. ***

Comment 29

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-271716-n.js

Christian, with your permission this will be added to the javascript test
library.

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

fine with me, but according to http://www.heise.de/newsticker/meldung/53644
where I got the testcase from it actually is from Berend-Jan Wever.

Comment 31

[Bob Clary [:bc] (inactive)]

Ok. I contacted him. Thanks.

Comment 32

[Bob Clary [:bc] (inactive)]

js1_5/Regress/regress-271716-n.js checked in.

Comment 33

[OstGote!]

(In reply to comment #27)
> (From update of attachment 168328 [details] [diff] [review] [edit])
> a=1.7.6 drivers

The patch has a=1.7.6 but is as I see still not checked in the 1.7 branch! I see
only checkins for bug 277069.

Comment 34

[Brendan Eich [:brendan]]

This didn't make 1.0.1 so it should not go in 1.7.6 -- the two are supposed to
have matching Gecko/JS/etc. back-ends.

Renominate if and when there is a blocking 1.7.x/1.0.2 flag pair.

/be

Comment 35

[Gerd Hafenbrack]

(In reply to comment #34)
> This didn't make 1.0.1 so it should not go in 1.7.6

Bad. It missed already 1.7.5.
Why didn't it make 1.0.1?
Daniel Veditz asked that in comment #26.

Comment 36

[Brendan Eich [:brendan]]

Sorry, this wasn't a priority for 1.0.1, which rolls up various security and
some topcrash fixes.  It should have been nominated for 1.0.1 as well as 1.7.6,
then it probably would have been considered.  The 1.0.1 drivers did not go over
all 1.7.6 nominations.

No point in saying "Bah" about this bug's fix missing 1.7.5, since the patch
wasn't developed until a month after Firefox 1.0 shipped, and 1.7.5
Gecko/JS/etc. must match Firefox/Thunderbird 1.0 Gecko/JS/etc.

/be

Comment 37

[Jesse Ruderman]

*** Bug 284615 has been marked as a duplicate of this bug. ***

Comment 38

[Frank Wein [:mcsmurf]]

*** Bug 271739 has been marked as a duplicate of this bug. ***

Comment 39

[Daniel Veditz [:dveditz]]

*** Bug 300715 has been marked as a duplicate of this bug. ***

Comment 40

[Brendan Eich [:brendan]]

Attachment: patch for 1.0.x and 1.7.x

Comment 41

[Wolfgang Rosenauer [:wolfiR]]

patch is checked in to branches

Comment 42

[Marcia Knous [:marcia]]

I know this bug was reported on Linux, but the testcase crashes today's Windows build - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060213 Firefox/1.0.8. I will go check the Linux behavior next.

Comment 43

[Marcia Knous [:marcia]]

Crashes on Today's Linux build (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060213 Firefox/1.0.8) as well with the testcase listed in the bug. In the terminal windows I saw this:
*** nsExtensionManager::_disableObsoleteExtensions - failure, catching exception  so finalize window can close
*** loading the extensions datasource
*** loading the extensions datasource
./run-mozilla.sh: line 159: 26241 Segmentation fault      "$prog" ${1+"$@"}

Talkback did not come up. Reopening and changing OS to All.

Comment 44

[Bob Clary [:bc] (inactive)]

I think that crash is the same as mentioned in comment 13.

Comment 45

[Bob Clary [:bc] (inactive)]

fwiw, my 2006-02-11 trunk debug build shows a stack like that in bug 294831 when running js1_5/Regress/regress-271716-n.js in the browser. You need to click on a menu item or someother browser ui to crash.

I think this is a new bug and this bug should be marked fixed again.

Comment 46

[Marcia Knous [:marcia]]

My Windows Talkback ID incident is 15139705. The Crash has js_MarkGCThing all over it.

Comment 47

[Bob Clary [:bc] (inactive)]

Marcia's stack looks like:

js_Mark  [c:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3978]
js_MarkGCThing  [c:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 865]
js_MarkGCThing  [c:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 919]
ditto.

Running 1.0.8/winxp/debug from this morning on the attached testcase I hit an assert

JS_PUBLIC_API(void *)
JS_GetPrivate(JSContext *cx, JSObject *obj)
{
    jsval v;

=>    JS_ASSERT(OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE);
    v = GC_AWARE_GET_SLOT(cx, obj, JSSLOT_PRIVATE);
    if (!JSVAL_IS_INT(v))
        return NULL;
    return JSVAL_TO_PRIVATE(v);
}

JS_GetPrivate(JSContext * 0x03bafcf0, JSObject * 0x03bbb700) line 2003 + 231 bytes
nsScriptSecurityManager::GetFunctionObjectPrincipal(JSContext * 0x03bafcf0, JSObject * 0x03bbb700, JSStackFrame * 0x0012e85c, nsIPrincipal * * 0x0012e3b8) line 1842 + 14 bytes
nsScriptSecurityManager::GetFramePrincipal(JSContext * 0x03bafcf0, JSStackFrame * 0x0012e85c, nsIPrincipal * * 0x0012e3b8) line 1916 + 24 bytes
nsScriptSecurityManager::GetPrincipalAndFrame(JSContext * 0x03bafcf0, nsIPrincipal * * 0x0012e3b8, JSStackFrame * * 0x0012e378) line 1940 + 20 bytes
nsScriptSecurityManager::GetSubjectPrincipal(JSContext * 0x03bafcf0, nsIPrincipal * * 0x0012e3b8) line 1980
nsScriptSecurityManager::GetSubjectPrincipal(nsScriptSecurityManager * const 0x00ee6340, nsIPrincipal * * 0x0012e3b8) line 1630
nsScriptSecurityManager::SubjectPrincipalIsSystem(nsScriptSecurityManager * const 0x00ee6340, int * 0x0012e3cc) line 1663 + 36 bytes
nsContentUtils::IsCallerChrome() line 920 + 21 bytes
PresShell::HandleEventInternal(nsEvent * 0x0012e748, nsIView * 0x03180070, unsigned int 0x00000001, nsEventStatus * 0x0012e574) line 6027 + 5 bytes
PresShell::HandleEvent(PresShell * const 0x032353d4, nsIView * 0x03180070, nsGUIEvent * 0x0012e748, nsEventStatus * 0x0012e574, int 0x00000001, int & 0x00000001) line 5921 + 25 bytes
nsViewManager::HandleEvent(nsView * 0x03180070, nsGUIEvent * 0x0012e748, int 0x00000000) line 2275
nsViewManager::DispatchEvent(nsViewManager * const 0x030fb540, nsGUIEvent * 0x0012e748, nsEventStatus * 0x0012e6ac) line 2061 + 20 bytes
HandleEvent(nsGUIEvent * 0x0012e748) line 77
nsWindow::DispatchEvent(nsWindow * const 0x031800fc, nsGUIEvent * 0x0012e748, nsEventStatus & nsEventStatus_eIgnore) line 1067 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012e748) line 1088
nsWindow::DispatchFocus(unsigned int 0x00000069, int 0x00000001) line 5451 + 15 bytes
nsWindow::ProcessMessage(unsigned int 0x00000007, unsigned int 0x003002da, long 0x00000000, long * 0x0012ebac) line 4194 + 23 bytes
nsWindow::WindowProc(HWND__ * 0x005a0388, unsigned int 0x00000007, unsigned int 0x003002da, long 0x00000000) line 1349 + 27 bytes
USER32! 77d48734()
USER32! 77d48816()
USER32! 77d4b4c0()
USER32! 77d4b50c()
NTDLL! 7c90eae3()
GlobalWindowImpl::Focus(GlobalWindowImpl * const 0x02e9dc8c) line 2779 + 25 bytes
nsWebShellWindow::HandleEvent(nsGUIEvent * 0x0012ef08) line 610
nsWindow::DispatchEvent(nsWindow * const 0x031bad6c, nsGUIEvent * 0x0012ef08, nsEventStatus & nsEventStatus_eIgnore) line 1067 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012ef08) line 1088
nsWindow::DispatchFocus(unsigned int 0x00000069, int 0x00000001) line 5451 + 15 bytes
nsWindow::ProcessMessage(unsigned int 0x00000007, unsigned int 0x00000000, long 0x00000000, long * 0x0012f36c) line 4194 + 23 bytes
nsWindow::WindowProc(HWND__ * 0x003002da, unsigned int 0x00000007, unsigned int 0x00000000, long 0x00000000) line 1349 + 27 bytes
USER32! 77d48734()
USER32! 77d48816()
USER32! 77d4b4c0()
USER32! 77d4b50c()
NTDLL! 7c90eae3()
USER32! 77d4b3f9()
USER32! 77d4b393()
nsWindow::DefaultWindowProc(HWND__ * 0x003002da, unsigned int 0x00000006, unsigned int 0x00000001, long 0x00000000) line 1375
USER32! 77d48734()
USER32! 77d48816()
USER32! 77d4c63f()
USER32! 77d4c665()
nsWindow::WindowProc(HWND__ * 0x003002da, unsigned int 0x00000006, unsigned int 0x00000001, long 0x00000000) line 1356 + 31 bytes
USER32! 77d48734()
USER32! 77d48816()
USER32! 77d4b4c0()
USER32! 77d4b50c()
NTDLL! 7c90eae3()
USER32! 77d49402()
PeekKeyAndIMEMessage(tagMSG * 0x0012f8d4 {msg=0x00000113 wp=0x00003f9b lp=0x01dc6f70}, HWND__ * 0x00000000) line 90 + 24 bytes
nsAppShell::Run(nsAppShell * const 0x02dcd788) line 128 + 11 bytes
nsAppShellService::Run(nsAppShellService * const 0x02dcd6c8) line 495
xre_main(int 0x00000003, char * * 0x003e6db8, const nsXREAppData * 0x0041e01c kAppData) line 1907 + 35 bytes
main(int 0x00000003, char * * 0x003e6db8) line 58 + 18 bytes
mainCRTStartup() line 338 + 17 bytes

I don't crash in 1.5.0, 1.5, trunk/winxp/debug builds from this morning.

Comment 48

[Brendan Eich [:brendan]]

Please file a new bug.  Marcia's stack is not this bug, this bug's fix is intact, so it makes no sense to reopen this bug.  Thanks,

/be

Comment 49

[Brendan Eich [:brendan]]

Bob, what is the class of that object, which lacks JSCLASS_HAS_PRIVATE?

/be

Comment 50

[Bob Clary [:bc] (inactive)]

filed Bug 327199 on the 1.0.8 crash.

Comment 51

[Bob Clary [:bc] (inactive)]

no crash mac    20060221 ff  on 1.7.13, 1.8.0.1, 1.8, 1.9a1
crash    win    20060221 ff  on 1.7.13
crash    linux  20060221 ff  on 1.7.13
crash    win    20060221 moz on 1.7.13

I can't get stacks on windows for ff win debug from 20060220. This is "probably" Bug 327199, but I'll leave it unverified.

Comment 52

[Bob Clary [:bc] (inactive)]

(In reply to comment #51)
To be complete...

no crash    win    20060221 ff  on 1.8.0.1, 1.8, 1.9a1
no crash    linux  20060221 ff  on 1.8.0.1, 1.8, 1.9a1

Comment 53

[Bob Clary [:bc] (inactive)]

v ff 1.0.8 per comment 48.

Comment 54

[Bob Clary [:bc] (inactive)]

Attachment: crashes and timeouts 1.8.0.5|1.8.1|1.9 20060630

The crashes on windows debug 1.9 are due to the new vc8 behavior of throwing an exception in |new| on out of memory. I tried to reproduce the 1.8.0.5 crash on windows and the trunk crashes on macppc but couldn't. Just attaching this here so you guys can look and see if you can figure out what is going on. Afaik, this isn't new behavior.

Comment 55

[Bob Clary [:bc] (inactive)]

filed bug 343842 on the 1.8.x issues.