877826 - crash in js::ion::BaselineScript::pcForReturnOffset

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Scoobidiver (away)]

It first showed up in 23.0a1/20130427 but is discontinuous across builds. The regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6104e0e5a2c&tochange=0e45f1b9521f
It's likely a regression from bug 857838 based on the stack trace.

A comment says: "using knockout's ko.toJSON method."

Signature 	js::ion::CompactBufferReader::readByte() More Reports Search
UUID	9896406e-b4fa-4d20-bfe4-d12482130530
Date Processed	2013-05-30 19:15:54
Uptime	200
Last Crash	3.5 minutes before submission
Install Age	4.6 hours since version was first installed.
Install Time	2013-05-30 14:40:34
Product	Firefox
Version	24.0a1
Build ID	20130530031141
Release Channel	nightly
OS	Windows NT
OS Version	6.1.7601 Service Pack 1
Build Architecture	x86
Build Architecture Info	GenuineIntel family 6 model 30 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x1010e000
App Notes 	
AdapterVendorID: 0x1002, AdapterDeviceID: 0x6899, AdapterSubsysID: 29701682, AdapterDriverVersion: 9.12.0.0
D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ 
Processor Notes 	sp-processor02_phx1_mozilla_com_32086:2012
EMCheckCompatibility	False
Adapter Vendor ID	0x1002
Adapter Device ID	0x6899
Total Virtual Memory	4294836224
Available Virtual Memory	3382218752
System Memory Use Percentage	33
Available Page File	6193532928
Available Physical Memory	2862821376

Frame 	Module 	Signature 	Source
0 	mozjs.dll 	js::ion::CompactBufferReader::readByte 	js/src/ion/CompactBuffer.h:57
1 	mozjs.dll 	js::ion::BaselineScript::pcForReturnOffset 	js/src/ion/BaselineJIT.cpp:618
2 	mozjs.dll 	js::ion::BaselineScript::pcForReturnAddress 	js/src/ion/BaselineJIT.cpp:638
3 	mozjs.dll 	js::ion::IonFrameIterator::baselineScriptAndPc 	js/src/ion/IonFrames.cpp:215
4 	mozjs.dll 	js::ion::GetPcScript 	js/src/ion/IonFrames.cpp:1025
5 	mozjs.dll 	MaybeCallMethod 	js/src/jsobj.cpp:4590
6 	mozjs.dll 	js::DefaultValue 	js/src/jsobj.cpp:4618
7 	mozjs.dll 	JSObject::defaultValue 	js/src/jsobjinlines.h:67
8 	mozjs.dll 	js_ReportOverRecursed 	js/src/jscntxt.cpp:520
9 	mozjs.dll 	js_ErrorFromException 	js/src/jsexn.cpp:479
10 	xul.dll 	XPCConvert::JSValToXPCException 	js/xpconnect/src/XPCConvert.cpp:1210
11 	xul.dll 	nsXPCWrappedJSClass::CheckForException 	js/xpconnect/src/XPCWrappedJSClass.cpp:972
12 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	js/xpconnect/src/XPCWrappedJSClass.cpp:1470
13 	xul.dll 	nsXPCWrappedJS::CallMethod 	js/xpconnect/src/XPCWrappedJS.cpp:573
14 	xul.dll 	PrepareAndDispatch 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:85
15 	xul.dll 	SharedStub 	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:112
16 	xul.dll 	jsds_ExecutionHookProc 	js/jsd/jsd_xpc.cpp:634

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Aion%3A%3ACompactBufferReader%3A%3AreadByte%28%29

Comment 1

[Scoobidiver (away)]

More reports also at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Aion%3A%3ACompactBufferReader%3A%3AreadVariableLength%28%29

Comment 2

[Scoobidiver (away)]

