RFR (M): 8112746: Followup to JDK-8059557 (JEP 245: Validate JVM Command-Line Flag Arguments)

[gerard ziemski]

Yep, already fixed - Sangheon pointed that as well couple of days ago.


cheers

On 08/10/2015 04:52 AM, Dmitry Dmitriev wrote:
> Hello Gerard,
>
> Changes looks good, but I noticed problem in old code. It seems that
> "is_int()" and "is_uint()" cases are missed in the following places:
> In CommandLineFlagConstraintList::check_constraints() function on lines
> 323-343 in src/share/vm/runtime/commandLineFlagConstraintList.cpp module
> In CommandLineFlagRangeList::check_ranges() function on lines 381-398 in
> src/share/vm/runtime/commandLineFlagRangeList.cpp
>
> Thanks,
> Dmitry
>
> On 07.08.2015 19:36, gerard ziemski wrote:
>> hi,
>>
>> Please review this webrev 3 of the follow-up fixes including:
>>
>> - Passing values by value, not pointer, to constraint functions. It was thought originally that me way want to “fix” the values, but it was decided to be out of scope for the JEP. (requested by Coleen)
>>
>> - Add print_error_as_needed() utility function to range/constraint code to improve code robustness (requested by Coleen)
>>
>> - Remove debug code (requested by Kim Barrett)
>>
>> - Implement Flag::flag_error_str in .cpp file (requested by Kim Barrett)
>>
>> - Only check constraint if range passes (requested by Kim Barrett)
>>
>> - Merged with Sangheon Kim fix for "Add additional validation after heap creation” (JDK-8130459)
>>
>>
>> Changes in webrev3 compared to webrev2:
>>
>> - Adding DBL_MAX - decided to let Sangheon handle this as part of his effort to add missing GC flags ranges/constraints
>>
>> - Do not bother checking for NULL in CommandLineFlagRangesList::check_ranges() and CommandLineFlagConstraintsList::check_constraints() - we crash on 32bit platforms if we do this, so can't do it. The problem exists because there are certain flags (ie. lp64_product) that have ranges (ie. ObjectAlignmentInBytes) that are defined as constants on 32 bit platforms, but still register as flags with ranges. I added comments to the code in question.
>>
>> - Implement missing range/constraints check in "int" and "uint" CommandLineFlagsEx::*AtPut() APIs
>>
>> - Factored out common error printing code
>>
>> - Use "ShouldNotReachHere()" in Flag::flag_error_str()
>>
>> - Consistent constraint error message formating
>>
>> - Consistent placement of "verbose" argument in APIs
>>
>> - Fix other APIs that used to pass value by reference
>>
>> - Merged with Roland Westlin fix for "CICompilerCount=1 when tiered is off is not allowed any more" (JDK-8130858)
>>
>>
>> This webrev passes "JPRT -testset hotspot” and “RBT vm.quick testlist”
>>
>>
>> References:
>>
>>     JEP 245:https://bugs.openjdk.java.net/browse/JDK-8122937
>>         bug:https://bugs.openjdk.java.net/browse/JDK-8112746
>>      webrev:http://cr.openjdk.java.net/~gziemski/8112746_rev3/
>>
>>
>> cheers
>>