Closed Bug 760457 Opened 12 years ago Closed 12 years ago

IonMonkey: Assertion failure: unsupported branch relocation, at js/src/ion/arm/Assembler-arm.cpp:531 or Crash [@ js::ion::Assembler::TraceJumpRelocations]

Categories

(Core :: JavaScript Engine, defect)

Other Branch
ARM
Linux
defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr10 --- unaffected

People

(Reporter: decoder, Unassigned)

References

Details

(Keywords: assertion, testcase)

Attachments

(1 file)

The following testcase asserts on ionmonkey-arm (private branch) revision  (run with --ion -n -m --ion-eager):


function startTest() {}
startTest();
gc();
I guess this could be the GC failure that Marty mentioned is still present even on his branch.


Crash info on opt-build:

Program received signal SIGSEGV, Segmentation fault.
js::ion::Assembler::TraceJumpRelocations (trc=0x340758, code=0x40a0d080, reader=<value optimized out>) at js/src/ion/arm/Assembler-arm.cpp:588
588             IonCode *child = CodeFromJump((Instruction *) (code->raw() + iter.offset()));
(gdb) x /i $pc
=> 0x1f2a04 <js::ion::Assembler::TraceJumpRelocations(JSTracer*, js::ion::IonCode*, js::ion::CompactBufferReader&)+56>: ldr.w   r3, [r0, #-4]
(gdb) info reg r3 r0
r3             0x52df000        86896640
r0             0x40dc8450       1088193616
(gdb) bt
#0  js::ion::Assembler::TraceJumpRelocations (trc=0x340758, code=0x40a0d080, reader=<value optimized out>) at js/src/ion/arm/Assembler-arm.cpp:588
#1  0x001b727e in js::ion::IonCode::trace (this=0x40a0d080, trc=0x340758) at js/src/ion/Ion.cpp:333
#2  0x0014000a in MarkChildren (this=0x340758, budget=..., tag=<value optimized out>, addr=1084280960) at js/src/gc/Marking.cpp:887
#3  js::GCMarker::processMarkStackOther (this=0x340758, budget=..., tag=<value optimized out>, addr=1084280960) at js/src/gc/Marking.cpp:1076
#4  0x00140632 in processMarkStackTop (this=0x340758, budget=...) at js/src/gc/Marking.cpp:1150
#5  js::GCMarker::drainMarkStack (this=0x340758, budget=...) at js/src/gc/Marking.cpp:1246
#6  0x000505c4 in NonIncrementalMark (rt=0x340678, incremental=<value optimized out>, budget=0, gckind=js::GC_NORMAL) at js/src/jsgc.cpp:3310
#7  GCCycle (rt=0x340678, incremental=<value optimized out>, budget=0, gckind=js::GC_NORMAL) at js/src/jsgc.cpp:3660
#8  0x00050a48 in Collect (rt=0x340678, incremental=<value optimized out>, budget=<value optimized out>, gckind=js::GC_NORMAL, reason=js::gcreason::API) at js/src/jsgc.cpp:3769
The issue here is that before and after linking, there need to be two separate methods for indexing into code buffers, and getting the next instruction.  This patch does the dumb thing, and re-indexes into the buffer every time.
Attachment #631666 - Flags: review?(Jacob.Bramley)
Comment on attachment 631666 [details] [diff] [review]
/home/mrosenberg/patches/early_gc-r0.patch

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

::: js/src/ion/arm/Assembler-arm.cpp
@@ +564,2 @@
>  {
> +    Instruction *load = iter->cur();

Call it load1 for consistency with load2. Actually, it'd be best to call them ins1, ins2 and so on, so that the abstraction still works if the instructions aren't loads or branches. For example, in the case where you have "str pc, [sp]", the instruction at "branch" is not a branch.

@@ +585,5 @@
>          // bx r_temp
>          // OR
>          // movw r_temp, #imm1
>          // movt r_temp, #imm2
>          // str pc, [sp]

Assert that the third instruction is what you expect.

@@ +670,5 @@
>  
>  void
>  Assembler::TraceJumpRelocations(JSTracer *trc, IonCode *code, CompactBufferReader &reader)
>  {
>      RelocationIterator iter(reader);

Consider renaming this to "relociter" (or something similar) to be consistent with institer.

@@ +734,5 @@
>              JS_ASSERT(code == IonCode::FromExecutable((uint8*)rp.target));
>          }
>      }
>      if (tmpDataRelocations_.length()) {
> +        ::TraceDataRelocations(trc, &m_buffer, &tmpDataRelocations_);

Did you mean dataRelocations_? (That's what the original code did here.)

@@ +2317,3 @@
>      Register dest;
>      Assembler::RelocStyle rs;
> +    const uint32 *val = getPtr32Target(&iter, &dest, &rs);

Why not use getPointer to avoid all the nasty casts?

::: js/src/ion/arm/Assembler-arm.h
@@ +1948,5 @@
> +    Instruction *i;
> +  public:
> +    InstructionIterator(Instruction *i_) : i(i_) {}
> +    void next() {
> +        i = i->next();

If next() returned a value, you could simplify calling code that retrieves sequences of instructions.
Attachment #631666 - Flags: review?(Jacob.Bramley) → review+
landed + fixed: http://hg.mozilla.org/projects/ionmonkey/rev/8a4f7d5a2fa1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.