Seemingly erroneous return value in HotSpot test

[David Holmes]

On 15/09/2023 12:02 pm, Julian Waters wrote:
> Just a heads up, there are even more instances of a method supposed to 
> return a jboolean returning JNI_ERR instead in other places as well, 
> which seems to be fairly concerning:

It is only error paths that should never really happen. But yes a bug. 
The same code returns JNI_OK for success instead of returning an actual 
bool e.g. `err == JNI_OK`

Cheers,
David

> === Output from failing command(s) repeated here ===
> * For target 
> support_test_hotspot_jtreg_native_support_libhs103t002_libhs103t002.o:
> libhs103t002.cpp
> d:\a\jdk\jdk\test\hotspot\jtreg\vmTestbase\nsk\jvmti\scenarios\hotswap\HS103\hs103t002\hs103t002.cpp(156): error C2220: the following warning is treated as an error
> d:\a\jdk\jdk\test\hotspot\jtreg\vmTestbase\nsk\jvmti\scenarios\hotswap\HS103\hs103t002\hs103t002.cpp(156): warning C4305: 'return': truncation from 'int' to 'jboolean'
> d:\a\jdk\jdk\test\hotspot\jtreg\vmTestbase\nsk\jvmti\scenarios\hotswap\HS103\hs103t002\hs103t002.cpp(159): warning C4305: 'return': truncation from 'int' to 'jboolean'
> d:\a\jdk\jdk\test\hotspot\jtreg\vmTestbase\nsk\jvmti\scenarios\hotswap\HS103\hs103t002\hs103t002.cpp(162): warning C4305: 'return': truncation from 'int' to 'jboolean'
>     ... (rest of output omitted)
> 
> best regards,
> Julian
> 
> On Fri, Sep 15, 2023 at 3:18 AM Chris Plummer <chris.plummer at oracle.com 
> <mailto:chris.plummer at oracle.com>> wrote:
> 
>     I think it should return false like is does with verify_tag(). This
>     will
>     cause the test to fail, which it what we would want in this case
> 
>     Chris
> 
>     On 9/13/23 11:20 PM, David Holmes wrote:
>      > Hi Julian,
>      >
>      > This is a serviceability issue - cc'd.
>      >
>      > On 14/09/2023 12:48 pm, Julian Waters wrote:
>      >> Hi all,
>      >>
>      >> In the HotSpot
>      >>
>     test /test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateThroughHeap/filter-tagged/HeapFilter.cpp,
>      >> the method occurance_expected, which returns a jboolean, returns
>      >> JNI_ERR in an error condition. The only place this is used is on
>     line
>      >> 383, at
>      >>
>     https://github.com/openjdk/jdk/blob/11d431b2c436d6b2a0aa7a00d676a93c1b87da0e/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateThroughHeap/filter-tagged/HeapFilter.cpp#L383 <https://github.com/openjdk/jdk/blob/11d431b2c436d6b2a0aa7a00d676a93c1b87da0e/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateThroughHeap/filter-tagged/HeapFilter.cpp#L383>
>      >>
>     <https://github.com/openjdk/jdk/blob/11d431b2c436d6b2a0aa7a00d676a93c1b87da0e/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateThroughHeap/filter-tagged/HeapFilter.cpp#L383 <https://github.com/openjdk/jdk/blob/11d431b2c436d6b2a0aa7a00d676a93c1b87da0e/test/hotspot/jtreg/vmTestbase/nsk/jvmti/IterateThroughHeap/filter-tagged/HeapFilter.cpp#L383>>.
>      >> There is no special handling for the JNI_ERR condition as far as I
>      >> can tell, and since jboolean is unsigned, the JNI_ERR value of -1
>      >> will end up becoming 255 when returning from this method, which is
>      >> then interpreted as JNI_TRUE on line 385. I caught this after
>      >> redefining jboolean to bool, which then caused the microsoft
>     compiler
>      >> to warn about truncating the value. This seems like a bug to me,
>      >> since I don't think it should return true on an error condition, is
>      >> there something I'm missing?
>      >
>      > That's a bug. Unclear whether they intended JNI_FALSE like
>     verify_tag,
>      > or whether this was really intended to be an error. If it is an
>     error
>      > then the test should abort somehow.
>      >
>      > Cheers,
>      > David
>      >
>      >> best regards,
>      >> Julian
>