Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

[Mandy Chung]

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