RFR(10)(S) 8177055: [TESTBUG] test/runtime/ReservedStack/ReservedStackTest.java sometimes fails on Oracle arm64 port after being enabled for open aarch64 port

[David Holmes]

Hi Chris,

Is there no property we can query that will give a different value for 
the two ports? Could we easily add anything? What does -Xinternalversion 
show? (though I suppose checking that is no different to checking how it 
responds to -XX:StackReservedPages=1)

That aside some issues with your logic:

202             if (isSupportedPlatform && !result.contains("PASSED")) {

251     public static boolean isSupportedPlatform;

266         boolean isSupportedPlatform = true;

270             isSupportedPlatform = false;

Line #202 is reading the static variable declared at line #251. But line 
#266 introduces a local variable with the same name, which may be set at 
line #270. So you never initialize the static AFAICS.

---

Nit: none of these static fields/members need to be public.

----

202             if (isSupportedPlatform && !result.contains("PASSED")) {

Why does the test check for a supported platform here instead of at the 
start of main (or in your case in initIsSupportedPlatform()) ??

  247     public static boolean isNeverSupportedPlatform() {
  248         return !isAlwaysSupportedPlatform() && !Platform.isAArch64();
  249     }

I don't understand what platform this is trying to isolate ??

Thanks,
David
-----

On 23/03/2017 4:52 PM, Chris Plummer wrote:
> Ping!
>
> On 3/21/17 6:15 PM, Chris Plummer wrote:
>> Hello,
>>
>> Please review the following:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8177055
>> http://cr.openjdk.java.net/~cjplummer/8177055/webrev.00/webrev.hotspot
>>
>> The test was recently enabled for aarch64 platforms since support for
>> stack reserved pages was added to the open aarch64 port. However, the
>> Oracle port still does not have this feature, causing the test to
>> sometimes fail when run on it. Note the test is always run on all
>> platforms. However, when run on an unsupported platform, "failures" do
>> not result in the test actually failing (although crashes will). This
>> is intentional and is how the test is suppose to work.
>>
>> The fix is to change how the test determines if it is running on a
>> supported platform. It used to use a fixed list of supported
>> platforms. Now I have it spawn a subprocess with
>> "-XX:StackReservedPages=1" to see if it produces the warning message
>> you should see on unsupported platforms:
>>
>> WARNING: Reserved Stack Area not supported on this platform
>>
>> If it doesn't appear, then the platform is assumed to support the
>> feature. I still use the old list of supported platforms (minus
>> aarch64) for sanity checks. Basically all platforms in the old list
>> (except aarch64) should never show the above warning, and all
>> platforms not in the old list (except aarch64) should always show the
>> above warning.
>>
>> thanks,
>>
>> Chris
>
>