help with changes to hotspot runtime to implement sealed types

[David Holmes]

On 12/12/2018 12:32 pm, Vicente Romero wrote:
> On 12/11/18 9:21 PM, David Holmes wrote:
>> Hi Vincente,
>>
>> On 12/12/2018 11:53 am, Vicente Romero wrote:
>>> Hi HS runtime,
>>>
>>> I'm working on the implementation of sealed types one of the projects 
>>> under the Amber umbrella. A sealed type is one that can only be 
>>> subtyped by a set of classes declared by the sealed type in Java:
>>>
>>> final class Sealed permits Sub {}
>>>     class Sub extends Sealed {}
>>
>> This has evolved :) I was expecting it to be restricted to subclassing 
>> by nestmates.
> 
> :) yep
> 
>>
>>> Both Sealed and Sub can coexist in the same compilation unit, package 
>>> or module. The class file for Sealed have an attribute named 
>>> PermittedSubtypes that contains a list with the, well, permitted 
>>> subtypes. In the case of the example it will be Sub only. On top of 
>>> that class Sealed will be final. So when loading / parsing class file 
>>> for Sub, we need to check, given that its supertype is final, if Sub 
>>> is listed as one of the permitted subtypes of Sealed or fail in other 
>>> case. And here is when I'm finding a roadblock. Currently the VM 
>>> doesn't allow any class to subtype a final class and this is checked at:
>>>
>>> ClassFileParser::post_process_parsed_stream
>>>
>>> so I removed the code that was doing this check at `ClassFileParser` 
>>> and created a separate method to do the checking at InstanceKlass 
>>> with the idea of enforcing the check after the class file was parsed 
>>> and the given class loaded. But regardless of my intents I always get 
>>> a SOE. The SOE happens when the subclass ask the super class if the 
>>> subclass is listed as one of its permitted subtypes. To answer this, 
>>> the superclass tries to load the subclass so I have a deadlock that I 
>>> suspect could be hard to solve, if possible. Please see attached what 
>>> I have done so far. Any ideas?
>>
>> +      if (name == k->name()) {
>> +        log_trace(class, sealed)("- Found it at 
>> permitted_subtypes[%d] => cp[%d]", i, cp_index);
>> +        // names match so check actual klass - this may trigger class 
>> loading if
>> +        // it doesn't match (but that should be impossible)
>> +        Klass* k2 = _constants->klass_at(cp_index, CHECK_false);
>>
>> You can't check the actual class in your case as if the names match 
>> then it refers to the subclass you are currently trying to load.
> 
> and what should I do? just check by name without doing:
> 
> Klass* k2 = _constants->klass_at(cp_index, CHECK_false);

Yes. All you can check is that the name of the current class matches an 
allowed subtype, and that the current class will be loaded into the same 
classloader as the supertype, and hence in the same package and module.

Note that the nestmates code you are copying deals with already loaded 
classes. Nest membership is not checked at classload time, only when a 
nestmate access check is needed. Whereas your code is dealing with a 
currently-being-loaded class, so needs to be more careful.

Cheers,
David

> 
>>
>> HTH,
>> David
> 
> Thanks,
> Vicente
>>
>>
>>> Thanks,
>>> Vicente
>>>
>>> PD, the code runs with no error with the slowdebug configuration
>