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

[Coleen Phillimore]

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
>>>>>>>
>>>>>
>>>
>>
>