473225 - "Assertion failed: ( int32_t(delta) == uint8_t(delta) )" on webreference.com

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Loading http://www.webreference.com/ triggers this fatal JavaScript Engine assertion:

Assertion failed: ( int32_t(delta) == uint8_t(delta) ) (/Users/jruderman/central/js/src/nanojit/LIR.cpp:253)

I have only tested this on mozilla-central.

Comment 1

[Andreas Gal :gal]

Confirmed with TM tip (debug build).

Comment 2

[Andreas Gal :gal]

#0  NanoAssertFail () at ../../../js/src/nanojit/avmplus.cpp:54
#1  0x0030a736 in nanojit::LIns::reference (this=0x6f9710, r=0x6f930c) at ../../../js/src/nanojit/LIR.cpp:253
#2  0x0030e88b in nanojit::LirBufWriter::insCall (this=0x199951d0, ci=0x35ef38, args=0xbfffcc98) at ../../../js/src/nanojit/LIR.cpp:1145
#3  0x0030e1b8 in nanojit::CseFilter::insCall (this=0x1914bcb0, ci=0x35ef38, args=0xbfffcc98) at ../../../js/src/nanojit/LIR.cpp:2060
#4  0x0028bf50 in nanojit::LirWriter::insCall (this=0x199f4820, call=0x35ef38, args=0xbfffcc98) at LIR.h:439
#5  0x002f4b85 in FuncFilter::insCall (this=0x199f4830, ci=0x35ef38, args=0xbfffcc98) at ../../../js/src/jstracer.cpp:815
#6  0x002e06bc in TraceRecorder::record_JSOP_GETELEM (this=0x187f20c0) at ../../../js/src/jstracer.cpp:6537
#7  0x002e9c36 in TraceRecorder::monitorRecording (cx=0x1f45c800, tr=0x187f20c0, op=JSOP_GETELEM) at jsopcode.tbl:176
#8  0x002131c0 in js_Interpret (cx=0x1f45c800) at ../../../js/src/jsinterp.cpp:2840
#9  0x00238926 in js_Execute (cx=0x1f45c800, chain=0x1b66f880, script=0x168bb000, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1564
#10 0x001c84c3 in JS_EvaluateUCScriptForPrincipals (cx=0x1f45c800, obj=0x1b66f880, principals=0x1916cca4, chars=0x19300008, length=241865, filename=0x19252c58 "http://kona.kontera.com/javascript/lib/KonaBase.js?00000000286", lineno=1, rval=0x0) at ../../../js/src/jsapi.cpp:5192
#11 0x132a1955 in nsJSContext::EvaluateString (this=0x1bb8edf0, aScript=@0x1bd8f794, aScopeObject=0x1b66f880, aPrincipal=0x1916cca0, aURL=0x19252c58 "http://kona.kontera.com/javascript/lib/KonaBase.js?00000000286", aLineNo=1, aVersion=0, aRetValue=0x0, aIsUndefined=0xbfffde14) at ../../../../dom/src/base/nsJSEnvironment.cpp:1588
#12 0x1307d620 in nsScriptLoader::EvaluateScript (this=0x1920bfc0, aRequest=0x1bd8f780, aScript=@0x1bd8f794) at ../../../../content/base/src/nsScriptLoader.cpp:653
#13 0x1307d88a in nsScriptLoader::ProcessRequest (this=0x1920bfc0, aRequest=0x1bd8f780) at ../../../../content/base/src/nsScriptLoader.cpp:567
#14 0x1307d916 in nsScriptLoader::ProcessPendingRequests (this=0x1920bfc0) at ../../../../content/base/src/nsScriptLoader.cpp:706
#15 0x1307dba0 in nsScriptLoader::OnStreamComplete (this=0x1920bfc0, aLoader=0x1912c940, aContext=0x1bd8f780, aStatus=0, aStringLen=241865, aString=0x18671008 "if(typeof Mogxe==\"undefined\"){var Mogxe=new Object();}if(typeof Mogxe.VEBNU==\"undefined\"){Mogxe.VEBNU=new Object();}if(typeof Mogxe.lIbMf==\"undefined\"){Mogxe.lIbMf=new Object();}Mogxe.j9GfP=function(Y"...) at ../../../../content/base/src/nsScriptLoader.cpp:884
#16 0x114026b1 in nsStreamLoader::OnStopRequest (this=0x1912c940, request=0x19920620, ctxt=0x1bd8f780, aStatus=0) at ../../../../netwerk/base/src/nsStreamLoader.cpp:108
#17 0x11426113 in nsHTTPCompressConv::OnStopRequest (this=0x1d87f790, request=0x19920620, aContext=0x1bd8f780, aStatus=0) at ../../../../netwerk/streamconv/converters/nsHTTPCompressConv.cpp:127
#18 0x114adb77 in nsHttpChannel::OnStopRequest (this=0x199205f0, request=0x192176c0, ctxt=0x1bd8f780, status=0) at ../../../../../netwerk/protocol/http/src/nsHttpChannel.cpp:4832
#19 0x113ceaf8 in nsInputStreamPump::OnStateStop (this=0x192176c0) at ../../../../netwerk/base/src/nsInputStreamPump.cpp:576
#20 0x113cec18 in nsInputStreamPump::OnInputStreamReady (this=0x192176c0, stream=0x190c516c) at ../../../../netwerk/base/src/nsInputStreamPump.cpp:401
#21 0x00443e1e in nsInputStreamReadyEvent::Run (this=0x1e535c00) at ../../../xpcom/io/nsStreamUtils.cpp:111
#22 0x00476520 in nsThread::ProcessNextEvent (this=0x714cd0, mayWait=0, result=0xbfffe1f4) at ../../../xpcom/threads/nsThread.cpp:510
#23 0x003ffb78 in NS_ProcessPendingEvents_P (thread=0x714cd0, timeout=20) at nsThreadUtils.cpp:180
#24 0x117cfec5 in nsBaseAppShell::NativeEventCallback (this=0x735030) at ../../../../widget/src/xpwidgets/nsBaseAppShell.cpp:121
#25 0x11787108 in nsAppShell::ProcessGeckoEvents (aInfo=0x735030) at ../../../../widget/src/cocoa/nsAppShell.mm:374
#26 0x90df65f5 in CFRunLoopRunSpecific ()
#27 0x90df6cd8 in CFRunLoopRunInMode ()
#28 0x90fa32c0 in RunCurrentEventLoopInMode ()
#29 0x90fa3012 in ReceiveNextEventCommon ()
#30 0x90fa2f4d in BlockUntilNextEventMatchingListInMode ()
#31 0x9013cd7d in _DPSNextEvent ()
#32 0x9013c630 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#33 0x9013566b in -[NSApplication run] ()
#34 0x117859ca in nsAppShell::Run (this=0x735030) at ../../../../widget/src/cocoa/nsAppShell.mm:693
#35 0x12482526 in nsAppStartup::Run (this=0x74f4b0) at ../../../../../toolkit/components/startup/src/nsAppStartup.cpp:192
#36 0x000e62e8 in XRE_main (argc=1, argv=0xbffff850, aAppData=0x70e630) at ../../../toolkit/xre/nsAppRunner.cpp:3272
#37 0x000027cb in main (argc=1, argv=0xbffff850) at ../../../browser/app/nsBrowserApp.cpp:156