(In reply to Scoobidiver from comment #0)
> It's likely a regression from bug 857838 based on the stack trace.
Confirmed by its first appearance in 23.0a1/20130423 on Mac

Mac reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Aion%3A%3ABaselineScript%3A%3ApcForReturnOffset%28JSScript*%2C+unsigned+int%29

Comment 3

[Scoobidiver (away)]

It's #3 browser crasher in 23.0b1 and #10 in 24.0a2 on Mac OS X.

Comment 4

[Jan de Mooij [:jandem]]

Kannan can you take a look at this? Comment 0 talks about this being a possible regression from bug 857838.

Comment 5

[Kannan Vijayan [:djvj]]

Taking a look now.

Comment 6

[Kannan Vijayan [:djvj]]

I downloaded the relevant release on OSX to try to reproduce this, but no luck.

Then I started looking through the crash reports.  One thing I noticed was that FireBug was heavily present in the crash reports.  In windows crashes, 43 of the first 50 (86%).. and in MacOSX 100% of the crashes checked.. had firebug installed.

I'll try browsing around with firebug installed and see if I can reproduce the issue.

Comment 7

[Kannan Vijayan [:djvj]]

I can't replicate this at all.  I need to have something to go on here.

I'm told that I can get access to coredumps as well as URLs corresponding to the crashes.  Can I get those?  I'm blindly browsing now, hoping for a crash, and it's not working.

Comment 8

[Kannan Vijayan [:djvj]]

Jan, do you know where I can get info mentioned in comment 7?

Comment 9

[Alex Xu]

Running: Firefox 25.0a1 from mozilla-central 137496:17fe59f6c54a (https://hg.mozilla.org/mozilla-central/rev/17fe59f6c54a) with non-code modifications, built with Clang 3.3 and configure flags:

--enable-application=browser --enable-optimize=-O2 --with-ccache --enable-gstreamer --with-arch=native --enable-official-branding --disable-gio --disable-gconf --disable-accessibility --disable-parental-controls --disable-safe-browsing --enable-debug-symbols --disable-install-strip --enable-clang-plugin --enable-warnings-as-errors

I can reliably reproduce this crash in my application by opening Firebug and calling a function with invalid parameters. Doing the same with the builtin console with Firebug not activated prints "Error".

I have not yet tried to reduce the size of the code base to a usable test case, but it is almost always reproducible.

I have a core dump saved and can reproduce the crash while running under gdb.

Program received signal SIGSEGV, Segmentation fault.
js::ion::BaselineScript::pcForReturnOffset (this=<optimized out>, script=<optimized out>, nativeOffset=206814820) at /home/alex/mozilla-central/js/src/ion/BaselineJIT.cpp:672
warning: Source file is more recent than executable.
672             uint8_t b = reader.readByte();
(gdb) bt
#0  js::ion::BaselineScript::pcForReturnOffset (this=<optimized out>, script=<optimized out>, nativeOffset=206814820) at /home/alex/mozilla-central/js/src/ion/BaselineJIT.cpp:672
#1  0x00007ffff6bbb2f8 in js::ion::IonFrameIterator::baselineScriptAndPc (this=<optimized out>, scriptRes=<optimized out>, pcRes=0x7fffffefe340) at /home/alex/mozilla-central/js/src/ion/IonFrames.cpp:219
#2  0x00007ffff6bbce49 in js::ion::GetPcScript (cx=<optimized out>, scriptRes=0x7fffffefe4f8, pcRes=0x7fffffefe4f0) at /home/alex/mozilla-central/js/src/ion/IonFrames.cpp:1060
#3  0x00007ffff6afa2a1 in currentScript (ppc=<optimized out>, allowCrossCompartment=<optimized out>, this=<optimized out>, this=<optimized out>, ppc=<optimized out>, allowCrossCompartment=<optimized out>) at /home/alex/mozilla-central/js/src/jscntxtinlines.h:554
#4  js_InferFlags (cx=0x7fffea6ca000, defaultFlags=0) at /home/alex/mozilla-central/js/src/jsobj.cpp:1704
#5  0x00007ffff6b00ba3 in CallResolveOp (flags=<optimized out>, recursedp=<optimized out>, flags=<optimized out>, recursedp=<optimized out>, cx=<optimized out>, obj=..., id=..., objp=..., propp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:3614
#6  LookupPropertyWithFlagsInline<1> (root=0x0, root=0x0, flags=<optimized out>, cx=<optimized out>, cx=<optimized out>, obj=..., id=..., flags=<optimized out>, objp=..., propp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:3701
#7  GetPropertyHelperInline<1> (getHow=<optimized out>, cx=<optimized out>, obj=..., receiver=..., id=..., getHow=<optimized out>, vp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:3982
#8  js::baseops::GetProperty (cx=0x7fffc2199200, obj=..., receiver=..., id=..., vp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:4090
#9  0x00007ffff6b043aa in getGeneric (cx=<optimized out>, obj=..., receiver=..., id=..., vp=...) at /home/alex/mozilla-central/js/src/jsobj.h:870
#10 MaybeCallMethod (cx=<optimized out>, obj=..., id=..., vp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:4695
#11 0x00007ffff6b03ff5 in js::DefaultValue (cx=0x7fffc2199200, hint=JSTYPE_STRING, obj=..., vp=...) at /home/alex/mozilla-central/js/src/jsobj.cpp:4723
#12 0x00007ffff6b3c3a0 in defaultValue (root=..., dummy=0, hint=<optimized out>, cx=<optimized out>, cx=<optimized out>, obj=..., hint=<optimized out>, vp=...) at /home/alex/mozilla-central/js/src/jsobj.h:1010
#13 ToPrimitive (root=0xfffbffffc77d66a0, preferredType=<optimized out>, cx=<optimized out>, preferredType=<optimized out>, vp=...) at /home/alex/mozilla-central/js/src/jsobjinlines.h:903
#14 js::ToStringSlow<(js::AllowGC)1> (cx=0x7fffc2199200, arg=...) at /home/alex/mozilla-central/js/src/jsstr.cpp:3778
#15 0x00007ffff6a7b160 in ToString<1> (root=..., dummy=0, cx=<optimized out>, v=...) at /home/alex/mozilla-central/js/src/jsstr.h:149
#16 JS_ValueToString (cx=0x7fffea6ca000, valueArg=...) at /home/alex/mozilla-central/js/src/jsapi.cpp:444
#17 0x00007ffff5c7c937 in XPCConvert::JSValToXPCException (ifaceName=0x7fffbf7c2420 "jsdIExecutionHook", methodName=0x7ffff1f862c8 "onExecute", exceptn=0x7fffffefe9b0, sArg=...) at /home/alex/mozilla-central/js/xpconnect/src/XPCConvert.cpp:1164
#18 0x00007ffff5c98b5e in nsXPCWrappedJSClass::CheckForException (ccx=..., aPropertyName=0x7ffff1f862c8 "onExecute", anInterfaceName=0x7fffbf7c2420 "jsdIExecutionHook", aForceReport=true) at /home/alex/mozilla-central/js/xpconnect/src/XPCWrappedJSClass.cpp:971
#19 0x00007ffff5c99fde in nsXPCWrappedJSClass::CallMethod (this=0x7fffbdbfabc0, wrapper=<optimized out>, methodIndex=3, info_=0x7ffff1f862a8, nativeParams=0x7fffffefeda0) at /home/alex/mozilla-central/js/xpconnect/src/XPCWrappedJSClass.cpp:1472
#20 0x00007ffff5c96bea in nsXPCWrappedJS::CallMethod (this=0x7fffe7510c80, methodIndex=3, info=0x7ffff1f862a8, params=0x7fffffefeda0) at /home/alex/mozilla-central/js/xpconnect/src/XPCWrappedJS.cpp:589
#21 0x00007ffff63e0cc2 in PrepareAndDispatch (self=0x7fffbdb9be80, methodIndex=<optimized out>, args=<optimized out>, gpregs=<optimized out>, fpregs=<optimized out>) at /home/alex/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp:122
#22 0x00007ffff63e011f in SharedStub () from /usr/local/lib64/firefox-25.0a1/libxul.so
#23 0x00007ffff5e1f28d in jsds_ExecutionHookProc (jsdc=0x7fffbe16a700, jsdthreadstate=<optimized out>, type=4, callerdata=<optimized out>, rval=0x7fffffefefa8) at /home/alex/mozilla-central/js/jsd/jsd_xpc.cpp:637
#24 0x00007ffff5e18513 in jsd_CallExecutionHook (hook=<optimized out>, hookData=<optimized out>, type=<optimized out>, rval=<optimized out>, cx=<optimized out>, hook=<optimized out>, hookData=<optimized out>, cx=<optimized out>, type=<optimized out>, 
    rval=<optimized out>, jsdc=<optimized out>) at /home/alex/mozilla-central/js/jsd/jsd_hook.cpp:146
#25 jsd_ThrowHandler (cx=<optimized out>, script=<optimized out>, pc=<optimized out>, rval=0x7fffffefefa8, closure=0x7fffbe16a700) at /home/alex/mozilla-central/js/jsd/jsd_hook.cpp:117
#26 0x00007ffff6ab2470 in js::DebugExceptionUnwind (cx=0x7fffc2199200, pc=0x7fffebd84ae4 ":", frame=...) at /home/alex/mozilla-central/js/src/jsdbgapi.cpp:148
#27 0x00007ffff6bbb75e in HandleException (frame=..., cx=<optimized out>, calledDebugEpilogue=<optimized out>, rfe=<optimized out>, cx=<optimized out>, frame=..., rfe=<optimized out>, calledDebugEpilogue=<optimized out>)
    at /home/alex/mozilla-central/js/src/ion/IonFrames.cpp:371
#28 js::ion::HandleException (rfe=0x7fffffeff5d0) at /home/alex/mozilla-central/js/src/ion/IonFrames.cpp:515
#29 0x00007ffff341bf4e in ?? ()
#30 0x00007fffc566a0c0 in ?? ()
#31 0x00007fffffeff5d0 in ?? ()
#32 0x00007fffffeff6a8 in ?? ()
#33 0x00007fffffeff650 in ?? ()
#34 0x00007fffffeff6a8 in ?? ()
#35 0x00007fff00000000 in ?? ()
#36 0x00007fffffeff6a8 in ?? ()
#37 0xfff9000000000000 in ?? ()
#38 0x00007ffff7a5feb8 in js::ion::DoGetIntrinsicFallbackInfo () from /usr/local/lib64/firefox-25.0a1/libxul.so
#39 0x00007fffe7b40d30 in ?? ()
#40 0x00007fffe6eb9055 in ?? ()
#41 0x0000000000000901 in ?? ()
#42 0x00007fffffeff660 in ?? ()
#43 0x00007fffe92c5158 in ?? ()
#44 0xfffbffffc77c2400 in ?? ()
#45 0xfffbffffc77c2400 in ?? ()
#46 0xfffbffffc5639ec0 in ?? ()
#47 0xfffbffffc566a0c0 in ?? ()
#48 0xfffaffffc77cc300 in ?? ()
#49 0xfffbffffc77c2400 in ?? ()
#50 0x0000000000000000 in ?? ()

Comment 10

[Alex Xu]

Missing from last comment: Firebug 1.11.4

The code is called from the console like `myprogram.modules.run("asdfasdfgarbagehere")`. The exact string does not matter. It looks like this:

myprogram.modules = {
  run: function (str) {
    var data = myprogram.data[str] || str,
        i = 0,
        rundata = function () {
          var dat = data[i++];
          console.log(dat);
          switch (typeof dat) {
            case "string":
              this.run(dat);
          }
        }
  }
}

Thus, calling it goes into infinite recursion, printing "a" all the while.

Interestingly, calling it from the Web Console does not cause a crash, *even if Firebug is open*. If this is done, Firebug prints (using Copy Error):

firebug-service ERROR in ERROR: InternalError: too much recursion chrome://firebug/content/js/debugger.js@2929
resource://firebug/firebug-service.js
Line 4521

Comment 11

[Alex Xu]

More interesting: running it once from Web Console appears to "fix" the issue. Running it again in Firebug after running it from the Web Console does not cause a crash.

Reloading doesn't reset it, but restarting the browser does.

Comment 12

[Kannan Vijayan [:djvj]]

Awesome, thanks Alex.  Will look into this on monday.

Comment 13

[Kannan Vijayan [:djvj]]

(In reply to Alex Xu from comment #11)
> More interesting: running it once from Web Console appears to "fix" the
> issue. Running it again in Firebug after running it from the Web Console
> does not cause a crash.
> 
> Reloading doesn't reset it, but restarting the browser does.

Alex: Just to confirm, those steps for replicating are for a 64-bit build on a Linux system?

Comment 14

[Alex Xu]

(In reply to Kannan Vijayan [:djvj] from comment #13)
> (In reply to Alex Xu from comment #11)
> > More interesting: running it once from Web Console appears to "fix" the
> > issue. Running it again in Firebug after running it from the Web Console
> > does not cause a crash.
> > 
> > Reloading doesn't reset it, but restarting the browser does.
> 
> Alex: Just to confirm, those steps for replicating are for a 64-bit build on
> a Linux system?

Yes.

Comment 15

[Kannan Vijayan [:djvj]]

(In reply to Alex Xu from comment #14)
> (In reply to Kannan Vijayan [:djvj] from comment #13)
> > (In reply to Alex Xu from comment #11)
> > > More interesting: running it once from Web Console appears to "fix" the
> > > issue. Running it again in Firebug after running it from the Web Console
> > > does not cause a crash.
> > > 
> > > Reloading doesn't reset it, but restarting the browser does.
> > 
> > Alex: Just to confirm, those steps for replicating are for a 64-bit build on
> > a Linux system?
> 
> Yes.

Well, I tried building on my Linux system with clang 3.1 as per your specifications and I can't get it to replicate.  3.1 is the latest packaged version I have so I'm building 3.3 from source right now.  In the meantime, is there any way I can get my hands on a core file from one of your crashes?

Comment 16

[Alex Xu]

I have successfully reproduced the crash on Firefox Nightly and have a core dump available. (currently compressing)

Where should I send this to?

Comment 17

[Alex Xu]

Hit submit too fast.

I meant to say: I have successfully reproduced the crash on Firefox Nightly (built from http://hg.mozilla.org/mozilla-central/rev/04d8c309fe72) with a clean profile with only Firebug installed. I have a core dump available, which is currently being uploaded to my host. (only 19M after xz)

Where can I send it off to? (still would prefer not to have it open to the world)

Comment 18

[Kannan Vijayan [:djvj]]

Send it to me.  My e-mail address should be visible to you.

Comment 19

[Kannan Vijayan [:djvj]]

(In reply to Alex Xu from comment #17)
> Where can I send it off to? (still would prefer not to have it open to the
> world)

Any luck on the upload?  I'm reasonably sure this bug is related to a bad return address being written out.  Jan pointed out BaselineFrame::initForOsr a a potential culprit.

I want to take a look at what the return address looks like in this code, and where it's mapping to, and how it compares to the ICEntry table.

My clang3.3 build is going poorly - llvm 3.3 and clang build fine, but they're throwing assertions when I try to compile firefox with them.. not sure what's going on.

Comment 20

[Alex Xu]

Sent it to you a few hours ago; maybe it got caught in spam box. Trying again from my personal domain.

Comment 21

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

Locking s-s per djvj's request on IRC.

Comment 22

[Kannan Vijayan [:djvj]]

Attachment: Handle rectifier frames in GetPcScript

Alex: Thanks a bunch for all the help in with reproducing this issue.  I was able to replicate it here, then get it replicating on a debug build, which led quite quickly to the problem.


Here's the issue:
When unwinding frames during exceptions, we update ionTop to point successively to the frame above the one we have just handled.

This can make it so that ionTop points to a Rectifier (or potentially Unwound_Rectifier) frame.

However, ion::GetPcScript doesn't handle the case where ionTop is at a rectifier frame.  It assumes that ionTop will always be either a Baseline, BaselineStub, or OptimizedJS frame (or Unwound variants thereof).  It tries reading doing script/pc retreival on the rectifier frame, thinking it's a baseline frame, and crashes.

This code path is invoked in particular by exception handling in debug mode, which seems to traverse the stack for some reason.

Comment 23

[Jan de Mooij [:jandem]]

Comment on attachment 773631 [details] [diff] [review]
Handle rectifier frames in GetPcScript

Review of attachment 773631 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh, good find!

Comment 24

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cad670b92cfe

Alex:  Thanks again for the invaluable help!

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cad670b92cfe

Possible to write a test for this?

Comment 26

[Kannan Vijayan [:djvj]]

Comment on attachment 773631 [details] [diff] [review]
Handle rectifier frames in GetPcScript

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
Bug 810824.

User impact if declined:
Potentially exploitable crash in debug mode (e.g. w/ firebug)

Testing completed (on m-c, etc.):
Green on m-i for two days.

Risk to taking this patch (and alternatives if risky):
Very low.  Handles a case that will otherwise always crash if encountered (or silently generate incorrect behaviour).
 
String or IDL/UUID changes made by this patch:
None.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/587ed5a42723
https://hg.mozilla.org/releases/mozilla-beta/rev/485792c15e17

Comment 28

[Kannan Vijayan [:djvj]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #25)
> https://hg.mozilla.org/mozilla-central/rev/cad670b92cfe
> 
> Possible to write a test for this?

I'll look into writing a test for this, but it might not be easy.  I will add it to my list of tests-needed bugs to look at.

Comment 29

[Kannan Vijayan [:djvj]]

A few weeks ago I tried for quite a while to write up a test case that revealed this error, and wasn't successful.  It's a pretty subtle set of interactions that lead to this, and they're proving very hard to replicate.

I'm removing the testsuite flag for this bug as I don't expect to be able to write a test case that triggers the issue this patch fixes.

Comment 30

[u279076]

Calling this verified fixed since I don't see any instances of the signatures in this bug for Firefox 24 or 25 in the last week.