6894807: problem with interfaces in C2 type system
Vladimir Kozlov
Vladimir.Kozlov at Sun.COM
Mon Oct 26 22:22:55 PDT 2009
John Rose wrote:
>
> 1. Is the second occurrence of this comment really true, or is it just a
> copy of the first occurrence (from Tom's fix)?
> // Happens in a CTW of rt.jar, 320-341, no extra flags
I think, Tom just copied it.
>
> 2. Why doesn't the bug happen in the original code? Oh, right: An
> object cannot have exactly an interface type.
>
> It's strange to have a phi with a constant type. Maybe the right answer
> for a filter() query is, if the phi's type ('kill' arg) is a
> Type::singleton, return it without further processing.
It is not Phi but input ConP (interface.class) of CastPP (which came from null_check_common):
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
Vladimir
>
> -- John
>
> On Oct 26, 2009, at 8:30 PM, Vladimir Kozlov wrote:
>
>> 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
>
More information about the hotspot-compiler-dev
mailing list