Closed Bug 605707 Opened 12 years ago Closed 11 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: gwagner, Assigned: mwu)

Details

(Keywords: crash, Whiteboard: fixed-in-tracemonkey)

Crash Data

Attachments

(2 files, 1 obsolete file)

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.
Can you post a longer stack?
That's all I get with "bt". I am building a debug version. Maybe there will be more information.
Attached file stack
Valgrind gives some more information.
I don't hit it in debug mode.
Severity: normal → critical
Keywords: crash
Summary: TM: crash at startup with gczeal at XDRChars → TM: crash at startup with gczeal at [@ XDRChars]
I still see this crash.
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
I got this crash on regular testing once on Windows, not during start-up.
I can't seem to hit this on startup on Linux x86-64. (opt, non-debug, with gczeal turned on)
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)
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?
(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?
(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.
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>,
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.
Sigh, the usual conservative stack scanning problem.
Attached patch Avoid GC after string creation (obsolete) — Splinter Review
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.
Assignee: general → mwu
Attachment #527637 - Flags: review?(cdleary)
(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);
(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.
(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.
(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.
Attachment #527637 - Attachment is obsolete: true
Attachment #527645 - Flags: review?
Attachment #527637 - Flags: review?(cdleary)
Attachment #527645 - Flags: review? → review?(igor)
OS: Mac OS X → All
Hardware: x86 → All
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.
Attachment #527645 - Flags: review?(igor) → review+
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.
Group: core-security
http://hg.mozilla.org/tracemonkey/rev/6a68d036e7f7
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/6a68d036e7f7
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Crash Signature: [@ XDRChars]
Group: core-security
You need to log in before you can comment on or make changes to this bug.