Skip to content

Commit b1137d5

Browse files
committed
Introduce a CheckedBehavior for StringIndexOutOfBoundsException.
Fundamentally, the primitive operation that causes language- mandated `StringIndexOutOfBoundsException`s is `String.charAt`. In order for the linker to recognize it and conditionally apply checked behavior semantics to it, we introduce a new IR `BinaryOp.String_charAt` for that operation. We use a compiler back-end hack to inject that IR node as the body of `String.charAt`. Using a dedicated primitive method in `scalajs.runtime`, like we did for `identityHashCode`, would not work so well here, as we want the binary operator to manipulate a *primitive* `StringType`, not a `ClassType(BoxedStringClass)`. For derived methods that are implemented on top of JS primitives, namely `codePointAt` and `substring`, we use a trick similar to what we did in `j.u.Arrays`: we introduce carefully-chosen no-op calls to `charAt`, with the only purpose of checking bounds, before delegating to the JS methods. Since `s.length()` is not known to be pure by the optimizer, we use clever tricks not to call it in the bounds checks of `substring`.
1 parent 8e6e6db commit b1137d5

File tree

21 files changed

+286
-43
lines changed

21 files changed

+286
-43
lines changed

compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2236,7 +2236,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
22362236
isJSFunctionDef(currentClassSym)) {
22372237
val flags = js.MemberFlags.empty.withNamespace(namespace)
22382238
val body = {
2239-
if (isImplClass(currentClassSym)) {
2239+
if (currentClassSym.get == HackedStringClass && methodName.name == charAtMethodName) {
2240+
// Hijack the body of String.charAt and replace it with a String_charAt binary op
2241+
js.BinaryOp(js.BinaryOp.String_charAt, genThis(), jsParams.head.ref)
2242+
} else if (isImplClass(currentClassSym)) {
22402243
val thisParam = jsParams.head
22412244
withScopedVars(
22422245
thisLocalVarIdent := Some(thisParam.name)
@@ -7059,6 +7062,9 @@ private object GenJSCode {
70597062
private val ObjectArgConstructorName =
70607063
MethodName.constructor(List(jstpe.ClassRef(ir.Names.ObjectClass)))
70617064

7065+
private val charAtMethodName =
7066+
MethodName("charAt", List(jstpe.IntRef), jstpe.CharRef)
7067+
70627068
private val thisOriginalName = OriginalName("this")
70637069

70647070
private object BlockOrAlone {

ir/shared/src/main/scala/org/scalajs/ir/Names.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,10 @@ object Names {
501501
val ArrayIndexOutOfBoundsExceptionClass: ClassName =
502502
ClassName("java.lang.ArrayIndexOutOfBoundsException")
503503

504+
/** The exception thrown by a `BinaryOp.String_charAt` that is out of bounds. */
505+
val StringIndexOutOfBoundsExceptionClass: ClassName =
506+
ClassName("java.lang.StringIndexOutOfBoundsException")
507+
504508
/** The exception thrown by an `AsInstanceOf` that fails. */
505509
val ClassCastExceptionClass: ClassName =
506510
ClassName("java.lang.ClassCastException")

ir/shared/src/main/scala/org/scalajs/ir/Printers.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,12 @@ object Printers {
411411
print(rhs)
412412
print(')')
413413

414+
case BinaryOp(BinaryOp.String_charAt, lhs, rhs) =>
415+
print(lhs)
416+
print('[')
417+
print(rhs)
418+
print(']')
419+
414420
case BinaryOp(op, lhs, rhs) =>
415421
import BinaryOp._
416422
print('(')

ir/shared/src/main/scala/org/scalajs/ir/Trees.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,9 @@ object Trees {
420420
final val Double_> = 56
421421
final val Double_>= = 57
422422

423+
// New in 1.11
424+
final val String_charAt = 58
425+
423426
def resultTypeOf(op: Code): Type = (op: @switch) match {
424427
case === | !== |
425428
Boolean_== | Boolean_!= | Boolean_| | Boolean_& |
@@ -439,6 +442,8 @@ object Trees {
439442
FloatType
440443
case Double_+ | Double_- | Double_* | Double_/ | Double_% =>
441444
DoubleType
445+
case String_charAt =>
446+
CharType
442447
}
443448
}
444449

ir/shared/src/test/scala/org/scalajs/ir/PrintersTest.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,9 @@ class PrintersTest {
538538
BinaryOp(Double_>, ref("x", DoubleType), ref("y", DoubleType)))
539539
assertPrintEquals("(x >=[double] y)",
540540
BinaryOp(Double_>=, ref("x", DoubleType), ref("y", DoubleType)))
541+
542+
assertPrintEquals("x[y]",
543+
BinaryOp(String_charAt, ref("x", StringType), ref("y", IntType)))
541544
}
542545

543546
@Test def printNewArray(): Unit = {

javalib/src/main/scala/java/lang/_String.scala

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@ final class _String private () // scalastyle:ignore
4848

4949
@inline
5050
def charAt(index: Int): Char =
51-
this.asInstanceOf[js.Dynamic].charCodeAt(index).asInstanceOf[Int].toChar
51+
throw new Error("stub") // body replaced by the compiler back-end
5252

5353
def codePointAt(index: Int): Int = {
5454
if (linkingInfo.esVersion >= ESVersion.ES2015) {
55+
charAt(index) // bounds check
5556
this.asInstanceOf[js.Dynamic].codePointAt(index).asInstanceOf[Int]
5657
} else {
5758
val high = charAt(index)
@@ -372,12 +373,26 @@ final class _String private () // scalastyle:ignore
372373
substring(beginIndex, endIndex)
373374

374375
@inline
375-
def substring(beginIndex: Int): String =
376+
def substring(beginIndex: Int): String = {
377+
// Bounds check (cleverly not using length() yet reporting errors in the appropriate cases)
378+
if (beginIndex != 0)
379+
charAt(beginIndex - 1)
380+
376381
thisString.jsSubstring(beginIndex)
382+
}
377383

378384
@inline
379-
def substring(beginIndex: Int, endIndex: Int): String =
385+
def substring(beginIndex: Int, endIndex: Int): String = {
386+
// Bounds check (cleverly not using length() yet reporting errors in the appropriate cases)
387+
if (beginIndex != 0)
388+
charAt(beginIndex - 1)
389+
if (endIndex != 0)
390+
charAt(endIndex - 1)
391+
if (endIndex < beginIndex)
392+
charAt(-1)
393+
380394
thisString.jsSubstring(beginIndex, endIndex)
395+
}
381396

382397
def toCharArray(): Array[Char] = {
383398
val len = length()

linker-interface/shared/src/main/scala/org/scalajs/linker/interface/Semantics.scala

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import Fingerprint.FingerprintBuilder
1818
final class Semantics private (
1919
val asInstanceOfs: CheckedBehavior,
2020
val arrayIndexOutOfBounds: CheckedBehavior,
21+
val stringIndexOutOfBounds: CheckedBehavior,
2122
val moduleInit: CheckedBehavior,
2223
val strictFloats: Boolean,
2324
val productionMode: Boolean,
@@ -31,6 +32,9 @@ final class Semantics private (
3132
def withArrayIndexOutOfBounds(behavior: CheckedBehavior): Semantics =
3233
copy(arrayIndexOutOfBounds = behavior)
3334

35+
def withStringIndexOutOfBounds(behavior: CheckedBehavior): Semantics =
36+
copy(stringIndexOutOfBounds = behavior)
37+
3438
def withModuleInit(moduleInit: CheckedBehavior): Semantics =
3539
copy(moduleInit = moduleInit)
3640

@@ -53,6 +57,7 @@ final class Semantics private (
5357
def optimized: Semantics = {
5458
copy(asInstanceOfs = this.asInstanceOfs.optimized,
5559
arrayIndexOutOfBounds = this.arrayIndexOutOfBounds.optimized,
60+
stringIndexOutOfBounds = this.stringIndexOutOfBounds.optimized,
5661
moduleInit = this.moduleInit.optimized,
5762
productionMode = true)
5863
}
@@ -61,6 +66,7 @@ final class Semantics private (
6166
case that: Semantics =>
6267
this.asInstanceOfs == that.asInstanceOfs &&
6368
this.arrayIndexOutOfBounds == that.arrayIndexOutOfBounds &&
69+
this.stringIndexOutOfBounds == that.stringIndexOutOfBounds &&
6470
this.moduleInit == that.moduleInit &&
6571
this.strictFloats == that.strictFloats &&
6672
this.productionMode == that.productionMode &&
@@ -74,26 +80,29 @@ final class Semantics private (
7480
var acc = HashSeed
7581
acc = mix(acc, asInstanceOfs.##)
7682
acc = mix(acc, arrayIndexOutOfBounds.##)
83+
acc = mix(acc, stringIndexOutOfBounds.##)
7784
acc = mix(acc, moduleInit.##)
7885
acc = mix(acc, strictFloats.##)
7986
acc = mix(acc, productionMode.##)
8087
acc = mixLast(acc, runtimeClassNameMapper.##)
81-
finalizeHash(acc, 6)
88+
finalizeHash(acc, 7)
8289
}
8390

8491
override def toString(): String = {
8592
s"""Semantics(
86-
| asInstanceOfs = $asInstanceOfs,
87-
| arrayIndexOutOfBounds = $arrayIndexOutOfBounds,
88-
| moduleInit = $moduleInit,
89-
| strictFloats = $strictFloats,
90-
| productionMode = $productionMode
93+
| asInstanceOfs = $asInstanceOfs,
94+
| arrayIndexOutOfBounds = $arrayIndexOutOfBounds,
95+
| stringIndexOutOfBounds = $stringIndexOutOfBounds,
96+
| moduleInit = $moduleInit,
97+
| strictFloats = $strictFloats,
98+
| productionMode = $productionMode
9199
|)""".stripMargin
92100
}
93101

94102
private def copy(
95103
asInstanceOfs: CheckedBehavior = this.asInstanceOfs,
96104
arrayIndexOutOfBounds: CheckedBehavior = this.arrayIndexOutOfBounds,
105+
stringIndexOutOfBounds: CheckedBehavior = this.stringIndexOutOfBounds,
97106
moduleInit: CheckedBehavior = this.moduleInit,
98107
strictFloats: Boolean = this.strictFloats,
99108
productionMode: Boolean = this.productionMode,
@@ -102,6 +111,7 @@ final class Semantics private (
102111
new Semantics(
103112
asInstanceOfs = asInstanceOfs,
104113
arrayIndexOutOfBounds = arrayIndexOutOfBounds,
114+
stringIndexOutOfBounds = stringIndexOutOfBounds,
105115
moduleInit = moduleInit,
106116
strictFloats = strictFloats,
107117
productionMode = productionMode,
@@ -217,6 +227,7 @@ object Semantics {
217227
new FingerprintBuilder("Semantics")
218228
.addField("asInstanceOfs", semantics.asInstanceOfs)
219229
.addField("arrayIndexOutOfBounds", semantics.arrayIndexOutOfBounds)
230+
.addField("stringIndexOutOfBounds", semantics.stringIndexOutOfBounds)
220231
.addField("moduleInit", semantics.moduleInit)
221232
.addField("strictFloats", semantics.strictFloats)
222233
.addField("productionMode", semantics.productionMode)
@@ -228,6 +239,7 @@ object Semantics {
228239
val Defaults: Semantics = new Semantics(
229240
asInstanceOfs = Fatal,
230241
arrayIndexOutOfBounds = Fatal,
242+
stringIndexOutOfBounds = Fatal,
231243
moduleInit = Unchecked,
232244
strictFloats = true,
233245
productionMode = false,

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/CoreJSLib.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,22 @@ private[emitter] object CoreJSLib {
892892
Return(If(x > 2147483647, 2147483647, If(x < -2147483648, -2147483648, x | 0)))
893893
},
894894

895+
condTree(semantics.stringIndexOutOfBounds != CheckedBehavior.Unchecked)(
896+
defineFunction2("charAt") { (s, i) =>
897+
val r = varRef("r")
898+
899+
val throwStringIndexOutOfBoundsException = {
900+
Throw(maybeWrapInUBE(semantics.stringIndexOutOfBounds,
901+
genScalaClassNew(StringIndexOutOfBoundsExceptionClass, IntArgConstructorName, i)))
902+
}
903+
904+
Block(
905+
const(r, Apply(genIdentBracketSelect(s, "charCodeAt"), List(i))),
906+
If(r !== r, throwStringIndexOutOfBoundsException, Return(r))
907+
)
908+
}
909+
),
910+
895911
condTree(allowBigIntsForLongs)(Block(
896912
defineFunction2("longDiv") { (x, y) =>
897913
If(y === bigInt(0), throwDivByZero, {

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,12 @@ object Emitter {
828828
StringArgConstructorName)
829829
},
830830

831-
cond(asInstanceOfs == Fatal || arrayIndexOutOfBounds == Fatal) {
831+
cond(stringIndexOutOfBounds != Unchecked) {
832+
instantiateClass(StringIndexOutOfBoundsExceptionClass,
833+
IntArgConstructorName)
834+
},
835+
836+
cond(asInstanceOfs == Fatal || arrayIndexOutOfBounds == Fatal || stringIndexOutOfBounds == Fatal) {
832837
instantiateClass(UndefinedBehaviorErrorClass,
833838
ThrowableArgConsructorName)
834839
},

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/EmitterNames.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ private[emitter] object EmitterNames {
3131
// Method names
3232

3333
val AnyArgConstructorName = MethodName.constructor(List(ClassRef(ObjectClass)))
34+
val IntArgConstructorName = MethodName.constructor(List(IntRef))
3435
val StringArgConstructorName = MethodName.constructor(List(ClassRef(BoxedStringClass)))
3536
val ThrowableArgConsructorName = MethodName.constructor(List(ClassRef(ThrowableClass)))
3637

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/FunctionEmitter.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,10 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
12711271
case _ => false
12721272
}
12731273

1274+
// String_charAt preserves pureness iff the semantics for stringIndexOutOfBounds are unchecked
1275+
case BinaryOp(BinaryOp.String_charAt, lhs, rhs) =>
1276+
(allowSideEffects || semantics.stringIndexOutOfBounds == Unchecked) && test(lhs) && test(rhs)
1277+
12741278
// Expressions preserving pureness
12751279
case Block(trees) => trees forall test
12761280
case If(cond, thenp, elsep) => test(cond) && test(thenp) && test(elsep)
@@ -2621,6 +2625,14 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
26212625

26222626
case Boolean_| => !(!js.BinaryOp(JSBinaryOp.|, newLhs, newRhs))
26232627
case Boolean_& => !(!js.BinaryOp(JSBinaryOp.&, newLhs, newRhs))
2628+
2629+
case String_charAt =>
2630+
semantics.stringIndexOutOfBounds match {
2631+
case CheckedBehavior.Compliant | CheckedBehavior.Fatal =>
2632+
genCallHelper("charAt", newLhs, newRhs)
2633+
case CheckedBehavior.Unchecked =>
2634+
js.Apply(genIdentBracketSelect(newLhs, "charCodeAt"), List(newRhs))
2635+
}
26242636
}
26252637

26262638
case NewArray(typeRef, lengths) =>

linker/shared/src/main/scala/org/scalajs/linker/checker/IRChecker.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,10 +513,12 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter) {
513513
Double_== | Double_!= |
514514
Double_< | Double_<= | Double_> | Double_>= =>
515515
DoubleType
516+
case String_charAt =>
517+
StringType
516518
}
517519
val expectedRhsType = (op: @switch) match {
518-
case Long_<< | Long_>>> | Long_>> => IntType
519-
case _ => expectedLhsType
520+
case Long_<< | Long_>>> | Long_>> | String_charAt => IntType
521+
case _ => expectedLhsType
520522
}
521523
typecheckExpect(lhs, env, expectedLhsType)
522524
typecheckExpect(rhs, env, expectedRhsType)

0 commit comments

Comments
 (0)