8000 Merge pull request #5108 from sjrd/fix-deser-hack-for-throw-from-1.8 · scala-js/scala-js@18277a7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 18277a7

Browse files
gzm0gzm0
authored
Merge pull request #5108 from sjrd/fix-deser-hack-for-throw-from-1.8
Fix #5107: Add missing `CheckNotNull`s in deserialization hacks.
2 parents 5eb1546 + 0b94e03 commit 18277a7

File tree

2 files changed

+80
-56
lines changed

2 files changed

+80
-56
lines changed

‎ir/shared/src/main/scala/org/scalajs/ir/Serializers.scala

Lines changed: 19 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,69 +1424,32 @@ object Serializers {
14241424
* so that `Throw(e)` only ever throws the value of `e`, while the NPE UB
14251425
* is specified by `UnwrapFromThrowable`. Among other things, this allows
14261426
* the user-space code `js.special.throw(e)` to indiscriminately throw `e`
1427-
* even if it is `null`.
1427+
* even if it is `null`. Later, in Scala.js 1.18, we further separated the
1428+
* null check of `UnwrapFromThrowable` to be an explicit `CheckNotNull`.
14281429
*
1429-
* With this hack, we patch `Throw(e)` where `e` is a nullable `Throwable`
1430-
* by inserting an appropriate `UnwrapFromThrowable`.
1430+
* With this hack, we patch `Throw(e)` by inserting an appropriate
1431+
* `CheckNotNull`.
14311432
*
1432-
* Naively, we would just return `UnwrapFromThrowable(e)`. Unfortunately,
1433-
* we cannot prove that this is type-correct when the type of `e` is a
1434-
* `ClassType(cls)`, as we cannot test whether `cls` is a subclass of
1435-
* `java.lang.Throwable`. So we have to produce the following instead:
1436-
*
1437-
* {{{
1438-
* if (expr === null) unwrapFromThrowable(null) else expr
1439-
* }}}
1440-
*
1441-
* except that evaluates `expr` twice. If it is a `VarRef`, which is a
1442-
* common case, that is fine. Otherwise, we have to wrap this pattern in
1443-
* an IIFE.
1444-
*
1445-
* We also have to avoid the transformation altogether when the `expr` is
1446-
* an `AnyType`. This happens when the previous Scala.js compiler already
1447-
* provides the unwrapped exception, which is either
1433+
* However, we must not do that when the previous Scala.js compiler
1434+
* already provides the *unwrapped* exception. This happened in two
1435+
* situations:
14481436
*
14491437
* - when automatically re-throwing an unhandled exception at the end of a
14501438
* `try..catch`, or
14511439
* - when throwing a maybe-JavaScriptException, with an explicit call to
14521440
* `runtime.package$.unwrapJavaScriptException(x)`.
1441+
*
1442+
* Fortunately, in both situations, the type of the `expr` is always
1443+
* `AnyType`. We can accurately use that test to know whether we need to
1444+
* apply the patch.
14531445
*/
14541446
private def throwArgumentHack8(expr: Tree)(implicit pos: Position): Tree = {
1455-
def unwrapFromThrowable(t: Tree): Tree =
1456-
UnaryOp(UnaryOp.UnwrapFromThrowable, t)
1457-
1458-
expr.tpe match {
1459-
case NullType =>
1460-
// Evaluate the expression then definitely run into an NPE UB
1461-
unwrapFromThrowable(expr)
1462-
1463-
case ClassType(_, _) =>
1464-
expr match {
1465-
case New(_, _, _) =>
1466-
// Common case (`throw new SomeException(...)`) that is known not to be `null`
1467-
expr
1468-
case VarRef(_) =>
1469-
/* Common case (explicit re-throw of the form `throw th`) where we don't need the IIFE.
1470-
* if (expr === null) unwrapFromThrowable(null) else expr
1471-
*/
1472-
If(BinaryOp(BinaryOp.===, expr, Null()), unwrapFromThrowable(Null()), expr)(AnyType)
1473-
case _ =>
1474-
/* General case where we need to avoid evaluating `expr` twice.
1475-
* ((x) => if (x === null) unwrapFromThrowable(null) else x)(expr)
1476-
*/
1477-
val x = LocalIdent(LocalName("x"))
1478-
val xParamDef = ParamDef(x, OriginalName.NoOriginalName, AnyType, mutable = false)
1479-
val xRef = xParamDef.ref
1480-
val closure = Closure(arrow = true, Nil, List(xParamDef), None, {
1481-
If(BinaryOp(BinaryOp.===, xRef, Null()), unwrapFromThrowable(Null()), xRef)(AnyType)
1482-
}, Nil)
1483-
JSFunctionApply(closure, List(expr))
1484-
}
1485-
1486-
case _ =>
1487-
// Do not transform expressions of other types, in particular `AnyType`
1488-
expr
1489-
}
1447+
if (expr.tpe == AnyType)
1448+
expr
1449+
else if (!expr.tpe.isNullable)
1450+
expr // no CheckNotNull needed; common case because of `throw new ...`
1451+
else
1452+
UnaryOp(UnaryOp.CheckNotNull, expr)
14901453
}
14911454

14921455
def readTrees(): List[Tree] =
@@ -1675,10 +1638,10 @@ object Serializers {
16751638
}
16761639

16771640
def arrayLength(t: Tree)(implicit pos: Position): Tree =
1678-
UnaryOp(UnaryOp.Array_length, t)
1641+
UnaryOp(UnaryOp.Array_length, UnaryOp(UnaryOp.CheckNotNull, t))
16791642

16801643
def getClass(t: Tree)(implicit pos: Position): Tree =
1681-
UnaryOp(UnaryOp.GetClass, t)
1644+
UnaryOp(UnaryOp.GetClass, UnaryOp(UnaryOp.CheckNotNull, t))
16821645

16831646
val jlClassRef = ClassRef(ClassClass)
16841647
val intArrayTypeRef = ArrayTypeRef(IntRef, 1)

‎linker/shared/src/test/scala/org/scalajs/linker/BackwardsCompatTest.scala

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,67 @@ class BackwardsCompatTest {
7575
test(classDefs, MainTestModuleInitializers)
7676
}
7777

78+
@Test
79+
def testThrowHackWithVariable_Issue5107(): AsyncResult = await {
80+
val Base64Class = ClassName("java.util.Base64")
81+
val DecoderClass = ClassName("java.util.Base64$Decoder")
82+
val ByteBufferClass = ClassName("java.nio.ByteBuffer")
83+
84+
val DecoderTypeRef = ClassRef(DecoderClass)
85+
val ByteBufferTypeRef = ClassRef(ByteBufferClass)
86+
val AB = ArrayTypeRef(ByteRef, 1)
87+
88+
val DecoderType = ClassType(DecoderClass, nullable = true)
89+
val ByteBufferType = ClassType(ByteBufferClass, nullable = true)
90+
91+
/* java.util.Base64.getDecoder().decode(java.nio.ByteBuffer.wrap(Array(65, 81, 73, 61)))
92+
* That is the only method I found in our javalib that contains a `throw e`,
93+
* as opposed to a `throw new ...`.
94+
*/
95+
val classDefs = Seq(
96+
mainTestClassDef(systemOutPrintln {
97+
Apply(
98+
EAF,
99+
ApplyStatic(EAF, Base64Class, m("getDecoder", Nil, DecoderTypeRef), Nil)(DecoderType),
100+
m("decode", List(ByteBufferTypeRef), ByteBufferTypeRef),
101+
List(
102+
ApplyStatic(EAF, ByteBufferClass, m("wrap", List(AB), ByteBufferTypeRef),
103+
List(ArrayValue(AB, List[Byte](65, 81, 73, 61).map(ByteLiteral(_)))))(ByteBufferType)
104+
)
105+
)(ByteBufferType)
106+
})
107+
)
108+
109+
test(classDefs, MainTestModuleInitializers)
110+
}
111+
112+
@Test
113+
def testArrayNewInstanceHacks_Issue5107(): AsyncResult = await {
114+
val ReflectArrayClass = ClassName("java.lang.reflect.Array")
115+
116+
val ClassClassRef = ClassRef(ClassClass)
117+
118+
val AI = ArrayTypeRef(IntRef, 1)
119+
120+
/* jlr.Array.newInstance(classOf[String], 5)
121+
* jlr.Array.newInstance(classOf[String], Array(5, 4))
122+
*/
123+
val classDefs = Seq(
124+
mainTestClassDef(Block(
125+
systemOutPrintln(
126+
ApplyStatic(EAF, ReflectArrayClass, m("newInstance", List(ClassClassRef, I), O),
127+
List(ClassOf(T), int(5)))(AnyType)
128+
),
129+
systemOutPrintln(
130+
ApplyStatic(EAF, ReflectArrayClass, m("newInstance", List(ClassClassRef, AI), O),
131+
List(ClassOf(T), ArrayValue(AI, List(int(5), int(4)))))(AnyType)
132+
)
133+
))
134+
)
135+
136+
test(classDefs, MainTestModuleInitializers)
137+
}
138+
78139
private def test(classDefs: Seq[ClassDef],
79140
moduleInitializers: Seq[ModuleInitializer]): Future[_] = {
80141
val classDefFiles = classDefs.map(MemClassDefIRFile(_))

0 commit comments

Comments
 (0)
0