JDK-8133564 : Runtime - 2nd followup to Validate JVM Command-Line Flag Arguments
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-08-13
  • Updated: 2019-06-20
  • Resolved: 2018-05-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11
11 b16Fixed
Related Reports
Cloners :  
JDK-8133565 - GC - 2nd followup to JDK-8059557
Relates :  
JDK-8112746 - Followup to JDK-8059557
Relates :  
JDK-8203939 - JVMFlag::printError missing ATTRIBUTE_PRINTF
Relates :  
JDK-8203938 - Fix build failures from JDK-8133564
Relates :  
JDK-8078555 - GC: implement ranges (optionally constraints) for those flags that have them missing
Relates :  
JDK-8078556 - Runtime: implement ranges (optionally constraints) for those flags that have them missing
Relates :  
JDK-8078554 - Compiler: implement ranges (optionally constraints) for those flags that have them missing
Description
2nd followup to JDK-8059557

There is a few more issues/cleanups that we can do, but shouldn't block JDK-8112746, which is itself blocks JDK-8078554, JDK-8078555 and JDK-8078556

We will deal with those issues here.
Comments
Fixed #1,#2, but the rest seem to be no longer applicable. Testing...
18-05-2018
Re #1 Even better - I would propose that we factor out the flag manipulation code out of globals.hpp/.cpp and leave those files to deal only with macros, and move the rest of the code into its own CommandLineFlags.hpp/.cpp file.
13-08-2015
Issues found by Kim Barrett in http://cr.openjdk.java.net/~gziemski/8112746_rev3/ #1 ------------------------------------------------------------------------------ src/share/vm/runtime/commandLineFlagRangeList.hpp src/share/vm/runtime/commandLineFlagRangeList.cpp class CommandLineError ... I think this isn't the right place for this class. I think a better place would be globals.[ch]pp, following class CommandLineFlags. I think such a change would eliminate the need for new #include of "runtime/commandLineFlagRangeList.hpp" in commandLineFlagConstraints{Compiler,GC,Runtime}.cpp, introduced in this latest revision. And of course, the definition of the print function should be moved to a corresponding location in globals.cpp. #2 ------------------------------------------------------------------------------ commandLineFlagConstraints{Compiler,GC,Runtime}.cpp With the refactoring for CommandLineError, I think these files no longer need to #include "utilities/defaultStream.hpp". #3 ------------------------------------------------------------------------------ src/share/vm/runtime/commandLineFlagConstraintList.cpp 307 bool CommandLineFlagConstraintList::check_constraints(CommandLineFlagConstraint::ConstraintType type) { ... 327 } else if (flag->is_int()) { 328 int value = flag->get_int(); 329 if (constraint->apply_int(value, true) != Flag::SUCCESS) status = false; 330 } else if (flag->is_uint()) { 331 uint value = flag->get_uint(); 332 if (constraint->apply_uint(value, true) != Flag::SUCCESS) status = false; ... [Latest version added missing cases.] Suggest adding a final else clause to the flag type dispatch that is a ShouldNotReachHere or some such. Similary in CommandLineFlagRangeList::check_ranges(). #4 ------------------------------------------------------------------------------ src/share/vm/runtime/commandLineFlagConstraintList.cpp 307 bool CommandLineFlagConstraintList::check_constraints(CommandLineFlagConstraint::ConstraintType type) { ... 319 Flag* flag = Flag::find_flag(name, strlen(name), true, true); 320 // We must check for NULL here as lp64_product flags on 32 bit architecture 321 // can generate constraint check (despite that they are declared as constants), 322 // but they will not be returned by Flag::find_flag() 323 if (flag != NULL) { This comment suggests to me that there is a bug in the underlying constraint registration infrastructure; why are we registering constraints for options that don't exist? Similary in CommandLineFlagRangeList::check_ranges(). I think this should be addressed as a followup RFE, rather than as further work on this change set. It's a pre-existing problem, not something introduced by this change set.
13-08-2015