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

[Coleen Phillimore]

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