[10] RFC: Relaxing access restrictions for VM annotations

[Vladimir Ivanov]

Thanks for the feedback, John.

The problem here is not related to bootstrapping (the checks for 
privileged classes can be just skipped), but because we need to make 
checks in the context of the class being loaded (in particular, 
determine the module the class belongs to).

I did a quick prototype of accessibility checks during parsing and ended 
up with the following:
   http://cr.openjdk.java.net/~vlivanov/vm_annotations/webrev.01/

Diff between versions:
   http://cr.openjdk.java.net/~vlivanov/vm_annotations/webrev.01_vs_00/

It required to duplicate code from IK::set_package(), IK::module(), and 
Reflection::verify_class_access(). Also, I had to change the loading 
sequence and compute (possibly, create) package early in the parsing phase.

Overall, it looked much messier and more brittle than wrapping class 
file paring into re-parsing logic.

Best regards,
Vladimir Ivanov

On 2/22/17 8:49 AM, John Rose wrote:
> Gating annotation effectiveness on the module system is brilliant.
>
> But I recommend not making too many compromises for the sake of java.base.
> Bootstrapping that particular code will always be hard and messy.
> (Yes, "always".  No magic bullets on any horizon, at least until we
> have full AOT with no backoff to bootstrapping from bytecode.
> That's pretty much "always".)   So to cut bootstrap loops you need
> a temporary first opinion about whatever it is that is getting bootstrapped,
> until you can get the permanent good facts from the system after it
> is bootstrapped.  (Getting good facts from the module system requires
> significant parts of java.base to be booted up.)
>
> Thus, expect to have some temporary oracle that can give answers
> on behalf of the module system, for exactly those annotations which
> are used in java.base.  You can record the answers of the oracle
> and assertion-check them much later, when the module system can
> be queried.  I don't think delaying from phase to phase within one
> class loading operation buys you anything but trouble.
>
> Does that help?
>
> — John
>
> On Feb 21, 2017, at 6:36 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>>
>> FYI I've been experimenting with relaxing restrictions on VM annotations - Java annotations which affect how VM work - @ReservedStackAccess, @Contended, @Stable, etc (most of them are located in jdk.internal.vm.annotation in 9).
>>
>> Right now, VM ignores any annotation unless it is declared on a class from privileged context (JDK classes or VM anonymous classes).
>>
>> Some features introduced adhoc flags to relax the restriction (e.g., -XX:RestrictContended & -XX:RestrictReservedStack), but with modules there's a universal way to expose them in controlled manner: rely on annotation class accessibility checks. In order to make some annotation available in non-privileged context, it is enough to add relevant export to java.base module (e.g., --add-exports java.base/jdk.internal.vm.annotation=ALL-UNNAMED).
>>
>> The problem I faced was that accessibility checks are performed on already loaded classes, but annotations are used during class parsing and the checks have to be performed against the class being loaded.
>>
>> One option is to re-implement accessibility checking logic and extract all necessary information from ClassFileParser.
>>
>> Another approach is to collect VM annotations during parsing and after the class is loaded apply them one-by-one doing accessibility checks. It works fine for most of the existing annotations, but problematic for @Contended - it affects instance layout which is computed during parsing.
>>
>> So, after some experiments, I decided to try different approach: optimistically assume all annotations are accessible and re-parse the class if any used annotation is not available:
>>
>>  http://cr.openjdk.java.net/~vlivanov/vm_annotations/webrev.00
>>
>> The benefit is that parsing & accessibility checking logic stay the same. Also, it enabled sanity checking logic for annotation usages almost for free.
>>
>> Overall, I'm quite satisfied with the result, but stumbled upon some nuisance when implementing re-parsing (e.g., tracing framework needs access to ClassFileParser and it brings unnecessary changes into ClassFileParser::parse_instance_klass_from_stream).
>>
>> Before investing into polishing the change, I'd like to discuss the approach I chose.
>>
>> Any problems with class file re-parsing or annotation resolution in VM I overlooked?
>>
>> Maybe there are strong arguments for other options (#1: accessibity checks in ClassFileParser & #2: apply annotations after the class is loaded)?
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>