6894807: problem with interfaces in C2 type system

[Vladimir Kozlov]

Thanks, Tom

I will do it.

Vladimir

Tom Rodriguez wrote:
> 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
>