RFR(XXS): 8214105: Invalid bit tests in jtreg

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR(XXS): 8214105: Invalid bit tests in jtreg

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


Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

Daniel D. Daugherty
On 11/20/18 9:34 AM, Simon Tooke wrote:

> 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


>
> Please let me know what you think.
>
> Thanks,
> -Simon
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

gary.adams@oracle.com
Looks OK to me.

On 11/20/18, 9:47 AM, Daniel D. Daugherty wrote:

> On 11/20/18 9:34 AM, Simon Tooke wrote:
>> 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
>
>
>>
>> Please let me know what you think.
>>
>> Thanks,
>> -Simon
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

serguei.spitsyn@oracle.com
In reply to this post by Simon Tooke
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


On 11/20/18 06:34, Simon Tooke wrote:

> 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
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

David Holmes
+1 on the fix.

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

Thanks,
David

On 21/11/2018 4:36 am, [hidden email] wrote:

> 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
>
>
> On 11/20/18 06:34, Simon Tooke wrote:
>> 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
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

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,
Jc

Ps: same for the other one he submitted

On Tue, Nov 20, 2018 at 2:58 PM David Holmes <[hidden email]> wrote:
+1 on the fix.

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

Thanks,
David

On 21/11/2018 4:36 am, [hidden email] wrote:
> 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
>
>
> On 11/20/18 06:34, Simon Tooke wrote:
>> 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
>>
>>
>


--

Thanks,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

David Holmes
On 21/11/2018 9:04 am, JC Beyler wrote:
> 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

> Jc
>
> Ps: same for the other one he submitted
>
> On Tue, Nov 20, 2018 at 2:58 PM David Holmes <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     +1 on the fix.
>
>     Simon is neither Committer nor Author so will need a sponsor.
>
>     Thanks,
>     David
>
>     On 21/11/2018 4:36 am, [hidden email]
>     <mailto:[hidden email]> wrote:
>      > 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
>      >
>      >
>      > On 11/20/18 06:34, Simon Tooke wrote:
>      >> 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
>      >>
>      >>
>      >
>
>
>
> --
>
> Thanks,
> Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR(XXS): 8214105: Invalid bit tests in jtreg

Severin Gehwolf
On Wed, 2018-11-21 at 09:20 +1000, David Holmes wrote:

> On 21/11/2018 9:04 am, JC Beyler wrote:
> > 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

> David
>
> > Jc
> >
> > Ps: same for the other one he submitted
> >
> > On Tue, Nov 20, 2018 at 2:58 PM David Holmes <[hidden email]
> > <mailto:[hidden email]>> wrote:
> >
> >     +1 on the fix.
> >
> >     Simon is neither Committer nor Author so will need a sponsor.
> >
> >     Thanks,
> >     David
> >
> >     On 21/11/2018 4:36 am, [hidden email]
> >     <mailto:[hidden email]> wrote:
> >      > 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
> >      >
> >      >
> >      > On 11/20/18 06:34, Simon Tooke wrote:
> >      >> 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
> >      >>
> >      >>
> >      >
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc