Need reviewers for changes in JVM initialization code (added check for -XX:StackShadowPages option)

[Pavel Tisnovsky]

Hi Coleen and Paul,

here are new versions of the patch:

for OpenJDK6:
http://cr.openjdk.java.net/~ptisnovs/StackXXXPagesCheck_OpenJDK6/

and for OpenJDK7 too:
http://cr.openjdk.java.net/~ptisnovs/StackXXXPagesCheck_OpenJDK7/

(the change itself is the same for both OpenJDKs, but line numbers are
different)

> I will push it through JPRT
does it mean that I need not do the push myself?

Thank you for your help
Pavel

Coleen Phillimore wrote:
> Yes, I agree they should stay intx.  Thank you for the change.  It looks
> fine to me.  If you update the patch, I will push it through JPRT.  Let
> me know when it is ready.
> thanks,
> Coleen
> 
> Paul Hohensee wrote:
>>  Leave them as intx, imo.  Their high bound value isn't anywhere near
>> maxint,
>> and if you change them to uintx you'll have to cast them to intx for the
>> calls to verify_min_value.  Plus there's some places where their
>> values are
>> assigned to ints that would have to be changed.
>>
>> Also, pls use INTX_FORMAT instead of UINTX_FORMAT in verify_min_value.
>>
>> Paul
>>
>> On 8/17/10 3:50 PM, Tom Rodriguez wrote:
>>> On Aug 6, 2010, at 7:05 AM, Pavel Tisnovsky wrote:
>>>
>>>> Tom Rodriguez wrote:
>>>>> I think you could put this check into arguments.cpp since I all
>>>>> platforms would require a positive number for the
>>>>> StackShadowPages.  The same should be true of StackRedPages and
>>>>> StackYellowPages.  Actually they all should be required to greater
>>>>> than 0 I think.  Other than that I don't see other obvious
>>>>> constraints on the values.  That bug report doesn't really show
>>>>> other problematic values, though I'm not sure I follow the point of
>>>>> the guarantee that's failing either.  What does the min stack size
>>>>> have to do with the number of guard pages?  I would expect it to be
>>>>> checking against ThreadStackSize and returning an error if it was
>>>>> too small like the other places that check against TheadStackSize.
>>>>>
>>>>> tom
>>>> Hi Tom,
>>>>
>>>> I've added check for all Stack*Pages parameters to arguments.cpp. Can
>>>> you please review the changes?
>>>>
>>>> http://cr.openjdk.java.net/~ptisnovs/StackXXXPagesCheck/
>>>>
>>>> Is there any reason why Stack*Pages parameters are of type int and not
>>>> unsigned int? I'm able to change its types (these variables are
>>>> generated by macro which accepts type as one of its parameter) but I'm
>>>> not sure if it would be correct in all cases.
>>> Would someone from runtime want to comment on this?  It seems ok to
>>> me.  Changing to uintx would also be ok but it might require other
>>> uses to be converted to unsigned as well.  I'd probably just leave it
>>> alone myself.
>>>
>>> tom
>>>
>>>
>>>> Cheers
>>>> Pavel
>>>>
>>>>> On Jul 27, 2010, at 8:15 AM, Pavel Tisnovsky wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> can anybody please review two quite simple changes in JVM
>>>>>> initialization code? Webrev is available at:
>>>>>> http://cr.openjdk.java.net/~ptisnovs/StackShadowPagesCheck/
>>>>>>
>>>>>> When -XX:StackShadowPages is set to negative integer or zero
>>>>>> value, JVM segfaulted on Linux and, according to
>>>>>> http://bugs.sun.com/view_bug.do?bug_id=6885308, hangs up on
>>>>>> Solaris (although I only check this issue on Linux)
>>>>>>
>>>>>> I also would like to add more check for -XX:StackRedPages,
>>>>>> -XX:StackShadowPages and -XX:StackYellowPages options to avoid
>>>>>> issue described in the bug report mentioned above
>>>>>> (http://bugs.sun.com/view_bug.do?bug_id=6885308) but I'm unable to
>>>>>> find relevant information about proper conditions (it may depends
>>>>>> on page sizes, VM stack size etc.). Any ideas?
>>>>>>
>>>>>> Cheers
>>>>>> Pavel Tisnovsky
>>>>>>