On JDK 21, ClassfileParser doesn't handle MethodParameter attributes without names

[cushon]

I'm seeing bad constant pool index: 0 crashes in scala with the latests OpenJDK 21 EA builds, with stack traces like:

	at scala.reflect.internal.Reporting.abort(Reporting.scala:69)
	at scala.reflect.internal.Reporting.abort$(Reporting.scala:65)
	at scala.reflect.internal.SymbolTable.abort(SymbolTable.scala:28)
	at scala.tools.nsc.symtab.classfile.ClassfileParser$ConstantPool.errorBadIndex(ClassfileParser.scala:385)
	at scala.tools.nsc.symtab.classfile.ClassfileParser$ConstantPool.getExternalName(ClassfileParser.scala:249)
	at scala.tools.nsc.symtab.classfile.ClassfileParser.readParamNames$1(ClassfileParser.scala:828)

The latest JDK 21 EA builds contain some system classes with MethodParameters attributes that contain empty names. This change was caused by the fix for JDK-8292275: javac does not emit SYNTHETIC and MANDATED flags for parameters by default.

This was allowed by the specification in earlier versions, but wasn't used in practice (at least by the JDK), and some class reading implementations made the assumption that the name was always present.

The relevant part of the JVMS is:

If the value of the name_index item is zero, then this parameters element indicates a formal parameter with no name.

https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.7.24

I think this is the logic that needs updating:

https://github.com/scala/scala/blob/0e30133df211bc383fb8c7b7f8f4162f514f5aad/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala#L843

The fix is probably something like:

        case tpnme.MethodParametersATTR =>
          def readParamNames(): Unit = {
            val paramCount = u1()
            val paramNames = new Array[NameOrString](paramCount)
            val paramNameAccess = new Array[Int](paramCount)
            var i = 0
            while (i < paramCount) {
-              paramNames(i) = pool.getExternalName(u2())
+              val paramIndex = u2()
+              paramNames(i) = if (paramIndex == 0) null else pool.getExternalName(paramIndex)

[SethTisue]

Thanks for the report — it's well timed, since there's likely still time for a fix to make the upcoming 2.13.11 release.

[SethTisue]

not sure if null or "" is more appropriate here

[SethTisue]

for reference, scala/scala#4735 is where the support originally landed

[SethTisue]

a quick way to reproduce the problem without involving sbt is simply to open the Scala REPL (using the default scala or scala-cli)

regardless, we should have a failing unit test that targets the issue specifically

reminder to self: Scala 3 will need the same fix

[SethTisue]

not sure if null or "" is more appropriate here

using "" (or rather, new NameOrString("")) seems implausible given that we later do param.name = paramNames.names(i).name.toTermName.encode (line 1357) — probably better to use null and then guard against it downstream — with any luck, line 1357 is the only place a guard would be needed

[SethTisue]

regardless, we should have a failing unit test that targets the issue specifically

or not... the problem affects the compiler itself, so any compiler on which this hypothetical unit test would fail, is a compiler that wouldn't even be able to compile the unit test

[som-snytt]

I've been through the desert on an arg with no name.