605707 - TM: crash at startup with gczeal at [@ XDRChars]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gregor Wagner [:gwagner]]

An optimized browser build with gczeal enabled crashes during startup.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x000000011675a000
0x0000000100fcc772 in XDRChars [inlined] () at /Users/idefix2/moz/zeal/js/src/jsxdrapi.cpp:435
435	            raw[i] = JSXDR_SWAB16(chars[i]);
(gdb) bt
#0  0x0000000100fcc772 in XDRChars [inlined] () at /Users/idefix2/moz/zeal/js/src/jsxdrapi.cpp:435
#1  0x0000000100fcc772 in JS_XDRString (xdr=0x103323240, strp=0x7fff5fbfe320) at /users/idefix2/moz/zeal/js/src/jsxdrapi.cpp:467
(gdb) 

I haven't tested a debug build yet.

Comment 1

[Andreas Gal :gal]

Can you post a longer stack?

Comment 2

[Gregor Wagner [:gwagner]]

That's all I get with "bt". I am building a debug version. Maybe there will be more information.

Comment 3

[Gregor Wagner [:gwagner]]

Attachment: stack

Valgrind gives some more information.
I don't hit it in debug mode.

Comment 4

[Gregor Wagner [:gwagner]]

I still see this crash.

Comment 5

[Philip Chee]

A SeaMonkey trunk/nightly tester reported this crash with the Scriptish extension installed (does not crash without Scriptish)

http://crash-stats.mozilla.com/report/index/bp-1ea4e02a-f6a2-404e-9ff1-31f9e2110103

Signature	XDRChars
UUID	1ea4e02a-f6a2-404e-9ff1-31f9e2110103
Time 	2011-01-03 12:09:34.625996
Uptime	5
Last Crash	140 seconds before submission
Install Age	17175 seconds (4.8 hours) since version was first installed.
Product	SeaMonkey
Version	2.1b2pre
Build ID	20110103003002
Branch	2.0
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 13
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x6872000

Comment 6

[Martijn Wargers (dead)]

I got this crash on regular testing once on Windows, not during start-up.

Comment 7

[Michael Wu [:mwu]]

I can't seem to hit this on startup on Linux x86-64. (opt, non-debug, with gczeal turned on)

Comment 8

[Gregor Wagner [:gwagner]]

I still get this:

Program received signal SIGSEGV, Segmentation fault.
XDRChars (xdr=0x7fffe43e4650, strp=0x7fffffffcdf0)
    at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:435
435	            raw[i] = JSXDR_SWAB16(chars[i]);
(gdb) bt
#0  XDRChars (xdr=0x7fffe43e4650, strp=0x7fffffffcdf0)
    at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:435
#1  JS_XDRString (xdr=0x7fffe43e4650, strp=0x7fffffffcdf0)
    at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:466
#2  0x00007ffff6f0c3ca in js_XDRRegExpObject (xdr=0x7fffe43e4650, 
    objp=0x7fffe37672e8) at /home/mozilla/gwagner/zeal/js/src/jsregexp.cpp:504
#3  0x00007ffff6f1ef20 in js_XDRScript (xdr=0x7fffe43e4650, 
    scriptp=0x7fffe5125260, hasMagic=<value optimized out>)
    at /home/mozilla/gwagner/zeal/js/src/jsscript.cpp:693
#4  0x00007ffff6e881d8 in js_XDRFunctionObject (xdr=0x7fffe43e4650, 
    objp=0x7fffe37693f8) at /home/mozilla/gwagner/zeal/js/src/jsfun.cpp:1901
#5  0x00007ffff6f1ee70 in js_XDRScript (xdr=0x7fffe43e4650, 
    scriptp=<value optimized out>, hasMagic=<value optimized out>)
    at /home/mozilla/gwagner/zeal/js/src/jsscript.cpp:680
#6  0x00007ffff6f4ca9b in JS_XDRScript (xdr=0x7ffff7ebe040, scriptp=0x0)
    at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:671
#7  0x00007ffff69cdea0 in WriteScriptToStream (this=<value optimized out>, 
    cache=0x7fffec9b7430, script=<value optimized out>, 
    component=<value optimized out>, uri=<value optimized out>, 
    cx=0x7fffe5fc6800)

Comment 9

[Gregor Wagner [:gwagner]]

Still crashing.
Igor you know the code around js_XDRRegExpObject much better. It seems we have an unrooted String. Do you wanna take a look?

Comment 10

[Igor Bukanov]

