[PATCH] [mips] Don't emit redundant mask instructions if the mask is already present.

[Mark Seaborn]

On 18 December 2014 at 13:50, Sasa Stankovic <Sasa.Stankovic at imgtec.com>
wrote:

> Yes. Currently two tests fail because of the redundant sandboxing -
> syscall_return_sandboxing_test and exception_test. For
> syscall_return_sandboxing_test, to make the test pass we can either remove
> sandboxing from the assembly, or apply this patch.


In that case, removing the sandboxing from the assembly makes sense.
That's the reason for implementing auto-sandboxing:  assembly code
shouldn't need any explicit sandboxing instructions.


> For exception test, neither removing the sandboxing nor applying the patch
> will work. Here is the problematic inline assembly from exception_test.c
> file:
>
> crash_at_known_address:
> and $zero, $zero, $t7
> prog_ctr_at_crash:
> sw $t0, 0($zero)
>

Incidentally, it's funny that the MIPS validator allows that instruction
sequence.  The "and" is a no-op, because modifying $zero has no effect.
The validator could safely allow "sw ..., 0($zero)" on its own as a "crash
instruction".  (BTW, I am not recommending changing the validator to do
that!)


> exception_test works by crashing when executing instruction labeled by
> "prog_ctr_at_crash", and then checking whether the saved program counter is
> identical to the value of "prog_ctr_at_crash". Test currently fails because
> sandboxing will add mask before "sw", moving the "sw" to the address
> prog_ctr_at_crash + 4. Removing the mask in the assembly obviously wouldn't
> make the test pass. But even with the patch applied this test would still
> fail. That's because patch is implemented so that it first saves the mask,
> and then emits it when emitting next instruction, surrounded by
> BundleLock() and BundleUnlock(). So patch would first save mask, then emit
> label "prog_ctr_at_crash", and then emit mask and "sw", placing "sw" at
> address prog_ctr_at_crash + 4.
>

OK, the key point is that the LLVM patch doesn't fix this test case. :-)


> The only way to make exception_test pass is either to add #ifdef for MIPS
> that would check that program counter has value prog_ctr_at_crash + 4,


That would be reasonable.


> or find instruction that doesn't require masking and that can cause
> program to crash.
>

That would be better.  Is there a MIPS breakpoint/trap instruction that
would work here?  (ARM's breakpoints/traps are complicated under NaCl.  I
can't remember how MIPS compares.)

Alternatively you can do memory accesses via sp without a mask, because sp
is pre-masked.  Something like this should work:

mov $sp, $zero
prog_ctr_at_crash:
sw $t0, ($sp)

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141218/5e375527/attachment.html>