RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

[John Rose]

On Jul 7, 2016, at 11:05 AM, Martin Buchholz <martinrb at google.com> wrote:

> private static final jdk.internal.misc.Unsafe java.util.concurrent.ConcurrentHashMap.U

Unless the security manager is turned on, you can do setAcc to pick up all sorts of private fields.

As Alan points out, it would be good to tighten this up, but that will require new logic which somehow avoids breaking existing legitimate uses.

(The new logic can't be some hack like "check for Unsafe".  Point-fixes like that just complicate the security model, creating dark corners for black hats to work in.)

The JVM and language are loose about mentioning inaccessible (or even non-existent) classes in field and method descriptors.
In particular, the JVM does not resolve or access-check names which occur as types of fields, parameters, or return values.
So accessing a field of type "Unsafe" (or even "NoSuchType") gets a pass, as long as the privacy of the field is somehow taken into account (via setAcc, here).

> jdk.internal.misc.Unsafe at 35851384

Here we get an object of a non-exported type.  That happens all the time; an anonymous inner class or lambda expression does this.

Again, the language and JVM are loose about letting objects of internal type "escape" via public APIs.  Arguably this is fundamental to OOP.

> class jdk.internal.misc.Unsafe

Here we inspect the class of the mysterious escaped object.  You can do this to AICs and lambdas too.

Our legacy reflection system says that Object.getClass returns the actual class of the object, without access check.
Again, that's loose, and difficult to tighten compatibly with existing applications.
In particular, adding an access check to getClass (which is the obvious move here) will perturb the performance model.
Currently, getClass requires one or two memory references.  An access check is likely to more than double that, with noticeable effects on dynamic languages.

Nothing here is impossible to fix, but everything is tricky, because of backward compatibility.

> Exception in thread "main" java.lang.IllegalAccessError: class GetUnsafe9
> (in unnamed module @0x649d209a) cannot access class
> jdk.internal.misc.Unsafe (in module java.base) because module java.base
> does not export jdk.internal.misc to unnamed module @0x649d209a
> at GetUnsafe9.main(GetUnsafe9.java:16)

Finally we get an error.  The specific error comes from an invisible cast which follows a successful call to the Class.cast method:

   //jdk.internal.misc.Unsafe U = unsafeClass.cast(theUnsafe);
   // ==>
   Object $tem = unsafeClass.cast(theUnsafe);  // descriptor is (Object)Object
   jdk.internal.misc.Unsafe U = (jdk.internal.misc.Unsafe) $tem; // javac inserts this cast

The "checkcast" instruction in the second statement fails because it must resolve the type Unsafe.

Even if there were other ways to get your hands on a well-typed Unsafe instance, as soon as you attempted to resolve an "invokevirtual" against it, you'd hit the same class resolution error as "checkcast" did.

The reflection API performs similar checks to the source compiler and JVM bytecode executor (verifier & link resolver).
However, since it works on "live" class pointers there is no resolution step to catch "bad" class references.
So Class.cast and Method.invoke either have to let the "bad" class operate (which Class.cast does) or perform an additional ad hoc access check (which Method.invoke does).
The additional access check is useful, but setAccessible nullifies it.
Paradoxically, Class.cast (which doesn't have the ad hoc check) is more restrictive than Method.invoke, since javac puts in the invisible cast (forcing resolution on Class.cast) while Method.invoke can be neutered by setAccessible.

Again, we are back to finding ways to rein in setAccessible.

HTH
— John