review for 7145024: Crashes in ucrypto related to C2

[Vladimir Kozlov]

Tom,

I don't get T_ARRAY argument processing in ComputeMoveOrder(). T_ARRAY has two 
out arguments length and array pointer so two moves are involved. I don't see 
how code handles it.

I think next code in break_cycle() could be simpler since it is cycle chain:

1416       MoveOperation* p = prev();
1417       if (p->_next != NULL) {
1418         p->_next->_prev = NULL;
1419       }

as this:

1416       MoveOperation* p = prev();
1417       assert(p->_next == this, "");
1418       _prev = NULL;

Why you don't want new test to be executed during regular testing? You used 
"manual" and double "##". Did you try to run it with jtreg.

Thanks,
Vladimir

Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/7145024
> 489 lines changed: 451 ins; 22 del; 16 mod; 3645 unchg
> 
> 7145024: Crashes in ucrypto related to C2
> Reviewed-by:
> 
> There are two issues here.  The first issue, that resulted in the
> asserts was incorrect incrementing of the slots when building the oop
> map.  Fixing that exposed the second issue which is that different
> signatures may require very different orders for the move to avoid
> clobbering other arguments.  There's no simple way to order them
> safely so I resurrected some old C1 code for computing a safe order
> for issuing stores and break any cycles in those stores.  The code
> itself is fairly general but it's not necessary on the other platforms
> so I kept it in the platform dependent code instead of moving it into
> a shared file.  Tested with a new manual test that exercises all
> permutations of 8 arguments that mix primtives and arrays on Solaris,
> Linux and Windows.  Also tested with failing
> javasoft.sqe.tests.api.java.security.Signature.updateTests test.
>