[9] RFR (S) 6762191: Setting stack size to 16K causes segmentation fault

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Wed Dec 3 18:58:24 UTC 2014


Hi Chris,

Approved with some minor nits, typos which needs correction.

yes java.c follows the JDK indenting as Alan pointed out.

TooSmallStackSize.java

Copyright should be 2014,

1.

   37  * stack size for the platform (as provided by the JVM error message when a very
   38  * small is used), and then verify that the JVM can be launched with that stack

s/small is/small stack is/

2.

  54      * Returns the minimum stack size this platform will allowed based on the

s/allowed/allow/

3.

58      * The TestResult argument must containthe result of having already run
s/containthe/contain the/

4.
92         if (verbose) System.out.println("*** Testing " + stackSize);

I know this is allowed in the HotSpot world but in JDK land we always
use with a LF + Indent, also in other places.

5.
85      * Returns the mimumum allowed stack size gleaned from the error message,
s/mimumum/minimum


Finally I am concerned with the integration, since it spans both
HotSpot and JDK, and it appears the test will fail if the HotSpot
changes are not integrated first, or has it already ?

Thanks
Kumar







On 12/1/2014 6:39 PM, Chris Plummer wrote:
> Sorry about the long delay in getting back to this. I ran into two 
> separate JPRT issues that were preventing me from testing these 
> changes, plus I was on vacation last week. Here's an updated webrev. 
> I'm not sure where we left things, so I'll just say what's changed 
> since the original version:
>
> 1. Rewrote the test to be in Java instead of a shell script.
> 2. Moved the test from hotspot/test/runtime/memory to 
> jdk/test/tools/launcher
> 3. Added STACK_SIZE_MINIMUM to java.c, allowing a makefile to override 
> the default 32k minimum value.
>
> https://bugs.openjdk.java.net/browse/JDK-6762191
> http://cr.openjdk.java.net/~cjplummer/6762191/webrev.02/
>
> thanks,
>
> Chris
>
> On 11/19/14 7:52 AM, Chris Plummer wrote:
>> 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 at 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
>>>>>>>
>>>>>
>>>>
>>
>



More information about the hotspot-runtime-dev mailing list