RFR (XXS) CR 8006997: ContendedPaddingWidth should be uintx

David Holmes david.holmes at oracle.com
Fri Feb 8 18:24:32 PST 2013


On 9/02/2013 2:45 AM, Aleksey Shipilev wrote:
> On 02/08/2013 05:30 PM, Aleksey Shipilev wrote:
>> On 02/08/2013 05:22 PM, David Holmes wrote:
>>> On 8/02/2013 8:39 PM, Aleksey Shipilev wrote:
>>>> Ok then. This is the new webrev:
>>>>    http://cr.openjdk.java.net/~shade/8006997/webrev.01/
>>>>
>>>> I haven't found another good place to stick the guarantee() to, so opted
>>>> to stick it before the actual use. I would be happy to stick it
>>>> somewhere else where the options are generally checked, but found no
>>>> non-platform/non-OS specific location.
>>>
>>> arguments.cpp - Arguments::parse_each_vm_init_arg - see
>>> -XX:MaxDirectMemorySize as an example.
>>
>> Ok, great, thanks! I will update the webrev and give it another jprt spin.
>
> Here's the update:
>    http://cr.openjdk.java.net/~shade/8006997/webrev.02/

Sorry but that's not quite going to work right. Now you are mixing 
assignment of a signed int and an unsigned jlong! I'm surprised there is 
no compiler warning about that. And if someone passes in a value greater 
than INT_MAX it will pass the range check then store a negative value 
into the variable due to the jlong -> int truncation.

When I referred you to MaxMemorySize as an example I didn't mean to 
literally copy it :) But now I see that parse_memory_size seems to be 
the only range checking done.

Will it suffice to change to:

2825       ArgsRange errcode = parse_memory_size(tail, 
&contended_padding_width, INT_MAX);

and

2833       FLAG_SET_CMDLINE(intx, ContendedPaddingWidth, (intx) 
contended_padding_width);

David
------

> Testing:
>   a) ad-hoc fastdebug build with negative value
>   b) JPRT/Hotspot full cycle
>
> -Aleksey.
>


More information about the hotspot-dev mailing list