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

RESOLVED FIXED in Firefox 51

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: decoder, Assigned: h4writer)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla52
ARM
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 unaffected, firefox49 wontfix, firefox50 wontfix, firefox51 fixed, firefox52 fixed)

Details

(Whiteboard: [jsbugmon:update], crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

3 years ago
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.

Updated

3 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

3 years ago
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)
Assignee

Comment 4

3 years ago
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)
Assignee

Comment 5

3 years ago
Posted 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)
Assignee

Comment 7

3 years ago
Posted 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+

Comment 10

3 years ago
Pushed by hv1989@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/683d731aad23
IonMonkey - Don't overflow BufferOffsets, r=sstangl

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/683d731aad23
Status: NEW → RESOLVED
Last Resolved: 3 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)
Assignee

Comment 13

3 years ago
Posted 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.