8264752: SIGFPE crash with option FlightRecorderOptions:threadbuffersize=30M

Reviewed-by: mgronlun

Author: Hui Shi

src/hotspot/share/jfr/recorder/service/jfrMemorySizer.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,13 +30,15 @@
 const julong MAX_ADJUSTED_GLOBAL_BUFFER_SIZE = 1 * M;
 const julong MIN_ADJUSTED_GLOBAL_BUFFER_SIZE_CUTOFF = 512 * K;
 const julong MIN_GLOBAL_BUFFER_SIZE = 64 * K;
+const julong MAX_GLOBAL_BUFFER_SIZE = 2 * G;
 // implies at least 2 * MIN_GLOBAL_BUFFER SIZE
 const julong MIN_BUFFER_COUNT = 2;
 // MAX global buffer count open ended
 const julong DEFAULT_BUFFER_COUNT = 20;
 // MAX thread local buffer size == size of a single global buffer (runtime determined)
 // DEFAULT thread local buffer size = 2 * os page size (runtime determined)
 const julong MIN_THREAD_BUFFER_SIZE = 4 * K;
+const julong MAX_THREAD_BUFFER_SIZE = 2 * G;
 const julong MIN_MEMORY_SIZE = 1 * M;
 const julong DEFAULT_MEMORY_SIZE = 10 * M;
 
@@ -305,6 +307,11 @@ static void thread_buffer_size(JfrMemoryOptions* options) {
   options->global_buffer_size = div_total_by_units(options->memory_size, options->buffer_count);
   if (options->thread_buffer_size > options->global_buffer_size) {
     options->global_buffer_size = options->thread_buffer_size;
+    if (options->memory_size_configured) {
+      options->buffer_count = div_total_by_per_unit(options->memory_size, options->global_buffer_size);
+    } else {
+      options->memory_size = multiply(options->global_buffer_size, options->buffer_count);
+    }
     options->buffer_count = div_total_by_per_unit(options->memory_size, options->global_buffer_size);
   }
   assert(options->global_buffer_size >= options->thread_buffer_size, "invariant");
@@ -324,7 +331,8 @@ static void assert_post_condition(const JfrMemoryOptions* options) {
   assert(options->memory_size % os::vm_page_size() == 0, "invariant");
   assert(options->global_buffer_size % os::vm_page_size() == 0, "invariant");
   assert(options->thread_buffer_size % os::vm_page_size() == 0, "invariant");
-  assert(options->buffer_count > 0, "invariant");
+  assert(options->buffer_count >= MIN_BUFFER_COUNT, "invariant");
+  assert(options->global_buffer_size >= options->thread_buffer_size, "invariant");
 }
 #endif
 
@@ -429,6 +437,10 @@ bool JfrMemorySizer::adjust_options(JfrMemoryOptions* options) {
     default:
       default_size(options);
   }
+  if (options->buffer_count < MIN_BUFFER_COUNT ||
+      options->global_buffer_size < options->thread_buffer_size) {
+    return false;
+  }
   DEBUG_ONLY(assert_post_condition(options);)
   return true;
 }

src/hotspot/share/jfr/recorder/service/jfrMemorySizer.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -32,6 +32,8 @@ extern const julong MIN_BUFFER_COUNT;
 extern const julong MIN_GLOBAL_BUFFER_SIZE;
 extern const julong MIN_MEMORY_SIZE;
 extern const julong MIN_THREAD_BUFFER_SIZE;
