RFR: 8178491: -Xss and -XX:ThreadStackSize argument parsing truncates bits

[Kim Barrett]

> On Jun 30, 2017, at 2:41 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> On 2017-06-30 00:56, Kim Barrett wrote:
>> src/share/vm/runtime/arguments.cpp
>> 2760   const julong min_ThreadStackSize = 0;
>> 2761   const julong max_ThreadStackSize = 1 * M;
>> 
>> Isn't there some data structure in the argument checking code that
>> contains the values registered in globals.hpp?
>> 
>> Hm, the data structure I was thinking of can obtained via
>> 
>>   CommandLineFlagRangeList::find("ThreadStackSize");
>> 
>> But it doesn't provide access to the range values. It only provides a
>> suite of check functions.
>> 
>> But that might be enough. Change the call to parse_memory_size to use
>> the type's min/max rather than (scaled) min/max for ThreadStackSize,
>> to just do parsing. Then look up the CommandLineFlagRange object, and
>> use it to perform the range check on the (aligned and descaled) parsed
>> value.
>> 
>> The point of all this is to avoid (hopefully) duplicating the range
>> information for ThreadStackSize here.
> 
> I understand the concern of the duplication, but unless there is a straight forward way to extract the range values out of the command line classes, then I'd like to leave this as an RFE for the maintainers of the command line range checking.

OK, as long as there’s a followup.

>> test/native/runtime/test_arguments.cpp
>>   45 TEST_F(ArgumentsTest, atojulong) {
>> 
>> Why is this test being fixturized?  It doesn't refer to anything
>> provided by the fixture, so far as I can tell.
> I want to put all the tests in the same test case (ArgumentsTest). This is what you get if you use TEST here:
> 
> "/home/stefank/hg/jdk10/hs/test/fmw/gtest/src/gtest.cc:1977: Failure
> Failed
> All tests in the same test case must use the same test fixture
> class, so mixing TEST_F and TEST in the same test case is
> illegal.  In test case ArgumentsTest,
> test parse_xss_test_vm is defined using TEST_F but
> test atojulong_test is defined using TEST.  You probably
> want to change the TEST to TEST_F or move it to another test
> case.”

There are a number of ways to deal with this, but the approach being taken
isn’t actually wrong or bad or anything; I just found it a little confusing because
the atojulong test wasn’t using anything from the fixture.  So ok as is.