Closed Bug 1300550 Opened 4 years ago Closed 4 years ago

Hit MOZ_CRASH(BOffImm offset out of range) at js/src/jit/arm/Assembler-arm.h:1013 or Crash [@ js::jit::BOffImm::BOffImm]

Categories

(Core :: JavaScript Engine, defect)

ARM
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox48 --- unaffected
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: decoder, Assigned: h4writer)

Details

(5 keywords, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(1 file, 2 obsolete files)

The following testcase crashes on mozilla-central revision 401ea746b1a9 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --thread-count=2 --disable-oom-functions --ion-offthread-compile=off --arm-asm-nop-fill=1):

var BUGNUMBER = 280769;
var N = 10 * (BUGNUMBER);
var a = new Array(N);
for (var i = 0; i != N; ++i) {
  a[i] = i;
}
var str = a.join('|');
var re = new RegExp(str);
re.exec(N - 1);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0843731e in js::jit::BOffImm::BOffImm (this=0xffff91f4, offset=-44923052) at js/src/jit/arm/Assembler-arm.h:1013
#0  0x0843731e in js::jit::BOffImm::BOffImm (this=0xffff91f4, offset=-44923052) at js/src/jit/arm/Assembler-arm.h:1013
#1  0x0843616a in js::jit::BufferOffset::diffB<js::jit::BOffImm> (other=..., this=<optimized out>) at js/src/jit/shared/IonAssemblerBuffer.h:51
#2  js::jit::Assembler::as_b (this=0xffffa1a8, l=0xffff996c, c=js::jit::Assembler::NotEqual) at js/src/jit/arm/Assembler-arm.cpp:2407
#3  0x08450654 in js::jit::MacroAssemblerARM::ma_b (this=0xffffa1a8, dest=0xffff996c, c=js::jit::Assembler::NotEqual) at js/src/jit/arm/MacroAssembler-arm.cpp:1403
#4  0x088f9ceb in js::jit::MacroAssembler::branch32<js::jit::Label*> (label=<optimized out>, rhs=..., lhs=..., cond=js::jit::Assembler::NotEqual, this=0xffffa1a8) at js/src/jit/arm/MacroAssembler-arm-inl.h:1062
#5  js::irregexp::NativeRegExpMacroAssembler::CheckNotCharacter (this=0xffffa18c, c=50, on_not_equal=0xffff996c) at js/src/irregexp/NativeRegExpMacroAssembler.cpp:611
#6  0x089358df in js::irregexp::RegExpNode::EmitQuickCheck (this=0xd51569e8, compiler=0xffff9d3c, trace=0xffff93e8, preload_has_checked_bounds=true, on_possible_success=0xc821dac0, details=0xc821dacc, fall_through_on_failure=false) at js/src/irregexp/RegExpEngine.cpp:4800
#7  0x089463cf in js::irregexp::ChoiceNode::Emit (this=0xe49b1360, compiler=0xffff9d3c, trace=0xffff96d0) at js/src/irregexp/RegExpEngine.cpp:4430
#8  0x08944e6c in js::irregexp::ActionNode::Emit (this=0xd5156a10, compiler=0xffff9d3c, trace=0xffff9790) at js/src/irregexp/RegExpEngine.cpp:4582
#9  0x08938048 in js::irregexp::ChoiceNode::EmitOutOfLineContinuation (this=0xd5156a58, compiler=0xffff9d3c, trace=0xffff98f8, alternative=..., alt_gen=0xffff9964, preload_characters=1, next_expects_preload=false) at js/src/irregexp/RegExpEngine.cpp:4552
#10 0x08946215 in js::irregexp::ChoiceNode::Emit (this=0xd5156a58, compiler=0xffff9d3c, trace=0xffff9bd0) at js/src/irregexp/RegExpEngine.cpp:4507
#11 0x089377a9 in js::irregexp::RegExpCompiler::Assemble (this=0xffff9d3c, cx=0xf7953000, assembler=0xffffa18c, start=0xd5156a58, capture_count=0) at js/src/irregexp/RegExpEngine.cpp:1755
#12 0x089485c6 in js::irregexp::CompilePattern (cx=0xf7953000, shared=0xf797b100, data=0xffffaaa4, sample=..., is_global=false, ignore_case=false, is_ascii=true, match_only=false, force_bytecode=false, sticky=false, unicode=false) at js/src/irregexp/RegExpEngine.cpp:1912
#13 0x086fd8f9 in js::RegExpShared::compile (this=0xf797b100, cx=0xf7953000, pattern=..., input=..., mode=js::RegExpShared::Normal, force=js::RegExpShared::DontForceByteCode) at js/src/vm/RegExpObject.cpp:594
#14 0x086fdcd3 in js::RegExpShared::compile (this=0xf797b100, cx=0xf7953000, input=..., mode=js::RegExpShared::Normal, force=js::RegExpShared::DontForceByteCode) at js/src/vm/RegExpObject.cpp:563
#15 0x086fdef4 in js::RegExpShared::compileIfNecessary (this=<optimized out>, cx=<optimized out>, input=..., mode=<optimized out>, force=<optimized out>) at js/src/vm/RegExpObject.cpp:616
#16 0x08701683 in js::RegExpShared::execute (this=0xf797b100, cx=0xf7953000, input=..., start=0, matches=0xffffb168, endIndex=0x0) at js/src/vm/RegExpObject.cpp:630
#17 0x089197be in ExecuteRegExpImpl (cx=cx@entry=0xf7953000, res=res@entry=0xf4153500, re=..., input=..., searchIndex=0, matches=0xffffb168, endIndex=0x0) at js/src/builtin/RegExp.cpp:126
#18 0x08919bd3 in ExecuteRegExp (cx=cx@entry=0xf7953000, regexp=..., string=..., string@entry=..., lastIndex=<optimized out>, matches=0xffffb168, endIndex=0x0, staticsUpdate=js::UpdateRegExpStatics) at js/src/builtin/RegExp.cpp:908
#19 0x08920528 in RegExpMatcherImpl (cx=cx@entry=0xf7953000, regexp=..., regexp@entry=..., string=..., string@entry=..., lastIndex=0, staticsUpdate=js::UpdateRegExpStatics, rval=...) at js/src/builtin/RegExp.cpp:930
#20 0x089208cf in js::RegExpMatcher (cx=0xf7953000, argc=3, vp=0xf401e168) at js/src/builtin/RegExp.cpp:967
#21 0x086cef6b in js::CallJSNative (cx=0xf7953000, native=0x89206c0 <js::RegExpMatcher(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
[...]
#54 main (argc=7, argv=0xffffcd54, envp=0xffffcd74) at js/src/shell/js.cpp:7659


