[JDK 11] RFR: (XS) 8193364: verify_special_jvm_flags should not cause an assertion failure when version is bumped

[Daniel D. Daugherty]

On 12/13/17 12:03 AM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8193364
> webrev: http://cr.openjdk.java.net/~dholmes/8193364/webrev/
>
> verify_special_jvm_flags operates only in debug builds and sanity 
> checks the entries in the deprecated/obsoleted/expired/aliased tables. 
> In addition it detects whether we have not yet removed the global 
> variables for flags that are obsolete/expired, and issues a warning. 
> In the latter case though it also returns false and causes the 
> assertion to fail. This make it impossible to bump the JDK version 
> without dealing with every flag en-masse.
>
> We should issue the warnings but not fail the assert in these cases.
>
> --- old/src/hotspot/share/runtime/arguments.cpp    2017-12-12 
> 23:59:14.039718085 -0500
> +++ new/src/hotspot/share/runtime/arguments.cpp    2017-12-12 
> 23:59:11.575576703 -0500
> @@ -710,7 +710,6 @@
>        if (!version_less_than(JDK_Version::current(), 
> flag.obsolete_in)) {
>          if (Flag::find_flag(flag.name) != NULL) {
>            warning("Global variable for obsolete special flag entry 
> \"%s\" should be removed", flag.name);
> -          success = false;

You should add a comment in place of the removed to line.
Otherwise, someone reading that function will think that
setting 'success' to false was just missed. Perhaps:

           // Do not set success to 'false' in this case since
           // we only want a warning here for ease of transition
           // to a new release.

> }
>        }
>      }
> @@ -720,7 +719,6 @@
>        if (!version_less_than(JDK_Version::current(), flag.expired_in)) {
>          if (Flag::find_flag(flag.name) != NULL) {
>            warning("Global variable for expired flag entry \"%s\" 
> should be removed", flag.name);
> -          success = false;

And perhaps:

           // Do not set success to 'false'; see above note.

Dan


> }
>        }
>      }
>
> This need was flagged when we went to JDK 10 (see JDK-8173421) but not 
> actioned. We need it now before we bump the version for JDK 11.
>
> Thanks,
> David