Discussion:
RFR(XXS): 8214105: Invalid bit tests in jtreg
Simon Tooke
2018-11-20 14:34:46 UTC
Permalink
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.  


    (status & JVMTI_CLASS_STATUS_INITIALIZED) == 1

Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead.  I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/

Please let me know what you think.

Thanks,
-Simon
Daniel D. Daugherty
2018-11-20 14:47:43 UTC
Permalink
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
    (status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead.  I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c
    Looks like you changed the tests for both
JVMTI_CLASS_STATUS_INITIALIZED
    and JVMTI_CLASS_STATUS_ERROR since both have the same error. Looks
good.

Thumbs up.

Dan
Post by Simon Tooke
Please let me know what you think.
Thanks,
-Simon
Gary Adams
2018-11-20 15:21:15 UTC
Permalink
Looks OK to me.
Post by Simon Tooke
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
(status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead. I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c
Looks like you changed the tests for both
JVMTI_CLASS_STATUS_INITIALIZED
and JVMTI_CLASS_STATUS_ERROR since both have the same error. Looks
good.
Thumbs up.
Dan
Post by Simon Tooke
Please let me know what you think.
Thanks,
-Simon
s***@oracle.com
2018-11-20 18:36:04 UTC
Permalink
Hi Simon,

The fix looks good.
Thank you for taking care about it!

Questions:
  - Do you have an Author status?
  - You probably need a sponsor for this, do you?

Thanks,
Serguei
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
    (status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead.  I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
Please let me know what you think.
Thanks,
-Simon
David Holmes
2018-11-20 22:56:28 UTC
Permalink
+1 on the fix.

Simon is neither Committer nor Author so will need a sponsor.

Thanks,
David
Post by s***@oracle.com
Hi Simon,
The fix looks good.
Thank you for taking care about it!
  - Do you have an Author status?
  - You probably need a sponsor for this, do you?
Thanks,
Serguei
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
     (status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead.  I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
Please let me know what you think.
Thanks,
-Simon
JC Beyler
2018-11-20 23:04:44 UTC
Permalink
Also +1 for the fix,

If the submit repo is enough for testing, I can do the legwork to test it
and push it once it passes,
Jc

Ps: same for the other one he submitted
Post by David Holmes
+1 on the fix.
Simon is neither Committer nor Author so will need a sponsor.
Thanks,
David
Post by s***@oracle.com
Hi Simon,
The fix looks good.
Thank you for taking care about it!
- Do you have an Author status?
- You probably need a sponsor for this, do you?
Thanks,
Serguei
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid bit test
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
Post by s***@oracle.com
Post by Simon Tooke
(status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if JVMTI_CLASS_STATUS_INITIALIZED
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead. I chose this instead of
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
Post by s***@oracle.com
Post by Simon Tooke
Please let me know what you think.
Thanks,
-Simon
--
Thanks,
Jc
David Holmes
2018-11-20 23:20:09 UTC
Permalink
Post by JC Beyler
Also +1 for the fix,
If the submit repo is enough for testing, I can do the legwork to test
it and push it once it passes,
Not sure if submit-repo will do much JVM TI testing ... I think we need
tier 3 for JVM TI and pretty sure submit-repo is only tier 1.

But thanks for the offer.

David
Post by JC Beyler
Jc
Ps: same for the other one he submitted
+1 on the fix.
Simon is neither Committer nor Author so will need a sponsor.
Thanks,
David
Post by s***@oracle.com
Hi Simon,
The fix looks good.
Thank you for taking care about it!
    - Do you have an Author status?
    - You probably need a sponsor for this, do you?
Thanks,
Serguei
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid
bit test
Post by s***@oracle.com
Post by Simon Tooke
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
Post by s***@oracle.com
Post by Simon Tooke
     (status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if
JVMTI_CLASS_STATUS_INITIALIZED
Post by s***@oracle.com
Post by Simon Tooke
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead.  I chose this
instead of
Post by s***@oracle.com
Post by Simon Tooke
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
Post by s***@oracle.com
Post by Simon Tooke
Please let me know what you think.
Thanks,
-Simon
--
Thanks,
Jc
Severin Gehwolf
2018-11-22 09:03:22 UTC
Permalink
Post by David Holmes
Post by JC Beyler
Also +1 for the fix,
If the submit repo is enough for testing, I can do the legwork to test
it and push it once it passes,
Not sure if submit-repo will do much JVM TI testing ... I think we need
tier 3 for JVM TI and pretty sure submit-repo is only tier 1.
But thanks for the offer.
Thanks everyone. I'll sponsor this for Simon.

Thanks,
Severin
Post by David Holmes
David
Post by JC Beyler
Jc
Ps: same for the other one he submitted
+1 on the fix.
Simon is neither Committer nor Author so will need a sponsor.
Thanks,
David
Post by s***@oracle.com
Hi Simon,
The fix looks good.
Thank you for taking care about it!
- Do you have an Author status?
- You probably need a sponsor for this, do you?
Thanks,
Serguei
Post by Simon Tooke
While compiling the JDK with GCC 8.1, I discovered an invalid
bit test
Post by s***@oracle.com
Post by Simon Tooke
in
test/hotspot/jtreg/serviceability/jvmti/StartPhase/AllowedFunctions/libAllowedFunctions.c.
Post by s***@oracle.com
Post by Simon Tooke
(status & JVMTI_CLASS_STATUS_INITIALIZED) == 1
Which only has a chance of being true if
JVMTI_CLASS_STATUS_INITIALIZED
Post by s***@oracle.com
Post by Simon Tooke
has a value 1 (its actual value is 4, but that's beside the point).
My proposed fix is to test for != 0 instead. I chose this
instead of
Post by s***@oracle.com
Post by Simon Tooke
testing for equality to JVMTI_CLASS_STATUS_INITIALIZED purely for
cosmetic reasons.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214105
http://cr.openjdk.java.net/~sgehwolf/webrevs/stooke/JDK-8214105/01/webrev/
Post by s***@oracle.com
Post by Simon Tooke
Please let me know what you think.
Thanks,
-Simon
--
Thanks,
Jc
Loading...