Comment 3

[Andreas Gal :gal]

Looks like an off by one:

253			NanoAssert(isU8(delta));
(gdb) p delta
$1 = 256
(gdb) list
248		}
249		
250		uint32_t LIns::reference(LIns *r) const
251		{
252			int delta = this-r-1;
253			NanoAssert(isU8(delta));
254			return delta;
255		}
256	
257	    LIns* LIns::deref(int32_t off) const
(gdb)

Comment 4

[Andreas Gal :gal]

The 2nd of 3 arguments.

#2  0x0030e88b in nanojit::LirBufWriter::insCall (this=0x199951d0, ci=0x35ef38, args=0xbfffcc98) at ../../../js/src/nanojit/LIR.cpp:1145
1145				*--offs = (uint8_t) l->i.reference(args[i]);
(gdb) list
1140			l->ci = ci;
1141	
1142			// call parameters laid in reverse order
1143			uint8_t* offs = (uint8_t*)l;
1144			for (int32_t i=0; i < argc; i++)
1145				*--offs = (uint8_t) l->i.reference(args[i]);
1146			NanoAssert((LInsp)offs>=_buf->next());


(gdb) p i
$2 = 1
(gdb) p argc
$3 = 3
(gdb)

Comment 5

[Andreas Gal :gal]

