RFR: 8336103: Sharper checks for <init> and <clinit> initializers [v2]

[David Holmes]

On Thu, 11 Jul 2024 08:58:11 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> All around Hotspot, we have calls to `method->is_initializer()`. That methods test for both instance and static initializers. In many cases, the uses imply we actually want to test for constructor (instance initializer), not static initializer. Sometimes we filter explicitly for `!m->is_static()`, sometimes we don't. Often we get lucky by never being exposed to static initializers on particular paths.
>> 
>> I would like to sharpen this. I went back and forth, and ultimately decided to remove `is_initializer` completely to avoid future confusion, and rewrite the uses appropriately.
>> 
>> Additional testing:
>>  - [x] Linux AArch64 server fastdebug, `all` (includes Fuzzer and CTW tests)
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Indenting

It is evident that people have been unfamiliar/sloppy with this API. This change should help prevent that in future. I have a concern about one change.

Thanks

src/hotspot/share/classfile/javaClasses.cpp line 3018:

> 3016:   int flags = (jushort)( m->access_flags().as_short() & JVM_RECOGNIZED_METHOD_MODIFIERS );
> 3017:   if (m->is_object_initializer()) {
> 3018:     flags |= java_lang_invoke_MemberName::MN_IS_CONSTRUCTOR;

I'm going to assume that `clinit` would already get filtered out at some point otherwise this would be a change in behaviour.

src/hotspot/share/runtime/reflection.cpp line 772:

> 770:   assert(!method()->is_object_initializer() &&
> 771:          (for_constant_pool_access || !method()->is_static_initializer()),
> 772:          "should call new_constructor instead");

Nit: existing -The assert message isn't really correct

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

PR Review: https://git.openjdk.org/jdk/pull/20120#pullrequestreview-2173741407
PR Review Comment: https://git.openjdk.org/jdk/pull/20120#discussion_r1675207989
PR Review Comment: https://git.openjdk.org/jdk/pull/20120#discussion_r1675249560