From e4b1cfe555454370656a6287d0d4054e1384cc6d Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Thu, 12 May 2022 20:33:10 -0700 Subject: [PATCH 1/2] Use names instead of type symbols when scanning for pickling annotations When scanning for Scala pickling annotations, all annotations in all classfiles (including those produced by the Java compiler) are considered. Before this commit, a Type and Symbol were created for each discovered annotation and comparared to the known Scala signature annotation types. In certain situations (as in tests/pos/i15166), this is problematic, as the denotation of an annotation symbol defined as a Java inner class may be forced before the inner class table is populated and setClassInfo is called on the class root. Comparing names (as strings) rather than type symbols avoids this situation. Fixes #15166 --- .../src/dotty/tools/dotc/core/Definitions.scala | 4 ---- .../dotc/core/classfile/ClassfileParser.scala | 16 +++++++++++----- .../i15166/InterfaceAudience_JAVA_ONLY_1.java | 8 ++++++++ .../i15166/InterfaceStability_JAVA_ONLY_1.java | 8 ++++++++ tests/pos/i15166/Test_2.scala | 4 ++++ 5 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java create mode 100644 tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java create mode 100644 tests/pos/i15166/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index ddcee28c5739..44506211e319 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -961,13 +961,9 @@ class Definitions { @tu lazy val NativeAnnot: ClassSymbol = requiredClass("scala.native") @tu lazy val RepeatedAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Repeated") @tu lazy val SourceFileAnnot: ClassSymbol = requiredClass("scala.annotation.internal.SourceFile") - @tu lazy val ScalaSignatureAnnot: ClassSymbol = requiredClass("scala.reflect.ScalaSignature") - @tu lazy val ScalaLongSignatureAnnot: ClassSymbol = requiredClass("scala.reflect.ScalaLongSignature") @tu lazy val ScalaStrictFPAnnot: ClassSymbol = requiredClass("scala.annotation.strictfp") @tu lazy val ScalaStaticAnnot: ClassSymbol = requiredClass("scala.annotation.static") @tu lazy val SerialVersionUIDAnnot: ClassSymbol = requiredClass("scala.SerialVersionUID") - @tu lazy val TASTYSignatureAnnot: ClassSymbol = requiredClass("scala.annotation.internal.TASTYSignature") - @tu lazy val TASTYLongSignatureAnnot: ClassSymbol = requiredClass("scala.annotation.internal.TASTYLongSignature") @tu lazy val TailrecAnnot: ClassSymbol = requiredClass("scala.annotation.tailrec") @tu lazy val ThreadUnsafeAnnot: ClassSymbol = requiredClass("scala.annotation.threadUnsafe") @tu lazy val ConstructorOnlyAnnot: ClassSymbol = requiredClass("scala.annotation.constructorOnly") diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 6c693aef1af7..9565a70494d5 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -50,6 +50,12 @@ object ClassfileParser { mapOver(tp) } } + + private inline def sigOfClassName(n: String) = s"L$n;" + val ScalaSignatureAnnot: String = sigOfClassName("scala.reflect.ScalaSignature") + val ScalaLongSignatureAnnot: String = sigOfClassName("scala.reflect.ScalaLongSignature") + val TASTYSignatureAnnot: String = sigOfClassName("scala.annotation.internal.TASTYSignature") + val TASTYLongSignatureAnnot: String = sigOfClassName("scala.annotation.internal.TASTYLongSignature") } class ClassfileParser( @@ -1004,19 +1010,19 @@ class ClassfileParser( val nAnnots = in.nextChar var i = 0 while (i < nAnnots) { - val attrClass = pool.getType(in.nextChar).typeSymbol + val attrSig = pool.getExternalName(in.nextChar).value val nArgs = in.nextChar var j = 0 while (j < nArgs) { val argName = pool.getName(in.nextChar) if (argName.name == nme.bytes) { - if (attrClass == defn.ScalaSignatureAnnot) + if attrSig == ScalaSignatureAnnot then return unpickleScala(parseScalaSigBytes) - else if (attrClass == defn.ScalaLongSignatureAnnot) + else if attrSig == ScalaLongSignatureAnnot then return unpickleScala(parseScalaLongSigBytes) - else if (attrClass == defn.TASTYSignatureAnnot) + else if attrSig == TASTYSignatureAnnot then return unpickleTASTY(parseScalaSigBytes) - else if (attrClass == defn.TASTYLongSignatureAnnot) + else if attrSig == TASTYLongSignatureAnnot then return unpickleTASTY(parseScalaLongSigBytes) } parseAnnotArg(skip = true) diff --git a/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java b/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java new file mode 100644 index 000000000000..2b6256620594 --- /dev/null +++ b/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java @@ -0,0 +1,8 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@InterfaceStability_JAVA_ONLY_1.Evolving(bytes="no") +public class InterfaceAudience_JAVA_ONLY_1 { + @Retention(RetentionPolicy.RUNTIME) + public @interface Public { String bytes(); } +} diff --git a/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java b/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java new file mode 100644 index 000000000000..5bdf9b90a0f3 --- /dev/null +++ b/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java @@ -0,0 +1,8 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@InterfaceAudience_JAVA_ONLY_1.Public(bytes="yes") +public class InterfaceStability_JAVA_ONLY_1 { + @Retention(RetentionPolicy.RUNTIME) + public @interface Evolving { String bytes(); } +} diff --git a/tests/pos/i15166/Test_2.scala b/tests/pos/i15166/Test_2.scala new file mode 100644 index 000000000000..53e6da69a42c --- /dev/null +++ b/tests/pos/i15166/Test_2.scala @@ -0,0 +1,4 @@ +// scalac: -Xfatal-warnings +object Test { + val x: InterfaceAudience_JAVA_ONLY_1.Public = ??? +} From bc767cc24b732c4136f6d78962efe76869a658c0 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Wed, 18 May 2022 13:54:37 -0700 Subject: [PATCH 2/2] Require seeing `ScalaSig` before scanning for Scala pickling annotations An alternative fix for #15166, which aligns with the behavior of the Scala 2 classfile parser. Before this commit, all classfiles, including those produced by the Java compiler and compilers of other JVM languages, were scanned for Scala pickling annotations. In certain situations (as in tests/pos/i15166), this is problematic, as the denotation of an annotation symbol defined as a Java inner class may be forced before the inner class table is populated and setClassInfo is called on the class root. We avoid this situation by only performing the pickling annotation scan for those classfiles having the `ScalaSig` attribute, i.e. those produced by the Scala 2 compiler. We also drop support for pickling TASTy using the classfile annotations `scala.annotation.internal.TASTYSignature` and `scala.annotation.internal.TASTYLongSignature`. These were never used by the compiler, there are no plans for future use, and preserving support would complicate this fix. --- .../dotty/tools/dotc/core/Definitions.scala | 2 ++ .../dotc/core/classfile/ClassfileParser.scala | 20 +++++-------------- .../internal/TASTYLongSignature.java | 1 + .../annotation/internal/TASTYSignature.java | 1 + 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 44506211e319..c44c6a764770 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -961,6 +961,8 @@ class Definitions { @tu lazy val NativeAnnot: ClassSymbol = requiredClass("scala.native") @tu lazy val RepeatedAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Repeated") @tu lazy val SourceFileAnnot: ClassSymbol = requiredClass("scala.annotation.internal.SourceFile") + @tu lazy val ScalaSignatureAnnot: ClassSymbol = requiredClass("scala.reflect.ScalaSignature") + @tu lazy val ScalaLongSignatureAnnot: ClassSymbol = requiredClass("scala.reflect.ScalaLongSignature") @tu lazy val ScalaStrictFPAnnot: ClassSymbol = requiredClass("scala.annotation.strictfp") @tu lazy val ScalaStaticAnnot: ClassSymbol = requiredClass("scala.annotation.static") @tu lazy val SerialVersionUIDAnnot: ClassSymbol = requiredClass("scala.SerialVersionUID") diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 9565a70494d5..1685e641945c 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -50,12 +50,6 @@ object ClassfileParser { mapOver(tp) } } - - private inline def sigOfClassName(n: String) = s"L$n;" - val ScalaSignatureAnnot: String = sigOfClassName("scala.reflect.ScalaSignature") - val ScalaLongSignatureAnnot: String = sigOfClassName("scala.reflect.ScalaLongSignature") - val TASTYSignatureAnnot: String = sigOfClassName("scala.annotation.internal.TASTYSignature") - val TASTYLongSignatureAnnot: String = sigOfClassName("scala.annotation.internal.TASTYLongSignature") } class ClassfileParser( @@ -876,7 +870,7 @@ class ClassfileParser( /** Parse inner classes. Expects `in.bp` to point to the superclass entry. * Restores the old `bp`. - * @return true iff classfile is from Scala, so no Java info needs to be read. + * @return Some(unpickler) iff classfile is from Scala, so no Java info needs to be read. */ def unpickleOrParseInnerClasses()(using ctx: Context, in: DataReader): Option[Embedded] = { val oldbp = in.bp @@ -1005,25 +999,21 @@ class ClassfileParser( // attribute isn't, this classfile is a compilation artifact. return Some(NoEmbedded) - if (scan(tpnme.RuntimeVisibleAnnotationATTR) || scan(tpnme.RuntimeInvisibleAnnotationATTR)) { + if (scan(tpnme.ScalaSignatureATTR) && scan(tpnme.RuntimeVisibleAnnotationATTR)) { val attrLen = in.nextInt val nAnnots = in.nextChar var i = 0 while (i < nAnnots) { - val attrSig = pool.getExternalName(in.nextChar).value + val attrClass = pool.getType(in.nextChar).typeSymbol val nArgs = in.nextChar var j = 0 while (j < nArgs) { val argName = pool.getName(in.nextChar) if (argName.name == nme.bytes) { - if attrSig == ScalaSignatureAnnot then + if attrClass == defn.ScalaSignatureAnnot then return unpickleScala(parseScalaSigBytes) - else if attrSig == ScalaLongSignatureAnnot then + else if attrClass == defn.ScalaLongSignatureAnnot then return unpickleScala(parseScalaLongSigBytes) - else if attrSig == TASTYSignatureAnnot then - return unpickleTASTY(parseScalaSigBytes) - else if attrSig == TASTYLongSignatureAnnot then - return unpickleTASTY(parseScalaLongSigBytes) } parseAnnotArg(skip = true) j += 1 diff --git a/library/src/scala/annotation/internal/TASTYLongSignature.java b/library/src/scala/annotation/internal/TASTYLongSignature.java index 2278da258a5d..e1acf8279637 100644 --- a/library/src/scala/annotation/internal/TASTYLongSignature.java +++ b/library/src/scala/annotation/internal/TASTYLongSignature.java @@ -7,6 +7,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) +@Deprecated public @interface TASTYLongSignature { public String[] bytes(); } diff --git a/library/src/scala/annotation/internal/TASTYSignature.java b/library/src/scala/annotation/internal/TASTYSignature.java index a6372f008397..ee042eff5f9e 100644 --- a/library/src/scala/annotation/internal/TASTYSignature.java +++ b/library/src/scala/annotation/internal/TASTYSignature.java @@ -7,6 +7,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) +@Deprecated public @interface TASTYSignature { public String bytes(); }