On 6/24/14, 4:41 AM, Frederic Parain wrote:
Hi Coleen,

It seems that there's still a reference to JVM_GetClassLoader
in file jdk/src/share/native/common/check_code.c. The code looks
like dead code, but it would be nice to clean it up.

I removed this code. There are no other instances of the macro BROKEN_JAVAC. I update the copyrights when I commit the changeset.

http://cr.openjdk.java.net/~coleenp/6642881_jdk_5/

Thanks!
Coleen

Thanks,

Fred

On 06/24/2014 01:45 AM, Coleen Phillimore wrote:

Please review a change to the JDK code for adding classLoader field to
the instances of java/lang/Class.  This change restricts reflection from
changing access to the classLoader field.  In the spec,
AccessibleObject.setAccessible() may throw SecurityException if the
accessibility of an object may not be changed:

http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean)


Only AccessibleObject.java has changes from the previous version of this
change.

open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
bug link https://bugs.openjdk.java.net/browse/JDK-6642881

Thanks,
Coleen

On 6/19/14, 11:01 PM, David Holmes wrote:
On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:
Hi Mandy,

On 19 jun 2014, at 22:34, Mandy Chung <mandy.ch...@oracle.com> wrote:

On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:
Hi,

On 19 jun 2014, at 20:46, Coleen Phillimore
<coleen.phillim...@oracle.com> wrote:
On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
Have you considered hiding the Class.classLoader field from
reflection? I’m not sure it is necessary but I’m not to keen on
the idea of people poking at this field with Unsafe (which should
go away in 9 but …).
I don't know how to hide the field from reflection. It's a
private final field so you need to get priviledges to make it
setAccessible.  If you mean injecting it on the JVM side, the
reason for this change is so that the JIT compilers can inline
this call and not have to call into the JVM to get the class loader.

There is sun.reflect.Reflection.registerFieldsToFilter() that hides
a field from being found using reflection. It might very well be
the case that private and final is enough, I haven’t thought this
through 100%. On the other hand, is there a reason to give users
access through the field instead of having to use
Class.getClassLoader()?

There are many getter methods that returns a private final field.
I'm not sure if hiding the field is necessary nor a good precedence
to set. Accessing a private field requires "accessDeclaredMember"
permission although it's a different security check (vs
"getClassLoader"
permission).  Arguably before this new classLoader field, one could
call Class.getClassLoader0() via reflection to get a hold of class
loader.

Perhaps you are concerned that the "accessDeclaredMember" permission
is too coarse-grained?  I think the security team is looking into
the improvement in that regards.

I think I’m a bit worried about two things, first as you wrote,
“accessDeclaredMember” isn’t the same as “getClassLoader”, but since
you could get the class loader through getClassLoader0() that
shouldn’t be a new issue.

The second thing is that IIRC there are ways to set a final field
after initialization. I’m not sure we need to care about that either
if you need Unsafe to do it.

Normal reflection can set a final field if you can successfully call
setAccessible(true) on the Field object.

David
-----


cheers
/Joel




Reply via email to