RFR 8187742 Minimal set of bootstrap methods for dynamic constants

John Rose john.r.rose at oracle.com
Tue Nov 7 20:59:41 UTC 2017

Good comments!  I can handle a couple of them…

On Nov 7, 2017, at 11:33 AM, Remi Forax <forax at univ-mlv.fr> wrote:
> I fail to see the point of using validateClassAccess(), if the BSM is used through an indy, the type is created only if the caller class can create it, so the test seems redundant.

That's true, but… 

> If it's not used by an indy, why do we need to test that ? Also, why it's not called in invoke ?

…Enum.valueOf doesn't do a security check; that is its choice.
This means that if you pass it an enum type that is not public
or not in a package exported to you, you can still peek at its
enum values.  Meanwhile, when javac emits a reference to
an enum, it does so with getstatic.  The getstatic bytecode
*does* perform access checks.  The call to validateClassAccess
performs those checks, for alignment with the semantics
of getstatic.  The internal use of Enum.valueOf is just a detail
of the emulation of getstatic in the case of an enum.

(Note to self:  Never use enums to implement a shared
secrets pattern.)

For bootstrap methods I prefer to use the most restrictive
set of applicable access rules, handshaking with the lookup.

In the case of enums it doesn't matter much, as you say,
because Enum.valueOf leaves the door open.

I could go either way on this one.

> There is also no reason to validate that the parameter type is the right one because the VM will validate the return type of the BSM is asTypable to the type sent as argument.

For condy, having the BSM validate types is a code smell
for the reason you mention.  Also, when primitives (and value
types) are in the mix, people usually code the validation
incorrectly, since Class.isInstance is the wrong tool, and
the right tool is non-obvious (MHs.identity.asType).

Are you commenting on the return type adjustment in invoke?
That's just a minor optimization to avoid (un)boxing, which
should have no semantic effect.

> Some methods use generics, so other don't. Given it's not useful to use generics if the methods is used as a BSM, i think the generics signature should be removed, otherwise, getstatic and invoke should use generics.

The generics are present to make normal calls from source
code type check.  Use cases:  Combinators, test code.

> to remove the @SuppressWarnings, type should be declared as a Class<Class<?>>.

Paul explained this one to me when I made the same objection.  Turns
out that the class literal "Class.class" (which is required to call that
BSM from source code) can only have a raw type.  Yuck.  So we
declare the formal with a matching amount of raw-ness.

> Why not using getstatic to retrieve the field Type of the wrapper instead ?

Because the descriptor is a more compact notation for a primitive type.
Primitive type references will be common and we want them to be small.

> If you have invoke(), you do not need enumConstant because you can cook a constant method handle Enum.valueOf and send it to invoke. The methods that returns VarHandles are trickier because you need a Lookup object. 

The set is minimal, but not that minimal.  We *could* express *everything*
using invoke but that would be grotesquely verbose and opaque.
The "extra" API points are thought to be helpful in reducing class file size,
and also clarifying the intentions of the affected constants.

> getstatic should be renamed to getConstant because this is what it does.

(I could go either way; since the JVM has ConstantValue attrs, "Constant"
is a Thing.  If it's getstatic I prefer getStatic, for alignment with pre-existing
names in jli.)

> wrapping the Throwable in a LinkageError seems wrong to me given that a calls to a BSM already does that, so getstatic can just declare "throws Throawble" like invoke and you have the same semantics.

The point is that we are emulating the semantics of a getstatic instruction,
which only throws LEs.  We don't want other stuff to leak out of the
implementation.  Remember that "bytecode behavior" is a normative
specification, to use when applicable.  It's applicable here.

Again, I could go either way on this one.

— John

More information about the core-libs-dev mailing list