RFR 8130459(M): Add additional validation after heap creation

[Kim Barrett]

On Jul 24, 2015, at 8:49 PM, sangheon.kim <sangheon.kim at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 07/24/2015 04:25 PM, Kim Barrett wrote:
>> On Jul 23, 2015, at 8:53 PM, sangheon.kim <sangheon.kim at oracle.com> wrote:
>>> Hi Kim,
>>> 
>>> Updated webrev includes:
>>> - Moved functions related to range/constraints from CommandLineFlags to CommandLineFlagConstraintList / CommandLineFlagRangeList.
>>> - 2 functions are changed to 'const'.
>>> - 2 typos.
>>> 
>>> webrev.04:
>>> http://cr.openjdk.java.net/~sangheki/8130459/webrev.04
>>> 
>>> Incremental:
>>> http://cr.openjdk.java.net/~sangheki/8130459/webrev.04_to_03
>> ------------------------------------------------------------------------------
>> src/share/vm/runtime/commandLineFlagConstraintList.cpp
>>  343   // Skip if we already checked.
>>  344   if (type < _validating_type) {
>>  345     return true;
>>  346   }
>> 
>> That's not quite what I had in mind when I suggested the type should
>> be verified to be less than _validating_type.  I think it's a program
>> error for that test to fail, e.g. it should be checked with a
>> assert/guarantee (I would use guarantee).  For example, if we were to
>> (presumably unintentionally) perform constraint checking out of order
>> then the out of order check would simply not be performed - ever -
>> with the code above.
> I see.
> 
> How about below?
>  // First check will be for 'AfterErgo' and initial value of '_validating_type' is 'AtParse'.
>  guarantee(type > _validating_type, "Constraint check is out of order.");
>  _validating_type = type;

Yes, that’s what I had in mind.

>> src/share/vm/runtime/thread.cpp
>> 3333   bool constraint_result = CommandLineFlagConstraintList::check_constraints(CommandLineFlagConstraint::AfterErgo);
>> 3334   Arguments::post_after_ergo_constraint_check(constraint_result);
>> 3335   if (!constraint_result) {
>> 3336     return JNI_EINVAL;
>> 3337   }
>> 
>> Simpler would be
>> 
>>   if (!CommandLineFlagConstraintList::check_constraints(CommandLineFlagConstraint::AfterErgo)) {
>>     return JNI_EINVAL;
>>   }
>>   Arguments::post_after_ergo_constraint_check();
>> 
>> with associated change of post_after_ergo_constraint_check to
>> eliminate the unused(!) argument.
> I don't have strong opinion on this as it is unused for now.
> However as you know its intend is to leave a way to utilize the result of constraint check for future use.
> Still we will have to cover runtime/gc/compiler team's flags. :)

We can change things later if there’s a need to do so.

However, in this case, I don’t think such a need will arise.  If the constraint check fails,
we should be heading toward the exit, and shouldn’t be doing post-check stuff anyway.