[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: [9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault
From:       Chris Plummer <chris.plummer () oracle ! com>
Date:       2014-11-19 15:52:11
Message-ID: 546CBCAB.7040101 () oracle ! com
[Download RAW message or body]

On 11/19/14 2:12 AM, David Holmes wrote:
> On 19/11/2014 6:49 PM, Chris Plummer wrote:
>> I've update the webrev to add STACK_SIZE_MINIMUM in place of the 32k
>> references, and also moved the test from hotspot/test/runtime to
>> jdk/test/tools/launcher as David requested. That required some
>> adjustments to the test script, since test_env.sh does not exist in
>> jdk/test, so I had to pull in the bits I needed into the script.
>
> Is there a reason this needs a shell script instead of using the 
> testlibrary tools to launch the VM and check the output?
Not that I'm aware of. I guess I just really didn't look at what it 
would take to make it all in java. I'll have a look at java examples and 
convert it.

Chris
>
> Sorry that should have been mentioned much earlier.
>
> David
>
>
>> http://cr.openjdk.java.net/~cjplummer/6762191/webrev.01/
>>
>> I still need to rerun through JPRT. I'll do so once there are no more
>> suggested changes.
>>
>> thanks,
>>
>> Chris
>>
>> On 11/18/14 2:08 PM, Chris Plummer wrote:
>>> Adding core-libs-dev@openjdk.java.net, since one of the changes is in
>>> java.c.
>>>
>>> Chris
>>>
>>> On 11/12/14 6:43 PM, David Holmes wrote:
>>>> Hi Chris,
>>>>
>>>> Sorry for the delay.
>>>>
>>>> On 13/11/2014 5:44 AM, Chris Plummer wrote:
>>>>> Hi,
>>>>>
>>>>> I'm still looking for reviewers.
>>>>
>>>> As the change is to the launcher it needs to be reviewed by the
>>>> launcher owner - which I think is serviceability (though also cc'd
>>>> Kumar :) ).
>>>>
>>>> Launcher change, and your rationale, seems okay to me. I'd probably
>>>> put the test in to jdk/test/tools/launcher/ though.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>
>>>>> On 11/7/14 7:53 PM, Chris Plummer wrote:
>>>>>> This is an initial review for 6762191. I'm guessing there will be
>>>>>> recommendations to fix in a different way, but thought this would 
>>>>>> be a
>>>>>> good time to start the discussion.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-6762191
>>>>>> http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.jdk/
>>>>>> http://cr.openjdk.java.net/~cjplummer/6762191/webrev.00.hotspot/
>>>>>>
>>>>>> The bug is that if the -Xss size is set to something very small 
>>>>>> (like
>>>>>> 16k), on linux there will be a crash due to overwriting the end 
>>>>>> of the
>>>>>> stack. This happens before hotspot can compute its stack needs and
>>>>>> verify that the stack is big enough.
>>>>>>
>>>>>> It didn't seem viable to move the hotspot stack size check 
>>>>>> earlier. It
>>>>>> depends on too much other work done before that point, and the 
>>>>>> changes
>>>>>> would have been disruptive. The stack size check is currently 
>>>>>> done in
>>>>>> os::init_2().
>>>>>>
>>>>>> What is needed is a check before the thread is created. That way we
>>>>>> can create a thread with a big enough stack to handle all needs 
>>>>>> up to
>>>>>> the point of the check in os::init_2(). This initial check does not
>>>>>> need to be the final check. It just needs to confirm that we have
>>>>>> enough stack to get us to the check in os::init_2().
>>>>>>
>>>>>> I decided to check in java.c if the -Xss size is too small, and 
>>>>>> set it
>>>>>> to a larger size if it is. I hard coded this size to 32k (I'll 
>>>>>> explain
>>>>>> why 32k later). I suspect this is the part that will result in some
>>>>>> debate. If you have better suggestions let me know. If it does stay
>>>>>> here, then probably the 32k needs to be a #define, and maybe even an
>>>>>> OS porting interface, but I'm not sure where to put it.
>>>>>>
>>>>>> The reason I chose 32k is because this is big enough for all 
>>>>>> platforms
>>>>>> to get to the stack size check in os::init_2(). It is also smaller
>>>>>> than the actual minimum stack size allowed on any platform. 32-bit
>>>>>> windows has the smallest requirement at 64k. I add some printfs to
>>>>>> print the minimum stack requirement, and then ran a simple JTReg 
>>>>>> test
>>>>>> with every JPRT supported platform to get the results.
>>>>>>
>>>>>> The TooSmallStackSize.sh will run "java -version" with -Xss16k,
>>>>>> -Xss32k, and -XXss<minsize>, where <minsize> is the size from the
>>>>>> error message produced by the JVM, such as in the following:
>>>>>>
>>>>>> $ java -Xss32k -version
>>>>>> The stack size specified is too small, Specify at least 100k
>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>
>>>>>> I ran this test through JPRT on all platforms, and they all pass.
>>>>>>
>>>>>> One thing to point out is that Windows behaves a bit different than
>>>>>> the other platforms. It always rounds the stack size up to a 
>>>>>> multiple
>>>>>> of 64k , so even if you specify -Xss16k, you get a 64k stack. On
>>>>>> 32-bit Windows with C1, 64k is also the minimum requirement, so 
>>>>>> there
>>>>>> is no error produced in this case. However, on 32-bit Windows 
>>>>>> with C2,
>>>>>> 68k is the minimum, so an error is produced since the stack will 
>>>>>> only
>>>>>> be 64k. There is no bug here. It's just a bit confusing.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> Chris
>>>>>
>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]