On 4/1/13 12:28 PM, Alan Bateman wrote:
On 27/03/2013 17:35, Mandy Chung wrote:
This is the JDK change for JEP 176: JEP 176: Mechanical Checking of
Caller-Sensitive Methods [1]. Christian has posted the webrev for
the hotspot VM change a couple weeks ago [2].
Webrev at:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7198429/webrev.00/
While it touches many files, the fix is simple and straight-forward
for review.
I went through the latest webrev.01 and this looks very good.
I guess I was initially surprised to see @CS on some of the
AccessController.doPrivileged methods as these usages aren't checked
(although they are caller sensitive).
These few methods are the special case that their usage are not
checked. This raises a good point how we could enforce the check and
whether it's appropriate to check in JVM_DoPrivileged. I will file a
bug to follow up this separately if you are okay.
Did you consider adding a constant, JVM_DEPTH (-1) or some name like
that, to jvm.h for the depth parameter?
Chris and I both agree it's a good thing to define this constant in
jvm.h. I have made the change in the jdk side and will file a bug to
update that in the hotspot repo.
I see Reflection.isCallerSensitive uses isExtClassLoader, we'll
probably have to re-visit this in the future.
Yes.
Doug and Chris are on this list but might not have seen that this
updates AtomicXXXXFieldUpdaters. They need to be aware of this for
future merges.
On the tests, does GetCallerClassTest really need to check the stack
trace? It seems unnecessary.
The stack trace check tries to catch if InternalError is thrown for
other reason. Such regression might be rare but I prefer to perform an
appropriate check while the test can reliably run.
One thing on the shell test (I read exchange about jtreg boot class
path support) is that it needs the GPL header.
Fixed.
I was surprised to see CallerSensitiveFinder in the webrev and I'm
curious how long it takes to run.
This is one discussion point I'd like to have. I was debating myself
initially if this test should be enabled or not. What I found it takes
5-14 sec.
Some sample timing on the jprt machines:
linux_i586 jdk_lang took 14 mins and CallerSensitiveFinder took 8.5 sec
macosx_x64 jdk_lang took 20.5 mins and CallerSensitiveFinder took
14.5 sec
solaris_i586 jdk_lang took 15.5 mins and CallerSensitiveFinder took
10 sec
windows_x64 jdk_lang took 16.5 mins and CallerSensitiveFinder took
10 sec
We discussed tagging stress tests or long running tests so that they can
be run on demand. I think this test would also be appropriate to be run
in post-build hudson job, kind of tests.
Mandy
