64-bit - lots of assertions, crashes - "Assertion failed: ( int32_t(offset) == int8_t(offset) ) (../nanojit/NativeX64.cpp:206)"

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {assertion})

Trunk
x86
Mac OS X
assertion
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-nanojit, fixed-in-tamarin, fixed-in-tracemonkey)

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
A day or two ago, 64-bit on 10.6 saw a lot of assertions / crashes on the fuzzers. This doesn't occur on 10.5 32-bit. Examples of assertions:

 35 Assertion failure: !JSVAL_IS_NULL(id), at ../jsscope.cpp:302
   7 Assertion failure: script->main <= target && target < script->code + script->length, at ../jsopcode.cpp:5524
   5 Assertion failure: ((jsval) obj & JSVAL_TAGMASK) == JSVAL_OBJECT, at ../jsapi.h:204
   4 Assertion failure: HAS_FUNCTION_CLASS(callee), at ../jstracer.cpp:5350
   2 Assertion failure: HAS_FUNCTION_CLASS(*(JSObject**)slot), at ../jstracer.cpp:2945

I don't yet have a testcase - need Jesse's help.
Created attachment 413414 [details]
assertion stack I just hit on 64-bit Linux mozilla-central on tinderboxpushlog

Assertion failed: ( int32_t(offset) == int8_t(offset) ) (/home/dbaron/builds/mozilla-central/mozilla/js/src/nanojit/NativeX64.cpp:206)
(Reporter)

Comment 2

8 years ago
It's this changeset that seems to be related:

changeset:   34704:b50c31eb3edd
user:        Nicholas Nethercote
date:        Mon Nov 16 13:14:39 2009 +1100
summary:     Bug 520712 - nanojit: print assembly code for X64 backend with TMFLAGS=assembly.  r=edwsmith.
Blocks: 520712
Bug 529218 has several follow-up fixes.  Looks like the patch has gone into nanojit-central but not yet made it into tracemonkey.  I'll merge it across ASAP.  Hopefully that'll fix things.
The patch in 529918 is now merged into tracemonkey.  It hasn't yet merged into mozilla-central, Sayre will have to do that.

Lesson from this incident:  if I break something and then fix it in nanojit-central, I should ensure that the fix gets merged into tracemonkey ASAP.
(Reporter)

Comment 5

8 years ago
There's now a straggler post bug 529918 patch landing:

Assertion failed: ( int32_t(offset) == int8_t(offset) ) (../nanojit/NativeX64.cpp:206)