+extern const julong MAX_GLOBAL_BUFFER_SIZE;
+extern const julong MAX_THREAD_BUFFER_SIZE;
 
 struct JfrMemoryOptions {
   julong memory_size;

src/hotspot/share/jfr/recorder/service/jfrOptionSet.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -393,34 +393,40 @@ static julong divide_with_user_unit(Argument& memory_argument, julong value) {
   return value;
 }
 
-template <typename Argument>
-static void log_lower_than_min_value(Argument& memory_argument, julong min_value) {
+static const char higher_than_msg[] = "This value is higher than the maximum size limited ";
+static const char lower_than_msg[] = "This value is lower than the minimum size required ";
+template <typename Argument, char const *msg>
+static void log_out_of_range_value(Argument& memory_argument, julong min_value) {
   if (memory_argument.value()._size != memory_argument.value()._val) {
     // has multiplier
     log_error(arguments) (
-      "This value is lower than the minimum size required " JULONG_FORMAT "%c",
+      "%s" JULONG_FORMAT "%c", msg,
       divide_with_user_unit(memory_argument, min_value),
       memory_argument.value()._multiplier);
     return;
   }
   log_error(arguments) (
-    "This value is lower than the minimum size required " JULONG_FORMAT,
+    "%s" JULONG_FORMAT, msg,
     divide_with_user_unit(memory_argument, min_value));
 }
 
+static const char default_val_msg[] = "Value default for option ";
+static const char specified_val_msg[] = "Value specified for option ";
 template <typename Argument>
 static void log_set_value(Argument& memory_argument) {
   if (memory_argument.value()._size != memory_argument.value()._val) {
     // has multiplier
     log_error(arguments) (
-      "Value specified for option \"%s\" is " JULONG_FORMAT "%c",
+      "%s\"%s\" is " JULONG_FORMAT "%c",
+      memory_argument.is_set() ? specified_val_msg: default_val_msg,
       memory_argument.name(),
       memory_argument.value()._val,
       memory_argument.value()._multiplier);
     return;
   }
   log_error(arguments) (
-    "Value specified for option \"%s\" is " JULONG_FORMAT,
+    "%s\"%s\" is " JULONG_FORMAT,
+    memory_argument.is_set() ? specified_val_msg: default_val_msg,
     memory_argument.name(), memory_argument.value()._val);
 }
 
@@ -541,6 +547,10 @@ static bool valid_memory_relations(const JfrMemoryOptions& options) {
         return false;
       }
     }
+  } else if (options.thread_buffer_size_configured && options.memory_size_configured) {
+    if (!ensure_first_gteq_second(_dcmd_memorysize, _dcmd_threadbuffersize)) {
+      return false;
+    }
   }
   return true;
 }
