6894807: problem with interfaces in C2 type system

[Tom Rodriguez]

I think it should include an extra test of !ftkp->klass_is_exact().   
This will cause it to return ft which is the original precise type.

tom

On Oct 26, 2009, at 3:07 PM, Vladimir Kozlov wrote:

> John, Tom
>
> I need your help with this.
>
> The original implementation of TypeOopPtr::filter() was done by John
> and pushed by Ross for:
>
> 6467870: Fixes to monotonically narrow or widen types during igvn
>
> Ross Knippel wrote:
> > This change is from John Rose (I'm just the medium.)
> > It's a piece extracted from his current work.
>
> It did not have the bug's problem since an interface could not be  
> exact.
>
> Then Tom added the klass part for:
>
> 6788347: C2Compiler crash 6u7
>
> Which causes the problem since interface klass could be exact  
> (constant,
> as in the bug's case) and casting j.l.O klass to interface's ptr
> returns incorrectly exact j.l.O klass:
>
>    return ktkp->cast_to_ptr_type(ftkp->ptr());
>
> The fix is either simple return original j.l.o klass:
>
>    return kills;
>
> or cast to ptrs meet:
>
>    return ktkp->cast_to_ptr_type(ktkp->meet_ptr(ftkp->ptr()));
>
> Both of them fixed the problem. But I am not sure since in
> both cases we loose the precision of interface klass which
> allows more ideal optimizations (for example, for CmpP).
>
> Thanks,
> Vladimir
>
> Vladimir Kozlov wrote:
>> I am investigating 6894807: No ClassCastException for  
>> HashAttributeSet constructors if run with -Xcomp
>> Because of Escape Analysis is on by default all constructors are  
>> inlined
>> in the test case and object scalar replaced. But due to, I think,
>> the problem in C2 type system the result is incorrect.
>> We should check if interface is exact before replacing
>> it with j.l.O in TypeOopPtr::filter() for the case
>> (exact interface klass)->filter(j.l.O klass).
>> Vladimir
>> ----------------------------------------------------------------
>> Test code expects ClassCastException exception:
>> import javax.print.attribute.Attribute;
>> import javax.print.attribute.AttributeSet;
>> import javax.print.attribute.DocAttribute;
>> import javax.print.attribute.HashAttributeSet;
>> import javax.print.attribute.standard.JobState;
>>        try {
>>            new MyHashAttributeSet(JobState.CANCELED,  
>> DocAttribute.class);
>>            System.out.println("No ClassCastException in t2!");
>>        } catch (ClassCastException _) {
>>        }
>> EA forces to inline all constructors for MyHashAttributeSet:
>> class MyHashAttributeSet extends HashAttributeSet{
>>    MyHashAttributeSet(Attribute attribute,
>>                           Class interfaceName){
>>        super(attribute, interfaceName);
>>    }
>> then
>>    protected HashAttributeSet(Attribute attribute, Class<?>  
>> interfaceName) {
>>        if (interfaceName == null) {
>>            throw new NullPointerException("null interface");
>>        }
>>        myInterface = interfaceName;
>>        add (attribute);
>>    }
>> add() is inlined also:
>>    public boolean add(Attribute attribute) {
>>        Object oldAttribute =
>>            attrMap.put(attribute.getCategory(),
>>                        AttributeSetUtilities.
>>                        verifyAttributeValue(attribute, myInterface));
>>        return (!attribute.equals(oldAttribute));
>>    }
>> and verifyAttributeValue(attribute, myInterface) is inlined as well:
>>    public static Attribute
>>        verifyAttributeValue(Object object, Class<?> interfaceName) {
>>        if (object == null) {
>>            throw new NullPointerException();
>>        }
>>        else if (interfaceName.isInstance (object)) {
>>            return (Attribute) object;
>>        } else {
>>            throw new ClassCastException();
>>        }
>>    }
>> object is JobState.CANCELED which is
>>    public static final JobState CANCELED = new JobState (7);
>> and interfaceName is DocAttribute.class where
>> public interface DocAttribute extends Attribute {}
>> ----------------------------------------------------------------
>> So we end up with
>> (DocAttribute.class).isInstance(JobState.CANCELED)
>> We have intrinsic for isInstance() for which we generated
>>  // Now load the mirror's klass metaobject, and null-check it.
>>  // Side-effects region with the control path if the klass is null.
>>  Node* kls = load_klass_from_mirror(mirror, never_see_null, nargs,
>>                                     region, _prim_path);
>>  case vmIntrinsics::_isInstance:
>>    // nothing is an instance of a primitive type
>>    query_value = gen_instanceof(obj, kls);
>> where due to NULL check we have next CastPP for DocAttribute.class:
>> [t at 19 l at 19]: print kls->dump(1)
>> 390    LoadKlass       === _  7  389  [[ 391  396 ]]   
>> @rawptr:BotPTR, idx=Raw; # *  Klass: * !jvms:  
>> AttributeSetUtilities::verifyAttributeValue @ bci:14  
>> HashAttributeSet::add @ bci:15 HashAttributeSet::<init> @ bci:36  
>> MyHashAttributeSet::<init> @ bci:3 InstanceCheck::t2 @ bci:10
>> 395    IfTrue  ===  393  [[ 370  396 ]] #1 !jvms:  
>> AttributeSetUtilities::verifyAttributeValue @ bci:14  
>> HashAttributeSet::add @ bci:15 HashAttributeSet::<init> @ bci:36  
>> MyHashAttributeSet::<init> @ bci:3 InstanceCheck::t2 @ bci:10
>> 396    CastPP  ===  395  390  [[ 409  409  413  432  427 ]]  #klass  
>> java/lang/Object: 0x0811c7e0 *  Klass:klass java/lang/Object:  
>> 0x0811c7e0 * !jvms: AttributeSetUtilities::verifyAttributeValue @  
>> bci:14 HashAttributeSet::add @ bci:15 HashAttributeSet::<init> @  
>> bci:36 MyHashAttributeSet::<init> @ bci:3 InstanceCheck::t2 @ bci:10
>> During IGVN LoadKlass(390) transformed to constant klass which is  
>> interface:
>> 702    ConP    ===  0  [[ 396  391 ]]  #precise klass javax/print/ 
>> attribute/DocAttribute: 0x08407298:Constant:exact *   
>> Interface:precise klass javax/print/attribute/DocAttribute:  
>> 0x08407298:Constant:exact *
>> 396    CastPP  ===  395  702  [[ 409  409  413  432  427 ]]  #klass  
>> java/lang/Object: 0x0811c7e0 *  Klass:klass java/lang/Object:  
>> 0x0811c7e0 * !jvms: AttributeSetUtilities::verifyAttributeValue @  
>> bci:14 HashAttributeSet::add @ bci:15 HashAttributeSet::<init> @  
>> bci:36 MyHashAttributeSet::<init> @ bci:3 InstanceCheck::t2 @ bci:10
>> ConstraintCastNode::Value() calls TypeOopPtr::filter() method
>> which, I think, returns incorrect result:
>> [t at 19 l at 19]: print ft->dump()
>> precise klass javax/print/attribute/DocAttribute:  
>> 0x08407298:Constant:exact *
>> [t at 19 l at 19]: print ftkp->klass()->print()
>> <ciInstanceKlass name=javax/print/attribute/DocAttribute loader=0x0  
>> loaded=true initialized=false finalized=false subklass=false size=9  
>> flags=public,interface,abstract mirror=PRESENT ident=655 PERM  
>> address=0x8407298>
>> [t at 19 l at 19]: print ktkp->klass()->print()
>> <ciInstanceKlass name=java/lang/Object loader=0x0 loaded=true  
>> initialized=true finalized=false subklass=true size=8  
>> flags=public,super mirror=PRESENT ident=558 PERM  
>> address=0x811c7e0>ktkp->klass()->print() = (void)
>> because of this code:
>>  // If we have an interface-typed Phi or cast and we narrow to a  
>> class type,
>>  // the join should report back the class.  However, if we have a J/ 
>> L/Object
>>  // class-typed Phi and an interface flows in, it's possible that  
>> the meet &
>>  // join report an interface back out.  This isn't possible but  
>> happens
>>  // because the type system doesn't interact well with interfaces.
>>  if (ftkp != NULL && ktkp != NULL &&
>>      ftkp->is_loaded() &&  ftkp->klass()->is_interface() &&
>>      ktkp->is_loaded() && !ktkp->klass()->is_interface()) {
>>    // Happens in a CTW of rt.jar, 320-341, no extra flags
>>    return ktkp->cast_to_ptr_type(ftkp->ptr());
>>  }
>> the result is
>> precise klass java/lang/Object: 0x0811c7e0:Constant:exact *
>> which leads to incorrect result from CmpP node:
>> 703    ConP    ===  0  [[ 704  432  704 ]]  #precise klass java/ 
>> lang/Object: 0x0811c7e0:Constant:exact *  Klass:precise klass java/ 
>> lang/Object: 0x0811c7e0:Constant:exact *
>> 704    CmpP    === _  703  703  [[ 705 ]]  !orig=[413] !jvms:  
>> AttributeSetUtilities::verifyAttributeValue @ bci:14  
>> HashAttributeSet::add @ bci:15 HashAttributeSet::<init> @ bci:36  
>> MyHashAttributeSet::<init> @ bci:3 InstanceCheck::t2 @ bci:10