RFR: 8256365: Clean up vtable initialization code

[Coleen Phillimore]

On Tue, 17 Nov 2020 16:34:46 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> I was looking through this code because of JDK-8061949 and want to do some minor cleanups.
>> 1. There's a function in the wrong place (is_override)
>> 2. methodHandles that use mh()->is_native(), with extra (),
>> 3. some methods declared with TRAPS, that don't trap
>> 4. some multi-clause conditionals with confusing formatting
>> 5. extra InstanceKlass::cast() casts
>> 6. some useless asserts
>> 7. and potentially a bug with RedefineClasses where the method being added to the vtable may have been redefined in the constraint verification call. (noreg-hard)
>> 
>> Tested with tier1-3.
>
> I don't think I like having nasty and subtle bug fixes and subtle behavioural changes hidden in what otherwise looks like a large (ish) cleanup patch. Could we do the bug fix first, and then rebase this mostly cleanup related change on top of that? As is, I have to read every line very carefully to see if it is just a cleanup or a subtle behaviour change.

The behavioral change is that that redefinition can make a method an old method in the safepoint that does constraint checking.  I can take this part out and leave the cleanup in this PR.  See my other comment about _klass vs. klass.

> src/hotspot/share/oops/klassVtable.cpp line 230:
> 
>> 228:           HandleMark hm(THREAD);
>> 229:           assert(default_methods->at(i)->is_method(), "must be a Method*");
>> 230:           methodHandle mh(THREAD, default_methods->at(i));
> 
> Maybe wrap the method in a {} block so you can see when it goes out of scope and don't get tempted to use it after it becomes invalid.

Do you mean this methodHandle mh?  vs the refetch of method below on line 232?  Since your main comment was to not mix the behavior change with the cleanup, I'm going to revert line 232 and do a different PR for that so that I can adequately describe the redefinition problem.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1236