6843347: Boundary values in some public GC options cause crashes

Setting some public integer options to specific values causes crashes or undefined GC behavior. This patchset adds the necessary argument checking for these options.

Reviewed-by: jmasa, brutisso

hotspot/src/cpu/sparc/vm/globals_sparc.hpp

@@ -74,7 +74,7 @@ define_pd_global(bool, RewriteFrequentPairs, true);
 define_pd_global(bool, UseMembar,            false);
 
 // GC Ergo Flags
-define_pd_global(intx, CMSYoungGenPerWorker, 16*M);  // default max size of CMS young gen, per GC worker thread
+define_pd_global(uintx, CMSYoungGenPerWorker, 16*M);  // default max size of CMS young gen, per GC worker thread
 
 #define ARCH_FLAGS(develop, product, diagnostic, experimental, notproduct) \
                                                                             \

hotspot/src/cpu/x86/vm/globals_x86.hpp

@@ -77,7 +77,7 @@ define_pd_global(bool, UseMembar,            false);
 #endif
 
 // GC Ergo Flags
-define_pd_global(intx, CMSYoungGenPerWorker, 64*M);  // default max size of CMS young gen, per GC worker thread
+define_pd_global(uintx, CMSYoungGenPerWorker, 64*M);  // default max size of CMS young gen, per GC worker thread
 
 #define ARCH_FLAGS(develop, product, diagnostic, experimental, notproduct) \
                                                                             \

hotspot/src/cpu/zero/vm/globals_zero.hpp

@@ -55,7 +55,7 @@ define_pd_global(bool,  RewriteFrequentPairs, true);
 define_pd_global(bool,  UseMembar,            true);
 
 // GC Ergo Flags
-define_pd_global(intx, CMSYoungGenPerWorker, 16*M);  // default max size of CMS young gen, per GC worker thread
+define_pd_global(uintx, CMSYoungGenPerWorker, 16*M);  // default max size of CMS young gen, per GC worker thread
 
 #define ARCH_FLAGS(develop, product, diagnostic, experimental, notproduct)
 

hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp

@@ -692,8 +692,7 @@ CMSCollector::CMSCollector(ConcurrentMarkSweepGeneration* cmsGen,
   _cmsGen ->init_initiating_occupancy(CMSInitiatingOccupancyFraction, CMSTriggerRatio);
 
   // Clip CMSBootstrapOccupancy between 0 and 100.
-  _bootstrap_occupancy = ((double)MIN2((uintx)100, MAX2((uintx)0, CMSBootstrapOccupancy)))
+  _bootstrap_occupancy = ((double)CMSBootstrapOccupancy)/(double)100;
-                         /(double)100;
 
   _full_gcs_since_conc_gc = 0;
 

hotspot/src/share/vm/gc_implementation/parallelScavenge/psMarkSweep.cpp

@@ -92,8 +92,8 @@ void PSMarkSweep::invoke(bool maximum_heap_compaction) {
   const bool clear_all_soft_refs =
     heap->collector_policy()->should_clear_all_soft_refs();
 
-  int count = (maximum_heap_compaction)?1:MarkSweepAlwaysCompactCount;
+  uint count = maximum_heap_compaction ? 1 : MarkSweepAlwaysCompactCount;
-  IntFlagSetting flag_setting(MarkSweepAlwaysCompactCount, count);
+  UIntFlagSetting flag_setting(MarkSweepAlwaysCompactCount, count);
   PSMarkSweep::invoke_no_policy(clear_all_soft_refs || maximum_heap_compaction);
 }
 

hotspot/src/share/vm/gc_implementation/parallelScavenge/psMarkSweepDecorator.cpp

@@ -88,8 +88,7 @@ void PSMarkSweepDecorator::precompact() {
    * by the MarkSweepAlwaysCompactCount parameter. This is a significant
    * performance improvement!
    */
-  bool skip_dead = (MarkSweepAlwaysCompactCount < 1)
+  bool skip_dead = ((PSMarkSweep::total_invocations() % MarkSweepAlwaysCompactCount) != 0);
-    || ((PSMarkSweep::total_invocations() % MarkSweepAlwaysCompactCount) != 0);
 
   size_t allowed_deadspace = 0;
   if (skip_dead) {

hotspot/src/share/vm/gc_implementation/shared/markSweep.cpp

@@ -30,7 +30,7 @@
 #include "oops/objArrayKlass.inline.hpp"
 #include "oops/oop.inline.hpp"
 
-unsigned int            MarkSweep::_total_invocations = 0;
+uint                    MarkSweep::_total_invocations = 0;
 
 Stack<oop, mtGC>              MarkSweep::_marking_stack;
 Stack<ObjArrayTask, mtGC>     MarkSweep::_objarray_stack;

hotspot/src/share/vm/gc_implementation/shared/markSweep.hpp

@@ -113,7 +113,7 @@ class MarkSweep : AllStatic {
   //
  protected:
   // Total invocations of a MarkSweep collection
-  static unsigned int _total_invocations;
+  static uint _total_invocations;
 
   // Traversal stacks used during phase1
   static Stack<oop, mtGC>                      _marking_stack;
@@ -147,7 +147,7 @@ class MarkSweep : AllStatic {
   static AdjustKlassClosure   adjust_klass_closure;
 
   // Accessors
-  static unsigned int total_invocations() { return _total_invocations; }
+  static uint total_invocations() { return _total_invocations; }
 
   // Reference Processing
   static ReferenceProcessor* const ref_processor() { return _ref_processor; }

hotspot/src/share/vm/memory/collectorPolicy.cpp

@@ -752,7 +752,7 @@ HeapWord* GenCollectorPolicy::satisfy_failed_allocation(size_t size,
   // free memory should be here, especially if they are expensive. If this
   // attempt fails, an OOM exception will be thrown.
   {
-    IntFlagSetting flag_change(MarkSweepAlwaysCompactCount, 1); // Make sure the heap is fully compacted
+    UIntFlagSetting flag_change(MarkSweepAlwaysCompactCount, 1); // Make sure the heap is fully compacted
 
     gch->do_collection(true             /* full */,
                        true             /* clear_all_soft_refs */,

hotspot/src/share/vm/memory/space.hpp

@@ -537,9 +537,8 @@ protected:
    * Occasionally, we want to ensure a full compaction, which is determined  \
    * by the MarkSweepAlwaysCompactCount parameter.                           \
    */                                                                        \
-  int invocations = MarkSweep::total_invocations();                          \
+  uint invocations = MarkSweep::total_invocations();                         \
-  bool skip_dead = (MarkSweepAlwaysCompactCount < 1)                         \
+  bool skip_dead = ((invocations % MarkSweepAlwaysCompactCount) != 0);       \
-    ||((invocations % MarkSweepAlwaysCompactCount) != 0);                    \
                                                                              \
   size_t allowed_deadspace = 0;                                              \
   if (skip_dead) {                                                           \

hotspot/src/share/vm/runtime/arguments.cpp

@@ -1901,7 +1901,7 @@ bool Arguments::check_vm_args_consistency() {
     status = false;
   }
 
-  status = status && verify_percentage(AdaptiveSizePolicyWeight,
+  status = status && verify_interval(AdaptiveSizePolicyWeight, 0, 100,
                               "AdaptiveSizePolicyWeight");
   status = status && verify_percentage(ThresholdTolerance, "ThresholdTolerance");
   status = status && verify_percentage(MinHeapFreeRatio, "MinHeapFreeRatio");
@@ -1961,8 +1961,6 @@ bool Arguments::check_vm_args_consistency() {
     FLAG_SET_DEFAULT(UseGCOverheadLimit, false);
   }
 
-  status = status && verify_percentage(GCHeapFreeLimit, "GCHeapFreeLimit");
-
   status = status && check_gc_consistency();
   status = status && check_stack_pages();
 
@@ -2056,6 +2054,52 @@ bool Arguments::check_vm_args_consistency() {
     status = status && verify_interval(G1ConcRSLogCacheSize, 0, 31,
                                        "G1ConcRSLogCacheSize");
   }
+  if (UseConcMarkSweepGC) {
+    status = status && verify_min_value(CMSOldPLABNumRefills, 1, "CMSOldPLABNumRefills");
+    status = status && verify_min_value(CMSOldPLABToleranceFactor, 1, "CMSOldPLABToleranceFactor");
+    status = status && verify_min_value(CMSOldPLABMax, 1, "CMSOldPLABMax");
+    status = status && verify_interval(CMSOldPLABMin, 1, CMSOldPLABMax, "CMSOldPLABMin");
+
+    status = status && verify_min_value(CMSYoungGenPerWorker, 1, "CMSYoungGenPerWorker");
+
+    status = status && verify_min_value(CMSSamplingGrain, 1, "CMSSamplingGrain");
+    status = status && verify_interval(CMS_SweepWeight, 0, 100, "CMS_SweepWeight");
+    status = status && verify_interval(CMS_FLSWeight, 0, 100, "CMS_FLSWeight");
+
+    status = status && verify_interval(FLSCoalescePolicy, 0, 4, "FLSCoalescePolicy");
+
+    status = status && verify_min_value(CMSRescanMultiple, 1, "CMSRescanMultiple");
+    status = status && verify_min_value(CMSConcMarkMultiple, 1, "CMSConcMarkMultiple");
+
+    status = status && verify_interval(CMSPrecleanIter, 0, 9, "CMSPrecleanIter");
+    status = status && verify_min_value(CMSPrecleanDenominator, 1, "CMSPrecleanDenominator");
+    status = status && verify_interval(CMSPrecleanNumerator, 0, CMSPrecleanDenominator - 1, "CMSPrecleanNumerator");
+
+    status = status && verify_percentage(CMSBootstrapOccupancy, "CMSBootstrapOccupancy");
+
+    status = status && verify_min_value(CMSPrecleanThreshold, 100, "CMSPrecleanThreshold");
+
+    status = status && verify_percentage(CMSScheduleRemarkEdenPenetration, "CMSScheduleRemarkEdenPenetration");
+    status = status && verify_min_value(CMSScheduleRemarkSamplingRatio, 1, "CMSScheduleRemarkSamplingRatio");
+    status = status && verify_min_value(CMSBitMapYieldQuantum, 1, "CMSBitMapYieldQuantum");
+    status = status && verify_percentage(CMSTriggerRatio, "CMSTriggerRatio");
+    status = status && verify_percentage(CMSIsTooFullPercentage, "CMSIsTooFullPercentage");
+  }
+
+  if (UseParallelGC || UseParallelOldGC) {
+    status = status && verify_interval(ParallelOldDeadWoodLimiterMean, 0, 100, "ParallelOldDeadWoodLimiterMean");
+    status = status && verify_interval(ParallelOldDeadWoodLimiterStdDev, 0, 100, "ParallelOldDeadWoodLimiterStdDev");
+
+    status = status && verify_percentage(YoungGenerationSizeIncrement, "YoungGenerationSizeIncrement");
+    status = status && verify_percentage(TenuredGenerationSizeIncrement, "TenuredGenerationSizeIncrement");
+
+    status = status && verify_min_value(YoungGenerationSizeSupplementDecay, 1, "YoungGenerationSizeSupplementDecay");
+    status = status && verify_min_value(TenuredGenerationSizeSupplementDecay, 1, "TenuredGenerationSizeSupplementDecay");
+
+    status = status && verify_min_value(ParGCCardsPerStrideChunk, 1, "ParGCCardsPerStrideChunk");
+
+    status = status && verify_min_value(ParallelOldGCSplitInterval, 0, "ParallelOldGCSplitInterval");
+  }
 #endif // INCLUDE_ALL_GCS
 
   status = status && verify_interval(RefDiscoveryPolicy,
@@ -2075,7 +2119,42 @@ bool Arguments::check_vm_args_consistency() {
 
   status = status && verify_interval(MarkStackSizeMax,
                                   1, (max_jint - 1), "MarkStackSizeMax");
+  status = status && verify_interval(NUMAChunkResizeWeight, 0, 100, "NUMAChunkResizeWeight");
 
+  status = status && verify_min_value(LogEventsBufferEntries, 1, "LogEventsBufferEntries");
+
+  status = status && verify_min_value(HeapSizePerGCThread, (uintx) os::vm_page_size(), "HeapSizePerGCThread");
+
+  status = status && verify_min_value(GCTaskTimeStampEntries, 1, "GCTaskTimeStampEntries");
+
+  status = status && verify_percentage(ParallelGCBufferWastePct, "ParallelGCBufferWastePct");
+  status = status && verify_interval(TargetPLABWastePct, 1, 100, "TargetPLABWastePct");
+
+  status = status && verify_min_value(ParGCStridesPerThread, 1, "ParGCStridesPerThread");
+
+  status = status && verify_min_value(MinRAMFraction, 1, "MinRAMFraction");
+  status = status && verify_min_value(InitialRAMFraction, 1, "InitialRAMFraction");
+  status = status && verify_min_value(MaxRAMFraction, 1, "MaxRAMFraction");
+  status = status && verify_min_value(DefaultMaxRAMFraction, 1, "DefaultMaxRAMFraction");
+
+  status = status && verify_interval(AdaptiveTimeWeight, 0, 100, "AdaptiveTimeWeight");
+  status = status && verify_min_value(AdaptiveSizeDecrementScaleFactor, 1, "AdaptiveSizeDecrementScaleFactor");
+
+  status = status && verify_interval(TLABAllocationWeight, 0, 100, "TLABAllocationWeight");
+  status = status && verify_min_value(MinTLABSize, 1, "MinTLABSize");
+  status = status && verify_min_value(TLABRefillWasteFraction, 1, "TLABRefillWasteFraction");
+
+  status = status && verify_percentage(YoungGenerationSizeSupplement, "YoungGenerationSizeSupplement");
+  status = status && verify_percentage(TenuredGenerationSizeSupplement, "TenuredGenerationSizeSupplement");
+
+  // the "age" field in the oop header is 4 bits; do not want to pull in markOop.hpp
+  // just for that, so hardcode here.
+  status = status && verify_interval(MaxTenuringThreshold, 0, 15, "MaxTenuringThreshold");
+  status = status && verify_interval(InitialTenuringThreshold, 0, MaxTenuringThreshold, "MaxTenuringThreshold");
+  status = status && verify_percentage(TargetSurvivorRatio, "TargetSurvivorRatio");
+  status = status && verify_percentage(MarkSweepDeadRatio, "MarkSweepDeadRatio");
+
+  status = status && verify_min_value(MarkSweepAlwaysCompactCount, 1, "MarkSweepAlwaysCompactCount");
 #ifdef SPARC
   if (UseConcMarkSweepGC || UseG1GC) {
     // Issue a stern warning if the user has explicitly set

hotspot/src/share/vm/runtime/globals.hpp

@@ -286,12 +286,12 @@ class CounterSetting {
 };
 
 
-class IntFlagSetting {
+class UIntFlagSetting {
-  intx val;
+  uintx val;
-  intx* flag;
+  uintx* flag;
  public:
-  IntFlagSetting(intx& fl, intx newValue) { flag = &fl; val = fl; fl = newValue; }
+  UIntFlagSetting(uintx& fl, uintx newValue) { flag = &fl; val = fl; fl = newValue; }
-  ~IntFlagSetting()                       { *flag = val; }
+  ~UIntFlagSetting()                         { *flag = val; }
 };
 
 
@@ -513,12 +513,12 @@ class CommandLineFlags {
   product(bool, ForceNUMA, false,                                           \
           "Force NUMA optimizations on single-node/UMA systems")            \
                                                                             \
-  product(intx, NUMAChunkResizeWeight, 20,                                  \
+  product(uintx, NUMAChunkResizeWeight, 20,                                 \
-          "Percentage (0-100) used to weight the current sample when "      \
+          "Percentage (0-100) used to weigh the current sample when "      \
           "computing exponentially decaying average for "                   \
           "AdaptiveNUMAChunkSizing")                                        \
                                                                             \
-  product(intx, NUMASpaceResizeRate, 1*G,                                   \
+  product(uintx, NUMASpaceResizeRate, 1*G,                                  \
           "Do not reallocate more that this amount per collection")         \
                                                                             \
   product(bool, UseAdaptiveNUMAChunkSizing, true,                           \
@@ -527,7 +527,7 @@ class CommandLineFlags {
   product(bool, NUMAStats, false,                                           \
           "Print NUMA stats in detailed heap information")                  \
                                                                             \
-  product(intx, NUMAPageScanRate, 256,                                      \
+  product(uintx, NUMAPageScanRate, 256,                                     \
           "Maximum number of pages to include in the page scan procedure")  \
                                                                             \
   product_pd(bool, NeedsDeoptSuspend,                                       \
@@ -715,7 +715,7 @@ class CommandLineFlags {
   diagnostic(bool, LogEvents, true,                                         \
              "Enable the various ring buffer event logs")                   \
                                                                             \
-  diagnostic(intx, LogEventsBufferEntries, 10,                              \
+  diagnostic(uintx, LogEventsBufferEntries, 10,                             \
              "Enable the various ring buffer event logs")                   \
                                                                             \
   product(bool, BytecodeVerificationRemote, true,                           \
@@ -1432,16 +1432,17 @@ class CommandLineFlags {
   product(bool, ParallelGCVerbose, false,                                   \
           "Verbose output for parallel GC.")                                \
                                                                             \
-  product(intx, ParallelGCBufferWastePct, 10,                               \
+  product(uintx, ParallelGCBufferWastePct, 10,                              \
-          "wasted fraction of parallel allocation buffer.")                 \
+          "Wasted fraction of parallel allocation buffer.")                 \
                                                                             \
   diagnostic(bool, ParallelGCRetainPLAB, false,                             \
              "Retain parallel allocation buffers across scavenges; "        \
              " -- disabled because this currently conflicts with "          \
              " parallel card scanning under certain conditions ")           \
                                                                             \
-  product(intx, TargetPLABWastePct, 10,                                     \
+  product(uintx, TargetPLABWastePct, 10,                                    \
-          "target wasted space in last buffer as pct of overall allocation")\
+          "Target wasted space in last buffer as percent of overall "       \
+          "allocation")                                                     \
                                                                             \
   product(uintx, PLABWeight, 75,                                            \
           "Percentage (0-100) used to weight the current sample when"       \
@@ -1519,7 +1520,7 @@ class CommandLineFlags {
   product(bool, AlwaysPreTouch, false,                                      \
           "It forces all freshly committed pages to be pre-touched.")       \
                                                                             \
-  product_pd(intx, CMSYoungGenPerWorker,                                    \
+  product_pd(uintx, CMSYoungGenPerWorker,                                   \
           "The maximum size of young gen chosen by default per GC worker "  \
           "thread available")                                               \
                                                                             \
@@ -1837,7 +1838,7 @@ class CommandLineFlags {
   product(bool, UseCMSInitiatingOccupancyOnly, false,                       \
           "Only use occupancy as a crierion for starting a CMS collection") \
                                                                             \
-  product(intx, CMSIsTooFullPercentage, 98,                                 \
+  product(uintx, CMSIsTooFullPercentage, 98,                                \
           "An absolute ceiling above which CMS will always consider the "   \
           "unloading of classes when class unloading is enabled")           \
                                                                             \
@@ -1876,7 +1877,7 @@ class CommandLineFlags {
   develop(uintx, PromotionFailureALotInterval, 5,                           \
           "Total collections between promotion failures alot")              \
                                                                             \
-  experimental(intx, WorkStealingSleepMillis, 1,                            \
+  experimental(uintx, WorkStealingSleepMillis, 1,                           \
           "Sleep time when sleep is used for yields")                       \
                                                                             \
   experimental(uintx, WorkStealingYieldsBeforeSleep, 5000,                  \
@@ -2020,7 +2021,7 @@ class CommandLineFlags {
           "Number of collections before the adaptive sizing is started")    \
                                                                             \
   product(uintx, AdaptiveSizePolicyOutputInterval, 0,                       \
-          "Collecton interval for printing information; zero => never")     \
+          "Collection interval for printing information; zero means never") \
                                                                             \
   product(bool, UseAdaptiveSizePolicyFootprintGoal, true,                   \
           "Use adaptive minimum footprint as a goal")                       \
@@ -3049,7 +3050,7 @@ class CommandLineFlags {
   product(uintx, MaxMetaspaceExpansion, ScaleForWordSize(4*M),              \
           "Max expansion of Metaspace without full GC (in bytes)")          \
                                                                             \
-  product(intx, QueuedAllocationWarningCount, 0,                            \
+  product(uintx, QueuedAllocationWarningCount, 0,                           \
           "Number of times an allocation that queues behind a GC "          \
           "will retry before printing a warning")                           \
                                                                             \
@@ -3077,7 +3078,7 @@ class CommandLineFlags {
           "either completely full or completely empty.  Par compact also"   \
           "has a smaller default value; see arguments.cpp.")                \
                                                                             \
-  product(intx, MarkSweepAlwaysCompactCount,     4,                         \
+  product(uintx, MarkSweepAlwaysCompactCount,     4,                        \
           "How often should we fully compact the heap (ignoring the dead "  \
           "space parameters)")                                              \
                                                                             \