We don't emit a trampoline here because from-to-1 == 255.

(gdb) p args[i]
$5 = (LInsp) 0x6f930c
(gdb) p from
$6 = (LInsp) 0x6f970c
(gdb) p $6-$5-1
$7 = 255
(gdb)

Comment 6

[Andreas Gal :gal]

(gdb) p from
$8 = (LInsp) 0x6f970c
(gdb) p l
$9 = (nanojit::$_18 *) 0x6f970c

Not sure why this is not asserted but the values do match.

Comment 7

[Graydon Hoare :graydon]

deja vu, memories of Bug 464862

Comment 8

[Andreas Gal :gal]

Attachment: trampolines can be 2 words

Comment 9

[Andreas Gal :gal]

Attachment: v2

Comment 10

[Graydon Hoare :graydon]

Ok. Talked to Andreas about this. Our best guess is that we're getting a far tramp inserted. The code-as-written only handles near tramps -- I didn't know about far tramps last time through -- and the tramp-writing loop is causing the LIR buffer too move its next() position past the worst-case estimate we made a few lines earlier. Solution is to multiply the worst-case guess of argc words by LIR_FAR_SLOTS.

Comment 11

[Graydon Hoare :graydon]

Comment on attachment 356630 [details] [diff] [review]
v2

r+ with an assert that l != from as soon as we calculate l.

Comment 12

[Andreas Gal :gal]

Pushed to TM.

http://hg.mozilla.org/tracemonkey/rev/8775c279e59c

Comment 13

[Andreas Gal :gal]

Comment on attachment 356630 [details] [diff] [review]
v2

Asking for ed's review to make sure this makes it over into tamarin soon.

Comment 14

[Andreas Gal :gal]

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

Comment 15

[Andreas Gal :gal]

Attachment: have to make sure we have room

Comment 16

[Andreas Gal :gal]

Pushed to TM.

http://hg.mozilla.org/tracemonkey/rev/4e72d3b2d6b6

Comment 18

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/4e72d3b2d6b6

Comment 19

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/261c21ca413a

Comment 20

[Bob Clary [:bc] (inactive)]

I spent a lot time trying to get a reduced test case for this but failed. If someone else cares to try, please do. otherwise its out of my queue.

Comment 21

[Graydon Hoare :graydon]

Hmm. The thing to try for a minimal case is to force the generation of a call record with its arguments *really* spread out in the instruction stream. So like a foo(x,y,z) to some builtin foo, where x, y and z are really involved expressions that will chew up more than 256 bytes of LIR to encode each of. I think. I'll try to cook something up.

Comment 22

[Graydon Hoare :graydon]

Wow, you're right, this one is pretty tricky. It's easy enough to force the generation of a short trampoline, but a long one has to be to a LIR instruction 16mb away. That's hard to wrangle given the memory cap of 16mb in the LIR buffer itself. The best guess I have is to try making 2 traces at very different times (thus in different mapped segments) and hope they're 16mb of virtual address apart, then use a value formed in one in a call record in the other.

Still, even if we manage to make a case that does so, it'll be extremely delicate and dependent on runtime behavior of the allocator, plus a dozen other assumptions baked into the code generation pipeline and subject to future change in a way that would not strictly represent "regression". So I tend to agree that this is "not minimally testable".

Comment 23

[Aakash Desai [:aakashd]]

If a reduced testcase can't be created, would going to the url provided in the bug and verifying the assertion does not come up be enough to verify this bug at this point?

Comment 24

[Andreas Gal :gal]

This url used t hit a very specific cache state, with code crossing a page boundary a very specific way. I don't think this is easy to reproduce once anything in the system changes, even if you back out the patch. I think we have to close this as fixed based on faith and lack of re-occurrence.

Comment 25

[Aakash Desai [:aakashd]]

If I get a seconded from someone else involved in this bug, I'll go ahead and move it to verified:fixed.

Comment 26

[Graydon Hoare :graydon]

Probably fine for now; this bug was actually the one that motivated me writing the LIR assembler over in bug 484142, though I'm not certain it makes it possible to write a testcase just yet. It's certainly moving in the right direction. I'll make a note in that bug to come back and check this one out for a motivating regression-test example.

Comment 27

[Aakash Desai [:aakashd]]

Ok, I'm moving this to verified FIXED per the comments and no re-occurences as-of-now. If it comes back, just re-open!