The second bug triggered by spectral-norm (bug 717193 is the other one). The problem is that generateVMWrapper takes registers from the non-volatile set, but OOL VM calls only push/pop the volatile regs. So OOL calls can clobber live registers in the non-volatile set. I discussed this a bit with bhackett, he suggested to push/pop all live registers for OOL calls, volatile and non-volatile.

Before a VM call you should always clobber Markable data such as Value, String and Objects and data stored in volatile registers, if generateVMWrapper is restricting it-self to volatile registers or is saving non-volatile registers. Otherwise all should be clobbered. This rule does not hold for JS Calls because we do not consider different state of registers, thus all should be clobbered. The simple thing to do, as we already discuss with dvander, was to clobber everything and to see later if there is any need of optimization. This optimization should avoid spilling non-volatile registers containing Double or Int around VM calls. To refer to bug 717193, Move group are not made to work with base offset of memory access. The point of MoveGroup was to handle some kind instantaneous move from one position to another among stack slots and registers. One of the (non-implemented) assertion made in the MoveGroup, is that the base register is the stack pointers. Unfortunately, due to the mis-aligned stack, we cannot rely on the stack pointer (at least on x86 and x64) to copy the arguments, and thus we need an extra register (argBase). By using volatile registers for argBase we make sure with are working with undefined behavior of the MoveGroup. One simple solution, is to restrict generateVMWrapper/argBase to non-volatile registers, such as we can avoid this bug. This is guarantee to work under the Move Resolver, because we have no intersection between argument registers (subset of volatile registers) and non-volatile registers.

As we discussed on IRC, this patch just saves all registers. I'll file a follow-up bug to optimize this.

(In reply to Jan de Mooij (:jandem) from comment #2) > As we discussed on IRC, this patch just saves all registers. I'll file a > follow-up bug to optimize this. Bug 717374.

Comment on attachment 587793 [details] [diff] [review] Patch Review of attachment 587793 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.