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

sangheon.kim sangheon.kim at oracle.com
Sat Jul 25 00:49:15 UTC 2015


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;

>
> ------------------------------------------------------------------------------
> 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. :)

Thanks,
Sangheon

>
> ------------------------------------------------------------------------------
>



More information about the hotspot-dev mailing list