[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-dev
Subject: Re: RFR: 8011397: JTREG needs to copy additional WhiteBox class file to JTwork/scratch/sun/hotspot
From: Andrey Zakharov <andrey.x.zakharov () oracle ! com>
Date: 2014-06-25 15:08:10
Message-ID: 53AAE5DA.2030700 () oracle ! com
[Download RAW message or body]
Hi, all
So in progress of previous email -
webrev:
http://cr.openjdk.java.net/~fzhinkin/azakharov/8011397/webrev.01/
Thanks.
On 16.06.2014 19:57, Andrey Zakharov wrote:
> Hi, all
> So issue is that when tests with WhiteBox API has been invoked with
> -Xverify:all it fails with Exception java.lang.NoClassDefFoundError:
> sun/hotspot/WhiteBox$WhiteBoxPermission
> Solutions that are observed:
> 1. Copy WhiteBoxPermission with WhiteBox. But
> > > Perhaps this is a good time to get rid of ClassFileInstaller
> altogether?
>
> 2. Using bootclasspath to hook pre-built whitebox (due @library
> /testlibrary/whitebox) . Some tests has @run main/othervm, some uses
> ProcessBuilder.
> - main/othervm/bootclasspath adds ${test.src} and ${test.classes}to
> options.
> - With ProcessBuilder we can just add ${test.classes}
> Question here is, can it broke some tests ? While testing this, I
> found only https://bugs.openjdk.java.net/browse/JDK-8046231, others
> looks fine.
>
> 3. Make ClassFileInstaller deal with inner classes like that:
> diff -r 6ed24aedeef0 -r c01651363ba8
> test/testlibrary/ClassFileInstaller.java
> --- a/test/testlibrary/ClassFileInstaller.java Thu Jun 05 19:02:56
> 2014 +0400
> +++ b/test/testlibrary/ClassFileInstaller.java Fri Jun 06 18:18:11
> 2014 +0400
> @@ -50,6 +50,16 @@
> }
> // Create the class file
> Files.copy(is, p, StandardCopyOption.REPLACE_EXISTING);
> +
> + for (Class<?> cls :
> Class.forName(arg).getDeclaredClasses()) {
> + //if (!Modifier.isStatic(cls.getModifiers())) {
> + String pathNameSub =
> cls.getCanonicalName().replace('.', '/').concat(".class");
> + Path pathSub = Paths.get(pathNameSub);
> + InputStream streamSub =
> cl.getResourceAsStream(pathNameSub);
> + Files.copy(streamSub, pathSub,
> StandardCopyOption.REPLACE_EXISTING);
> + //}
> + }
> +
> }
> }
> }
>
> Works fine for ordinary classes, but fails for WhiteBox due
> Class.forName initiate Class. WhiteBox has "static" section, and
> initialization fails as it cannot bind to native methods
> "registerNatives" and so on.
>
>
> So, lets return to first one option? Just add everywhere
> * @run main ClassFileInstaller sun.hotspot.WhiteBox
> + * @run main ClassFileInstaller sun.hotspot.WhiteBox$WhiteBoxPermission
>
> Thanks.
>
>
> On 10.06.2014 19:43, Igor Ignatyev wrote:
> > Andrey,
> >
> > I don't like this idea, since it completely changes the tests.
> > 'run/othervm/bootclasspath' adds all paths from CP to BCP, so the
> > tests whose main idea was testing WB methods themselves (sanity,
> > compiler/whitebox, ...) don't check that it's possible to use WB when
> > the application isn't in BCP.
> >
> > Igor
> >
> > On 06/09/2014 06:59 PM, Andrey Zakharov wrote:
> > > Hi, everybody
> > > I have tested my changes on major platforms and found one bug, filed:
> > > https://bugs.openjdk.java.net/browse/JDK-8046231
> > > Also, i did another try to make ClassFileInstaller to copy all inner
> > > classes within parent, but this fails for WhiteBox due its static
> > > "registerNatives" dependency.
> > >
> > > Please, review suggested changes:
> > > - replace ClassFileInstaller and run/othervm with
> > > "run/othervm/bootclasspath".
> > > bootclasspath parameter for othervm adds-Xbootclasspath/a:
> > > option with ${test.src} and ${test.classes}according to
> > > http://hg.openjdk.java.net/code-tools/jtreg/file/31003a1c46d9/src/share/classes/com/sun/javatest/regtest/MainAction.java. \
> > >
> > > Is this suitable for our needs - give to test compiled WhiteBox?
> > > - replace explicit -Xbootclasspath option values (".") in
> > > ProcessBuilder invocations to ${test.classes} where WhiteBox has been
> > > compiled.
> > >
> > > Webrev:
> > > http://cr.openjdk.java.net/~fzhinkin/azakharov/8011397/webrev.00/
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8011397
> > > Thanks.
> > >
> > >
> > > On 23.05.2014 15:40, Andrey Zakharov wrote:
> > > >
> > > > On 22.05.2014 12:47, Igor Ignatyev wrote:
> > > > > Andrey,
> > > > >
> > > > > 1. You changed dozen of tests, have you tested your changes?
> > > > Locally, aurora on the way.
> > > > >
> > > > > 2. Your changes of year in copyright is wrong. it has to be
> > > > > $first_year, [$last_year, ], see Mark's email[1] for details.
> > > > >
> > > > > [1]
> > > > > http://mail.openjdk.java.net/pipermail/jdk7-dev/2010-May/001321.html
> > > > Thanks, fixed. will be uploaded soon.
> > > >
> > > >
> > > > >
> > > > > Igor
> > > > >
> > > > > On 05/21/2014 07:37 PM, Andrey Zakharov wrote:
> > > > > >
> > > > > > On 13.05.2014 14:43, Andrey Zakharov wrote:
> > > > > > > Hi
> > > > > > > So here is trivial patch -
> > > > > > > removing ClassFileInstaller sun.hotspot.WhiteBox and adding
> > > > > > > main/othervm/bootclasspath
> > > > > > > where this needed
> > > > > > >
> > > > > > > Also, some tests are modified as
> > > > > > > - "-Xbootclasspath/a:.",
> > > > > > > + "-Xbootclasspath/a:" + System.getProperty("test.classes"),
> > > > > > >
> > > > > > > Thanks.
> > > > > > webrev: http://cr.openjdk.java.net/~jwilhelm/8011397/webrev.02/
> > > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8011397
> > > > > > Thanks.
> > > > > >
> > > > > > >
> > > > > > > On 09.05.2014 12:13, Mikael Gerdin wrote:
> > > > > > > > On Thursday 08 May 2014 19.28.13 Igor Ignatyev wrote:
> > > > > > > > > // cc'ing hotspot-dev instaed of compiler, runtime and gc lists.
> > > > > > > > >
> > > > > > > > > On 05/08/2014 07:09 PM, Filipp Zhinkin wrote:
> > > > > > > > > > Andrey,
> > > > > > > > > >
> > > > > > > > > > I've CC'ed compiler and runtime mailing list, because you're
> > > > > > > > > > changes
> > > > > > > > > > affect test for other components as too.
> > > > > > > > > >
> > > > > > > > > > I don't like your solution (but I'm not a reviewer, so treat my
> > > > > > > > > > words
> > > > > > > > > > just as suggestion),
> > > > > > > > > > because we'll have to write more meta information for each test
> > > > > > > > > > and it
> > > > > > > > > > is very easy to
> > > > > > > > > > forget to install WhiteBoxPermission if you don't test your test
> > > > > > > > > > with
> > > > > > > > > > some security manager.
> > > > > > > > > >
> > > > > > > > > > From my point of view, it will be better to extend
> > > > > > > > > > ClassFileInstaller
> > > > > > > > > >
> > > > > > > > > > so it will copy not only
> > > > > > > > > > a class whose name was passed as an arguments, but also all
> > > > > > > > > > inner
> > > > > > > > > > classes of that class.
> > > > > > > > > > And if someone want copy only specified class without inner
> > > > > > > > > > classes,
> > > > > > > > > > then some option
> > > > > > > > > > could be added to ClassFileInstaller to force such behaviour.
> > > > > > > > Perhaps this is a good time to get rid of ClassFileInstaller
> > > > > > > > altogether?
> > > > > > > >
> > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8009117
> > > > > > > >
> > > > > > > > The reason for its existence is that the WhiteBox class needs
> > > > > > > > to be
> > > > > > > > on the
> > > > > > > > boot class path.
> > > > > > > > If we can live with having all the test's classes on the boot
> > > > > > > > class
> > > > > > > > path then
> > > > > > > > we could use the /bootclasspath option in jtreg as stated in
> > > > > > > > the RFE.
> > > > > > > >
> > > > > > > > /Mikael
> > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > > Filipp.
> > > > > > > > > >
> > > > > > > > > > On 05/08/2014 04:47 PM, Andrey Zakharov wrote:
> > > > > > > > > > > Hi!
> > > > > > > > > > > Suggesting patch with fixes for
> > > > > > > > > > > https://bugs.openjdk.java.net/browse/JDK-8011397
> > > > > > > > > > >
> > > > > > > > > > > webrev:
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > >
> > > > > > > > > > > patch:
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > mission
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Thanks.
> > > > > > >
> > > > > >
> > > >
> > >
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic