Skip to content

Commit

Permalink
JacoDB produces incorrect 3-address code for IINC instruction #146
Browse files Browse the repository at this point in the history
  • Loading branch information
@lehvolk
lehvolk committed Aug 30, 2023
1 parent 576f852 commit 841eebb
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 60 deletions.
21 changes: 12 additions & 9 deletions jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ class RawInstListBuilder(
for ((variable, value) in assignments) {
if (value != frame[variable]) {
if (insn.isBranchingInst || insn.isTerminateInst) {
addInstruction(insn, JcRawAssignInst(method, value, frame[variable]!!), insnList.lastIndex)
insnList.addInst(insn, JcRawAssignInst(method, value, frame[variable]!!), insnList.lastIndex)
} else {
addInstruction(insn, JcRawAssignInst(method, value, frame[variable]!!))
insnList.addInst(insn, JcRawAssignInst(method, value, frame[variable]!!))

Check warning on line 257 in jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt

View check run for this annotation

Codecov / codecov/patch

jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt#L257 

Added line #L257 was not covered by tests
}
}
}
Expand All @@ -265,9 +265,9 @@ class RawInstListBuilder(
for ((variable, value) in assignments) {
if (value != frame.stack[variable]) {
if (insn.isBranchingInst || insn.isTerminateInst) {
insnList.add(insnList.lastIndex, JcRawAssignInst(method, value, frame.stack[variable]))
insnList.addInst(insn, JcRawAssignInst(method, value, frame.stack[variable]), insnList.lastIndex)

Check warning on line 268 in jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt

View check run for this annotation

Codecov / codecov/patch

jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt#L268 

Added line #L268 was not covered by tests
} else {
insnList += JcRawAssignInst(method, value, frame.stack[variable])
insnList.addInst(insn, JcRawAssignInst(method, value, frame.stack[variable]))

Check warning on line 270 in jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt

View check run for this annotation

Codecov / codecov/patch

jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt#L270 

Added line #L270 was not covered by tests
}
}
}
Expand Down Expand Up @@ -403,20 +403,23 @@ class RawInstListBuilder(
private fun instructionList(insn: AbstractInsnNode) = instructions.getOrPut(insn, ::mutableListOf)

private fun addInstruction(insn: AbstractInsnNode, inst: JcRawInst, index: Int? = null) {
instructionList(insn).addInst(insn, inst, index)
}

private fun MutableList<JcRawInst>.addInst(node: AbstractInsnNode, inst: JcRawInst, index: Int? = null) {

Check warning on line 409 in jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt

View check run for this annotation

Codecov / codecov/patch

jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/RawInstListBuilder.kt#L409 

Added line #L409 was not covered by tests
if (index != null) {
instructionList(insn).add(index, inst)
add(index, inst)
} else {
instructionList(insn).add(inst)
add(inst)
}
if (postfixInstructions.isNotEmpty()) {
when {
insn.isBranchingInst -> postfixInstructions.forEach {
instructionList(insn).add(0, it.value)
node.isBranchingInst -> postfixInstructions.forEach {
instructionList(node).add(0, it.value)
}

inst !is JcRawReturnInst -> postfixInstructions.forEach {
instructionList(insn).add(it.value)
instructionList(node).add(it.value)
}
}
postfixInstructions.clear()
Expand Down
104 changes: 57 additions & 47 deletions jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/BaseInstructionsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@ import org.jacodb.api.ext.isKotlin
import org.jacodb.api.ext.packageName
import org.jacodb.impl.bytecode.JcDatabaseClassWriter
import org.jacodb.impl.cfg.MethodNodeBuilder
import org.jacodb.impl.features.InMemoryHierarchy
import org.jacodb.impl.features.classpaths.StringConcatSimplifier
import org.jacodb.impl.features.hierarchyExt
import org.jacodb.testing.BaseTest
import org.jacodb.testing.WithDB
import org.junit.jupiter.api.Assertions
import org.objectweb.asm.ClassWriter
import org.objectweb.asm.util.CheckClassAdapter
Expand All @@ -37,68 +34,81 @@ import java.net.URLClassLoader
import java.nio.file.Files
import java.nio.file.Paths

abstract class BaseInstructionsTest: BaseTest() {

companion object : WithDB(InMemoryHierarchy, StringConcatSimplifier)
abstract class BaseInstructionsTest : BaseTest() {

private val target = Files.createTempDirectory("jcdb-temp")

val ext = runBlocking { cp.hierarchyExt() }

protected fun testClass(klass: JcClassOrInterface) = try {
val classNode = klass.asmNode()
classNode.methods = klass.declaredMethods.filter { it.enclosingClass == klass }.map {
if (it.isAbstract || it.name.contains("$\$forInline")) {
it.asmNode()
} else {
try {
protected fun testClass(klass: JcClassOrInterface) {
testAndLoadClass(klass, false)
}

protected fun testAndLoadClass(klass: JcClassOrInterface): Class<*> {
return testAndLoadClass(klass, true)!!
}

private fun testAndLoadClass(klass: JcClassOrInterface, loadClass: Boolean): Class<*>? {
try {
val classNode = klass.asmNode()
classNode.methods = klass.declaredMethods.filter { it.enclosingClass == klass }.map {
if (it.isAbstract || it.name.contains("$\$forInline")) {
it.asmNode()
} else {
try {
// val oldBody = it.body()
// println()
// println("Old body: ${oldBody.print()}")
val instructionList = it.rawInstList
it.instList.forEachIndexed { index, inst ->
Assertions.assertEquals(index, inst.location.index, "indexes not matched for $it at $index")
}
val instructionList = it.rawInstList
it.instList.forEachIndexed { index, inst ->
Assertions.assertEquals(index, inst.location.index, "indexes not matched for $it at $index")
}
// println("Instruction list: $instructionList")
val graph = it.flowGraph()
if (!it.enclosingClass.isKotlin) {
graph.instructions.forEach {
Assertions.assertTrue(it.lineNumber > 0, "$it should have line number")
val graph = it.flowGraph()
if (!it.enclosingClass.isKotlin) {
val methodMsg = "$it should have line number"
graph.instructions.forEach {
Assertions.assertTrue(it.lineNumber > 0, methodMsg)
}
}
}
graph.applyAndGet(OverridesResolver(ext)) {}
JcGraphChecker(it, graph).check()
graph.applyAndGet(OverridesResolver(ext)) {}
JcGraphChecker(it, graph).check()
// println("Graph: $graph")
// graph.view("/usr/bin/dot", "/usr/bin/firefox", false)
// graph.blockGraph().view("/usr/bin/dot", "/usr/bin/firefox")
val newBody = MethodNodeBuilder(it, instructionList).build()
val newBody = MethodNodeBuilder(it, instructionList).build()
// println("New body: ${newBody.print()}")
// println()
newBody
} catch (e: Exception) {
throw IllegalStateException("error handling $it", e)
newBody
} catch (e: Exception) {
throw IllegalStateException("error handling $it", e)
}

}
}
val cw = JcDatabaseClassWriter(cp, ClassWriter.COMPUTE_FRAMES)
val checker = CheckClassAdapter(cw)
try {
classNode.accept(checker)
} catch (ex: Throwable) {
println(ex)
}
val targetDir = target.resolve(klass.packageName.replace('.', '/'))
val targetFile = targetDir.resolve("${klass.simpleName}.class").toFile().also {
it.parentFile?.mkdirs()
}
targetFile.writeBytes(cw.toByteArray())
if (loadClass) {

val cp = listOf(target.toUri().toURL()) + System.getProperty("java.class.path")
.split(File.pathSeparatorChar)
.map { Paths.get(it).toUri().toURL() }
val allClassLoader = URLClassLoader(cp.toTypedArray(), null)
return allClassLoader.loadClass(klass.name)
}
} catch (e: NoClassInClasspathException) {
System.err.println(e.localizedMessage)
}
val cw = JcDatabaseClassWriter(cp, ClassWriter.COMPUTE_FRAMES)
val checker = CheckClassAdapter(cw)
try {
classNode.accept(checker)
} catch (ex: Throwable) {
println(ex)
}
val targetDir = target.resolve(klass.packageName.replace('.', '/'))
val targetFile = targetDir.resolve("${klass.simpleName}.class").toFile().also {
it.parentFile?.mkdirs()
}
targetFile.writeBytes(cw.toByteArray())

val cp = listOf(target.toUri().toURL()) + System.getProperty("java.class.path").split(File.pathSeparatorChar)
.map { Paths.get(it).toUri().toURL() }
val allClassLoader = URLClassLoader(cp.toTypedArray(), null)
allClassLoader.loadClass(klass.name)
} catch (e: NoClassInClasspathException) {
System.err.println(e.localizedMessage)
return null
}
}
1 change: 1 addition & 0 deletions jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/IRTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.jacodb.testing.cfg


import org.jacodb.api.*
import org.jacodb.api.cfg.*
import org.jacodb.api.ext.HierarchyExtension
Expand Down
8 changes: 4 additions & 4 deletions jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/InstructionsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class InstructionsTest : BaseInstructionsTest() {
fun `iinc should work`() {
val clazz = cp.findClass<Incrementation>()

val javaClazz = testClass(clazz) as Class<*>
val javaClazz = testAndLoadClass(clazz)
val method = javaClazz.methods.first { it.name == "iinc" }
val res = method.invoke(null, 0)
assertEquals(0, res)
Expand All @@ -282,7 +282,7 @@ class InstructionsTest : BaseInstructionsTest() {
fun `iinc arrayIntIdx should work`() {
val clazz = cp.findClass<Incrementation>()

val javaClazz = testClass(clazz) as Class<*>
val javaClazz = testAndLoadClass(clazz)
val method = javaClazz.methods.first { it.name == "iincArrayIntIdx" }
val res = method.invoke(null)
assertArrayEquals(intArrayOf(1, 0, 2), res as IntArray)
Expand All @@ -292,7 +292,7 @@ class InstructionsTest : BaseInstructionsTest() {
fun `iinc arrayByteIdx should work`() {
val clazz = cp.findClass<Incrementation>()

val javaClazz = testClass(clazz) as Class<*>
val javaClazz = testAndLoadClass(clazz)
val method = javaClazz.methods.first { it.name == "iincArrayByteIdx" }
val res = method.invoke(null)
assertArrayEquals(intArrayOf(1, 0, 2), res as IntArray)
Expand All @@ -302,7 +302,7 @@ class InstructionsTest : BaseInstructionsTest() {
fun `iinc for`() {
val clazz = cp.findClass<Incrementation>()

val javaClazz = testClass(clazz) as Class<*>
val javaClazz = testAndLoadClass(clazz)
val method = javaClazz.methods.first { it.name == "iincFor" }
val res = method.invoke(null)
assertArrayEquals(intArrayOf(0, 1, 2, 3, 4), res as IntArray)
Expand Down

0 comments on commit 841eebb

Please sign in to comment.