6843347: Boundary values in some public GC options cause crashes

Summary: 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

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) \
                                                                             \

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) \
                                                                             \

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)
 

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)))
-                         /(double)100;
+  _bootstrap_occupancy = ((double)CMSBootstrapOccupancy)/(double)100;
 
   _full_gcs_since_conc_gc = 0;
 

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;
-  IntFlagSetting flag_setting(MarkSweepAlwaysCompactCount, count);
+  uint count = maximum_heap_compaction ? 1 : MarkSweepAlwaysCompactCount;
+  UIntFlagSetting flag_setting(MarkSweepAlwaysCompactCount, count);
   PSMarkSweep::invoke_no_policy(clear_all_soft_refs || maximum_heap_compaction);
 }
 

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)
-    || ((PSMarkSweep::total_invocations() % MarkSweepAlwaysCompactCount) != 0);
+  bool skip_dead = ((PSMarkSweep::total_invocations() % MarkSweepAlwaysCompactCount) != 0);
 
   size_t allowed_deadspace = 0;
   if (skip_dead) {

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;

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; }

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 */,

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();                          \
-  bool skip_dead = (MarkSweepAlwaysCompactCount < 1)                         \
-    ||((invocations % MarkSweepAlwaysCompactCount) != 0);                    \
+  uint invocations = MarkSweep::total_invocations();                         \
+  bool skip_dead = ((invocations % MarkSweepAlwaysCompactCount) != 0);       \
                                                                              \
   size_t allowed_deadspace = 0;                                              \
   if (skip_dead) {                                                           \

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

src/share/vm/runtime/globals.hpp

@@ -286,12 +286,12 @@ class CounterSetting {
 };
 
 
-class IntFlagSetting {
-  intx val;
-  intx* flag;
+class UIntFlagSetting {
+  uintx val;
+  uintx* flag;
  public:
-  IntFlagSetting(intx& fl, intx newValue) { flag = &fl; val = fl; fl = newValue; }
-  ~IntFlagSetting()                       { *flag = val; }
+  UIntFlagSetting(uintx& fl, uintx newValue) { flag = &fl; val = fl; fl = newValue; }
+  ~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,                                  \
-          "Percentage (0-100) used to weight the current sample when "      \
+  product(uintx, NUMAChunkResizeWeight, 20,                                 \
+          "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,                               \
-          "wasted fraction of parallel allocation buffer.")                 \
+  product(uintx, ParallelGCBufferWastePct, 10,                              \
+          "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,                                     \
-          "target wasted space in last buffer as pct of overall allocation")\
+  product(uintx, TargetPLABWastePct, 10,                                    \
+          "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)")                                              \
                                                                             \