RFR(L): 8032410: compiler/uncommontrap/TestStackBangRbp.java times out on Solaris-Sparc V9

[Christian Thalinger]

On Mar 14, 2014, at 6:42 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:

> 
> Hi Vladimir,
> 
>>>> With +1 you will touch yellow page because it is inclusive if I read it
>>>> right:
>>>> 
>>>>   while (bang_offset <= bang_end) {
>>>> 
>>>> Can you test with StackShadowPages=1?
>>> 
>>> Are you suggesting I run with StackShadowPages=1 to check if:
>>> 
>>> 137     int bang_end = (StackShadowPages+1)*page_size;
>>> 
>>> is ok?
>> 
>> Yes, because you may be creating hole in banging if compiled code called from interpreter. It should be consistent with AbstractInterpreterGenerator::bang_stack_shadow_pages().
> 
> The VM doesn’t like running with StackShadowPages=1. Every crash that I have running the regression tests with (StackShadowPages+1)*page_size, I can reproduce with StackShadowPages*page_size.
> 
> The bad thing that could happen would be for stack banging to hit the red zone or even past the red zone, right? I tried to write a test case that would cause this to happen and my conclusion is that it’s not possible as long as we have more than one yellow page. Let’s say StackShadowPages=1. Worst case is that sp points somewhere in the shadow page when we enter a compiled method. Then we bang sp + 2 pages which hits in the second yellow page.
> 
> This said, I don’t understand why 8026775 changed, in macroAssembler_<arch>.cpp:
> 
> -  for (int i = 0; i< StackShadowPages-1; i++) {
> 
> to
> 
> +  for (int i = 1; i <= StackShadowPages; i++) {
> 
> for the stack banging during deoptimizations. To me:
> 
> +  for (int i = 1; i < StackShadowPages; i++) {
> 
> would have been good enough. So what would make sense to me is to use:
> 
> +  for (int i = 1; i < StackShadowPages; i++) {
> 
> for the stack banging from the deopt blobs. And:
> 
> int bang_end = StackShadowPages*page_size;
> 
> for the stack banging from compiled code.

This was very hairy to get right and hopefully Mikael still has all the details swapped in.

> 
> Roland.