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

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue May 13 17:12:37 UTC 2014


On 5/13/14 4:18 AM, Dmitry Samersoff wrote:
> Dan,
>
> Thank you for the review!
>
> 1. I use exit(7) to make the situation when the test fails because of
> some error condition clear visible.
>
> 2. I'm not sure it's a good idea to pass test if compilation fails, but
> I make this changes to handle it the same way as other tests does
>
> 3. Agree with rest of comments. See new webrev.
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8042155/webrev.02/


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

test/runtime/StackGuardPages/invoke.c
     I'm good with most of the changes.

     One thing I did notice is that some of the error messages are
     identical which would make it more difficult for someone
     investigating a future failure to know which failure happened:

     102     fprintf(stderr, "Test ERROR. Can't load class DoOverflow\n");
   144     printf("Test ERROR. Can't load class DoOverflow\n");
You can make them unique by including the function
         name that generated them

     I just noticed that some error messages still go to stdout,
     e.g., line 144 above.

     Looks like this comment from the first round was also missed
     (the line number is now 243):

 >     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);
 >     }


test/runtime/StackGuardPages/testme.sh
     No comments.

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
     No comments on the deleted files.


Dan

>
> -Dmitry
>
>
> On 2014-05-13 01:51, Daniel D. Daugherty wrote:
>> 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
>>>
>



More information about the hotspot-runtime-dev mailing list