Revision3: Corrected: RFR 8059557 (XL): Validate JVM Command-Line Flag Arguments
Gerard Ziemski
gerard.ziemski at oracle.com
Tue Jun 16 21:11:16 UTC 2015
Coleen, Dmitry, David, Kim, Derek and Bengt,
I have create an incremental webrev as requested:
http://cr.openjdk.java.net/~gziemski/8059557_rev2_to_rev3
As per suggestion from Kim and Coleen, I will not be spinning webrev4
unless there is a blocker in the code that we haven't found yet.
The plan right now is to check in rev3 and have
https://bugs.openjdk.java.net/browse/JDK-8112746 track any follow up
issues, which I already populated with comments from Coleen, Dmitry and Kim.
Can I have a final sign off on this webrev rev3 from all of you please?
cheers
On 6/14/2015 9:31 PM, David Holmes wrote:
> Hi Gerard,
>
> Any chance you could produce and incremental webrev please?
>
> Thanks,
> David
>
> On 14/06/2015 12:32 AM, Gerard Ziemski wrote:
>> hi all,
>>
>> Thank you for the reviews so far!
>>
>> Here is a revision 3 of the feature taking into account feedback from
>> Kim Barrett, David Holmes, Derek White and Bengt Rutisson. I have
>> also merged the changes to the latest jdk9.
>>
>> 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 mechanism 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_rev3
>> 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
>>
>> Note: due to "awk" limit of 50 pats the Frames diff is not available
>> for "src/share/vm/runtime/arguments.cpp” and
>> "src/share/vm/runtime/globals.cpp”
>>
>>
>> Followup issues:
>>
>> JDK-8085866: There are 99 uses of FLAG_SET_ERGO, should they check
>> the return value?
>> JDK-8085864: FLAG_SET_CMDLINE in TestGenCollectorPolicy() currently
>> ignore the return values
>> JDK-8081842: Find a better place for
>> EMIT_{CONSTRAINTS,RANGES}_FOR_GLOBALS_EXT
>> JDK-8081833: There is a large amount of code near-duplication among
>> the various CommandLineFlagRange_<type> classes
>> JDK-8081519: Split globals.hpp to factor out the Flag values needed
>> by JDK-8059557
>>
>>
>> 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 | 2 +-
>> 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/g1/g1_globals.cpp | 14 +-
>> src/share/vm/gc/g1/g1_globals.hpp | 38 +-
>> src/share/vm/opto/c2_globals.cpp | 12 +-
>> src/share/vm/opto/c2_globals.hpp | 40 +-
>> src/share/vm/prims/whitebox.cpp | 12 +-
>> src/share/vm/runtime/arguments.cpp | 765
>> ++++++----------
>> src/share/vm/runtime/arguments.hpp | 24 +-
>> src/share/vm/runtime/commandLineFlagConstraintList.cpp | 286
>> ++++++
>> src/share/vm/runtime/commandLineFlagConstraintList.hpp | 80 +
>> src/share/vm/runtime/commandLineFlagConstraintsCompiler.cpp | 44 +
>> src/share/vm/runtime/commandLineFlagConstraintsCompiler.hpp | 39 +
>> src/share/vm/runtime/commandLineFlagConstraintsGC.cpp | 235
>> +++++
>> src/share/vm/runtime/commandLineFlagConstraintsGC.hpp | 58 +
>> src/share/vm/runtime/commandLineFlagConstraintsRuntime.cpp | 65 +
>> src/share/vm/runtime/commandLineFlagConstraintsRuntime.hpp | 41 +
>> src/share/vm/runtime/commandLineFlagRangeList.cpp | 367
>> ++++++++
>> src/share/vm/runtime/commandLineFlagRangeList.hpp | 71 +
>> src/share/vm/runtime/globals.cpp | 721
>> ++++++++++++---
>> src/share/vm/runtime/globals.hpp | 343
>> ++++++-
>> src/share/vm/runtime/globals_extension.hpp | 105 +-
>> src/share/vm/runtime/init.cpp | 5 +-
>> 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 | 12 +-
>> src/share/vm/services/diagnosticCommand.cpp | 3 +-
>> src/share/vm/services/management.cpp | 6 +-
>> src/share/vm/services/memoryService.cpp | 4 +-
>> src/share/vm/services/writeableFlags.cpp | 183 ++-
>> src/share/vm/services/writeableFlags.hpp | 60 +-
>> test/compiler/c2/7200264/Test7200264.sh | 5 +-
>> test/compiler/startup/NumCompilerThreadsCheck.java | 2 +-
>> test/gc/arguments/TestHeapFreeRatio.java | 23 +-
>> test/gc/arguments/TestSurvivorAlignmentInBytesOption.java | 6 +-
>> test/gc/g1/TestStringDeduplicationTools.java | 6 +-
>> test/runtime/CompressedOops/CompressedClassSpaceSize.java | 4 +-
>> test/runtime/CompressedOops/ObjectAlignment.java | 9 +-
>> test/runtime/contended/Options.java | 10 +-
>> 49 files changed, 2851 insertions(+), 943 deletions(-)
>>
>
More information about the hotspot-dev
mailing list