RFR 8157592: StackTraceLogging fails with stack overflow on 32-bit Windows

[George Triantafillou]

Hi Coleen,

Thanks for your comments.

New webrev: 
http://cr.openjdk.java.net/~gtriantafill/8157592/webrev.01/index.html 
<http://cr.openjdk.java.net/%7Egtriantafill/8157592/webrev.01/index.html>

-George

On 6/17/2016 11:48 AM, Coleen Phillimore wrote:
>
> For this
>
> http://cr.openjdk.java.net/~gtriantafill/8157592/webrev/test/runtime/Throwable/StackTraceLogging.java.udiff.html 
>
>
> Can you change the comment:
>
>   44         // These depths match the ones in TestThrowable.java
>
>
> to
>
>   44         // These depths match the ones in TestThrowable.java, 
> except the one greater than 1024
>
>
> And add 1023 to the list in 45?
>
> thanks,
> Coleen
>
>
> On 6/17/16 11:36 AM, George Triantafillou wrote:
>> New webrev: http://cr.openjdk.java.net/~gtriantafill/8157592/webrev/ 
>> <http://cr.openjdk.java.net/%7Egtriantafill/8157592/webrev/>
>>
>> -George
>>
>> On 6/17/2016 10:43 AM, George Triantafillou wrote:
>>> Hi David,
>>>
>>> Thanks for your comments.  Based on your comments and Coleen's 
>>> comments on the purpose of the test, I've made the following changes:
>>>
>>> TestThrowable.java:
>>> - removed the explicit check for StackOverflowError
>>> - changed the maximum depth check from 2042 to 1025
>>>
>>> StackTraceLogging.java:
>>> - removed unused updateEnvironment method
>>>
>>> Tested locally with 32-bit Windows and Linux.
>>>
>>> -George
>>>
>>> On 6/16/2016 5:11 PM, Coleen Phillimore wrote:
>>>>
>>>>
>>>> On 6/16/16 5:02 PM, David Holmes wrote:
>>>>> Hi George,
>>>>>
>>>>> On 17/06/2016 5:22 AM, George Triantafillou wrote:
>>>>>> Hi David,
>>>>>>
>>>>>> I did an experiment by printing the stack depth at smaller 
>>>>>> intervals and
>>>>>> running TestThrowable.java with
>>>>>> -XX:AbortVMOnException=java.lang.StackOverflowError.  In one failing
>>>>>> run, there were 1888 recursions.  Here's the stack of the failing 
>>>>>> thread
>>>>>> from the hs_err file:
>>>>>>
>>>>>> Stack: [0x15980000,0x159d0000],  sp=0x1598b9bc,  free space=46k
>>>>>>
>>>>>> If you divide by the stack size on windows 32, that amounts to
>>>>>> approximately 173 bytes per frame, which doesn’t seem unreasonable.
>>>>>>
>>>>>> (gdb) print (0x159d0000-0x15980000)/1888
>>>>>> $2 = 173
>>>>>>
>>>>>> Factor in that the frames are compiled, which use less space than
>>>>>> interpreted and whether the frames are compiled in time for the 
>>>>>> test to
>>>>>> run can also add variability.  C2 optimizes stack usage more than 
>>>>>> C1,
>>>>>> and the test case appears to compile these methods with C1. Also, 
>>>>>> when
>>>>>> run with jtreg, there are frames on the stack for the jtreg test 
>>>>>> framework.
>>>>>>
>>>>>> The stack usage for this test isn’t unreasonable. There’s no 
>>>>>> change to
>>>>>> stack usage that would have made this test fail now. It’s just 
>>>>>> normal
>>>>>> variability on 32-bit Windows:
>>>>>>
>>>>>> java -XX:+PrintFlagsFinal -version | grep Stack | grep Pages
>>>>>>
>>>>>> intx StackRedPages                             = 1
>>>>>> intx StackShadowPages                          = 9
>>>>>> intx StackYellowPages                          = 3
>>>>>>
>>>>>> 13*4k = 52k which results in stack overflow.
>>>>>
>>>>> Thanks for doing the experiment. Do we have numbers for other 
>>>>> 32-bit platforms to compare against?
>>>>>
>>>>> I'd still rather see the test keep well away from triggering SOE 
>>>>> than to just catch it and ignore it. That way the test continues 
>>>>> to act as a means to detect excessive stack use changes.
>>>>
>>>> The purpose of the test is not to generate stack overflow exception 
>>>> or check the stack.   It's supposed to check that logging the stack 
>>>> depth is correct.   Since I used recursion when writing this test, 
>>>> adding a catch for SOE for cases where it occurs will make it 
>>>> robust against other changes in the VM. We have other stack 
>>>> overflow tests in the system. This is not one of them!
>>>>
>>>> I think George's changes are fine and fix the test, for which I'm 
>>>> grateful.
>>>>
>>>> Coleen
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>> -George
>>>>>>
>>>>>> On 6/9/2016 9:08 PM, David Holmes wrote:
>>>>>>> Hi George,
>>>>>>>
>>>>>>> On 10/06/2016 4:09 AM, George Triantafillou wrote:
>>>>>>>> Please review this small change to fix test failures on 32-bit 
>>>>>>>> Windows.
>>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8157592
>>>>>>>> Open webrev: 
>>>>>>>> http://cr.openjdk.java.net/~gtriantafill/8157592/webrev/
>>>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8157592/webrev/>
>>>>>>>>
>>>>>>>> TestThrowable.java: Added catch for intermittent 
>>>>>>>> StackOverflowError
>>>>>>>> errors on 32-bit Windows.
>>>>>>>
>>>>>>> This does not seem to be addressing the problem - why do we 
>>>>>>> suddenly
>>>>>>> start getting SOE when running this test? I asked that question 
>>>>>>> in the
>>>>>>> bug report. If something has changed to trigger this then we 
>>>>>>> need to
>>>>>>> ensure that change was intended/desirable and then look at how to
>>>>>>> adjust the test - not simply catch the SOE and ignore it.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>
>>>>>>>> StackTraceLogging.java: Removed unused updateEnvironment method.
>>>>>>>>
>>>>>>>> Tested locally on 32-bit Windows as well as RBT.
>>>>>>>>
>>>>>>>> -George
>>>>>>>>
>>>>>>
>>>>
>>>
>>
>