RFR: 8011397: JTREG needs to copy additional WhiteBox class file to JTwork/scratch/sun/hotspot
Andrey Zakharov
andrey.x.zakharov at oracle.com
Mon Jun 16 15:57:56 UTC 2014
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:
>>>>>>>>>> https://bugs.openjdk.java.net/secure/attachment/20275/8011397.tgz
>>>>>>>>>>
>>>>>>>>>> patch:
>>>>>>>>>> https://bugs.openjdk.java.net/secure/attachment/20274/8011397.WhiteBoxPer
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> mission
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks.
>>>>>>
>>>>>
>>>
>>
More information about the hotspot-dev
mailing list