Program received signal SIGABRT, Aborted.
0x00007fff85ed7fe6 in __kill ()
(gdb) bt
#0  0x00007fff85ed7fe6 in __kill ()
#1  0x00007fff85f78e32 in abort ()
#2  0x00000001001931f9 in NanoAssertFail () at ../nanojit/avmplus.cpp:77
#3  0x0000000100196db7 in nanojit::Assembler::emit_target8 (this=0x100a9aa08, underrun=0, op=34339947158700034, target=0x10075302f "H#\025ʟ##I#V\b###27Ҝ#H#M#####E########t ../nanojit/NativeX64.cpp:206
#4  0x00000001001979e1 in nanojit::Assembler::JP8 (this=0x100a9aa08, n=0, t=0x10075302f "H#\025ʟ##I#V\b###27Ҝ#H#M#####E########t ../nanojit/NativeX64.cpp:531
#5  0x00000001001a428e in nanojit::Assembler::asm_fbranch (this=0x100a9aa08, onFalse=false, cond=0x100965ba8, target=0x100775907 "L#}###u###m###e###}#########05*#&") at ../nanojit/NativeX64.cpp:1177
#6  0x00000001001a46df in nanojit::Assembler::asm_branch (this=0x100a9aa08, onFalse=false, cond=0x100965ba8, target=0x100775907 "L#}###u###m###e###}#########05*#&") at ../nanojit/NativeX64.cpp:1025
#7  0x0000000100184d2d in nanojit::Assembler::gen (this=0x100a9aa08, reader=0x7fff5fbfe670) at ../nanojit/Assembler.cpp:1328
#8  0x000000010018525f in nanojit::Assembler::assemble (this=0x100a9aa08, frag=0x10094c8c8, reader=0x7fff5fbfe670) at ../nanojit/Assembler.cpp:717
#9  0x000000010018fb2d in nanojit::compile (assm=0x100a9aa08, frag=0x10094c8c8, alloc=@0x100581000, labels=0x100887a08) at ../nanojit/LIR.cpp:2050
#10 0x00000001001555f0 in TraceRecorder::compile (this=0x101840640) at ../jstracer.cpp:4285
#11 0x00000001001564a8 in TraceRecorder::closeLoop (this=0x101840640, slotMap=@0x7fff5fbfe870, exit=0x100a1c9d8) at ../jstracer.cpp:4710
#12 0x00000001001567a2 in TraceRecorder::closeLoop (this=0x101840640, exit=0x100a1c9d8) at ../jstracer.cpp:4599
#13 0x0000000100156cc4 in TraceRecorder::closeLoop (this=0x101840640) at ../jstracer.cpp:4591
#14 0x0000000100156e3f in TraceRecorder::checkTraceEnd (this=0x101840640, pc=0x100423383 "\b##V") at ../jstracer.cpp:5131
#15 0x0000000100167895 in TraceRecorder::relational (this=0x101840640, op=nanojit::LIR_flt, tryBranchAfterCond=true) at ../jstracer.cpp:8813
#16 0x000000010016796f in TraceRecorder::record_JSOP_LT (this=0x101840640) at ../jstracer.cpp:10015
#17 0x000000010016e5d9 in TraceRecorder::monitorRecording (this=0x101840640, op=JSOP_LT) at jsopcode.tbl:139
#18 0x000000010007652b in js_Interpret (cx=0x100411450) at jsops.cpp:78
#19 0x00000001000a2348 in js_Execute (cx=0x100411450, chain=0x1003db000, script=0x100a4b000, down=0x0, flags=0, result=0x0) at jsinterp.cpp:1618
#20 0x0000000100011521 in JS_ExecuteScript (cx=0x100411450, obj=0x1003db000, script=0x100a4b000, rval=0x0) at ../jsapi.cpp:4949
#21 0x0000000100009b86 in Process (cx=0x100411450, obj=0x1003db000, filename=0x7fff5fbff9d6 "tmp2/original.js", forceTTY=0) at ../../shell/js.cpp:439
#22 0x000000010000a7cc in ProcessArgs (cx=0x100411450, obj=0x1003db000, argv=0x7fff5fbff870, argc=2) at ../../shell/js.cpp:848
#23 0x000000010000aae3 in main (argc=2, argv=0x7fff5fbff870, envp=0x7fff5fbff888) at ../../shell/js.cpp:4860
(gdb) f 3    
#3  0x0000000100196db7 in nanojit::Assembler::emit_target8 (this=0x100a9aa08, underrun=0, op=34339947158700034, target=0x10075302f "H#\025ʟ##I#V\b###27Ҝ#H#M#####E########t ../nanojit/NativeX64.cpp:206
206             NanoAssert(isS8(offset));
(gdb) l
201
202         void Assembler::emit_target8(size_t underrun, uint64_t op, NIns* target) {
203             underrunProtect(underrun); // must do this before calculating offset
204             // Nb: see emit_target32() for why we use _nIns here.
205             int64_t offset = target - _nIns;
206             NanoAssert(isS8(offset));
207             emit(op | uint64_t(offset)<<56);
208         }
209
210         void Assembler::emit_target32(size_t underrun, uint64_t op, NIns* target) {
(gdb) x offset
0xfffffffffffa005a:     Cannot access memory at address 0xfffffffffffa005a
(gdb)
Keywords: assertion
Summary: 64-bit - lots of assertions, crashes → 64-bit - lots of assertions, crashes - "Assertion failed: ( int32_t(offset) == int8_t(offset) ) (../nanojit/NativeX64.cpp:206)"
Created attachment 413528 [details] [diff] [review]
patch

I can't reproduce the assertion without a test case, but this patch will
hopefully fix it nonetheless.

- The change in JMP() is just a consistency clean-up.

- The removal of the underrunProtect() is also a clean-up -- it's not needed
  because the isTargetWithinS8() that follows immediately calls
  underrunProtect().

- The addition of underrunProtect(16) is what should fix the assertion.  It
  should also fix dbaron's attached assertion, ie. it's the same assertion.
Attachment #413528 - Flags: review?(edwsmith)
(Reporter)

Comment 7

8 years ago
(In reply to comment #6)
> Created an attachment (id=413528) [details]
> patch
> 
> I can't reproduce the assertion without a test case, but this patch will
> hopefully fix it nonetheless.

I've a ~250 line testcase that is still pending reduction, but a quick test shows this patch does indeed seem to fix the assertion. :)

Updated

8 years ago
Attachment #413528 - Flags: review?(edwsmith) → review+
Committed to nanojit-central:
http://hg.mozilla.org/projects/nanojit-central/rev/3d99d6e37ba8

I'll merge it to tracemonkey as soon as I can, but tracemonkey currently has all kinds of breakage going on.
Whiteboard: fixed-in-nanojit

Comment 9

8 years ago
pushed to tamarin http://hg.mozilla.org/tamarin-redux/rev/3ac43401490f
Whiteboard: fixed-in-nanojit → fixed-in-nanojit fixed-in-tamarin
http://hg.mozilla.org/tracemonkey/rev/91555af5f8b8
Whiteboard: fixed-in-nanojit fixed-in-tamarin → fixed-in-nanojit, fixed-in-tamarin, fixed-in-tracemonkey
(Reporter)

Comment 11

8 years ago
This was pushed to mozilla-central some time ago:

http://hg.mozilla.org/mozilla-central/rev/91555af5f8b8
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.