--- old/src/os/windows/vm/os_windows.cpp 2017-06-29 13:28:52.459344280 +0200 +++ new/src/os/windows/vm/os_windows.cpp 2017-06-29 13:28:52.251337673 +0200 @@ -4043,7 +4043,7 @@ // If stack_commit_size is 0, windows will reserve the default size, // but only commit a small portion of it. - size_t stack_commit_size = round_to(ThreadStackSize*K, os::vm_page_size()); + size_t stack_commit_size = align_size_up_((size_t)ThreadStackSize * K, (size_t)os::vm_page_size()); size_t default_reserve_size = os::win32::default_stack_size(); size_t actual_reserve_size = stack_commit_size; if (stack_commit_size < default_reserve_size) { --- old/src/share/vm/runtime/arguments.cpp 2017-06-29 13:28:52.819355714 +0200 +++ new/src/share/vm/runtime/arguments.cpp 2017-06-29 13:28:52.619349362 +0200 @@ -644,10 +644,9 @@ } } -Arguments::ArgsRange Arguments::check_memory_size(julong size, julong min_size) { +Arguments::ArgsRange Arguments::check_memory_size(julong size, julong min_size, julong max_size) { if (size < min_size) return arg_too_small; - // Check that size will fit in a size_t (only relevant on 32-bit) - if (size > max_uintx) return arg_too_big; + if (size > max_size) return arg_too_big; return arg_in_range; } @@ -2617,9 +2616,10 @@ Arguments::ArgsRange Arguments::parse_memory_size(const char* s, julong* long_arg, - julong min_size) { + julong min_size, + julong max_size) { if (!atojulong(s, long_arg)) return arg_unreadable; - return check_memory_size(*long_arg, min_size); + return check_memory_size(*long_arg, min_size, max_size); } // Parse JavaVMInitArgs structure @@ -2750,6 +2750,56 @@ return JNI_OK; } +// Parse -Xss memory string parameter and convert to ThreadStackSize in K. +jint Arguments::parse_xss(const JavaVMOption* option, const char* tail, intx* out_ThreadStackSize) { + assert(is_size_aligned_((size_t)os::vm_page_size(), K), "Implementation assumption"); + + // The min and max sizes match the values in globals.hpp. + const julong min_size = 1000; + // Must limit the max_size to match multiplications + // and alignments in the os_.cpp files, so that + // the calculation stored in a size_t (same size as uintx) + // doesn't overflow. + // Must also make sure that when *value is max_size it + // doesn't overflow intx in the calculation further down + // in this function. + const julong max_ThreadStackSize = align_size_down_((julong)max_uintx, (size_t)os::vm_page_size()) / K; + const julong max_size = max_ThreadStackSize * K; + + julong size = 0; + ArgsRange errcode = parse_memory_size(tail, &size, min_size, max_size); + if (errcode != arg_in_range) { + bool silent = option == NULL; // Allow testing to silence error messages + if (!silent) { + jio_fprintf(defaultStream::error_stream(), + "Invalid thread stack size: %s\n", option->optionString); + describe_range_error(errcode); + } + return JNI_EINVAL; + } + + // Internally track ThreadStackSize in units of 1024 bytes. + const julong size_aligned = align_size_up_(size, K); + assert(size <= size_aligned, + "Overflow: " JULONG_FORMAT " " JULONG_FORMAT, + size, size_aligned); + + const julong size_in_K = size_aligned / K; + assert(size_in_K < (julong)max_intx, + "size_in_K doesn't fit in the type of ThreadStackSize: " JULONG_FORMAT, + size_in_K); + + // Check that code expanding ThreadStackSize to a page aligned number of bytes won't overflow. + const julong max_expanded = align_size_up_(size_in_K * K, (size_t)os::vm_page_size()); + assert(max_expanded < max_uintx && max_expanded > size_in_K, + "Expansion overflowed: " JULONG_FORMAT " " JULONG_FORMAT, + max_expanded, size_in_K); + + *out_ThreadStackSize = (intx)size_in_K; + + return JNI_OK; +} + jint Arguments::parse_each_vm_init_arg(const JavaVMInitArgs* args, bool* patch_mod_javabase, Flag::Flags origin) { // For match_option to return remaining or value part of option string const char* tail; @@ -3013,17 +3063,12 @@ } // -Xss } else if (match_option(option, "-Xss", &tail)) { - julong long_ThreadStackSize = 0; - ArgsRange errcode = parse_memory_size(tail, &long_ThreadStackSize, 1000); - if (errcode != arg_in_range) { - jio_fprintf(defaultStream::error_stream(), - "Invalid thread stack size: %s\n", option->optionString); - describe_range_error(errcode); - return JNI_EINVAL; + intx value = 0; + jint err = parse_xss(option, tail, &value); + if (err != JNI_OK) { + return err; } - // Internally track ThreadStackSize in units of 1024 bytes. - if (FLAG_SET_CMDLINE(intx, ThreadStackSize, - round_to((int)long_ThreadStackSize, K) / K) != Flag::SUCCESS) { + if (FLAG_SET_CMDLINE(intx, ThreadStackSize, value) != Flag::SUCCESS) { return JNI_EINVAL; } // -Xoss, -Xsqnopause, -Xoptimize, -Xboundthreads, -Xusealtsigs --- old/src/share/vm/runtime/arguments.hpp 2017-06-29 13:28:53.143366005 +0200 +++ new/src/share/vm/runtime/arguments.hpp 2017-06-29 13:28:52.971360542 +0200 @@ -322,6 +322,7 @@ friend class VMStructs; friend class JvmtiExport; friend class CodeCacheExtensions; + friend class ArgumentsTest; public: // Operation modi enum Mode { @@ -514,6 +515,7 @@ static jint parse_java_options_environment_variable(ScopedVMInitArgs* vm_args); static jint parse_vm_options_file(const char* file_name, ScopedVMInitArgs* vm_args); static jint parse_options_buffer(const char* name, char* buffer, const size_t buf_len, ScopedVMInitArgs* vm_args); + static jint parse_xss(const JavaVMOption* option, const char* tail, intx* out_ThreadStackSize); static jint insert_vm_options_file(const JavaVMInitArgs* args, const char* vm_options_file, const int vm_options_file_pos, @@ -542,9 +544,9 @@ } static void describe_range_error(ArgsRange errcode); - static ArgsRange check_memory_size(julong size, julong min_size); + static ArgsRange check_memory_size(julong size, julong min_size, julong max_size); static ArgsRange parse_memory_size(const char* s, julong* long_arg, - julong min_size); + julong min_size, julong max_size = max_uintx); // Parse a string for a unsigned integer. Returns true if value // is an unsigned integer greater than or equal to the minimum // parameter passed and returns the value in uintx_arg. Returns --- old/src/share/vm/runtime/globals.hpp 2017-06-29 13:28:53.419374772 +0200 +++ new/src/share/vm/runtime/globals.hpp 2017-06-29 13:28:53.235368927 +0200 @@ -3295,7 +3295,7 @@ \ product_pd(intx, ThreadStackSize, \ "Thread Stack Size (in Kbytes)") \ - range(0, (max_intx-os::vm_page_size())/(1 * K)) \ + range(0, intx(align_size_down_(max_uintx, os::vm_page_size())/K)) \ \ product_pd(intx, VMThreadStackSize, \ "Non-Java Thread Stack Size (in Kbytes)") \ --- old/test/native/runtime/test_arguments.cpp 2017-06-29 13:28:53.735384809 +0200 +++ new/test/native/runtime/test_arguments.cpp 2017-06-29 13:28:53.567379473 +0200 @@ -27,7 +27,22 @@ #include "unittest.hpp" #include "utilities/globalDefinitions.hpp" -TEST(arguments, atojulong) { +class ArgumentsTest : public ::testing::Test { +public: + static intx parse_xss_inner_annotated(const char* str, jint expected_err, const char* file, int line_number); + + // Expose the private Arguments functions. + + static Arguments::ArgsRange check_memory_size(julong size, julong min_size, julong max_size) { + return Arguments::check_memory_size(size, min_size, max_size); + } + + static jint parse_xss(const JavaVMOption* option, const char* tail, intx* out_ThreadStackSize) { + return Arguments::parse_xss(option, tail, out_ThreadStackSize); + } +}; + +TEST_F(ArgumentsTest, atojulong) { char ullong_max[32]; int ret = jio_snprintf(ullong_max, sizeof(ullong_max), JULONG_FORMAT, ULLONG_MAX); ASSERT_NE(-1, ret); @@ -70,3 +85,119 @@ ASSERT_EQ(valid_strings[i].expected_value, value); } } + +TEST_F(ArgumentsTest, check_memory_size__min) { + EXPECT_EQ(check_memory_size(999, 1000, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size(1000, 1000, max_uintx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(1001, 1000, max_uintx), Arguments::arg_in_range); + + EXPECT_EQ(check_memory_size(max_intx - 2, max_intx - 1, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size(max_intx - 1, max_intx - 1, max_uintx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_intx - 0, max_intx - 1, max_uintx), Arguments::arg_in_range); + + EXPECT_EQ(check_memory_size(max_intx - 1, max_intx, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size(max_intx , max_intx, max_uintx), Arguments::arg_in_range); + + NOT_LP64( + EXPECT_EQ(check_memory_size((julong)max_intx + 1, max_intx, max_uintx), Arguments::arg_in_range); + + EXPECT_EQ(check_memory_size( max_intx - 1, (julong)max_intx + 1, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size( max_intx , (julong)max_intx + 1, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size((julong)max_intx + 1, (julong)max_intx + 1, max_uintx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size((julong)max_intx + 2, (julong)max_intx + 1, max_uintx), Arguments::arg_in_range); + ); + + EXPECT_EQ(check_memory_size(max_uintx - 2, max_uintx - 1, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size(max_uintx - 1, max_uintx - 1, max_uintx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_uintx , max_uintx - 1, max_uintx), Arguments::arg_in_range); + + EXPECT_EQ(check_memory_size(max_uintx - 1, max_uintx, max_uintx), Arguments::arg_too_small); + EXPECT_EQ(check_memory_size(max_uintx , max_uintx, max_uintx), Arguments::arg_in_range); +} + +TEST_F(ArgumentsTest, check_memory_size__max) { + EXPECT_EQ(check_memory_size(max_uintx - 1, 1000, max_uintx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_uintx , 1000, max_uintx), Arguments::arg_in_range); + + EXPECT_EQ(check_memory_size(max_intx - 2 , 1000, max_intx - 1), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_intx - 1 , 1000, max_intx - 1), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_intx , 1000, max_intx - 1), Arguments::arg_too_big); + + EXPECT_EQ(check_memory_size(max_intx - 1 , 1000, max_intx), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size(max_intx , 1000, max_intx), Arguments::arg_in_range); + + NOT_LP64( + EXPECT_EQ(check_memory_size((julong)max_intx + 1 , 1000, max_intx), Arguments::arg_too_big); + + EXPECT_EQ(check_memory_size( max_intx , 1000, (julong)max_intx + 1), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size((julong)max_intx + 1 , 1000, (julong)max_intx + 1), Arguments::arg_in_range); + EXPECT_EQ(check_memory_size((julong)max_intx + 2 , 1000, (julong)max_intx + 1), Arguments::arg_too_big); + ); +} + +// A random value - used to verify the output when parsing is expected to fail. +static const intx no_value = 4711; + +inline intx ArgumentsTest::parse_xss_inner_annotated(const char* str, jint expected_err, const char* file, int line_number) { + intx value = no_value; + jint err = parse_xss(NULL /* Silence error messages */, str, &value); + EXPECT_EQ(err, expected_err) << "Failure from: " << file << ":" << line_number; + return value; +} + +// Wrapper around the help function - gives file and line number when a test failure occurs. +#define parse_xss_inner(str, expected_err) ArgumentsTest::parse_xss_inner_annotated(str, expected_err, __FILE__, __LINE__) + +static intx calc_expected(julong small_xss_input) { + assert(small_xss_input <= max_julong / 2, "Sanity"); + + // Match code in arguments.cpp + julong julong_ret = align_size_up_(small_xss_input, K) / K; + assert(julong_ret <= (julong)max_intx, "Overflow: " JULONG_FORMAT, julong_ret); + return (intx)julong_ret; +} + +static char buff[100]; +static char* to_string(julong value) { + jio_snprintf(buff, sizeof(buff), JULONG_FORMAT, value); + return buff; +} + +TEST_VM_F(ArgumentsTest, parse_xss) { + // Test the maximum input value - should fail. + { + EXPECT_EQ(parse_xss_inner(to_string(max_julong), JNI_EINVAL), no_value); + NOT_LP64(EXPECT_EQ(parse_xss_inner(to_string(max_uintx), JNI_EINVAL), no_value)); + } + + // Test values "far" away from the uintx boundary. + { + LP64_ONLY(EXPECT_EQ(parse_xss_inner(to_string(max_julong / 2), JNI_OK), calc_expected(max_julong / 2))); + EXPECT_EQ(parse_xss_inner(to_string(INT_MAX), JNI_OK), calc_expected(INT_MAX)); + EXPECT_EQ(parse_xss_inner(to_string(INT_MAX / 2), JNI_OK), calc_expected(INT_MAX / 2)); + } + + // Test value aligned both to K and vm_page_size. + { + EXPECT_TRUE(is_size_aligned(32 * M, K)); + EXPECT_TRUE(is_size_aligned(32 * M, (size_t)os::vm_page_size())); + EXPECT_EQ(parse_xss_inner(to_string(32 * M), JNI_OK), (intx)(32 * M / K)); + } + + // Test values around the uintx boundary. + { + // Reverse calculation of ThreadStackSize in os_windows.cpp + julong page_aligned_max_uintx = align_size_down_((julong)max_uintx, (size_t)os::vm_page_size()); + julong expected_max = align_size_down_(page_aligned_max_uintx, K) / K; + ASSERT_LE(expected_max, (julong)max_intx) << "expected_max will overflow intx"; + + // Calculation used in os_windows.cpp + julong max_expanded = align_size_up_(expected_max * K, (size_t)os::vm_page_size()); + ASSERT_LE(max_expanded, (julong)max_uintx) << "max_expanded will overflow uintx"; + + // Test exact max and values around max + EXPECT_EQ(parse_xss_inner(to_string(max_expanded), JNI_OK), (intx)expected_max); + EXPECT_EQ(parse_xss_inner(to_string(max_expanded - 1), JNI_OK), (intx)expected_max); + EXPECT_EQ(parse_xss_inner(to_string(max_expanded + 1), JNI_EINVAL), no_value); + } +}