This is a controlled crash in both debug and optimized builds, so likely not a security problem.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/6b1e076dbcb7
user:        Hannes Verschore
date:        Tue May 24 07:43:20 2016 +0200
summary:     Bug 1269313: IonMonkey - Use TI to break alias between instructions, r=jandem

This iteration took 223.186 seconds to run.
Too late to fix for 49. We could still take a patch for 50 though.
Hannes, is bug 1269313 a likely regressor?
Blocks: 1269313
Flags: needinfo?(hv1989)
I think the regression goes back even further (maybe start of ARM build?). Cannot put a revision on it.
No longer blocks: 1269313
Flags: needinfo?(hv1989)
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → hv1989
Attachment #8792431 - Flags: review?(sstangl)
Comment on attachment 8792431 [details] [diff] [review]
Patch

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

Assembler::bind() is missing a check that nextLink() is false, and can read uninitialized memory.

If an INVALID offset is bound, compilation should fail, right? Where is the point at which these INVALID offsets are detected?
Flags: needinfo?(hv1989)
Attached patch Patch (obsolete) — Splinter Review
Retry. I now call m_buffer.fail_bail() whenever calling that function causes an incorrect offset. Not sure if that fixes everything or if this patch is too invasive. But I think it should do the job?
Attachment #8792431 - Attachment is obsolete: true
Attachment #8792431 - Flags: review?(sstangl)
Flags: needinfo?(hv1989)
Attachment #8801721 - Flags: review?(sstangl)
we are a week away from RC, too late, this is now a wontfix for 50. Also, I don't see any instances of this crash on 50 in last 7 days.
Comment on attachment 8801721 [details] [diff] [review]
Patch

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

This looks sufficient. I wish we had a Rust-like Result-style return type that required error-checking.
Attachment #8801721 - Flags: review?(sstangl) → review+
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/683d731aad23
IonMonkey - Don't overflow BufferOffsets, r=sstangl
https://hg.mozilla.org/mozilla-central/rev/683d731aad23
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
We could still land this in 51 if you want to uplift to aurora. Hannes, what do you think?
Flags: needinfo?(hv1989)
Attached patch PatchSplinter Review
Approval Request Comment
[Feature/regressing bug #]:
Start of IonMonkey or something. Long long time ago.

[User impact if declined]:
Crashes on ARM builds.

[Describe test coverage new/current, TreeHerder]:
In tree for 6 days

[Risks and why]: 
Could be that not everything is fixed. Though the status quo with this patch is better than the status quo without this patch. 

[String/UUID change made/needed]:
/
Attachment #8801721 - Attachment is obsolete: true
Flags: needinfo?(hv1989)
Attachment #8807498 - Flags: review+
Attachment #8807498 - Flags: approval-mozilla-aurora?
Comment on attachment 8807498 [details] [diff] [review]
Patch

Crash fix, let's take this for aurora 51.
Attachment #8807498 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.