Skip to content

Commit

Permalink
Fix unknown classes fields (#260)
Browse files Browse the repository at this point in the history
[jacodb-core] Force use of UnknownClassMethodsAndFields conditionally 

If the UnknownClasses classpath feature is configured to be used, then
force use of the UnknownClassMethodsAndFields feature as well.

[jacodb-core] Run IRTest with the UnknownClasses feature

Added dependency on the commons-compress 1.21 which requires
the UnknownClasses feature.

[jacodb-core] Avoid using `try {} catch (e: Exception) {}`

[jacodb-core] Add and use JcClasspath.registeredLocationIds

[jacodb-core] Run ClassesTest with in-RAM backend
  • Loading branch information
Saloed authored Aug 14, 2024
1 parent 3661d03 commit fce4d81
Show file tree
Hide file tree
Showing 17 changed files with 79 additions and 46 deletions.
7 changes: 7 additions & 0 deletions buildSrc/src/main/kotlin/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ object Versions {

// libs for tests only
const val jgit_test_only_version = "5.9.0.202009080501-r"
const val commons_compress_test_only_version = "1.21"
}

fun dep(group: String, name: String, version: String): String = "$group:$name:$version"
Expand Down Expand Up @@ -332,6 +333,12 @@ object Libs {
name = "org.eclipse.jgit",
version = Versions.jgit_test_only_version
)

val commons_compress_test_only_lib = dep(
group = "org.apache.commons",
name = "commons-compress",
version = Versions.commons_compress_test_only_version
)
}

