-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add jvm backend check for UTF-8 Constant length #19622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package dotty.tools.dotc | ||
package transform | ||
|
||
import MegaPhase.MiniPhase | ||
import GenericSignatures.* | ||
import core.Contexts.* | ||
import core.Decorators.* | ||
import core.Constants.* | ||
import core.Symbols.* | ||
import util.SrcPos | ||
|
||
final class CheckJVMSpecification extends MiniPhase: | ||
import ast.tpd.* | ||
|
||
override def phaseName: String = CheckJVMSpecification.name | ||
override def description: String = CheckJVMSpecification.description | ||
|
||
|
||
override def transformDefDef(tree: DefDef)(using Context): Tree = | ||
val sym = tree.symbol | ||
val memberTpe = atPhase(ctx.base.erasurePhase) { sym.owner.denot.thisType.memberInfo(sym) } | ||
for sig <- GenericSignatures.javaSig(sym, memberTpe) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean we'll compute the Java generic signature twice for every |
||
checkUTF8MaximumLength(sig, tree.srcPos, prefix = "Method signature") | ||
tree | ||
|
||
override def transformLiteral(tree: Literal)(using Context): Tree = | ||
val Literal(cste) = tree | ||
if cste.tpe == defn.StringType then | ||
checkUTF8MaximumLength(cste.stringValue, tree.srcPos.focus, prefix = "String") | ||
tree | ||
|
||
/** See https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.4.7 */ | ||
private def checkUTF8MaximumLength(str: String, pos: SrcPos, prefix: String)(using Context) = | ||
val UTF8_MAX_LENGTH = 65535 | ||
if str.length > UTF8_MAX_LENGTH then | ||
Comment on lines
+34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, so this is not precise enough. It will work for ASCII strings without null characters, but not for all strings. The referenced spec says that the We should simulate the encoding and count the number of bytes actually taken. As a fast-path, we should succeed early if |
||
report.error(em"""${prefix} length exceed the maximum length allowed by the JVM specification. | ||
|Allowed: ${UTF8_MAX_LENGTH} | ||
|Actual : ${str.length}""", pos) | ||
|
||
object CheckJVMSpecification: | ||
val name = "checkJVMspec" | ||
val description = "check issues related to the jvm specification" | ||
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
-- Error: tests/neg/i15850-b.scala:9:4 --------------------------------------------------------------------------------- | ||
9 |def crash(x: Level7): Unit = () // error | ||
|^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|Method signature length exceed the maximum length allowed by the JVM specification. | ||
|Allowed: 65535 | ||
|Actual : 496979 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's an awesome error. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
type Level1 = Tuple4[Unit, Unit, Unit, Unit] | ||
type Level2 = Tuple4[Level1, Level1, Level1, Level1] | ||
type Level3 = Tuple4[Level2, Level2, Level2, Level2] | ||
type Level4 = Tuple4[Level3, Level3, Level3, Level3] | ||
type Level5 = Tuple4[Level4, Level4, Level4, Level4] | ||
type Level6 = Tuple4[Level5, Level5, Level5, Level5] | ||
type Level7 = Tuple4[Level6, Level6, Level6, Level6] | ||
|
||
def crash(x: Level7): Unit = () // error |
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider an
end CheckJVMSpecification
marker for this class. It spans more than one screen.