Skip to content

Commit 0b432f9

Browse files
committed
SI-8601 Avoid over-eager optimization of LOAD_FIELD
It can NPE or trigger static class initilization, we can't elimiate it without changing semantics.
1 parent ee611cd commit 0b432f9

File tree

4 files changed

+18
-6
lines changed

4 files changed

+18
-6
lines changed

src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,10 @@ abstract class ClosureElimination extends SubComponent {
5656
case (BOX(t1), UNBOX(t2)) if (t1 == t2) =>
5757
Some(Nil)
5858

59-
case (LOAD_FIELD(sym, isStatic), DROP(_)) if !sym.hasAnnotation(definitions.VolatileAttr) =>
60-
if (isStatic)
61-
Some(Nil)
62-
else
63-
Some(DROP(REFERENCE(definitions.ObjectClass)) :: Nil)
59+
// Can't eliminate (LOAD_FIELD, DROP) without eliding side effects:
60+
// - static class initialization
61+
// - NPE if the receiver is null
62+
// We could replace the LOAD/DROP with a null check to preserve semantics.
6463

6564
case _ => None
6665
}

src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ abstract class DeadCodeElimination extends SubComponent {
167167
set += idx
168168
localStores(key) = set
169169

170-
case RETURN(_) | JUMP(_) | CJUMP(_, _, _, _) | CZJUMP(_, _, _, _) | STORE_FIELD(_, _) |
170+
case RETURN(_) | JUMP(_) | CJUMP(_, _, _, _) | CZJUMP(_, _, _, _) | STORE_FIELD(_, _) | LOAD_FIELD(_, _) | // Why LOAD_FIELD? It can NPE!
171171
THROW(_) | LOAD_ARRAY_ITEM(_) | STORE_ARRAY_ITEM(_) | SCOPE_ENTER(_) | SCOPE_EXIT(_) | STORE_THIS(_) |
172172
LOAD_EXCEPTION(_) | SWITCH(_, _) | MONITOR_ENTER() | MONITOR_EXIT() | CHECK_CAST(_) =>
173173
moveToWorkList()

test/files/run/t8601c.flags

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-optimize

test/files/run/t8601c.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
object Test {
2+
def loadField(x: scala.runtime.IntRef): Unit = x.elem
3+
def storeField(x: scala.runtime.IntRef): Unit = x.elem = 42
4+
5+
def check(x: => Any) = try { x; sys.error("failed to throw NPE!") } catch { case _: NullPointerException => }
6+
7+
def main(args: Array[String]) {
8+
check(loadField(null)) // bug: did not NPE under -Ydead-code
9+
check(storeField(null))
10+
11+
}
12+
}

0 commit comments

Comments
 (0)