585309 - Crash [@ JSString::flatten() ]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[pal-moz]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100807 Minefield/4.0b4pre ID:20100807040603

http://crash-stats.mozilla.com/report/index/bp-baefcd7b-5d83-493b-8074-84e242100807

OK : http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1281160098/
NG : http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1281164227/

changeset : ddedaa587215

[pushlog]
ddedaa587215 Robert Sayre — Merge.
← 49 hidden changesets [Collapse]
a60658f8364a Robert Sayre — Merge tracemonkey to mozilla-central.
d3719a8716f5 Andreas Gal — Ensure that JSOPTION_UNROOTED_GLOBAL is set when we cycle collect (stop-gap measure for bug 584495, r=brendan).
1108781b8682 Dave Herman — bug 584786, r=shu: s/CompilerContext/StaticContext/g
0e1d9698a6ac David Anderson — Fixed assertion failure in callNative (bug 579740, r=jorendorff).
8c22931336c5 Dave Herman — bug 585208, r=taustin: reference to undefined Object.getOwnProperties
d695985dc913 Luke Wagner — Bug 585231 - Remove ArgsPrivateNative (r=dmandelin)
5a254bcd79f2 Robert Sayre — Backed out changeset 504bc84513b0. Andreas Gal – Ensure that JSOPTION_UNROOTED_GLOBAL is set when we cycle collect (stop-gap measure for bug 584495, r=brendan). default tip
504bc84513b0 Andreas Gal — Ensure that JSOPTION_UNROOTED_GLOBAL is set when we cycle collect (stop-gap measure for bug 584495, r=brendan).
049f29c354e7 Dave Herman — bug 584103, r=taustin: Narcissus Harmony mode
c8623a2babe4 Robert Sayre — Merge tracemonkey to mozilla-central.
f46f2b605fb9 Dave Herman — bug 583001, r=pwalton: narcissus REPL
c329b30abba8 Robert Sayre — Merge mozilla-central to tracemonkey.
720b966a8384 Paul Biggar — Bug 584993 - Expose js_DumpObject as a shell function. r=jorendorff.
db386e3aa0b9 Tom Austin — Bug 584807 - Simplify Narcissus testing setup. (r=gal)
e555673c8119 Chris Leary — Unbreak narcissus. (r=red)
90f23f644ef2 Chris Leary — Submillisecond now() shell function for benchmark stability. (r=jorendorff)
324b0d7b7ded Jason Orendorff — Bug 584565 - TM: When f.length is resolved at record time, we can fail to emit a shape guard. r=brendan.
f6c1d5ab87f9 Bob Clary — bug 584868 - do not use fake document object in test js1_5/extensions/regress-361964.js as it can interfere with test reporting object detection, r=sparky.
c15ed7c71e27 Andreas Gal — Inline unit string comparison (577883, r=njn).
d712417ee50b Andreas Gal — Fix int -> int32, msvc is being picky about it (follow-up for 584499, r=me).
fd0411f5ce7f Andreas Gal — Optimize string[idx] on trace (584499, r=lw).
a99f99f9f614 Nicholas Nethercote — Bug 584275 - nanojit: preparation for adding many more access regions (TM-specific part). r=gal.
7bc5d62c6b0b Nicholas Nethercote — Update nanojit-import-rev stamp.
0b1a3c68e7a5 Nicholas Nethercote — Fix more Windows bustage for bug 584275. r=me.
cf81ac9d4256 Nicholas Nethercote — Fix Windows bustage for bug 584275. r=me.
b6a753a0037c Nicholas Nethercote — Bug 584275 - nanojit: preparation for adding many more access regions. r=edwsmith.
b0f7fbb6a899 Dan Witte — Fix review comment.
af3cf4beaa0a Dan Witte — Bug 573066 - Fix ctypes stdcall closure tests. r=bsmedberg, a=blocker
bd38a23c5f41 Dan Witte — Bug 573087 - Automangle ctypes stdcall symbols. r=bsmedberg, a=blocker
9912fe3c7935 Dan Witte — Bug 564966 - ctypes stdcall tests borked on windows. r=benjamn, a=blocker
a3a5c0f64c80 Dan Witte — Update libffi.patch.
6fc27fff0097 Dan Witte — Apply libffi.patch.
0d9f290e5b0e Dan Witte — Mark script files as executable.
e2265b714a02 Dan Witte — Pull libffi git revision 3aeecc9eb1a6feba6549849cdd335c926415a4fc to fix bug 528129, bug 556902, bug 538002, bug 581909, bug 573066, bug 564966, and bug 556521. r+a=bsmedberg
736566d21998 Dan Witte — Bug 583099 - toolkit/components/ctypes uses the wrong global object. r=jorendorff, a=bsmedberg
93f2360f3f5d Gregor Wagner — Bug 581589 - TM: add defaultCompartment to compartments array (r=gal)
3899c73806fe Luke Wagner — Bug 584252 - disallow non-standard extensions to float syntax (r=jorendorff)
3c4234a49726 Olli Pettay — Bug 583777 - Fix violations of the new JS request model. r=igor
63b9ba3664a0 Blake Kaplan — Bug 584261 - Initialize the tab element before attaching the browser. r=gavin
a4c2d83e4dc9 Blake Kaplan — Bug 584551 - Do things that require entering a request on the old compartment before entering a request on the new one. r=igor
52f61ac62bba Blake Kaplan — Bug 584260 - Delay creating the outer window JS object until the last possible second. r=jst
6ea9b217883a Blake Kaplan — Bug 546573 - EnsureInnerWindow from wrappers. r=jst sr=bzbarsky
35fd077aad03 Blake Kaplan — Bug 574975 - Split inner/outer scriptable helpers, make the outer window not global. r=jst.
479cedf27101 Bob Clary — bug 583155 - reduce log noise from jsreftest in the browser, r=mrbkap,jorendorff.
564a6574d520 Igor Bukanov — bug 583763 - conservative GC cleanup and better reporting of missing conservative roots. r=anygregor
785488c82d41 Chris Leary — Bug 583868: increase heap size to avoid V8 OOM in shell. (r=anygregor)
8bd848198658 Shu-yu Guo — Bug 584264 - Whitespace cleanup for narcissus and small fix for parser. r=pwalton
87ddaf82dbd0 Ginn Chen — Bug 584219 Sun Studio doesn't support __attribute__ packed for enum r=lw
cb31ec3a3325 Luke Wagner — Bug 584168 - consider canonicalizing nans passed to the JSAPI (r=brendan)

