RE: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
Hi Daniel
> -Original Message-
> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> Subject: Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit
> tests
>
> Hi Frank,
>
> I am intrigued by these change which do not seem
> to have anything to do with the rest. I mean, I'm not opposed
> as long as they are intended and that Joe validates them:
I corrected, cleaned up some code during I addressed the major task, there are
also some others besides you found, I even couldn't
recall all of them now. Sorry it confused you...
>
>
>
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.fram
> es.html
>
> 118 spf.setNamespaceAware(false);
>
>
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.fra
me
> s.html
>
>
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.h
t
> ml
>
> http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/publish.xml.frames.html
> This looks like a desirable change - but what does it have to do
> with enabling security manager?
>
They are unrelated to sm enabling, XMLReaderNSTableTest was not added with
@Test, so it was never run, after I added, I had to
correct the golden file to let it passed.
spf.setNamespaceAware(false); is redundant, I will replace with a comment line
to state NamespaceAware is false by default.
> Also:
>
>
> http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java.frames.html
>
>70
> tf.setFeature("http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions";,
> true);
>
> Is this needed only when there is a security manager?
>
>
>
> http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java.html
>
>
> 119 /*
> 120 addPermission(new RuntimePermission("getClassLoader"));
> 121 addPermission(new RuntimePermission("createClassLoader"));
> 122 addPermission(new RuntimePermission("createSecurityManager"));
> 123 addPermission(new RuntimePermission("modifyThread"));
> 124 addPermission(new PropertyPermission("*", "read,write"));
> 125
> 126 addPermission(new RuntimePermission("setIO"));
> 127 addPermission(new RuntimePermission("setContextClassLoader"));
> 128 addPermission(new
> RuntimePermission("accessDeclaredMembers"));*/
>
>
> Please remove before pushing.
>
>
Yes, I will. Forgot yesterday...
>
>
> test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest-in.xml
> test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest.xsl
>
> It seems these two files have been removed, but shouldn't they have
> been moved to test/javax/xml/jaxp/internaltest/javax/xml/transform
> instead?
> I see they are used by
> test/javax/xml/jaxp/internaltest/javax/xml/transform/CLITest.java
>
> Did you forget to hg add them?
>
The new destination directory which CLITest.java is moved to, contains same
files. So don't need to move them.
>
>
>
> Otherwise the new JAXPPolicyManager & its Policy implementation
> look good. This is much simpler and better than the first
> iteration :-)
>
> This was a *very* long patch - so congratulations for seeing
> this through!
>
Really thank you very much for having reviewed my changes and given me so much
valuable comments!
Frank
> cheers,
>
> -- daniel
>
>
> On 02/08/16 11:20, Frank Yuan wrote:
> > Hi Joe and Daniel
> >
> > I have finished the rework as your comments, please check
> > http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
> >
> > JAXP tests use Policy classes, as well as 3 other patterns provided by
> > JAXPTestUtilities:
> > 1. runWithAllPerm methods, are only used for user setup code, never run
> > jaxp code with them.
> > 2. tryRunWithPolicyManager method, is used to run some test methods with
> > security manager and run the others without security
> > manager in single test class
> > 3. runWithTmpPermission methods, which may run jaxp code with some limited
> > permissions, those permissions belong to user
permissions
> > and jaxp code won't doPrivileged for them. E.g.
> > PropertyPermission("MyInputFactory", "read") or
> > FilePermission("/tmp/this/does/not/exist/but/that/is/ok", "read")
> >
> >
> > Btw, some tests or test methods are disabled or commented sm support
> > temporarily for some known jaxp security bugs.
> >
> > Thanks
> > Frank
> >
Re: RFR(XS): 8162670: make of jtreg_tests fails if no tests are run, causing jprt test runs to also fail
Looks good.
Thanks,
David
On 3/08/2016 12:50 AM, Chris Plummer wrote:
On 8/1/16 9:30 PM, Chris Plummer wrote:
On 8/1/16 7:25 PM, David Holmes wrote:
On 2/08/2016 12:11 PM, Chris Plummer wrote:
On 8/1/16 5:58 PM, David Holmes wrote:
Hi Chris,
On 2/08/2016 8:46 AM, Chris Plummer wrote:
Hello,
Please review this simple change:
https://bugs.openjdk.java.net/browse/JDK-8162670
http://cr.openjdk.java.net/~cjplummer/8162670/webrev-00/
You've split a compound expression with your code:
227 jtregExitCode=$$? && \
228 if [ $${jtregExitCode} == 1 ]; then \
229 jtregExitCode=0; \
230 fi ; \
231 _summary="$(SUMMARY_TXT)"; \
I'm not clear exactly why the && was needed here but rather than find
out later I suggest rearranging the above to:
jtregExitCode=$$? && \
_summary="$(SUMMARY_TXT)"; \
if [ $${jtregExitCode} == 1 ]; then \
jtregExitCode=0; \
fi ; \
Yeah, that makes sense. I'll make the change. However, it's really
unclear what the use case for && is here. How can jtregExitCode=$$?
ever
fail?
I wonder if it evaluates to the $? value and so only sets _summary if
we had a zero exit code? (Not that I understand why we would only set
_summary in that context.)
I tried the following:
bash-4.1$ x=0 && echo "foo";
And "foo" is always printed. I also tried non-zero values for x. Here
are some other examples:
bash-4.1$ (exit 1) && echo "foo"
bash-4.1$ (exit 0) && echo "foo"
foo
Commands evaluate to true if the exit status is 0, and false
otherwise, so I guess you could say commands evaluate to !?$. For &&,
the rhs is only executed if the lhs has a zero exit status.
Updated webrev:
http://cr.openjdk.java.net/~cjplummer/8162670/webrev-01/
thanks,
Chris
Chris
David
thanks,
Chris
Thanks,
David
Note the copyright dates haven't been updated in this webrev, but
I did
update them locally after noticing that.
Tested with jprt test case given in the CR, and also with a jprt run
using "testset -hotspot" to make sure I didn't break anything.
thanks,
Chris
Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
other than that previously discussed,
in
test/javax/xml/jaxp/functional/javax/xml/parsers/ptests/DocumentBuilderFactoryTest.java
the comments related to the removed code can be removed as well
test/javax/xml/jaxp/functional/javax/xml/parsers/ptests/SAXParserTest.java
@Test(expectedExceptions = SAXException.class, dataProvider =
"parser-provider", enabled = false) //disabled for 8162848
-- just a reminder, as we discussed, the test needs to be enabled,
and 8162848 closed (also one in
test/javax/xml/jaxp/unittest/common/Sources.java)
what are runWithSecurityManager and runWithoutSecurityManager in some of
the tests? I thought the whole test suite will be run with and without
security manager, isn't that the plan?
Once again, great work and nice to clean up the old security-testing code.
Best,
Joe
On 8/2/16, 2:56 PM, Joe Wang wrote:
On 8/2/16, 5:30 AM, Daniel Fuchs wrote:
Hi Frank,
I am intrigued by these change which do not seem
to have anything to do with the rest. I mean, I'm not opposed
as long as they are intended and that Joe validates them:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.frames.html
118 spf.setNamespaceAware(false);
Not sure why this was changed.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.frames.html
The test itself wasn't very clear on the content it tests. If it only
verifies whether a resolver is used as is said, then this and related
changes in ResolverTest and publish.xml are fine. To the extent it
verifies, it didn't have to output to a file.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.html
We have to let Frank explain why namespace was turned off for these
tests :-) In this case, XMLReaderNSTableTest. In general, I would say
enabling security shouldn't change tests or gold files in this case.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/publish.xml.frames.html
This looks like a desirable change - but what does it have to do
with enabling security manager?
Probably to remove the reference to that particular server? Seems to
be fine as a cleanup. Again, it (the original test) only verifies a
resolve is used, it didn't even need to write out a file to prove that.
Also:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java.frames.html
70
tf.setFeature("http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions";,
true);
Is this needed only when there is a security manager?
Yes, when Security Manager is present, this feature is turned off by
default.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java.html
119 /*
120 addPermission(new RuntimePermission("getClassLoader"));
121 addPermission(new RuntimePermission("createClassLoader"));
122 addPermission(new
RuntimePermission("createSecurityManager"));
123 addPermission(new RuntimePermission("modifyThread"));
124 addPermission(new PropertyPermission("*", "read,write"));
125
126 addPermission(new RuntimePermission("setIO"));
127 addPermission(new
RuntimePermission("setContextClassLoader"));
128 addPermission(new
RuntimePermission("accessDeclaredMembers"));*/
Please remove before pushing.
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest-in.xml
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest.xsl
It seems these two files have been removed, but shouldn't they have
been moved to test/javax/xml/jaxp/internaltest/javax/xml/transform
instead?
I see they are used by
test/javax/xml/jaxp/internaltest/javax/xml/transform/CLITest.java
Did you forget to hg add them?
Otherwise the new JAXPPolicyManager & its Policy implementation
look good. This is much simpler and better than the first
iteration :-)
This was a *very* long patch - so congratulations for seeing
this through!
Very long patch, indeed! Thanks for the great effort! Very well done
with the unified Policy/Permission management.
Best,
Joe
cheers,
-- daniel
On 02/08/16 11:20, Frank Yuan wrote:
Hi Joe and Daniel
I have finished the rework as your comments, please check
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
JAXP tests use Policy classes, as well as 3 other patterns provided
by JAXPTestUtilities:
1. runWithAllPerm methods, a
Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java
> On Aug 1, 2016, at 2:47 PM, Kim Barrett wrote: > > This updated webrev addresses concerns about the performance of the VM > API used by the reference processor thread in the original webrev. > > New webrevs: > full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/ > http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03/ > incr: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03.incr/ > http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03.incr/ Sorry, typo in the incremental webrev URLs — “incr” => “inc”.
Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
On 8/2/16, 5:30 AM, Daniel Fuchs wrote:
Hi Frank,
I am intrigued by these change which do not seem
to have anything to do with the rest. I mean, I'm not opposed
as long as they are intended and that Joe validates them:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.frames.html
118 spf.setNamespaceAware(false);
Not sure why this was changed.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.frames.html
The test itself wasn't very clear on the content it tests. If it only
verifies whether a resolver is used as is said, then this and related
changes in ResolverTest and publish.xml are fine. To the extent it
verifies, it didn't have to output to a file.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.html
We have to let Frank explain why namespace was turned off for these
tests :-) In this case, XMLReaderNSTableTest. In general, I would say
enabling security shouldn't change tests or gold files in this case.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/publish.xml.frames.html
This looks like a desirable change - but what does it have to do
with enabling security manager?
Probably to remove the reference to that particular server? Seems to be
fine as a cleanup. Again, it (the original test) only verifies a resolve
is used, it didn't even need to write out a file to prove that.
Also:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java.frames.html
70
tf.setFeature("http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions";,
true);
Is this needed only when there is a security manager?
Yes, when Security Manager is present, this feature is turned off by
default.
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java.html
119 /*
120 addPermission(new RuntimePermission("getClassLoader"));
121 addPermission(new RuntimePermission("createClassLoader"));
122 addPermission(new
RuntimePermission("createSecurityManager"));
123 addPermission(new RuntimePermission("modifyThread"));
124 addPermission(new PropertyPermission("*", "read,write"));
125
126 addPermission(new RuntimePermission("setIO"));
127 addPermission(new
RuntimePermission("setContextClassLoader"));
128 addPermission(new
RuntimePermission("accessDeclaredMembers"));*/
Please remove before pushing.
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest-in.xml
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest.xsl
It seems these two files have been removed, but shouldn't they have
been moved to test/javax/xml/jaxp/internaltest/javax/xml/transform
instead?
I see they are used by
test/javax/xml/jaxp/internaltest/javax/xml/transform/CLITest.java
Did you forget to hg add them?
Otherwise the new JAXPPolicyManager & its Policy implementation
look good. This is much simpler and better than the first
iteration :-)
This was a *very* long patch - so congratulations for seeing
this through!
Very long patch, indeed! Thanks for the great effort! Very well done
with the unified Policy/Permission management.
Best,
Joe
cheers,
-- daniel
On 02/08/16 11:20, Frank Yuan wrote:
Hi Joe and Daniel
I have finished the rework as your comments, please check
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
JAXP tests use Policy classes, as well as 3 other patterns provided
by JAXPTestUtilities:
1. runWithAllPerm methods, are only used for user setup code, never
run jaxp code with them.
2. tryRunWithPolicyManager method, is used to run some test methods
with security manager and run the others without security
manager in single test class
3. runWithTmpPermission methods, which may run jaxp code with some
limited permissions, those permissions belong to user permissions
and jaxp code won't doPrivileged for them. E.g.
PropertyPermission("MyInputFactory", "read") or
FilePermission("/tmp/this/does/not/exist/but/that/is/ok", "read")
Btw, some tests or test methods are disabled or commented sm support
temporarily for some known jaxp security bugs.
Thanks
Frank
Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Hi Sean,
thanks a lot for your reply.
You're right - I'm still waiting for a review of the hotspot part. Any
volunteers :)
Regarding the noreg label: I used 'noreg-other' because there already
exist AES tests (they have never been part of this original change).
Is that the right label? There are simply too many different noreg
labels without documentation so if I selected the wrong one, please
advice which one to choose.
Thanks,
Volker
On Tue, Aug 2, 2016 at 9:58 AM, Seán Coffey wrote:
> Volker,
>
> Have the jdk8u hotspot edits been reviewed ? Looks like they should be.
>
> Can you add a suitable noreg label to the master bug report also.
>
> Regards,
> Sean.
>
>
> On 29/07/2016 19:58, Volker Simonis wrote:
>>
>> Ping!
>>
>> Can I please have an approval for backporting this change to 8u-dev?
>>
>> Thanks,
>> Volker
>>
>>
>> On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
>> wrote:
>>>
>>> Hi,
>>>
>>> could you please approve the backport of the following, mostly ppc64
>>> change to jdk8u-dev:
>>>
>>> 8152172: PPC64: Support AES intrinsics
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
>>> Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
>>> Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
>>> Review:
>>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
>>> URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f
>>>
>>> As you can see, the change consists of two parts - the main one in the
>>> hotpsot repo and a small part in the jdk repo.
>>>
>>> The jdk part applied cleanly to jdk8u (with the usual directory
>>> shuffling).
>>>
>>> The hotspot part required a small tweak, but only in the ppc-only
>>> part. This is because the feature detection for the AES instructions
>>> was already active in jdk9 when the original change was made but is
>>> not available in jdk8u until now. You can find the additional changes
>>> at the end of this mail for your convenience.
>>>
>>> The required shared changes cleanly apply to jdk8u.
>>>
>>> As Vladimir pointed out in a previous thread, shared Hotspot changes
>>> have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
>>> that and synchronously push both parts.
>>>
>>> @Hiroshii: could you please verify that you are fine with the change?
>>> I've done some tests on Power8LE but just want to be sure this
>>> downport satisfies your needs as well.
>>>
>>> Thank you and best regards,
>>> Volker
>>>
>>> =
>>>
>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
>>> --- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
>>> @@ -452,6 +476,7 @@
>>> a->popcntw(R7, R5); // code[7] -> popcntw
>>> a->fcfids(F3, F4); // code[8] -> fcfids
>>> a->vand(VR0, VR0, VR0); // code[9] -> vand
>>> + a->vcipher(VR0, VR1, VR2); // code[10] -> vcipher
>>> a->blr();
>>>
>>> // Emit function to set one cache line to zero. Emit function
>>> descriptor and get pointer to it.
>>> @@ -495,6 +520,7 @@
>>> if (code[feature_cntr++]) features |= popcntw_m;
>>> if (code[feature_cntr++]) features |= fcfids_m;
>>> if (code[feature_cntr++]) features |= vand_m;
>>> + if (code[feature_cntr++]) features |= vcipher_m;
>>>
>>> // Print the detection code.
>>> if (PrintAssembly) {
>>> diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
>>> --- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
>>> +++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
>>> @@ -42,6 +42,7 @@
>>> fcfids,
>>> vand,
>>> dcba,
>>> +vcipher,
>>> num_features // last entry to count features
>>> };
>>> enum Feature_Flag_Set {
>>> @@ -56,6 +57,7 @@
>>> fcfids_m = (1 << fcfids ),
>>> vand_m= (1 << vand ),
>>> dcba_m= (1 << dcba ),
>>> +vcipher_m = (1 << vcipher),
>>> all_features_m= -1
>>> };
>>> static int _features;
>>> @@ -83,6 +85,7 @@
>>> static bool has_fcfids() { return (_features & fcfids_m) != 0; }
>>> static bool has_vand(){ return (_features & vand_m) != 0; }
>>> static bool has_dcba(){ return (_features & dcba_m) != 0; }
>>> + static bool has_vcipher() { return (_features & vcipher_m) != 0; }
>>>
>>> static const char* cpu_features() { return _features_str; }
>
>
Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java
> On Aug 2, 2016, at 8:55 AM, Peter Levart wrote: > > Hi Kim, > > This looks very good. I like the way you dealt with race between the > ReferenceHandler thread and threads waiting for it to do some cleanup > progress. I think the VM API is suitable for possible further development on > JDK-8149925. It's also nice that the whole pending list is obtained in one > native call, so further optimizations on Java side are possible. > > Regards, Peter Thanks! > > On 08/01/2016 08:47 PM, Kim Barrett wrote: >> This updated webrev addresses concerns about the performance of the VM >> API used by the reference processor thread in the original webrev. >> >> New webrevs: >> full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/ >> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03/ >> incr: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03.incr/ >> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03.incr/ [snip]
Re: JDK 9 RFR of JDK-8162817: Annotation toString output not reusable for source input
Hi Peter,
On 8/2/2016 1:59 AM, Peter Levart wrote:
Hi Joe,
I wonder why you compare the type obtained from an value.getClass()
with class literals for primitive types (lines 174, 176, 178):
165 Class type = value.getClass();
166 if (!type.isArray()) {
167 // primitive value, string, class, enum const, or
annotation
168 if (type == Class.class)
169 return toSourceString((Class) value);
170 else if (type == String.class)
171 return toSourceString((String) value);
172 if (type == Character.class)
173 return toSourceString((char) value);
174 else if (type == double.class)
175 return toSourceString((double) value);
176 else if (type == float.class)
177 return toSourceString((float) value);
178 else if (type == long.class)
179 return toSourceString((long) value);
180 else
181 return value.toString();
182 } else {
They will never match!
Indeed! Just goes to show the value of making sure there are no coverage
gaps in one's regression tests ;-) In an updated version of the patch,
http://cr.openjdk.java.net/~darcy/8162817.2/
I replaced
174 else if (type == double.class)
175 return toSourceString((double) value);
176 else if (type == float.class)
177 return toSourceString((float) value);
178 else if (type == long.class)
179 return toSourceString((long) value);
180 else
181 return value.toString();
with
174 else if (type == Double.class)
175 return toSourceString((double) value);
176 else if (type == Float.class)
177 return toSourceString((float) value);
178 else if (type == Long.class)
179 return toSourceString((long) value);
180 else
181 return value.toString();
and added a test case to check the following annotation:
76 @MostlyPrimitive(
77 c0='a',
78 c1='\'',
79 i0=1,
80 i1=2,
81 f0=1.0f,
82 f1=Float.NaN,
83 d0=0.0,
84 d1=2.0/0.0,
85 l0=5,
86 l1=Long.MAX_VALUE,
87 s0="Hello world.",
88 s1="a\"b",
89 class0=Obj[].class
90 )
Many of the new toString forms of these members differ from the current
ones.
Also, sometimes you use "else if (...)" and sometimes just "if (...)".
They are both logically correct as you always "return" in the body of
the previous if statement, but it is not very consistent...
Otherwise looks good.
The existing code in this class (most of it dating back to 2003!), was
pretty consistent in its "if {return ...} - if {return ...}" usage.
However, when there are logical alternatives, I find it marginally
clearer to use a "if {return ...} else if {return ...}. ..." structure.
(If there was switching on Class objects, this class would be a great
candidate to use it.)
However, I didn't think it was justified to update the rest of the class
to use the if-else pattern.
Thanks for the review,
-Joe
Regards, Peter
On 08/01/2016 11:39 PM, joe darcy wrote:
This change should cover 99 44/100 % of the annotation values that
appear in practice; limited efforts were taken quoting characters in
strings, etc.
The basic approach is to introduce a family of overloaded
toSourceString methods to wrap/filter different kinds of values
coupled with methods to convert the various primitive arrays to
Stream for final processing by a shared method to surround an
array by "{" and "}" and add comma separators as needed.
Thanks,
-Joe
Re: RFR(XS): 8162670: make of jtreg_tests fails if no tests are run, causing jprt test runs to also fail
On 8/1/16 9:30 PM, Chris Plummer wrote:
On 8/1/16 7:25 PM, David Holmes wrote:
On 2/08/2016 12:11 PM, Chris Plummer wrote:
On 8/1/16 5:58 PM, David Holmes wrote:
Hi Chris,
On 2/08/2016 8:46 AM, Chris Plummer wrote:
Hello,
Please review this simple change:
https://bugs.openjdk.java.net/browse/JDK-8162670
http://cr.openjdk.java.net/~cjplummer/8162670/webrev-00/
You've split a compound expression with your code:
227 jtregExitCode=$$? && \
228 if [ $${jtregExitCode} == 1 ]; then \
229 jtregExitCode=0; \
230 fi ; \
231 _summary="$(SUMMARY_TXT)"; \
I'm not clear exactly why the && was needed here but rather than find
out later I suggest rearranging the above to:
jtregExitCode=$$? && \
_summary="$(SUMMARY_TXT)"; \
if [ $${jtregExitCode} == 1 ]; then \
jtregExitCode=0; \
fi ; \
Yeah, that makes sense. I'll make the change. However, it's really
unclear what the use case for && is here. How can jtregExitCode=$$?
ever
fail?
I wonder if it evaluates to the $? value and so only sets _summary if
we had a zero exit code? (Not that I understand why we would only set
_summary in that context.)
I tried the following:
bash-4.1$ x=0 && echo "foo";
And "foo" is always printed. I also tried non-zero values for x. Here
are some other examples:
bash-4.1$ (exit 1) && echo "foo"
bash-4.1$ (exit 0) && echo "foo"
foo
Commands evaluate to true if the exit status is 0, and false
otherwise, so I guess you could say commands evaluate to !?$. For &&,
the rhs is only executed if the lhs has a zero exit status.
Updated webrev:
http://cr.openjdk.java.net/~cjplummer/8162670/webrev-01/
thanks,
Chris
Chris
David
thanks,
Chris
Thanks,
David
Note the copyright dates haven't been updated in this webrev, but
I did
update them locally after noticing that.
Tested with jprt test case given in the CR, and also with a jprt run
using "testset -hotspot" to make sure I didn't break anything.
thanks,
Chris
Re: RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"
On 02/08/16 15:03, Roger Riggs wrote:
Hi Daniel,
Those were added in a previous diagnostic iteration.
They do a separate ps which might be too late to provide any using
information.
The first perl script retains and prints the suspect processes and
should be enough debugging info.
(There is no harm in retaining this extra info but I didn't think it was
particularly useful.)
Thanks Roger. No issue removing this code then.
+1
-- daniel
Thanks, Roger
On 8/2/2016 9:59 AM, Daniel Fuchs wrote:
Hi Roger,
Running in othervm looks good to me.
The only thing I wonder about is these lines which are
removed:
79 // Log remaining processes
80 ProcessBuilder pb = new ProcessBuilder("/bin/ps",
"-ef");
81 pb.inheritIO();
82 Process p2 = pb.start();
83 p2.waitFor();
Isn't that removing diagnostic information?
best regards,
-- daniel
On 02/08/16 14:45, Roger Riggs wrote:
Please review a test fix for an intermittently failing ProcessBuilder
test.
The test should be run in a separate vm so it only sees Zombies from
processes it creates.
When run in-process with jtreg, it may see Zombies from other processes
created by jtreg.
Additional diagnostic information is added as well in case of
recurrence.
The test is moved back to tier1.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-zombies-8160151/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8160151
Thanks, Roger
Re: RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"
Hi Daniel,
Those were added in a previous diagnostic iteration.
They do a separate ps which might be too late to provide any using
information.
The first perl script retains and prints the suspect processes and
should be enough debugging info.
(There is no harm in retaining this extra info but I didn't think it was
particularly useful.)
Thanks, Roger
On 8/2/2016 9:59 AM, Daniel Fuchs wrote:
Hi Roger,
Running in othervm looks good to me.
The only thing I wonder about is these lines which are
removed:
79 // Log remaining processes
80 ProcessBuilder pb = new ProcessBuilder("/bin/ps",
"-ef");
81 pb.inheritIO();
82 Process p2 = pb.start();
83 p2.waitFor();
Isn't that removing diagnostic information?
best regards,
-- daniel
On 02/08/16 14:45, Roger Riggs wrote:
Please review a test fix for an intermittently failing ProcessBuilder
test.
The test should be run in a separate vm so it only sees Zombies from
processes it creates.
When run in-process with jtreg, it may see Zombies from other processes
created by jtreg.
Additional diagnostic information is added as well in case of
recurrence.
The test is moved back to tier1.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-zombies-8160151/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8160151
Thanks, Roger
Re: RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"
Hi Roger,
Running in othervm looks good to me.
The only thing I wonder about is these lines which are
removed:
79 // Log remaining processes
80 ProcessBuilder pb = new ProcessBuilder("/bin/ps", "-ef");
81 pb.inheritIO();
82 Process p2 = pb.start();
83 p2.waitFor();
Isn't that removing diagnostic information?
best regards,
-- daniel
On 02/08/16 14:45, Roger Riggs wrote:
Please review a test fix for an intermittently failing ProcessBuilder test.
The test should be run in a separate vm so it only sees Zombies from
processes it creates.
When run in-process with jtreg, it may see Zombies from other processes
created by jtreg.
Additional diagnostic information is added as well in case of recurrence.
The test is moved back to tier1.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-zombies-8160151/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8160151
Thanks, Roger
RFR 9: [TESTBUG] 8160151: java/lang/ProcessBuilder/Zombies.java failed with error "1 zombies!"
Please review a test fix for an intermittently failing ProcessBuilder test. The test should be run in a separate vm so it only sees Zombies from processes it creates. When run in-process with jtreg, it may see Zombies from other processes created by jtreg. Additional diagnostic information is added as well in case of recurrence. The test is moved back to tier1. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-zombies-8160151/ Issue: https://bugs.openjdk.java.net/browse/JDK-8160151 Thanks, Roger
Re: RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java
Hi Kim, This looks very good. I like the way you dealt with race between the ReferenceHandler thread and threads waiting for it to do some cleanup progress. I think the VM API is suitable for possible further development on JDK-8149925. It's also nice that the whole pending list is obtained in one native call, so further optimizations on Java side are possible. Regards, Peter On 08/01/2016 08:47 PM, Kim Barrett wrote: This updated webrev addresses concerns about the performance of the VM API used by the reference processor thread in the original webrev. New webrevs: full: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03/ incr: http://cr.openjdk.java.net/~kbarrett/8156500/jdk.03.incr/ http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.03.incr/ The originally offered approach was an attempt to minimize changes. I was trying to be as similar to the existing code as possible, while moving part of it into the VM, where we have the necessary control over suspension and the like. We already know we want to make changes in this area, in order to eliminate the use of jdk.internal.ref.Cleaner (JDK-8149925). But we've also agreed that JDK-8149925 wasn't urgent and to defer it to JDK 10. I don't think we should revisit that. As Peter pointed out, my original change introduces a small performance regression on a microbenchmark for reference processing throughput. It also showed a small performance benefit on a different microbenchmark (allocation rate for a stress test of direct byte buffers), as noted in the original RFR. I can reproduce both of those. I think reference processing thread throughput is the right measure to use, so long as others don't become horrible. Focusing on that, it's better to just let the reference processing thread do the processing, rather than slowing it down to allow for the rare case when there's another thread that could possibly help. This is especially true now that Cleaner has such limited usage. This leads to a different API for other threads; rather than tryHandlePending that other threads can call to help and to examine progress, we now have waitForReferenceProcessing. The VM API is also different; rather than popReferencePendingList to get or wait, we now have getAndClearReferencePendingList and checkReferencePendingList, the latter with an optional wait. The splitting of the VM API allows us to avoid a narrow race condition discussed by Peter in his prototypes. Peter suggested this race was ok because java.nio.Bits makes several retries. However, those retries are only done before throwing OOME. There are no retries before invoking GC, so this race could lead to unnecessary successive GCs. Doing all the processing on the reference processing thread eliminates execution of Cleaners on application threads, though that's not nearly as much an issue now that the use of Cleaner is so restricted. We've also eliminated a pre-existing issue where a helper could report no progress even though the reference processing thread (and other helpers) had removed a pending reference, but not yet processed it. This could result in the non-progressing helper invoking GC or reporting failure, even though it might have succeeded if it had waited for the other thread(s) to complete processing the reference(s) being worked on. I think the new waitForReferenceProcessing could be used to fix JDK-6306116, though that isn't part of this change, and was not really a motivating factor. I don't know if the new waitForReferenceProcessing will be used by whatever we eventually do for JDK-8149925, but I think the new VM API will suffice for that. That is, I think JDK-8149925 might require changes to the core-libs API used by nio.Bits, and won't require further VM API changes.
Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
Hi Frank,
I am intrigued by these change which do not seem
to have anything to do with the rest. I mean, I'm not opposed
as long as they are intended and that Joe validates them:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.frames.html
118 spf.setNamespaceAware(false);
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.frames.html
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.html
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/publish.xml.frames.html
This looks like a desirable change - but what does it have to do
with enabling security manager?
Also:
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java.frames.html
70
tf.setFeature("http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions";,
true);
Is this needed only when there is a security manager?
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java.html
119 /*
120 addPermission(new RuntimePermission("getClassLoader"));
121 addPermission(new RuntimePermission("createClassLoader"));
122 addPermission(new RuntimePermission("createSecurityManager"));
123 addPermission(new RuntimePermission("modifyThread"));
124 addPermission(new PropertyPermission("*", "read,write"));
125
126 addPermission(new RuntimePermission("setIO"));
127 addPermission(new RuntimePermission("setContextClassLoader"));
128 addPermission(new
RuntimePermission("accessDeclaredMembers"));*/
Please remove before pushing.
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest-in.xml
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest.xsl
It seems these two files have been removed, but shouldn't they have
been moved to test/javax/xml/jaxp/internaltest/javax/xml/transform
instead?
I see they are used by
test/javax/xml/jaxp/internaltest/javax/xml/transform/CLITest.java
Did you forget to hg add them?
Otherwise the new JAXPPolicyManager & its Policy implementation
look good. This is much simpler and better than the first
iteration :-)
This was a *very* long patch - so congratulations for seeing
this through!
cheers,
-- daniel
On 02/08/16 11:20, Frank Yuan wrote:
Hi Joe and Daniel
I have finished the rework as your comments, please check
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
JAXP tests use Policy classes, as well as 3 other patterns provided by
JAXPTestUtilities:
1. runWithAllPerm methods, are only used for user setup code, never run jaxp
code with them.
2. tryRunWithPolicyManager method, is used to run some test methods with
security manager and run the others without security
manager in single test class
3. runWithTmpPermission methods, which may run jaxp code with some limited
permissions, those permissions belong to user permissions
and jaxp code won't doPrivileged for them. E.g. PropertyPermission("MyInputFactory",
"read") or
FilePermission("/tmp/this/does/not/exist/but/that/is/ok", "read")
Btw, some tests or test methods are disabled or commented sm support
temporarily for some known jaxp security bugs.
Thanks
Frank
Re: JDK 9 RFR of 8161970, 8157664: Remove 4 tools tests from ProblemList.txt
Amy, Looks good for me! -Dmitry On 2016-08-02 08:57, Amy Lu wrote: > tools/jlink/JLinkOptimTest.java > This test has been removed in JDK-8160829 > > sun/tools/jinfo/JInfoSanityTest.java > sun/tools/jinfo/JInfoRunningProcessFlagTest.java > sun/tools/jmap/heapconfig/JMapHeapConfigTest.java > These tests have been removed in JDK-8155091 > > Please review the patch to delete these tests from ProblemList.txt. > > bug: > https://bugs.openjdk.java.net/browse/JDK-8161970 > https://bugs.openjdk.java.net/browse/JDK-8157664 > > webrev: http://cr.openjdk.java.net/~amlu/8161970-8157664/webrev.00/ > > Thanks, > Amy > > --- old/test/ProblemList.txt 2016-08-02 13:44:34.0 +0800 > +++ new/test/ProblemList.txt 2016-08-02 13:44:34.0 +0800 > @@ -318,7 +318,7 @@ > > > > -# jdk_tools > +# core_tools > > tools/pack200/CommandLineTests.java > 7143279,8059906 generic-all > > @@ -368,20 +368,14 @@ > > sun/tools/jcmd/TestJcmdSanity.java 8031482 > windows-all > > -sun/tools/jmap/heapconfig/JMapHeapConfigTest.java > 8072131,8132452 generic-all > - > sun/tools/jstatd/TestJstatdExternalRegistry.java8046285 > generic-all > > sun/tools/jps/TestJpsJar.java 8160923 > generic-all > > sun/tools/jps/TestJpsJarRelative.java 6456333 > generic-all > > -sun/tools/jinfo/JInfoRunningProcessFlagTest.java6734748 > generic-all > - > sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java8057732 > generic-all > > -sun/tools/jinfo/JInfoSanityTest.java8059035 > generic-all > - > demo/jvmti/compiledMethodLoad/CompiledMethodLoadTest.java 8151899 > generic-all > > > @@ -391,9 +385,3 @@ > com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370 > linux-i586,macosx-all > > > - > -# core_tools > - > -tools/jlink/JLinkOptimTest.java 8159264 > generic-all > - > - > > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR 8159487: Add JAVA_VERSION, OS_NAME, OS_ARCH properties in release file
+1 > On Aug 2, 2016, at 8:39 AM, Sundararajan Athijegannathan > wrote: > > Please review http://cr.openjdk.java.net/~sundar/8159487/webrev.00/ for > https://bugs.openjdk.java.net/browse/JDK-8159487 > > OS_NAME, OS_ARCH, OS_VERSION properties are already added due to another > fix. Just adding "JAVA_VERSION" and a test change to check these > properties exist in release file. > > Thanks > > -Sundar >
Re: [9] RFR: 8162797: Code clean-up in IncludeLocalesPlugin
+1 -Sundar On 8/2/2016 2:15 AM, Naoto Sato wrote: > Hello, > > Please review this small change for the bug: > > https://bugs.openjdk.java.net/browse/JDK-8162797 > > The fix is located at: > > http://cr.openjdk.java.net/~naoto/8162797/webrev.00/ > > Naoto
Re: JDK 9 RFR of 8161970, 8157664: Remove 4 tools tests from ProblemList.txt
+1 for the jlink test being removed from the problem test. -Sundar On 8/2/2016 11:27 AM, Amy Lu wrote: > tools/jlink/JLinkOptimTest.java > This test has been removed in JDK-8160829 > > sun/tools/jinfo/JInfoSanityTest.java > sun/tools/jinfo/JInfoRunningProcessFlagTest.java > sun/tools/jmap/heapconfig/JMapHeapConfigTest.java > These tests have been removed in JDK-8155091 > > Please review the patch to delete these tests from ProblemList.txt. > > bug: > https://bugs.openjdk.java.net/browse/JDK-8161970 > https://bugs.openjdk.java.net/browse/JDK-8157664 > > webrev: http://cr.openjdk.java.net/~amlu/8161970-8157664/webrev.00/ > > Thanks, > Amy > > --- old/test/ProblemList.txt 2016-08-02 13:44:34.0 +0800 > +++ new/test/ProblemList.txt 2016-08-02 13:44:34.0 +0800 > @@ -318,7 +318,7 @@ > > > > -# jdk_tools > +# core_tools > > tools/pack200/CommandLineTests.java > 7143279,8059906 generic-all > > @@ -368,20 +368,14 @@ > > sun/tools/jcmd/TestJcmdSanity.java 8031482 > windows-all > > -sun/tools/jmap/heapconfig/JMapHeapConfigTest.java > 8072131,8132452 generic-all > - > sun/tools/jstatd/TestJstatdExternalRegistry.java8046285 > generic-all > > sun/tools/jps/TestJpsJar.java 8160923 > generic-all > > sun/tools/jps/TestJpsJarRelative.java 6456333 > generic-all > > -sun/tools/jinfo/JInfoRunningProcessFlagTest.java6734748 > generic-all > - > sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java8057732 > generic-all > > -sun/tools/jinfo/JInfoSanityTest.java8059035 > generic-all > - > demo/jvmti/compiledMethodLoad/CompiledMethodLoadTest.java 8151899 > generic-all > > > @@ -391,9 +385,3 @@ > com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370 > linux-i586,macosx-all > > > - > -# core_tools > - > -tools/jlink/JLinkOptimTest.java 8159264 > generic-all > - > - >
RFR 8159487: Add JAVA_VERSION, OS_NAME, OS_ARCH properties in release file
Please review http://cr.openjdk.java.net/~sundar/8159487/webrev.00/ for https://bugs.openjdk.java.net/browse/JDK-8159487 OS_NAME, OS_ARCH, OS_VERSION properties are already added due to another fix. Just adding "JAVA_VERSION" and a test change to check these properties exist in release file. Thanks -Sundar
RE: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
Hi Joe and Daniel
I have finished the rework as your comments, please check
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
JAXP tests use Policy classes, as well as 3 other patterns provided by
JAXPTestUtilities:
1. runWithAllPerm methods, are only used for user setup code, never run jaxp
code with them.
2. tryRunWithPolicyManager method, is used to run some test methods with
security manager and run the others without security
manager in single test class
3. runWithTmpPermission methods, which may run jaxp code with some limited
permissions, those permissions belong to user permissions
and jaxp code won't doPrivileged for them. E.g.
PropertyPermission("MyInputFactory", "read") or
FilePermission("/tmp/this/does/not/exist/but/that/is/ok", "read")
Btw, some tests or test methods are disabled or commented sm support
temporarily for some known jaxp security bugs.
Thanks
Frank
Re: JDK 9 RFR of JDK-8162817: Annotation toString output not reusable for source input
Hi Joe,
I wonder why you compare the type obtained from an value.getClass() with
class literals for primitive types (lines 174, 176, 178):
165 Class type = value.getClass();
166 if (!type.isArray()) {
167 // primitive value, string, class, enum const, or
annotation
168 if (type == Class.class)
169 return toSourceString((Class) value);
170 else if (type == String.class)
171 return toSourceString((String) value);
172 if (type == Character.class)
173 return toSourceString((char) value);
174 else if (type == double.class)
175 return toSourceString((double) value);
176 else if (type == float.class)
177 return toSourceString((float) value);
178 else if (type == long.class)
179 return toSourceString((long) value);
180 else
181 return value.toString();
182 } else {
They will never match!
Also, sometimes you use "else if (...)" and sometimes just "if (...)".
They are both logically correct as you always "return" in the body of
the previous if statement, but it is not very consistent...
Otherwise looks good.
Regards, Peter
On 08/01/2016 11:39 PM, joe darcy wrote:
This change should cover 99 44/100 % of the annotation values that
appear in practice; limited efforts were taken quoting characters in
strings, etc.
The basic approach is to introduce a family of overloaded
toSourceString methods to wrap/filter different kinds of values
coupled with methods to convert the various primitive arrays to
Stream for final processing by a shared method to surround an
array by "{" and "}" and add comma separators as needed.
Thanks,
-Joe
Re: [8u] request for approval: "8152172: PPC64: Support AES intrinsics"
Volker,
Have the jdk8u hotspot edits been reviewed ? Looks like they should be.
Can you add a suitable noreg label to the master bug report also.
Regards,
Sean.
On 29/07/2016 19:58, Volker Simonis wrote:
Ping!
Can I please have an approval for backporting this change to 8u-dev?
Thanks,
Volker
On Fri, Jul 22, 2016 at 11:08 AM, Volker Simonis
wrote:
Hi,
could you please approve the backport of the following, mostly ppc64
change to jdk8u-dev:
8152172: PPC64: Support AES intrinsics
Bug: https://bugs.openjdk.java.net/browse/JDK-8152172
Webrev: http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_hs/
Webrev http://cr.openjdk.java.net/~simonis/webrevs/2016/8152172_8u_jdk/
Review:
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2016-March/thread.html#22032
URL:http://hg.openjdk.java.net/jdk9/hs-comp/jdk/rev/74bc7be0777b
URL:http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/68394bf0a09f
As you can see, the change consists of two parts - the main one in the
hotpsot repo and a small part in the jdk repo.
The jdk part applied cleanly to jdk8u (with the usual directory shuffling).
The hotspot part required a small tweak, but only in the ppc-only
part. This is because the feature detection for the AES instructions
was already active in jdk9 when the original change was made but is
not available in jdk8u until now. You can find the additional changes
at the end of this mail for your convenience.
The required shared changes cleanly apply to jdk8u.
As Vladimir pointed out in a previous thread, shared Hotspot changes
have to go through JPRT even in jdk8u-dev. So I need a sponsor to do
that and synchronously push both parts.
@Hiroshii: could you please verify that you are fine with the change?
I've done some tests on Power8LE but just want to be sure this
downport satisfies your needs as well.
Thank you and best regards,
Volker
=
diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.cpp
--- a/src/cpu/ppc/vm/vm_version_ppc.cpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.cpp Fri Jul 22 10:32:36 2016 +0200
@@ -452,6 +476,7 @@
a->popcntw(R7, R5); // code[7] -> popcntw
a->fcfids(F3, F4); // code[8] -> fcfids
a->vand(VR0, VR0, VR0); // code[9] -> vand
+ a->vcipher(VR0, VR1, VR2); // code[10] -> vcipher
a->blr();
// Emit function to set one cache line to zero. Emit function
descriptor and get pointer to it.
@@ -495,6 +520,7 @@
if (code[feature_cntr++]) features |= popcntw_m;
if (code[feature_cntr++]) features |= fcfids_m;
if (code[feature_cntr++]) features |= vand_m;
+ if (code[feature_cntr++]) features |= vcipher_m;
// Print the detection code.
if (PrintAssembly) {
diff -r 15928d255046 src/cpu/ppc/vm/vm_version_ppc.hpp
--- a/src/cpu/ppc/vm/vm_version_ppc.hpp Wed Jul 13 00:47:40 2016 -0700
+++ b/src/cpu/ppc/vm/vm_version_ppc.hpp Fri Jul 22 10:32:36 2016 +0200
@@ -42,6 +42,7 @@
fcfids,
vand,
dcba,
+vcipher,
num_features // last entry to count features
};
enum Feature_Flag_Set {
@@ -56,6 +57,7 @@
fcfids_m = (1 << fcfids ),
vand_m= (1 << vand ),
dcba_m= (1 << dcba ),
+vcipher_m = (1 << vcipher),
all_features_m= -1
};
static int _features;
@@ -83,6 +85,7 @@
static bool has_fcfids() { return (_features & fcfids_m) != 0; }
static bool has_vand(){ return (_features & vand_m) != 0; }
static bool has_dcba(){ return (_features & dcba_m) != 0; }
+ static bool has_vcipher() { return (_features & vcipher_m) != 0; }
static const char* cpu_features() { return _features_str; }