@@ -609,7 +619,7 @@ template <typename Argument>
 static bool ensure_gteq(Argument& memory_argument, const jlong value) {
   if ((jlong)memory_argument.value()._size < value) {
     log_set_value(memory_argument);
-    log_lower_than_min_value(memory_argument, value);
+    log_out_of_range_value<Argument, lower_than_msg>(memory_argument, value);
     return false;
   }
   return true;
@@ -640,14 +650,38 @@ static bool ensure_valid_minimum_sizes() {
   return true;
 }
 
+template <typename Argument>
+static bool ensure_lteq(Argument& memory_argument, const jlong value) {
+  if ((jlong)memory_argument.value()._size > value) {
+    log_set_value(memory_argument);
+    log_out_of_range_value<Argument, higher_than_msg>(memory_argument, value);
+    return false;
+  }
+  return true;
+}
+
+static bool ensure_valid_maximum_sizes() {
+  if (_dcmd_globalbuffersize.is_set()) {
+    if (!ensure_lteq(_dcmd_globalbuffersize, MAX_GLOBAL_BUFFER_SIZE)) {
+      return false;
+    }
+  }
+  if (_dcmd_threadbuffersize.is_set()) {
+    if (!ensure_lteq(_dcmd_threadbuffersize, MAX_THREAD_BUFFER_SIZE)) {
+      return false;
+    }
+  }
+  return true;
+}
+
 /**
  * Starting with the initial set of memory values from the user,
  * sanitize, enforce min/max rules and adjust to a set of consistent options.
  *
  * Adjusted memory sizes will be page aligned.
  */
 bool JfrOptionSet::adjust_memory_options() {
-  if (!ensure_valid_minimum_sizes()) {
+  if (!ensure_valid_minimum_sizes() || !ensure_valid_maximum_sizes()) {
     return false;
   }
   JfrMemoryOptions options;
@@ -656,6 +690,24 @@ bool JfrOptionSet::adjust_memory_options() {
     return false;
   }
   if (!JfrMemorySizer::adjust_options(&options)) {
+    if (options.buffer_count < MIN_BUFFER_COUNT || options.global_buffer_size < options.thread_buffer_size) {
+      log_set_value(_dcmd_memorysize);
+      log_set_value(_dcmd_globalbuffersize);
+      log_error(arguments) ("%s \"%s\" is " JLONG_FORMAT,
+        _dcmd_numglobalbuffers.is_set() ? specified_val_msg: default_val_msg,
+        _dcmd_numglobalbuffers.name(), _dcmd_numglobalbuffers.value());
+      log_set_value(_dcmd_threadbuffersize);
+      if (options.buffer_count < MIN_BUFFER_COUNT) {
+        log_error(arguments) ("numglobalbuffers " JULONG_FORMAT " is less than minimal value " JULONG_FORMAT,
+          options.buffer_count, MIN_BUFFER_COUNT);
+        log_error(arguments) ("Decrease globalbuffersize/threadbuffersize or increase memorysize");
+      } else {
+        log_error(arguments) ("globalbuffersize " JULONG_FORMAT " is less than threadbuffersize" JULONG_FORMAT,
+          options.global_buffer_size, options.thread_buffer_size);
+        log_error(arguments) ("Decrease globalbuffersize or increase memorysize or adjust global/threadbuffersize");
+      }
+      return false;
+    }
     if (!check_for_ambiguity(_dcmd_memorysize, _dcmd_globalbuffersize, _dcmd_numglobalbuffers)) {
       return false;
     }

test/jdk/jdk/jfr/startupargs/TestBadOptionValues.java

@@ -26,6 +26,7 @@
 import jdk.test.lib.Asserts;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
+import sun.hotspot.WhiteBox;
 
 /**
  * @test
@@ -37,7 +38,9 @@
  *          java.management
  *          jdk.jfr
  *
- * @run main jdk.jfr.startupargs.TestBadOptionValues
+ * @build sun.hotspot.WhiteBox
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI jdk.jfr.startupargs.TestBadOptionValues
  */
 public class TestBadOptionValues {
 
@@ -119,6 +122,31 @@ public static void main(String[] args) throws Exception {
         test(START_FLIGHT_RECORDING, "Parsing error memory size value: invalid value",
             "maxsize=");
 
+        // globalbuffersize exceeds limit
+        test(FLIGHT_RECORDER_OPTIONS, "This value is higher than the maximum size limit",
+            "globalbuffersize=4G");
+
+        // threadbuffersize exceeds limit
+        test(FLIGHT_RECORDER_OPTIONS, "This value is higher than the maximum size limit",
+            "threadbuffersize=4G");
+
+        // computed numglobalbuffers smaller than MIN_BUFFER_COUNT
+        test(FLIGHT_RECORDER_OPTIONS, "Decrease globalbuffersize/threadbuffersize or increase memorysize",
+            "memorysize=1m,globalbuffersize=1m");
+
+        // memorysize smaller than threadbuffersize
+        test(FLIGHT_RECORDER_OPTIONS, "The value for option \"threadbuffersize\" should not be larger than the value specified for option \"memorysize\"",
+            "memorysize=1m,threadbuffersize=2m");
+
+        // computed globalbuffersize smaller than threadbuffersize
+        // test is on when vm page isn't larger than 4K, avoiding both buffer sizes align to vm page size
+        WhiteBox wb = WhiteBox.getWhiteBox();
+        long smallPageSize = wb.getVMPageSize();
+        if (smallPageSize <= 4096) {
+            test(FLIGHT_RECORDER_OPTIONS, "Decrease globalbuffersize or increase memorysize or adjust global/threadbuffersize",
+                "memorysize=1m,numglobalbuffers=256");
+        }
+
         test(FLIGHT_RECORDER_OPTIONS, "Parsing error memory size value: invalid value",
             "threadbuffersize=a",
             "globalbuffersize=G",

test/jdk/jdk/jfr/startupargs/TestMemoryOptions.java

@@ -638,6 +638,11 @@ public static void runTestCase(TestCase tc) throws Exception {
         tc.setGlobalBufferSizeTestParam(64, 'k');
         tc.setGlobalBufferCountTestParam(16, 'b');
         testCases.add(tc);
+
+        // threadbuffersize exceeds default memorysize
+        tc = new TestCase("ThreadBufferSizeExceedMemorySize", false);
+        tc.setThreadBufferSizeTestParam(30, 'm');
+        testCases.add(tc);
     }
 
     public static void main(String[] args) throws Exception {