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

[Stefan Karlsson]

Hi all,

Please review this patch to fix bugs in the -Xss parsing.

http://cr.openjdk.java.net/~stefank/8178491/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8178491

I found this bug while working on an Enhancement to the align functions:
https://bugs.openjdk.java.net/browse/JDK-8178489

In arguments.cpp we have the following code in the section that converts 
the Xss value to a ThreadStackSize value:
  round_to((int)long_ThreadStackSize, K) / K) != Flag::SUCCESS)

This explicit narrowing cast throws away the upper 32 bits of 
long_ThreadStackSize, the resulting int is then promoted to an intptr_t 
used as a argument to round_to. This causes weird error messages and 
behavior. For example, running:
java -Xss2147483648 -version

Yields:
intx ThreadStackSize=18014398507384832 is outside the allowed range [ 0 
... 9007199254740987 ]

My initial reaction was that this could be solved by simply fixing the 
cast, but further inspection of the code parsing -Xss and 
ThreadStackSize revealed more issues.

Changes in this patch:

1) Extract the -Xss parsing code into a separate, testable function. 
Unit tests added.

2) Extended parse_memory_size and check_memory_size, to take the maximum 
input value. Unit tests added.

3) Provide an upper limit for the values allowed in the call to 
parse_memory_size in the -Xss parsing. The upper limit is calculated 
backwards from the the upper limit of the ThreadStackSize.

4) The max input value for ThreadStackSize was artificially limited by 
the fact that round_to was used, and that limited values to fit in an 
intptr_t, even though size_t is used when the ThreadStackSize is finally 
expanded into page size aligned bytes. This meant that we had an 
extremly high upper limit, but it wasn't the absolute upper limit. I 
changed this to the absolute upper limit, so that it's easier to reason 
about why this limit was chosen. To make this possible, I replaced the 
usages round_to and round_down, with align_size_up_ and align_size_down_ 
macros. In future patches, when the input parameter types of our align 
functions have been fixed, I'll convert the usages of these macros.

5) More asserts to check for unintended overflows.

IMHO, It's a bit silly to allow this high values for -Xss, and we could 
probably limit it to more practical values. I experimented with that, 
but I found that the code and tests became more unintuitive than they 
already are. So, I'd like to leave that as a future enhancement if the 
maintainers of this code allows it.

Tested with JPRT, TooSmallStackSize.java, and TestThreadStackSizes.java. 
Any suggestions on how to further test this?

Thanks,
StefanK