RFR(S): JDK-8042155 Tests for stack guard pages have to be cleaned up

[Daniel D. Daugherty]

On 5/1/14 8:21 AM, Dmitry Samersoff wrote:
> Hi Everyone,
>
> Please, review test changes:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8042155/webrev.01/


test/runtime/StackGuardPages/testme.sh
     You need to check the command status after running gcc_cmd
     and if it is not zero, then gratuitously pass the test.
     Jerry made a similar test fix last week:

+if [ $? -ne 0 ] ; then
+    echo "Compile failed, Ignoring failed compilation and forcing the 
test to pass"
+    exit 0
+fi

     This will catch the case where a 64-bit Linux test machine
     has a compiler, but not the 32-bit build env.

test/runtime/StackGuardPages/DoOverflow.java
     No comments.

test/runtime/StackGuardPages/invoke.c
     line 28:  * than detaching from vm thread and overflow stack once 
again.
         Typo?: 'than detaching' -> 'Then we detach'

     line 92: (*_jvm)->AttachCurrentThread(_jvm, (void**)&env, NULL);
     line 125: (*_jvm)->AttachCurrentThread(_jvm, (void **)&env, NULL);
         What happens if the JNI AttachCurrentThread() call fails?

     line 96: printf("Test ERROR. Can't load class DoOverflow\n");
     line 102: printf("Test ERROR. Can't find method DoOverflow.printIt\n");
     line 129: printf("Test ERROR. Can't load class DoOverflow\n");
     line 135: printf("Test ERROR. Can't find method 
DoOverflow.printAlive\n");
     line 192: printf("Test ERROR. Can't create JavaVM\n");
     line 224: printf("Test ERROR. Unknown parameter should be 
test_java_overflow or test_native_overflow\n");
         Don't errors usually get printed on stderr?

     line 106: (*env)->CallStaticVoidMethod(env, class_id, method_id, NULL);
         What happens if this call fails? Other than the intentional
         StackOverflowError, but wait, you catch and swallow that error...

     line 108: (*_jvm)->DetachCurrentThread(_jvm);
     line 150   (*_jvm)->DetachCurrentThread(_jvm);
         What happens if this call fails?

     line 139: (*env)->CallStaticVoidMethod (env, class_id, method_id, 
NULL);
         What happens if this call fails?

     line 153: // For non-initial thread we doesn't unmap
         Typo: "we doesn't unmap"
            -> "we don't unmap"

     line 154: ...same SEGV_ACCERR tring to access it
         Typos, perhaps: same SEGV signal trying to access it

     line 156: // We have no ways to check this, so bail out, marking 
test as succeded
         Typo: 'no ways' -> 'no way'
         Typo: 'as succeded' -> 'as succeeded'

     line 157: printf("Test OK. ...
     line 171: printf("Test OK. ...
         Should "OK" be "PASSED"?

     line 168: printf("Test FAILED. Stack guard page is still there\n");
         Don't failures usually get printed on stderr?

     line 225: You'll end up here if no arguments are passed or if the
         wrong arguments are passed. You should have a better usage
         message, e.g.:

     void usage() {
         fprintf(stderr, "Usage: invoke test_java_overflow\n");
         fprintf(stderr, "       invoke test_native_overflow\n");
         exit(1);
     }

     I noticed that all the exit() calls pass '7'. Why '7'?

     Also, why 'return 0' or 'return 1' instead of 'exit(0)' or 'exit(1)'?


test/runtime/6929067/T.java
test/runtime/6929067/Test6929067.sh
test/runtime/6929067/invoke.c
test/runtime/InitialThreadOverflow/DoOverflow.java
test/runtime/InitialThreadOverflow/invoke.c
test/runtime/InitialThreadOverflow/testme.sh
     OK I can see why you want to combine the two tests.
     I'm guessing that 'InitialThreadOverflow' was
     derived from the much older '6929067'...


Dan

>
> The fix combine tests for 6929067 and 8009062 into single, more
> sophisticated test.
>
> -Dmitry
>