Review request 7157734 testcase corrections (use TESTVMOPTS).

[Andrew Hughes]

----- Original Message -----
> On 24/08/12 14:30, Andrew Hughes wrote:
> > ----- Original Message -----
> >> Hi,
> >>
> >> Various hotpsot .sh testcase scripts do not use the env var
> >> TESTVMOPTS,
> >> which is passed by jtreg.  They therefore don't set -d64 when we
> >> want
> >> to
> >> test a 64-bit JVM and on Solaris at least this means we don't test
> >> what
> >> we think we're testing.  We actually run a 32-bit JVM, and likely
> >> not
> >> even the one we just built to test.
> >>
> >> What some testcases do do, is to read $HOME/JDK64BIT if it exists,
> >> and
> >> use the file contents as command-line arguments, or even just as a
> >> flag
> >> to know that we should set -d64 when we run java.  It seems best
> >> to
> >> get
> >> rid of this practice.  I have asked around and found nobody who
> >> can
> >> say
> >> that technique is still in use.
> >>
> >> http://cr.openjdk.java.net/~kevinw/7157734.1/webrev/
> >>
> >> Thanks
> >> Kevin
> >>
> > I've come across your changes to Test6929067.sh as they conflicted
> > with
> > our changes, which we posted here:
> >
> > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2011-May/002163.html
> >
> > but which were never accepted.
> >
> > Have you tested this on anything but a x86 GNU/Linux box?
> >
> > This test:
> >
> > ${TESTJAVA}/bin/java ${TESTVMOPTS} -version 2>1 | grep
> > "64-Bit">/dev/null
> > if [ "$?" = "0" ]
> > then
> >    ARCH=amd64
> > else
> >    ARCH=i386
> > fi
> >
> > is flawed.  Something that returns "64-Bit" could also be SPARC64
> > on GNU/Linux
> > or (via Zero) PPC64, etc.
> >
> > I also don't see how:
> >
> > gcc -o invoke -I${TESTJAVA}/include -I${TESTJAVA}/include/linux
> > invoke.c ${TESTJAVA}/jre/lib/${ARCH}/client/libjvm.so
> >
> > will work on x86_64 as there is no client VM.
> >
> > Our fix wasn't perfect either, but, from our perspective, it's
> > better than this.  Can
> > we perhaps come up with something between the two that works for
> > everyone?
> >
> > Also, if this is only building on GNU/Linux, you can drop:
> >
> > /usr/openwin/lib:/usr/dt/lib
> >
> > from LD_LIBRARY_PATH.
> 
> Hi Andrew,
> 
> Yes, I'm sure we can get the best of all these changes in there...  I
> was mainly just trying to banish the use of BIT_FLAG and use
> TESTVMOPTS
> where it had been ignored before.  Actually yes there are a few more
> issues that need fixing!...
> 

Wow, thanks for doing this so quickly!

> 
> If we do "${TESTJAVA}/bin/java -d64" and check the return code, we're
> assuming that 64-bit is not possible when jtreg is testing 32-bit.
>  So
> that's similar to the old way this would only run the 32-bit JVM,
> ignoring the TESTVMOPTS from jtreg saying it wanted 64.  So I think
> we
> need to run ${TESTJAVA}/bin/java ${TESTVMOPTS} -version and see what
> bitness it reports.
> 

Yes, this is why I was thinking ours isn't perfect either... :-)

> I'm not sure we can include both client and server on the compile
> line.
> Both might be present, but TESTVMOPTS might or might not specify
> -server.  Or we might get server by default.  Oh, need again to parse
> -version output.  (That's probably why it was hardcoded to client
> originally, for simplicity, but that's out of date if we aren't
> always
> getting client by default.)
> 

That seems a good fix.  So we get now whatever matches the output
of java -version in all cases.

> uname -m might return x86_64, although we might be testing 32-bit
> i386.
> Another wrinkle for the ARCH switch...
> 

Ah, true.  I don't know about the other archs, I guess the same could
happen with SPARC and PPC?  So we probably need something similar for
those, and probably aarch64 at some point.

For example, uname -m on my PowerMac returns ppc64 but the userland
is 32-bit.

All this gets messed up if the third line doesn't come from HotSpot,
but it is a HotSpot test ;-)

> I put a combined suggestion here:
> http://cr.openjdk.java.net/~kevinw/0001/webrev/
> 
> Do you think that captures everything?  I think you're saying that
> the
> arm and ppc architectures fall through the switch and get used as
> they
> are in the the later paths.
> 

Yes, they would in our patch (while in yours, there were set to i386).
I guess if we need to handle 32-bit on 64-bit, we need some switches
for those too.

I've added a few extra cases here:

http://cr.openjdk.java.net/~andrew/6929067/webrev.01/

and also fixed a typo (585/586 :-)

Pavel also spotted that you may need to add /usr/lib64 to LD_LIBRARY_PATH
as well as /usr/lib

> Let me know what you think and I'll create a bug...
> 
> Thanks!
> Kevin
> 
> 
> 
> 
> 
> 
> 
> 

Thanks!
-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07