Comment 1

[M** A****]

Confirm using Vista Business 
http://crash-stats.mozilla.com/report/index/bp-900c3ee7-8838-4818-be04-03b2e2100807

Comment 2

[IU]

Also crashes on XP.

Comment 3

[IU]

Tracemonkey pushlog: http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=785488c82d41&tochange=c15ed7c71e27

Comment 4

[IU]

Caused by bug 577883

http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=d712417ee50b&tochange=c15ed7c71e27

Comment 5

[Andreas Gal :gal]

Thanks for the bisect.

Comment 6

[Andreas Gal :gal]

Some steps to reproduce would be helpful.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #6)
> Some steps to reproduce would be helpful.

I got this to reproduce on a Mac 10.6 32-bit nightly as well. Just load the URL and occasionally refresh if necessary. It sometimes crashes on first load, but sometimes after repeated refreshes. And no, I couldn't get it to crash on a local copy of the site.

Comment 8

[IU]

(In reply to comment #6)
> Some steps to reproduce would be helpful.

1. Use a new profile
2. Load http://img836.imageshack.us/i/capture2n.jpg/
3. If the page is blank (sometimes happens) reload
4. Once the page loads, simply wait for it to crash (takes a few seconds).

Comment 9

[Ken]

(In reply to comment #6)
> Some steps to reproduce would be helpful.

FWIW
 When I had Adblock plus disabled (unpatched version stopped working with recent build)...
 The Imageshack web page will crash almost all the time if you wait long enough for the ad's to change/display.
 With the patched Adblock plus, Imageshack no longer crashes for me.

So, if trying to look for the code that crashes... I might start with the code they use to display advertisements.

Comment 10

[Andreas Gal :gal]

For the life of me I can't get this to crash on macosx with a debug build. Also, the bisected changeset doesn't make sense since it doesn't touches the path thats crashing here.

Comment 11

[Robert Sayre]

this doesn't crash for me with an opt build from TM tip either, but I do crash in the nightly.

Comment 12

[Andreas Gal :gal]

I am on TM tip too. I don't see anything in the log that might have fixed this between the nightly and tip, so this is more likely hidden by something but not fixed.

Comment 13

[Robert Sayre]

Attachment: valgrind stack

Did get valgrind to complain on the mozilla-central nightly.

Comment 14

[Robert Sayre]

you know, I bet both of our builds are 64-bit.  But the nightly is 32-bit.

Comment 15

[WildcatRay]

FWIW, my crashes were using 32-bit builds on Win7 64-bit OS:

bp-31000a94-8770-4de8-9007-5469f2100807
bp-1b7428e4-c7bd-46d7-a83f-3bbdd2100807
bp-b29dbfb7-e24b-45a2-ba17-b74022100807
bp-b53f8cfe-2f68-4263-9f2d-849f22100807

Comment 16

[IU]

Attachment: WinDbg log of crash

Windows XP SP3

Comment 17

[[not reading bugmail]]

crashing for me using nightly on 32bit using 32bit Win 7.

http://crash-stats.mozilla.com/report/index/bp-9409c780-cdc5-442c-89c2-737c22100807
http://crash-stats.mozilla.com/report/index/bp-2fac8d69-abfd-4018-b802-79f212100807

Shows it stopping at here:
http://hg.mozilla.org/mozilla-central/annotate/81c119fb86c7/js/src/jsstr.cpp#l125

125 topNode->convertToInteriorNode(NULL);

126 JSString *str = topNode, *next;

127 size_t pos = 0;

Comment 18

[Andreas Gal :gal]

Can anyone catch this in a debugger?

Comment 19

[IU]

(In reply to comment #18)
> Can anyone catch this in a debugger?

You mean like the one I just attached? :-)

Comment 20

[Andreas Gal :gal]

I meant could you stop in the debugger and find me? I have a couple questions about the state :)

Comment 21

[IU]

(In reply to comment #20)
> I meant could you stop in the debugger and find me? I have a couple questions
> about the state :)

What do you mean by "find me?"  Are you wanting me to start it up in a debugger then contact you on IRC or something?

Also, you should be able to reproduce this crash with XP in VirtualBox.

Comment 22

[Robert Sayre]

I have this caught in gdb in on a 32-bit mac debug build from TM tip.

JS_Assert (s=0x3559c0e "isRope()", file=0x3559a44 "/Users/sayrer/dev/tracemonkey/js/src/jsstr.cpp", ln=103)

Comment 23

[Robert Sayre]

Attachment: 32-bit mac mozconfig

set the environment variable MOZCONFIG to this file's path, and then "make -f client.mk build"

Comment 24

[gabe]

http://www2.wspa.com/news/problem-solver/2010/jul/12/dangling-cable-line-worries-residents-greenville-ar-569671/

open page wait 30 seconds crash not sure if same bug but simlier stack

Comment 25

[Andreas Gal :gal]

I have a build spinning. Sayrer says that the string is NULL here. That points towards the right regressor being fingered above after all. js_Flatten is just a red herring. str is null and thats the bug. Stay tuned.

Comment 26

[Andreas Gal :gal]

trying to attach another branch to the tree (hits = 0)
Looking for compat peer 29@341, from 0x63c254c (ip: 0x545f5f1)
checking vm types 0x63c254c (ip: 0x545f5f1): args0=F/F args1=O/O args2=O/O args3=O/O args4=I/I arguments0=N/N scopeChain0=O/O var0=S/S var1=O/O var2=O/O var3=U/U var4=S/S var5=I/I var6=I/I var7=I/I var8=O/O var9=O/O var10=O/O var11=O/O var12=O/O var13=O/O var14=I/I var15=I/I var16=I/I var17=S/S var18=I/I var19=I/I var20=U/U var21=U/U global0=F/F global1=F/F 
args0: function<0x1fd01c00:unnamed> args1: object<0x1fd01a80:Object> args2: object<0x1fd00270:Array> args3: object<0x1b483810:Date> args4: int<1> arguments0: null scopeChain0: object<0x1b483840:Call> var0: string<0x1f96a460> var1: object<0x1b483cf0:Array> var2: object<0x1b483d50:Array> var3: undefined var4: string<0x1f96a4e0> var5: int<2> var6: int<4> var7: int<20> var8: object<0x1b4839f0:Array> var9: object<0x1b483a20:Object> var10: object<0x1b483a50:Array> var11: object<0x1b483ae0:Array> var12: object<0x1b483b10:Array> var13: object<0x1b483b40:Array> var14: int<0> var15: int<2> var16: int<0> var17: string<0x1f96a470> var18: int<2> var19: int<1> var20: undefined var21: undefined global0: function<0x1b4c51b0:Object> global1: function<0x1b4ce288:Array> 
entering trace at http://tap-cdn.rubiconproject.com/partner/scripts/rubicon/page_parser.js:29@341, native stack slots: 44 code: 0x1ac7b579
Assertion failure: isRope(), at ../../../js/src/jsstr.cpp:103

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x0402a57f in JS_Assert (s=0x40f1e95 "isRope()", file=0x40fc9ac "../../../js/src/jsstr.cpp", ln=103) at ../../../js/src/jsutil.cpp:81
81	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) up
#1  0x0400b110 in JSString::flatten (this=0x1f96a9f0) at ../../../js/src/jsstr.cpp:103
103	    JS_ASSERT(isRope());
(gdb) p *this
$1 = {
  mLengthAndFlags = 212, 
  {
    mChars = 0x250d2e70, 
    mLeft = 0x250d2e70
  }, 
  {
    mInlineStorage = {15, 0, 0, 0}, 
    e = {
      {
        mCapacity = 15, 
        mParent = 0xf, 
        mBufferWithInfo = 0xf
      }, 
      {
        mBase = 0x0, 
        mRight = 0x0
      }
    }
  }, 
  static FLAT = 0, 
  static DEPENDENT = 1, 
  static INTERIOR_NODE = 2, 
  static TOP_NODE = 3, 
  static ROPE_BIT = 2, 
  static ATOMIZED = 4, 
  static MUTABLE = 8, 
  static FLAGS_LENGTH_SHIFT = 4, 
  static ROPE_TRAVERSAL_COUNT_SHIFT = 2, 
  static ROPE_TRAVERSAL_COUNT_MASK = 12, 
  static ROPE_TRAVERSAL_COUNT_UNIT = 4, 
  static TYPE_MASK = 3, 
  static MAX_LENGTH = 268435455, 
  static INVALID_SMALL_CHAR = 255 '?', 
  static fromSmallChar = 0x412c840, 
  static toSmallChar = 0x412c7c0 '?' <repeats 48 times>, 
  static unitStringTable = 0x41177e0, 
  static length2StringTable = 0x41187e0, 
  static hundredStringTable = 0x41287e0, 
  static intStringTable = 0x41291a0, 
  static deflatedIntStringTable = 0x410e100 "100", 
  static deflatedUnitStringTable = 0x410e380 "", 
  static deflatedLength2StringTable = 0x410b100 "00"
}
(gdb) up
#2  0x0400c3b6 in js_Flatten (str=0x1f96a9f0) at ../../../js/src/jsstr.cpp:184
184	    str->flatten();
(gdb) up
#3  0x1ac7b99f in ?? ()
(gdb) disass $pc-32 $pc+32
Dump of assembler code from 0x1ac7b97f to 0x1ac7b9bf:
0x1ac7b97f:	mov    %eax,-0x14(%ebp)
0x1ac7b982:	jo     0x1ad38371
0x1ac7b988:	mov    %eax,0x48(%edi)
0x1ac7b98b:	mov    %esi,%ebx
0x1ac7b98d:	and    $0x2,%ebx
0x1ac7b990:	mov    -0x20(%ebp),%ebx
0x1ac7b993:	je     0x1ac7b9aa
0x1ac7b995:	mov    -0x1c(%ebp),%edi
0x1ac7b998:	mov    %edi,%ecx
0x1ac7b99a:	call   0x400c3a2 <_Z10js_FlattenP8JSString>
0x1ac7b99f:	mov    -0x10(%ebp),%ecx
0x1ac7b9a2:	mov    %edi,%edx
0x1ac7b9a4:	mov    -0x18(%ebp),%edi
0x1ac7b9a7:	mov    -0x14(%ebp),%eax
0x1ac7b9aa:	movl   $0x17800000,-0x14(%ebp)
0x1ac7b9b1:	shr    $0x4,%esi
0x1ac7b9b4:	cmp    %esi,%eax
0x1ac7b9b6:	jae    0x1ac7b9d7
0x1ac7b9b8:	mov    0x4(%edx),%esi
0x1ac7b9bb:	movzwl (%esi,%eax,2),%esi

Comment 28

[Andreas Gal :gal]

Another testcase:

http://yfrog.com/mfq5nj

Comment 29

[Andreas Gal :gal]

Attachment: patch

mLengthAndFlags and mChars are not constant. We have to reload them. This kinda sucks. Maybe nick has an idea how we can beef up the alias analysis here without completely throwing it out of the window (js_Flatten changes mChars and mLengthAndFlags).

Comment 30

[Andreas Gal :gal]

Attachment: patch

Comment 31

[Alan Pierce]

Comment on attachment 463876 [details] [diff] [review]
patch

Looks good.

Comment 32

[Andreas Gal :gal]

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

Comment 33

[Nicholas Nethercote [inactive]]

(In reply to comment #29)
> 
> mLengthAndFlags and mChars are not constant. We have to reload them. This kinda
> sucks.

If you lie to the compiler, the compiler will get its revenge :/  We check AccSet annotations quite a bit with checkAccSet(), but I don't know if we can check LOAD_CONST annotations -- any ideas are welcome, I don't want this kind of thing to happen again if possible.

> Maybe nick has an idea how we can beef up the alias analysis here
> without completely throwing it out of the window (js_Flatten changes mChars and
> mLengthAndFlags).

Bug 584279 will create an access region that covers just the contents of all JSString structs, which is good.  But I don't think losing the CONST annotation actually hurts in this case.  I ran my compare-generated-LIR script for SunSpider and AFAICT there is no difference in the code generated after this fix was applied.  Ie. there were no CSE opportunities missed.

Comment 34

[Jim Jeffery not reading bug-mail 1/2/11]

Any chance this can land in m-c before the next tm->m-c merge?  Several of us that use m-c nightly builds are hitting this bug.

Comment 35

[Michael Lefevre]

Current top crashes for Firefox4.0b4pre:
JSString::flatten()                            866
mozilla::FrameLayerBuilder::DrawThebesLayer    156
JSString::flatten                              52
F957328252________________________             25

Comment 36

[Brendan Eich [:brendan]]

http://hg.mozilla.org/mozilla-central/rev/032af4368fcb

/be

Comment 37

[WildcatRay]

Using the hourly with the patch, it is not crashing. Thanks!

Comment 38

[Andreas Gal :gal]

Thanks for all the reports and testing help.

Comment 40

[Martijn Wargers (dead)]

Was this fixed on trunk or not? I'm still crashing with this stacktrace, using:
Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre

Comment 41

[pal-moz]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100822 Minefield/4.0b5pre ID:20100822040607

no crash here.

Comment 42

[Scoobidiver (away)]

These crashes reappeared in b7pre/20100921 build at a daily rate of about 5 crashes/day.
The regression range is :
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b1d56e69f4d9&tochange=8528ce3f97ce

From b8pre/20101015 build, there is a spike in crashes. It becomes #6 top crasher.
The regression range for the spike is :
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ad0a0be8be74&tochange=19cb42fa4554

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=JSString%3A%3Aflatten%28%29&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=JSString%3A%3Aflatten%28%29

Comment 43

[Jesse Ruderman]

File a new bug report. This one is fixed.

Comment 44

[juan becerra [:juanb]]

I crashed on Mac after applying latest nightly from 20101019, consistently.

See: bp-5f7f882e-ee3f-41da-b2df-39f8b2101019, bp-b4bd881f-b51e-46a2-be06-20bf62101019