SI-9403 fix ICodeReader for negative BIPUSH / SIPUSH values

The byte value of a BIPUSH instruction and the (byte1 << 8) | byte2 value of a SIPUSH instruction are signed, see [1] and [2]. Similar for the increment value of IINC [3]. [1] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.bipush [2] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.sipush [3] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-6.html#jvms-6.5.iinc
adriaanm · Jul 24, 2015 · 2678d34 · 2678d34
1 parent 0e9525a
commit 2678d34
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 3 deletions.
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -84,6 +84,9 @@ abstract class ClassfileParser {
   protected final def u2(): Int = in.nextChar.toInt
   protected final def u4(): Int = in.nextInt
 
+  protected final def s1(): Int = in.nextByte.toInt // sign-extend the byte to int
+  protected final def s2(): Int = (in.nextByte.toInt << 8) | u1 // sign-extend and shift the first byte, or with the unsigned second byte
+
   private def readInnerClassFlags() = readClassFlags()
   private def readClassFlags()      = JavaAccFlags classFlags u2
   private def readMethodFlags()     = JavaAccFlags methodFlags u2

diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
@@ -326,8 +326,8 @@ abstract class ICodeReader extends ClassfileParser {
         case JVM.dconst_0    => code emit CONSTANT(Constant(0.0))
         case JVM.dconst_1    => code emit CONSTANT(Constant(1.0))
 
-        case JVM.bipush      => code.emit(CONSTANT(Constant(u1))); size += 1
-        case JVM.sipush      => code.emit(CONSTANT(Constant(u2))); size += 2
+        case JVM.bipush      => code.emit(CONSTANT(Constant(s1))); size += 1
+        case JVM.sipush      => code.emit(CONSTANT(Constant(s2))); size += 2
         case JVM.ldc         => code.emit(CONSTANT(pool.getConstant(u1))); size += 1
         case JVM.ldc_w       => code.emit(CONSTANT(pool.getConstant(u2))); size += 2
         case JVM.ldc2_w      => code.emit(CONSTANT(pool.getConstant(u2))); size += 2
@@ -466,7 +466,7 @@ abstract class ICodeReader extends ClassfileParser {
           size += 2
           val local = code.getLocal(u1, INT)
           code.emit(LOAD_LOCAL(local))
-          code.emit(CONSTANT(Constant(u1)))
+          code.emit(CONSTANT(Constant(s1)))
           code.emit(CALL_PRIMITIVE(Arithmetic(ADD, INT)))
           code.emit(STORE_LOCAL(local))
 

diff --git a/test/files/run/t9403.flags b/test/files/run/t9403.flags
@@ -0,0 +1 @@
+-Ybackend:GenASM -optimize
diff --git a/test/files/run/t9403/C_1.scala b/test/files/run/t9403/C_1.scala
@@ -0,0 +1,5 @@
+package p
+class C {
+  @inline final def f(x: Int): Long = 10L / (if (x < 0) -2 else 2)
+  @inline final def g(x: Int): Long = 3000L / (if (x < 0) -300 else 300)
+}
diff --git a/test/files/run/t9403/Test_2.scala b/test/files/run/t9403/Test_2.scala
@@ -0,0 +1,29 @@
+import p.C
+import scala.tools.asm.Opcodes
+import scala.tools.partest.BytecodeTest
+import scala.tools.partest.ASMConverters._
+
+
+object Test extends BytecodeTest {
+  def foo(c: C, x: Int) = c.f(x)
+  def goo(c: C, x: Int) = c.g(x)
+
+  def has(i: Instruction, c: String, m: String) = {
+    val cls = loadClassNode(c)
+    val mth = convertMethod(getMethod(cls, m))
+    assert(mth.instructions.contains(i))
+  }
+
+  def show(): Unit = {
+    assert(foo(new C, -2) == -5L)
+    assert(goo(new C, -2) == -10L)
+
+    val bipush2 = IntOp(Opcodes.BIPUSH, -2)
+    has(bipush2, "p.C", "f")
+    has(bipush2, "Test$", "foo")
+
+    val sipush300 = IntOp(Opcodes.SIPUSH, -300)
+    has(sipush300, "p.C", "g")
+    has(sipush300, "Test$", "goo")
+  }
+}