Bug 79104 – Unaligned userspace access for SH4 platforms

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[thouraya]

Unaligned access memory when running sunspider

[thouraya]

Created attachment 127968 [details]
fix unaligend access memory issue in YARR JIT

[thouraya]

Hi,

Any update on the patch?

Regards,
Thouraya.

[Filip Pizlo]

Comment on attachment 127968 [details]
fix unaligend access memory issue in YARR JIT

View in context: https://bugs.webkit.org/attachment.cgi?id=127968&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:978
> +        m_assembler.ensureSpace(m_assembler.maxInstructionSize + 68, sizeof(uint32_t));
> +        move(scr, SH4Registers::r0);
> +        m_assembler.andlImm8r(0x3, SH4Registers::r0);
> +        m_assembler.cmpEqImmR0(0x0, SH4Registers::r0);
> +        m_assembler.branch(BF_OPCODE, 2 + extraInst);
> +
> +        if (dest != SH4Registers::r0)
> +            move(scr1, SH4Registers::r0);
> +
> +        load32(scr, dest);
> +        m_assembler.branch(BRA_OPCODE, 25 + extraInst);
> +        m_assembler.nop();
> +
> +        m_assembler.andlImm8r(0x1, SH4Registers::r0);
> +        m_assembler.cmpEqImmR0(0x0, SH4Registers::r0);
> +
> +        if (dest != SH4Registers::r0)
> +            move(scr1, SH4Registers::r0);
> +
> +        m_assembler.branch(BF_OPCODE, 8);

Why aren't you just using the macro assembler API instead of having to manually compute branch offsets?

[thouraya]

Created attachment 132284 [details]
patch

Hi,

attached the patch to fix the unaligned user space access and some changes to use the macro assembler API instead of  computing manually branch offsets.


Regards,
Thouraya.

[thouraya]

Hi,

Please,
Could you have a look on the patch ?

Thanks a lot.
thouraya.

(In reply to comment #3)
> (From update of attachment 127968 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127968&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:978
> > +        m_assembler.ensureSpace(m_assembler.maxInstructionSize + 68, sizeof(uint32_t));
> > +        move(scr, SH4Registers::r0);
> > +        m_assembler.andlImm8r(0x3, SH4Registers::r0);
> > +        m_assembler.cmpEqImmR0(0x0, SH4Registers::r0);
> > +        m_assembler.branch(BF_OPCODE, 2 + extraInst);
> > +
> > +        if (dest != SH4Registers::r0)
> > +            move(scr1, SH4Registers::r0);
> > +
> > +        load32(scr, dest);
> > +        m_assembler.branch(BRA_OPCODE, 25 + extraInst);
> > +        m_assembler.nop();
> > +
> > +        m_assembler.andlImm8r(0x1, SH4Registers::r0);
> > +        m_assembler.cmpEqImmR0(0x0, SH4Registers::r0);
> > +
> > +        if (dest != SH4Registers::r0)
> > +            move(scr1, SH4Registers::r0);
> > +
> > +        m_assembler.branch(BF_OPCODE, 8);
> 
> Why aren't you just using the macro assembler API instead of having to manually compute branch offsets?

[thouraya]

Hi,

Any updates?

Regards.

[WebKit Review Bot]

Comment on attachment 132284 [details]
patch

Clearing flags on attachment: 132284

Committed r112192: <http://trac.webkit.org/changeset/112192>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.