`java.lang.VerifyError: Bad type on operand stack` on Microsoft JDK 17 when running with code coverage enabled

[pshirshov]

It seems like the compiler might produce incorrect bytecode in some cases which leads to really cryptic stacktraces in run time.

This only reproduces on Microsoft JVM 17.0.3 running on Linux, 11.0.15 works just fine. I haven't tested on other vendor's JDKs.

Seems like this does NOT happen on macOS with:

❯ java -version
openjdk version "17.0.5" 2022-10-18
OpenJDK Runtime Environment GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08)
OpenJDK 64-Bit Server VM GraalVM CE 22.3.0 (build 17.0.5+8-jvmci-22.3-b08, mixed mode, sharing)

I don't have an isolated repro yet and, as usually, it might be complicated to get it.

The build log: https://github.com/7mind/izumi/actions/runs/4145278506/jobs/7169427138#step:3:34787
A commit where the failure reproduces: 7mind/izumi@9d3aacb , the commit itself is irrelevant, the issue manifested before. I belive it's a regression in 3.2.2, I haven't seen anything like this before and my environment didn't change.

Error:  java.lang.VerifyError: Bad type on operand stack
Error:  Exception Details:
Error:    Location:
Error:      izumi/functional/lifecycle/Lifecycle$LiftF.<init>(Lscala/Function0;Lizumi/functional/quasi/QuasiApplicative;)V @25: invokedynamic
Error:    Reason:
Error:      Type uninitializedThis (current frame, stack[1]) is not assignable to 'izumi/functional/lifecycle/Lifecycle$LiftF'
Error:    Current Frame:
Error:      bci: @25
Error:      flags: { flagThisUninit }
Error:      locals: { uninitializedThis, 'scala/Function0', 'izumi/functional/quasi/QuasiApplicative' }
Error:      stack: { uninitializedThis, uninitializedThis, 'scala/Function0' }
Error:    Bytecode:
Error:      0000000: b200 2711 0f55 1229 b600 2db2 0027 110f
Error:      0000010: 5412 29b6 002d 2a2a 2bba 0040 0000 032c
Error:      0000020: b700 42b1                              
Error:  
Error:  	at izumi.functional.lifecycle.Lifecycle$.liftF(Lifecycle.scala:313)
Error:  	at izumi.distage.model.Injector.produceCustomF(Injector.scala:230)
Error:  	at izumi.distage.model.Injector.produceCustomF$(Injector.scala:22)
Error:  	at izumi.distage.InjectorDefaultImpl.produceCustomF(InjectorDefaultImpl.scala:22)
Error:  	at izumi.distage.model.Injector.produce(Injector.scala:190)
Error:  	at izumi.distage.model.Injector.produce$(Injector.scala:22)
Error:  	at izumi.distage.InjectorDefaultImpl.produce(InjectorDefaultImpl.scala:22)

[smarter]

It seems the issue only happens when code coverage is enabled which is why you might have trouble reproducing it.

[smarter]

I tried switching to 3.2.1 but the compiler crashed when compiling distage-core-api, so it never got to running the failing test I saw with 3.2.2

[smarter]

LiftF has a secondary constructor which contains a lambda:
https://github.com/7mind/izumi/blob/9d3aacbab6e669bf681a9c3199073b25c463b0ca/fundamentals/fundamentals-bio/src/main/scala/izumi/functional/lifecycle/Lifecycle.scala#L669

When code coverage is disabled, the HoistSuperArgs phase of the compiler recognizes that this is an expression that isn't allowed in a secondary constructor and moves it into into a separate static method.

But when code coverage is enabled (which adds some calls to Invoker.invoked(...) into the constructor), this phase no longer detects the problem which leads to invalid bytecode being generated.

It seems that HoistSuperArgs assumes that the first statement in a constructor is always the super-call, which isn't the case when code coverage is enabled, e.g.:
https://github.com/lampepfl/dotty/blob/3d251d6fd1bb1e98d66760ee50b824861a87ca57/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala#L213

/cc @TheElectronWill, do you think you could have a look at that?

[pshirshov]

Thanks for the explanation. My bad, I forgot about code coverage :)

[TheElectronWill]

I'll finally have time to take a look 😄
I wonder if it would be simpler to move InstrumentCoverage later in the compilation pipeline, to avoid issues like this that potentially require tweaking the coverage with many conditions that depends on the implementation of later phases 🤔 (here, fixing HoistSuperArgs should be possible, but I'm speaking in general)

[smarter]

I wonder if it would be simpler to move InstrumentCoverage later in the compilation pipeline

That could make sense yes, I think the only issue is that it means we could potentially generate a lot more instrumentation calls for synthetic nodes that don't show up in the coverage report, I see there's already logic to avoid instrumenting trees when span.isZeroExtent so that might not be too bad in practice?

[TheElectronWill]

Synthetic nodes are (should) always be marked with the Synthetic flags, aren't they?

[sjrd]

Tree nodes do not have flags. Only definitions (symbols) have.