object Plugins {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface JcClasspath : Closeable, CommonProject {
/** locations of this classpath */
val locations: List<JcByteCodeLocation>
val registeredLocations: List<RegisteredLocation>
val registeredLocationIds: Set<Long>
val features: List<JcClasspathFeature>?

/**
Expand Down
1 change: 1 addition & 0 deletions jacodb-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ dependencies {
testFixturesImplementation(Libs.jetbrains_annotations)
testFixturesImplementation(Libs.kotlinx_coroutines_core)
testFixturesImplementation(Libs.jgit_test_only_lib)
testFixturesImplementation(Libs.commons_compress_test_only_lib)
}

tasks {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class JcClasspathImpl(

override val locations: List<JcByteCodeLocation> = locationsRegistrySnapshot.locations.mapNotNull { it.jcLocation }
override val registeredLocations: List<RegisteredLocation> = locationsRegistrySnapshot.locations

override val registeredLocationIds: Set<Long> = locationsRegistrySnapshot.ids
private val classpathVfs = ClasspathVfs(globalClassVFS, locationsRegistrySnapshot)
private val featuresChain = run {
val strictFeatures = features.filter { it !is UnknownClasses }
Expand Down
41 changes: 32 additions & 9 deletions jacodb-core/src/main/kotlin/org/jacodb/impl/JcDatabaseImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,31 @@

package org.jacodb.impl

import kotlinx.coroutines.*
import org.jacodb.api.jvm.*
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.joinAll
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.jacodb.api.jvm.JavaVersion
import org.jacodb.api.jvm.JcByteCodeLocation
import org.jacodb.api.jvm.JcClasspath
import org.jacodb.api.jvm.JcClasspathFeature
import org.jacodb.api.jvm.JcDatabase
import org.jacodb.api.jvm.JcDatabasePersistence
import org.jacodb.api.jvm.JcFeature
import org.jacodb.api.jvm.RegisteredLocation
import org.jacodb.impl.features.classpaths.ClasspathCache
import org.jacodb.impl.features.classpaths.KotlinMetadata
import org.jacodb.impl.features.classpaths.MethodInstructionsFeature
import org.jacodb.impl.features.classpaths.UnknownClassMethodsAndFields
import org.jacodb.impl.features.classpaths.UnknownClasses
import org.jacodb.impl.fs.JavaRuntime
import org.jacodb.impl.fs.asByteCodeLocation
import org.jacodb.impl.fs.filterExisting
Expand Down Expand Up @@ -76,14 +96,17 @@ class JcDatabaseImpl(
}

private fun List<JcClasspathFeature>?.appendBuiltInFeatures(): List<JcClasspathFeature> {
if (this != null && any { it is ClasspathCache }) {
return this + listOf(KotlinMetadata, MethodInstructionsFeature(settings.keepLocalVariableNames))
return mutableListOf<JcClasspathFeature>().also { result ->
result += orEmpty()
if (!result.any { it is ClasspathCache }) {
result += ClasspathCache(settings.cacheSettings)
}
result += KotlinMetadata
result += MethodInstructionsFeature(settings.keepLocalVariableNames)
if (result.any { it is UnknownClasses } && !result.any { it is UnknownClassMethodsAndFields }) {
result += UnknownClassMethodsAndFields
}
}
return listOf(
ClasspathCache(settings.cacheSettings),
KotlinMetadata,
MethodInstructionsFeature(settings.keepLocalVariableNames)
) + orEmpty()
}

override suspend fun classpath(dirOrJars: List<File>, features: List<JcClasspathFeature>?): JcClasspath {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ open class LocationsRegistrySnapshot(
val locations: List<RegisteredLocation>
) : Closeable {

val ids = locations.map { it.id }.toHashSet()
val ids = locations.mapTo(mutableSetOf()) { it.id }

override fun close() = registry.close(this)

Expand Down
19 changes: 7 additions & 12 deletions jacodb-core/src/main/kotlin/org/jacodb/impl/bytecode/KMetadata.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,15 @@ val JcMethod.kmConstructor: KmConstructor?

val JcParameter.kmParameter: KmValueParameter?
get() {
try {
method.kmFunction?.let {
// Shift needed to properly handle extension functions
val shift = if (it.receiverParameterType != null) 1 else 0
method.kmFunction?.let {
// Shift needed to properly handle extension functions
val shift = if (it.receiverParameterType != null) 1 else 0

// index - shift could be out of bounds if generated JVM parameter is fictive
// E.g., see how extension functions and coroutines are compiled
return it.valueParameters.getOrNull(index - shift)
}

return method.kmConstructor?.valueParameters?.get(index)
} catch (e: Exception) {
return null
// index - shift could be out of bounds if generated JVM parameter is fictive
// E.g., see how extension functions and coroutines are compiled
return it.valueParameters.getOrNull(index - shift)
}
return method.kmConstructor?.valueParameters?.getOrNull(index)
}

// If parameter is a receiver parameter, it doesn't have KmValueParameter instance, but we still can get KmType for it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ object Builders : JcFeature<Set<String>, BuildersResponse> {
}

fun syncQuery(classpath: JcClasspath, req: Set<String>): Sequence<BuildersResponse> {
val locationIds = classpath.registeredLocations.map { it.id }
val locationIds = classpath.registeredLocationIds
val persistence = classpath.db.persistence
return sequence {
val result = persistence.read { context ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ suspend fun JcClasspath.duplicatedClasses(): Map<String, Int> {
sqlAction = { jooq ->
jooq.select(SYMBOLS.NAME, DSL.count(SYMBOLS.NAME)).from(CLASSES)
.join(SYMBOLS).on(SYMBOLS.ID.eq(CLASSES.NAME))
.where(CLASSES.LOCATION_ID.`in`(registeredLocations.map { it.id }))
.where(CLASSES.LOCATION_ID.`in`(registeredLocationIds))
.groupBy(SYMBOLS.NAME)
.having(DSL.count(SYMBOLS.NAME).greaterThan(1))
.fetch()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ suspend fun JcClasspath.hierarchyExt(): HierarchyExtension {
fun JcClasspath.asyncHierarchyExt(): Future<HierarchyExtension> = GlobalScope.future { hierarchyExt() }

internal fun JcClasspath.allClassesExceptObject(context: JCDBContext, direct: Boolean): Sequence<ClassSource> {
val locationIds = registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = registeredLocationIds
return context.execute(
sqlAction = { jooq ->
if (direct) {
Expand Down Expand Up @@ -162,7 +162,7 @@ private class HierarchyExtensionERS(cp: JcClasspath) : HierarchyExtensionBase(cp
if (name == JAVA_OBJECT) {
cp.allClassesExceptObject(context, !entireHierarchy)
} else {
val locationIds = cp.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = cp.registeredLocationIds
val nameId = name.asSymbolId(persistence.symbolInterner)
if (entireHierarchy) {
entireHierarchy(txn, nameId, mutableSetOf())
Expand Down Expand Up @@ -251,7 +251,7 @@ private fun JcClasspath.subClasses(name: String, entireHierarchy: Boolean): Sequ
if (name == JAVA_OBJECT) {
allClassesExceptObject(context, !entireHierarchy)
} else {
val locationIds = registeredLocations.joinToString(", ") { it.id.toString() }
val locationIds = registeredLocationIds.joinToString(", ") { it.toString() }
val dslContext = context.dslContext
BatchedSequence(defaultBatchSize) { offset, batchSize ->
val query = when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ object InMemoryHierarchy : JcFeature<InMemoryHierarchyReq, ClassSource> {
}
}

val locationIds = classpath.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = classpath.registeredLocationIds
val classSymbolId = persistence.findSymbolId(req.name)

val allSubclasses = hashSetOf<Long>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ object Usages : JcFeature<UsageFeatureRequest, UsageFeatureResponse> {
}

fun syncQuery(classpath: JcClasspath, req: UsageFeatureRequest): Sequence<UsageFeatureResponse> {
val locationIds = classpath.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = classpath.registeredLocationIds
val persistence = classpath.db.persistence
val symbolInterner = persistence.symbolInterner
val name = req.methodName ?: req.field ?: throw IllegalArgumentException("Callee name should be specified")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ class SQLitePersistenceImpl(

private val JcClasspath.clause: Condition
get() {
val ids = registeredLocations.map { it.id }
return CLASSES.LOCATION_ID.`in`(ids)
return CLASSES.LOCATION_ID.`in`(registeredLocationIds)
}

private fun JcDatabase.classSources(clause: Condition, single: Boolean = false): List<ClassSource> = read { context ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class ErsPersistenceImpl(
}

private fun findClassSourcesImpl(context: JCDBContext, cp: JcClasspath, fullName: String): Sequence<ClassSource> {
val ids = cp.registeredLocations.mapTo(hashSetOf()) { it.id }
val ids = cp.registeredLocationIds
return context.txn.find("Class", "nameId", findSymbolId(fullName).compressed)
.asSequence().filter { it.getCompressed<Long>("locationId") in ids }
.map { it.toClassSource(cp.db, fullName) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import org.junit.jupiter.api.Test

class ClassesTest : DatabaseEnvTest() {

companion object : WithGlobalDB()
companion object : WithGlobalRAMDB()

override val cp: JcClasspath = runBlocking { db.classpath(allClasspath) }

Expand Down
23 changes: 11 additions & 12 deletions jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/IRTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@

package org.jacodb.testing.cfg

import org.jacodb.api.common.cfg.CommonAssignInst
import org.jacodb.api.common.cfg.CommonCallInst
import org.jacodb.api.common.cfg.CommonExpr
import org.jacodb.api.common.cfg.CommonGotoInst
import org.jacodb.api.common.cfg.CommonIfInst
import org.jacodb.api.common.cfg.CommonInst
import org.jacodb.api.common.cfg.CommonReturnInst
import org.jacodb.api.jvm.JavaVersion
import org.jacodb.api.jvm.JcClassType
import org.jacodb.api.jvm.JcMethod
Expand Down Expand Up @@ -61,10 +54,12 @@ import org.jacodb.impl.cfg.Simplifier
import org.jacodb.impl.cfg.util.ExprMapper
import org.jacodb.impl.features.classpaths.ClasspathCache
import org.jacodb.impl.features.classpaths.StringConcatSimplifier
import org.jacodb.impl.features.classpaths.UnknownClasses
import org.jacodb.impl.fs.JarLocation
import org.jacodb.testing.WithDB
import org.jacodb.testing.WithRAMDB
import org.jacodb.testing.asmLib
import org.jacodb.testing.commonsCompressLib
import org.jacodb.testing.guavaLib
import org.jacodb.testing.jgitLib
import org.jacodb.testing.kotlinStdLib
Expand All @@ -73,7 +68,6 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import java.io.File
Expand All @@ -95,8 +89,8 @@ class OverridesResolver(
return methods.firstOrNull { typedMethod ->
val jcMethod = typedMethod.method
jcMethod.name == name &&
jcMethod.returnType.typeName == returnType.typeName &&
jcMethod.parameters.map { param -> param.type.typeName } == argTypes.map { it.typeName }
jcMethod.returnType.typeName == returnType.typeName &&
jcMethod.parameters.map { param -> param.type.typeName } == argTypes.map { it.typeName }
} ?: error("Could not find a method with correct signature")
}

Expand Down Expand Up @@ -350,6 +344,11 @@ abstract class IRTest : BaseInstructionsTest() {
runAlongLib(jgitLib)
}

@Test
fun `get ir of commons compress`() {
runAlongLib(commonsCompressLib)
}

@Test
fun `get ir of asm`() {
runAlongLib(asmLib, muteGraphChecker = true)
Expand Down Expand Up @@ -392,10 +391,10 @@ abstract class IRTest : BaseInstructionsTest() {

class IRSqlTest : IRTest() {

companion object : WithDB(StringConcatSimplifier)
companion object : WithDB(StringConcatSimplifier, UnknownClasses)
}

class IRRAMTest : IRTest() {

companion object : WithRAMDB(StringConcatSimplifier)
companion object : WithRAMDB(StringConcatSimplifier, UnknownClasses)
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ val jgitLib: File
}
}

val commonsCompressLib: File
get() {
val commonsCompressUrl = classpath.first { it.contains("commons-compress-") }
return File(commonsCompressUrl).also {
Assertions.assertTrue(it.isFile && it.exists())
}
}

val asmLib: File
get() {
val asmUrl = classpath.first { it.contains("${File.separator}asm${File.separator}") }
Expand Down

0 comments on commit fce4d81

Please sign in to comment.