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

[Paul Hohensee]

  You still should cast the first argument to verify_min_value in 
check_stack_pages
to intx, vis

// Check stack pages settings
bool Arguments::check_stack_pages()
{
   bool status = true;
   status = status&&  verify_min_value((intx)StackYellowPages, 1, "StackYellowPages");
   status = status&&  verify_min_value((intx)StackRedPages, 1, "StackRedPages");
   status = status&&  verify_min_value((intx)StackShadowPages, 1, "StackShadowPages");
   return status;
}

lest some versions of gcc reject the code because of the implicit 
unsigned -> signed
conversion.

Paul

On 8/19/10 8:43 AM, Pavel Tisnovsky wrote:
> 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
>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-dev/attachments/20100819/0ab5cbbc/attachment.html