Skip to content

Commit 2a19cd5

Browse files
committed
SI-2464 Resiliance against missing InnerClass attributes
A classfile in the wild related to Vaadin lacked the InnerClasses attribute. As such, our class file parser treated a nested enum class as top-level, which led to a crash when trying to find its linked module. More details of the investigation are available in the JIRA comments. The test introduces a new facility to rewrite classfiles. This commit turns this situation into a logged warning, rather than crashing. Code by @paulp, test by yours truly.
1 parent 5312d63 commit 2a19cd5

File tree

5 files changed

+99
-8
lines changed

5 files changed

+99
-8
lines changed

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,10 +588,14 @@ abstract class ClassfileParser {
588588
// sealed java enums
589589
if (jflags.isEnum) {
590590
val enumClass = sym.owner.linkedClassOfClass
591-
if (!enumClass.isSealed)
592-
enumClass setFlag (SEALED | ABSTRACT)
593-
594-
enumClass addChild sym
591+
enumClass match {
592+
case NoSymbol =>
593+
devWarning(s"no linked class for java enum $sym in ${sym.owner}. A referencing class file might be missing an InnerClasses entry.")
594+
case linked =>
595+
if (!linked.isSealed)
596+
linked setFlag (SEALED | ABSTRACT)
597+
linked addChild sym
598+
}
595599
}
596600
}
597601
}

src/partest/scala/tools/partest/BytecodeTest.scala

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@ package scala.tools.partest
22

33
import scala.tools.nsc.util.JavaClassPath
44
import scala.collection.JavaConverters._
5-
import scala.tools.asm
6-
import asm.{ ClassReader }
7-
import asm.tree.{ClassNode, MethodNode, InsnList}
8-
import java.io.InputStream
5+
import scala.tools.asm.{ClassWriter, ClassReader}
6+
import scala.tools.asm.tree.{ClassNode, MethodNode, InsnList}
7+
import java.io.{FileOutputStream, FileInputStream, File => JFile, InputStream}
98
import AsmNode._
109

1110
/**
@@ -127,3 +126,31 @@ abstract class BytecodeTest extends ASMConverters {
127126
new JavaClassPath(containers, DefaultJavaContext)
128127
}
129128
}
129+
130+
object BytecodeTest {
131+
/** Parse `file` as a class file, transforms the ASM representation with `f`,
132+
* and overwrites the orginal file.
133+
*/
134+
def modifyClassFile(file: JFile)(f: ClassNode => ClassNode) {
135+
val rfile = new reflect.io.File(file)
136+
def readClass: ClassNode = {
137+
val cr = new ClassReader(rfile.toByteArray())
138+
val cn = new ClassNode()
139+
cr.accept(cn, 0)
140+
cn
141+
}
142+
143+
def writeClass(cn: ClassNode) {
144+
val writer = new ClassWriter(0)
145+
cn.accept(writer)
146+
val os = rfile.bufferedOutput()
147+
try {
148+
os.write(writer.toByteArray)
149+
} finally {
150+
os.close()
151+
}
152+
}
153+
154+
writeClass(f(readClass))
155+
}
156+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package test;
2+
3+
@Connect(loadStyle = Connect.LoadStyle.EAGER)
4+
public class Annotated {
5+
}

test/files/run/t2464/Connect.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test;
2+
3+
import java.lang.annotation.ElementType;
4+
import java.lang.annotation.Retention;
5+
import java.lang.annotation.RetentionPolicy;
6+
import java.lang.annotation.Target;
7+
8+
9+
@Retention(RetentionPolicy.RUNTIME)
10+
@Target(ElementType.TYPE)
11+
public @interface Connect {
12+
13+
LoadStyle loadStyle() default LoadStyle.EAGER;
14+
15+
public enum LoadStyle {
16+
EAGER,
17+
DEFERRED,
18+
LAZY
19+
}
20+
}

test/files/run/t2464/Test.scala

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import scala.reflect.io.Streamable
2+
import scala.tools.asm.{ClassWriter, ClassReader}
3+
import scala.tools.asm.tree.ClassNode
4+
import scala.tools.partest._
5+
import scala.tools.partest.BytecodeTest.modifyClassFile
6+
import java.io.{FileOutputStream, FileInputStream, File}
7+
8+
object Test extends DirectTest {
9+
def code = ???
10+
11+
def compileCode(code: String) = {
12+
val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
13+
compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code)
14+
}
15+
16+
def app = """
17+
object O {
18+
new test.Annotated
19+
}
20+
"""
21+
22+
def show(): Unit = {
23+
compileCode(app)
24+
modifyClassFile(new File(testOutput.toFile, "test/Annotated.class")) {
25+
(cn: ClassNode) =>
26+
// As investigated https://issues.scala-lang.org/browse/SI-2464?focusedCommentId=64521&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-64521
27+
// classfiles in the wild sometimes lack the required InnerClass attribute for nested enums that
28+
// are referenced in an annotation. I don't know what compiler or bytecode processor leaves things
29+
// that way, but this test makes sure we don't crash.
30+
cn.innerClasses.clear()
31+
cn
32+
}
33+
compileCode(app)
34+
}
35+
}

0 commit comments

Comments
 (0)