RFR(XS): JDK-6904403 assert(f == k->has_finalizer(), "inconsistent has_finalizer") with debug VM

[Coleen Phillimore]

Ok, then.  I don't need to see the copyright changes after you add 
them.  My code review is complete.
thanks!
Coleen

On 5/27/14, 11:45 AM, Dmitry Samersoff wrote:
> Coleen,
>
> please see below.
>
> On 2014-05-27 19:02, Coleen Phillimore wrote:
>> Dmitry,
>> This is excellent.  Thank you for adding a test.  I have a couple
>> comments but they are sort of simple things.
>>
>> 1.  In InstanceKlass::has_redefined_super() can you make "klass' an
>> InstanceKlass* and cast to InstanceKlass on line 1491 instead to avoid
>> the casting.   You'll only have InstanceKlass on the way up.
> InstanceKlass klass->super() return Klass *, so we have to cast it anyway:
>
>      klass = InstanceKlass::cast(klass->super());
>
>   Current approach is a bit more readable on my view and similar to code
> above.
>
>> 2.  The new test files need copyrights!   Except maybe the manifest one
>> (idk).
> OK.
>
>> 3.  Can you make jtreg commands to do the things in the script?  I
>> didn't see anything very unusual.
> I tried it and ended up to bunch of java code instead of 10 lines of
> straightforward shell script.
>
> jtreg compiles classes with tools.jar from COMPILEJAVA, not from
> TESTJAVA. It might be a problem if COMPILE java doesn't exactly match
> TESTJAVA (e.g. cross compilation) and I didn't find a way to change it.
>
> I didn't find a way to instruct jtreg to build a jar file required for
> agent. I have to do it programatically and then use process builder to
> launch one more VM.
>
> So I would prefer to stay with a shell here.
>
>> Thank you for doing this.  It's really helpful and it seems it found a
>> bug also (super classes?)
> Initially this fix was intended as very quick band-aid to address jcov
> problem important for SQE. But as soon as we go deeper I decide to make
> better fix to cover other possible cases with redefined finalizers.
>
> -Dmitry
>
>> Coleen
>>
>> On 5/24/14, 1:17 PM, Dmitry Samersoff wrote:
>>> Coleen,
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-6904403/webrev.03/
>>>
>>> This is new version of the fix.
>>>
>>> Changes:
>>>
>>> 1. Added testcase
>>>
>>> 2. Check entire class hierarchy for redefined classes
>>>      as any of supers could be redefined and cause assert.
>>>
>>> -Dmitry
>>>
>>> On 2014-04-16 16:56, Dmitry Samersoff wrote:
>>>> Hi Everybody,
>>>>
>>>> Please review small changes.
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-6904403/webrev.01/
>>>>
>>>> This fix tells to JVM to don't assert if the class was redefined.
>>>>
>>>> Please notice, these changes doesn't make redefined finalyzers working.
>>>> It's a large project which is out of scope of this changes.
>>>>
>>>> -Dmitry
>>>>
>