Corrected: RFR 8059557 (XL): Validate JVM Command-Line Flag Arguments
Coleen Phillimore
coleen.phillimore at oracle.com
Fri May 15 20:49:04 UTC 2015
Gerard,
This is significant work! The macro re-expansions are daunting but the
simpler user interface makes it worth while. Someone better at macros
should review this in more detail to see if there's any gotchas,
especially wrt to C++11 and beyond. Hopefully there aren't any
surprises. I think there's some things people should know about
globals.hpp:
*+ /* NB: The default value of UseLinuxPosixThreadCPUClocks may be */ \*
*+ /* overridden in Arguments::parse_each_vm_init_arg. */ \*
* product(bool, UseLinuxPosixThreadCPUClocks, true, \*
* "enable fast Linux Posix clocks where available") \*
* /* NB: The default value of UseLinuxPosixThreadCPUClocks may be \*
* overridden in Arguments::parse_each_vm_init_arg. */ \*
It looks like if you have a comment for an option, do you need to have
it above the option, or is this just nicer?
In
http://cr.openjdk.java.net/~gziemski/8059557_rev0/src/share/vm/runtime/globals.cpp.udiff.html
This should be out->print_cr()
*+ if (printRanges == false) {*
* out->print_cr("[Global flags]");*
*+ } else {*
*+ tty->print_cr("[Global flags ranges]");*
*+ }*
*+*
There's some ifdef debugging code left in but I think that's ok for now,
because it's not much and may be helpful but not helpful enough in the
long run to add a XX:OnlyPrintProductFlags option.
The name checkAllRangesAndConstraints should be
check_all_ranges_and_constraints as per the coding standard, but the
constraint functions in
http://cr.openjdk.java.net/~gziemski/8059557_rev0/src/share/vm/runtime/commandLineFlagConstraintsGC.hpp.html
seem okay in mixed case so you can find them when you're grepping for
the flag.
I didn't review the tests yet. If someone else reviews them I'd be
glad. This looks very good with these minor suggestions!
thanks,
Coleen
On 5/14/15, 3:57 PM, Gerard Ziemski wrote:
> hi all,
>
> We introduce a new mechanism that allows specification of a valid
> range per flag that is then used to automatically validate given
> flag's value every time it changes. Ranges values must be constant and
> can not change. Optionally, a constraint can also be specified and
> applied every time a flag value changes for those flags whose valid
> value can not be trivially checked by a simple min and max (ex.
> whether it's power of 2, or bigger or smaller than some other flag
> that can also change)
>
> I have chosen to modify the table macros (ex. RUNTIME_FLAGS in
> globals.hpp) instead of using a more sophisticated solution, such as
> C++ templates, because even though macros were unfriendly when
> initially developing, once a solution was arrived at, subsequent
> additions to the tables of new ranges, or constraint are trivial from
> developer's point of view. (The intial development unfriendliness of
> macros was mitigated by using a pre-processor, which for those using a
> modern IDE like Xcode, is easily available from a menu). Using macros
> also allowed for more minimal code changes.
>
> The presented solution is based on expansion of macros using variadic
> functions and can be readily seen in
> runtime/commandLineFlagConstraintList.cpp and
> runtime/commandLineFlagRangeList.cpp
>
> In commandLineFlagConstraintList.cpp or commandLineFlagRangesList.cpp,
> there is bunch of classes and methods that seems to beg for C++
> template to be used. I have tried, but when the compiler tries to
> generate code for both uintx and size_t, which happen to have the same
> underlying type (on BSD), it fails to compile overridden methods with
> same type, but different name. If someone has a way of simplifying the
> new code via C++ templates, however, we can file a new enhancement
> request to address that.
>
> This webrev represents only the initial range checking framework and
> only 100 or so flags that were ported from an existing ad hoc range
> checking code to this new mechanism. There are about 250 remaining
> flags that still need their ranges determined and ported over to this
> new mechansim and they are tracked by individual subtasks.
>
> I had to modify several existing tests to change the error message
> that they expected when VM refuses to run, which was changed to
> provide uniform error messages.
>
> To help with testing and subtask efforts I have introduced a new
> runtime flag:
>
> PrintFlagsRanges: "Print VM flags and their ranges and exit VM"
>
> which in addition to the already existing flags: "PrintFlagsInitial"
> and "PrintFlagsFinal" allow for thorough examination of the flags
> values and their ranges.
>
> The code change builds and passes JPRT (-testset hotspot) and UTE
> (vm.quick.testlist)
>
>
> References:
>
> Webrev: http://cr.openjdk.java.net/~gziemski/8059557_rev0/
> note: due to "awk" limit of 50 pats the Frames diff is not
> available for "src/share/vm/runtime/arguments.cpp"
>
> JEP:https://bugs.openjdk.java.net/browse/JDK-8059557
> Compiler subtask:https://bugs.openjdk.java.net/browse/JDK-8078554
> GC subtask:https://bugs.openjdk.java.net/browse/JDK-8078555
> Runtime subtask:https://bugs.openjdk.java.net/browse/JDK-8078556
>
>
> hgstat:
>
> src/cpu/ppc/vm/globals_ppc.hpp | 2 +-
> src/cpu/sparc/vm/globals_sparc.hpp | 2 +-
> src/cpu/x86/vm/globals_x86.hpp | 2 +-
> src/cpu/zero/vm/globals_zero.hpp | 3 +-
> src/os/aix/vm/globals_aix.hpp | 4 +-
> src/os/bsd/vm/globals_bsd.hpp | 29 +-
> src/os/linux/vm/globals_linux.hpp | 9 +-
> src/os/solaris/vm/globals_solaris.hpp | 4 +-
> src/os/windows/vm/globals_windows.hpp | 5 +-
> src/share/vm/c1/c1_globals.cpp | 4 +-
> src/share/vm/c1/c1_globals.hpp | 17 +-
> src/share/vm/gc_implementation/g1/g1_globals.cpp | 16 +-
> src/share/vm/gc_implementation/g1/g1_globals.hpp | 38 ++-
> src/share/vm/opto/c2_globals.cpp | 12 +-
> src/share/vm/opto/c2_globals.hpp | 39 ++-
> src/share/vm/prims/whitebox.cpp | 12 +-
> src/share/vm/runtime/arguments.cpp | 711
> +++++++++++++++++++++++++++------------------------------------
> src/share/vm/runtime/arguments.hpp | 24 +-
> src/share/vm/runtime/commandLineFlagConstraintList.cpp | 242
> +++++++++++++++++++++
> src/share/vm/runtime/commandLineFlagConstraintList.hpp | 72 ++++++
> src/share/vm/runtime/commandLineFlagConstraintsGC.cpp | 226
> ++++++++++++++++++++
> src/share/vm/runtime/commandLineFlagConstraintsGC.hpp | 57 +++++
> src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp | 50 ++++
> src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp | 39 +++
> src/share/vm/runtime/commandLineFlagRangeList.cpp | 304
> +++++++++++++++++++++++++++
> src/share/vm/runtime/commandLineFlagRangeList.hpp | 67 ++++++
> src/share/vm/runtime/globals.cpp | 681
> ++++++++++++++++++++++++++++++++++++++++++++++++------------
> src/share/vm/runtime/globals.hpp | 302
> +++++++++++++++++++++-----
> src/share/vm/runtime/globals_extension.hpp | 101
> +++++++-
> src/share/vm/runtime/init.cpp | 6 +-
> src/share/vm/runtime/os.hpp | 17 +
> src/share/vm/runtime/os_ext.hpp | 7 +-
> src/share/vm/runtime/thread.cpp | 6 +
> src/share/vm/services/attachListener.cpp | 4 +-
> src/share/vm/services/classLoadingService.cpp | 6 +-
> src/share/vm/services/diagnosticCommand.cpp | 3 +-
> src/share/vm/services/management.cpp | 6 +-
> src/share/vm/services/memoryService.cpp | 2 +-
> src/share/vm/services/writeableFlags.cpp | 161
> ++++++++++----
> src/share/vm/services/writeableFlags.hpp | 52 +---
> test/compiler/c2/7200264/Test7200264.sh | 5 +-
> test/compiler/startup/NumCompilerThreadsCheck.java | 2 +-
> test/gc/arguments/TestHeapFreeRatio.java | 23 +-
> test/gc/g1/TestStringDeduplicationTools.java | 8 +-
> test/runtime/CompressedOops/CompressedClassSpaceSize.java | 4 +-
> test/runtime/CompressedOops/ObjectAlignment.java | 9 +-
> test/runtime/contended/Options.java | 12 +-
> 47 files changed, 2572 insertions(+), 835 deletions(-)
>
>
>
>
More information about the hotspot-dev
mailing list