RFR(xs): 8196920: VerifierTest_1A.java and VerifierTest_1B.java fail: assert(receiver_klass->is_subtype_of(static_receiver_klass))

[Ioi Lam]

On 2/13/18 8:15 PM, David Holmes wrote:
> On 14/02/2018 1:52 PM, Ioi Lam wrote:
>> There is one set of braces for each method. This is pretty common in 
>> the HotSpot tests. E.g.
>>
>> http://hg.openjdk.java.net/jdk/hs/file/e0d3be81da47/test/hotspot/jtreg/runtime/BootstrapMethod/BSMCalledTwice.java#l128 
>
>
> I suspect most of these were written by the same person and then 
> copied as examples.
>
> It is a very odd style to me.
>

It's the format output by

     java jdk.internal.org.objectweb.asm.util.ASMifier HelloWorld.class

When a classfile can have hundreds of methods, you want to separate them 
in some way.

- Ioi

>> So we should leave the braces and indentation as is.
>
> Ok. Not worth arguing over. :)
>
> David
>
>> Thanks
>>
>> - Ioi
>>
>>
>>
>> On 2/13/18 7:21 PM, David Holmes wrote:
>>> On 14/02/2018 12:19 PM, Calvin Cheung wrote:
>>>> On 2/13/18, 5:48 PM, David Holmes wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> Changes look good to me.
>>>> Thanks for your review, David.
>>>>>
>>>>> Aside: why does this test have unnecessary {} scopes ??
>>>> Not sure since I didn't write the original test. Perhaps they're 
>>>> there for better code readability?
>>>> I can replace them with blank lines like the following:
>>>>>         cw.visit(V1_8, ACC_ABSTRACT + ACC_INTERFACE, 
>>>>> "UnverifiableIntf", null, "java/lang/Object", null);
>>>>>
>>>>>             fv = cw.visitField(ACC_PUBLIC + ACC_FINAL + 
>>>>> ACC_STATIC, "x", "LVerifierTest0;", null, null);
>>>>>             fv.visitEnd();
>>>>>
>>>>>             mv = cw.visitMethod(ACC_STATIC, "<clinit>", "()V", 
>>>>> null, null);
>>>>>             mv.visitCode();
>>>>>             mv.visitTypeInsn(NEW, "VerifierTest0");
>>>>>             mv.visitInsn(DUP);
>>>>>             mv.visitMethodInsn(INVOKESPECIAL, "VerifierTest0", 
>>>>> "<init>", "()V", false);
>>>>>             mv.visitFieldInsn(PUTSTATIC, "UnverifiableIntf", "x", 
>>>>> "LVerifierTest0;");
>>>>>             mv.visitInsn(RETURN);
>>>>>             mv.visitMaxs(2, 0);
>>>>>             mv.visitEnd();
>>>>>
>>>>>         addBadMethod(cw);
>>>>>         cw.visitEnd();
>>>> Do you prefer the above?
>>>
>>> Sure - with indent fixed too of course. :)
>>>
>>> But don't waste too much time on it.
>>>
>>> Thanks,
>>> David
>>>
>>>> thanks,
>>>> Calvin
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 14/02/2018 9:23 AM, Calvin Cheung wrote:
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8196920
>>>>>> (sorry, the bug was marked confidential)
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8196920/webrev.00/
>>>>>> (the above change was suggested by Ioi)
>>>>>>
>>>>>> By design, the test created a unverifiable class:
>>>>>>
>>>>>> Expected exception:
>>>>>> java.lang.VerifyError: Call to wrong <init> method
>>>>>> Exception Details:
>>>>>>    Location:
>>>>>>      UnverifiableBase.<clinit>()V @4: invokespecial
>>>>>>    Reason:
>>>>>>      Type 'java/lang/Object' (constant pool 4) is not assignable 
>>>>>> to 'VerifierTest0' (constant pool 14)
>>>>>>
>>>>>> Due to some recent changes, the test doesn't work with the 
>>>>>> following vm options combination in hs-tier4 testing:
>>>>>> -Xcomp -XX:-TieredCompilation -Xverify:none
>>>>>> and resulting in assert in sharedRuntime.cpp:
>>>>>> assert(receiver_klass->is_subtype_of(static_receiver_klass)) 
>>>>>> failed: actual receiver must be subclass of static receiver klass
>>>>>>
>>>>>> A fix is to replace the above with a "bad" method which will 
>>>>>> never be executed during runtime:
>>>>>>
>>>>>> Expected exception:
>>>>>> java.lang.VerifyError: Operand stack underflow
>>>>>> Exception Details:
>>>>>>    Location:
>>>>>>      UnverifiableBase.bad()V @0: areturn
>>>>>>    Reason:
>>>>>>      Attempt to pop empty stack.
>>>>>>
>>>>>> thanks,
>>>>>> Calvin
>>