(In reply to comment #9)
> It seems we have an unrooted String. Do you wanna take a look?

I will try to reproduce this tomorrow. In the mean time could you disable the background free thread and see how it would affect the behavior?

Comment 11

[Gregor Wagner [:gwagner]]

(In reply to comment #10)
> (In reply to comment #9)
> > It seems we have an unrooted String. Do you wanna take a look?
> 
> I will try to reproduce this tomorrow. 

It's an opt build with gczeal=2 in Runtime::init

> In the mean time could you disable the
> background free thread and see how it would affect the behavior?

I changed the background free to a normal free and see the same behavior.

Comment 12

[Gregor Wagner [:gwagner]]

I still see the crash with following stack:

Program received signal SIGSEGV, Segmentation fault.
js_GetDeflatedUTF8StringLength (cx=0x7fffe3f02c00, chars=0x7fffe4100000, 
    nchars=4294944767, useCESU8=true)
    at /home/mozilla/gwagner/zeal/js/src/jsstr.cpp:4167
4167	        c = *chars;
(gdb) bt
#0  js_GetDeflatedUTF8StringLength (cx=0x7fffe3f02c00, chars=0x7fffe4100000, 
    nchars=4294944767, useCESU8=true)
    at /home/mozilla/gwagner/zeal/js/src/jsstr.cpp:4167
#1  0x00007ffff7056cde in JS_XDRString (xdr=0x7fffe3fa04c0, strp=0x7fffffffccf0)
    at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:430
#2  0x00007ffff7015829 in js_XDRRegExpObject (xdr=0x7fffe3fa04c0, objp=0x7fffe3fae588)
    at /home/mozilla/gwagner/zeal/js/src/jsregexp.cpp:449
#3  0x00007ffff70290df in js_XDRScript (xdr=0x7fffe3fa04c0, 
    scriptp=<value optimized out>) at /home/mozilla/gwagner/zeal/js/src/jsscript.cpp:690
#4  0x00007ffff6f8e616 in js_XDRFunctionObject (xdr=0x7fffe3fa04c0, objp=0x7fffe3fb75c8)
    at /home/mozilla/gwagner/zeal/js/src/jsfun.cpp:1904
#5  0x00007ffff702903d in js_XDRScript (xdr=0x7fffe3fa04c0, 
    scriptp=<value optimized out>) at /home/mozilla/gwagner/zeal/js/src/jsscript.cpp:677
#6  0x00007ffff7056b6d in JS_XDRScriptObject (xdr=0x7fffe3fa04c0, 
    scriptObjp=0x7fffffffd298) at /home/mozilla/gwagner/zeal/js/src/jsxdrapi.cpp:735
#7  0x00007ffff6a526a4 in WriteScriptToStream (this=<value optimized out>, 
    cache=0x7fffeccb7430, scriptObj=<value optimized out>, 
    component=<value optimized out>, uri=<value optimized out>, cx=0x7fffe3f02c00)
    at /home/mozilla/gwagner/zeal/js/src/xpconnect/loader/mozJSComponentLoader.cpp:417
#8  mozJSComponentLoader::WriteScript (this=<value optimized out>,

Comment 13

[Michael Wu [:mwu]]

Found the issue.

GC is done on http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l449 which messes up the string created at http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l443

I'm guessing that things are getting optimized and str isn't on the stack by the time NewBuiltinClassInstance is called.

Comment 14

[Jeff Walden [:Waldo]]

Sigh, the usual conservative stack scanning problem.

Comment 15

[Michael Wu [:mwu]]

Attachment: Avoid GC after string creation

This is really subtle, but I'm not sure how else to fix it. I had a patch which made create take a JSString** instead of JSString* but that involved more changes elsewhere.

Comment 16

[Gregor Wagner [:gwagner]]

(In reply to comment #13)
> Found the issue.
> 
> GC is done on
> http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l449
> which messes up the string created at
> http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l443
> 
> I'm guessing that things are getting optimized and str isn't on the stack by
> the time NewBuiltinClassInstance is called.

The reference has to be somewhere and we also scan the registers so this shouldn't be a problem.
Who is responsible for marking the string after this line?
AlreadyIncRefed<RegExp> re = RegExp::create(cx, str, flags);

Comment 17

[Michael Wu [:mwu]]

(In reply to comment #16)
> (In reply to comment #13)
> > Found the issue.
> > 
> > GC is done on
> > http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l449
> > which messes up the string created at
> > http://hg.mozilla.org/mozilla-central/file/357593f3abbd/js/src/jsregexpinlines.h#l443
> > 
> > I'm guessing that things are getting optimized and str isn't on the stack by
> > the time NewBuiltinClassInstance is called.
> 
> The reference has to be somewhere and we also scan the registers so this
> shouldn't be a problem.

AIUI, the only reference left is in |AlreadyIncRefed<RegExp> re|, which isn't GC allocated.

Comment 18

[Igor Bukanov]

(In reply to comment #15)
> This is really subtle

The problem is that the compiler stores the only instance of the created str in the malloc-allocated RegExp memory before allocating the object. The proper fix would be to use Anchor.

Comment 19

[Michael Wu [:mwu]]

(In reply to comment #18)
> (In reply to comment #15)
> > This is really subtle
> 
> The problem is that the compiler stores the only instance of the created str in
> the malloc-allocated RegExp memory before allocating the object. The proper fix
> would be to use Anchor.

Ah nice, this is exactly the API I was looking for.

Comment 20

[Michael Wu [:mwu]]

Attachment: Anchor the regex string

Comment 21

[Igor Bukanov]

Comment on attachment 527645 [details] [diff] [review]
Anchor the regex string

>@@ -443,6 +443,7 @@ RegExp::createObjectNoStatics(JSContext 
>     JSString *str = js_NewStringCopyN(cx, chars, length);
>     if (!str)
>         return NULL;
>+    JS::Anchor<JSString *> anchor(str);
>     AlreadyIncRefed<RegExp> re = RegExp::create(cx, str, flags);


We need a comment explaining why we need Anchor. r+ with that.

Comment 22

[Chris Leary [:cdleary] (not checking bugmail)]

js::RegExp is deceptive that way... just heapified guts that get refcounted by the wrapping JSObjects, and the conservative stack scanner has no reasonable way of recognizing it since it's not a GCThing.

Comment 23

[Michael Wu [:mwu]]

http://hg.mozilla.org/tracemonkey/rev/6a68d036e7f7

Comment 24

[Michael Wu [:mwu]]

http://hg.mozilla.org/mozilla-central/rev/6a68d036e7f7