review for 7047961: JSR 292 MethodHandleWalk swap args doesn't handle T_LONG and T_DOUBLE properly

[Vladimir Kozlov]

Why you used DEBUG instead of ASSERT? For fastdebug build DEBUG is not defined.

Not all ctor initialize ArgToken._value field.

Could you add comment to next check because it looks strange since it narrow 
error cases? In other words, for example, why it is OK to have bad argument 
index with conv_op <= OP_RETYPE_RAW:

+       if ((arg_slot < 0 || arg_slot >= _outgoing.length()) &&
+           conv_op > java_lang_invoke_AdapterMethodHandle::OP_RETYPE_RAW &&
+           conv_op != java_lang_invoke_AdapterMethodHandle::OP_COLLECT_ARGS &&
+           conv_op != java_lang_invoke_AdapterMethodHandle::OP_FOLD_ARGS) {
+         lose(err_msg("bad argument index %d", arg_slot), CHECK_(empty));
         }

verify_args_and_signature() should be under "ifdef ASSERT".

Vladimir

Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/7047961
> 316 lines changed: 213 ins; 27 del; 76 mod; 5061 unchg
> 
> 7047961: JSR 292 MethodHandleWalk swap args doesn't handle T_LONG and T_DOUBLE properly
> Reviewed-by:
> 
> Several more errors were found with handling op arguments in
> MethodHandleWalk.  swap wasn't dealing with the double word arguments
> properly.  collect and fold are allowed to have 0 length operations.
> rotate was using the wrong type to determine the size of the
> operation.  boxing operations are allowed on subword types.  nop
> conversion operations should just do nothing.  I also removed the
> SlotState wrapper since its only purpose seemed to be to keep track of
> the arg types when symbolic was used and I modified tt_symbolic to
> keep accurate track of the type instead of claiming to be a long.  I
> also added more detail to most of the failure messages and fixed a
> missing ResourceMark along with adding a lot more sanity checks.
> Tested with vm/mlvm and java/lang/invoke regression tests.
>