RFR: 7176515: ExceptionInInitializerError for an enum with multiple switch statements [v11]

[Jaikiran Pai]

On Tue, 21 Mar 2023 17:19:23 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> This patch is both a minor optimization and a fix for JDK-7176515.
>> 
>> JDK-7176515 relates to how javac handles `switch` statements on `Enum` values, which are effectively "syntactic sugar".
>> 
>> The simple approach would be to just get the enum's `ordinal()` value and then revert to a normal integer value switch on that. However, this snapshots the ordinal values at compile-time, and thus can fail if the `Enum` class is recompiled later.
>> 
>> Presumably to avoid that possibility (are there any other reasons?) the compiler instead uses a lookup table. This table is dynamically constructed at runtime by a static initializer in a synthetic class. This avoids the "stale ordinal" problem of the simple approach, but it also creates a potential class initialization loop if there happens to an `Enum` switch inside an `Enum` class constructor, as demonstrated by the bug.
>> 
>> This patch makes the following change: If we are handling an `Enum` switch, and the `Enum` class we are switching on is also the class we are compiling, then just go with the simple approach, because the "stale ordinal" problem can't happen in this case so there's no need to build a runtime lookup table.
>> 
>> This results in two improvements:
>> * It avoids building the lookup table for switches on self
>> * It fixes JDK-7176515 as stated
>> 
>> Although the reproducing test case provided in JDK-7176515 gets fixed by this patch, the underlying issue is still there and can still be triggered with a slightly more complicated test case (included, but commented out).
>> 
>> So JDK-7176515 could be left open (or a new bug created) and a separate discussion had about how to "really" fix it.
>> 
>> Part of this discussion should be defining what that means, i.e., the boundaries of the bug. There are some scenarios that are basically impossible to fix, for example, two `Enum` classes whose constructors take as a parameter instances of the other `Enum` class and then switch on it. That cycle has nothing to do with how switches are handled.
>
> Archie L. Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update regression test to not look for classfile no longer being generated.

Hello Archie,

> > I'm still seeing test: test/hotspot/jtreg/runtime/cds/appcds/jvmti/ClassFileLoadHookTest.java failing with this patch
> 
> Strange... on github everything is passing now: https://github.com/archiecobbs/jdk/actions/runs/4473109724
> 
> But when I run that specific test on my laptop, it fails for me too.

GitHub actions job for the jdk repo is configured to run only `tier1` tests. The tests are grouped into different tiers in a file called `TEST.groups`. For hotspot that file resides here and the tier1 definition is this https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/TEST.groups#L545. You will notice that it's configured to run (among other things), the `tier1_runtime` which is defined in that same file https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/TEST.groups#L364. That group is defined to run everything under `runtime/` directory except the ones marked with a `-` (minus) sign and one of those is https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/TEST.groups#L404 `tier1_runtime_appcds_exclude` which is defined here https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/TEST.groups#L513, which except for a few tests, excludes everything else under `runtime/cds/appcds/` directory, so I believe that's why this `test/hotspot/jtreg/runtime/
 cds/appcds/jvmti/ClassFileLoadHookTest.java` isn't run in that job.

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

PR Comment: https://git.openjdk.org/jdk/pull/10797#issuecomment-1478996188