Skip to content

Add support for classes with hidden fields #563 #592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
24 changes: 8 additions & 16 deletions utbot-core/src/main/kotlin/org/utbot/common/KClassUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,14 @@ import java.lang.reflect.Method

val Class<*>.packageName: String get() = `package`?.name?:""

fun Class<*>.findField(name: String): Field =
findFieldOrNull(name) ?: error("Can't find field $name in $this")

fun Class<*>.findFieldOrNull(name: String): Field? = generateSequence(this) { it.superclass }
.mapNotNull {
try {
it.getField(name)
} catch (e: NoSuchFieldException) {
try {
it.getDeclaredField(name)
} catch (e: NoSuchFieldException) {
null
}
}
}
.firstOrNull()
/**
* Returns the Field that would be used by compiler when you try to direct access [fieldName] from code, i.e.
* when you write `${this.name}.$fieldName`.
* If there is no field named [fieldName] in the class, null is returned
*/
fun Class<*>.findDirectAccessedFieldOrNull(fieldName: String): Field? = generateSequence(this) { it.superclass }
.flatMap { it.declaredFields.asSequence() }
.firstOrNull { it.name == fieldName }

fun Method.invokeCatching(obj: Any?, args: List<Any?>) = try {
Result.success(invoke(obj, *args.toTypedArray()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import org.utbot.framework.plugin.api.util.charClassId
import org.utbot.framework.plugin.api.util.constructor
import org.utbot.framework.plugin.api.util.doubleClassId
import org.utbot.framework.plugin.api.util.executableId
import org.utbot.framework.plugin.api.util.findFieldOrNull
import org.utbot.framework.plugin.api.util.field
import org.utbot.framework.plugin.api.util.floatClassId
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.intClassId
Expand All @@ -30,6 +30,7 @@ import org.utbot.framework.plugin.api.util.jClass
import org.utbot.framework.plugin.api.util.longClassId
import org.utbot.framework.plugin.api.util.method
import org.utbot.framework.plugin.api.util.primitiveTypeJvmNameOrNull
import org.utbot.framework.plugin.api.util.safeField
import org.utbot.framework.plugin.api.util.shortClassId
import org.utbot.framework.plugin.api.util.toReferenceTypeBytecodeSignature
import org.utbot.framework.plugin.api.util.voidClassId
Expand Down Expand Up @@ -347,7 +348,7 @@ data class UtCompositeModel(
if (fields.isNotEmpty()) {
append(" ")
append(fields.entries.joinToString(", ", "{", "}") { (field, value) ->
if (value.classId != classId || value.isNull()) "${field.name}: $value" else "${field.name}: not evaluated"
if (value.classId != classId || value.isNull()) "(${field.declaringClass}) ${field.name}: $value" else "${field.name}: not evaluated"
}) // TODO: here we can get an infinite recursion if we have cyclic dependencies.
}
if (mocks.isNotEmpty()) {
Expand Down Expand Up @@ -519,6 +520,7 @@ data class UtDirectSetFieldModel(
val fieldId: FieldId,
val fieldModel: UtModel,
) : UtStatementModel(instance) {

override fun toString(): String = withToStringThreadLocalReentrancyGuard {
val modelRepresentation = when (fieldModel) {
is UtAssembleModel -> fieldModel.modelName
Expand Down Expand Up @@ -855,7 +857,7 @@ open class FieldId(val declaringClass: ClassId, val name: String) {
return result
}

override fun toString() = declaringClass.findFieldOrNull(name).toString()
override fun toString() = safeField.toString()
}

inline fun <T> withReflection(block: () -> T): T {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.utbot.framework.plugin.api.util

import org.utbot.common.findFieldOrNull
import org.utbot.framework.plugin.api.BuiltinClassId
import org.utbot.framework.plugin.api.BuiltinMethodId
import org.utbot.framework.plugin.api.ClassId
Expand Down Expand Up @@ -286,9 +285,17 @@ val ClassId.isMap: Boolean
val ClassId.isIterableOrMap: Boolean
get() = isIterable || isMap

fun ClassId.findFieldOrNull(fieldName: String): Field? = jClass.findFieldOrNull(fieldName)
fun ClassId.findFieldByIdOrNull(fieldId: FieldId): Field? {
if (isNotSubtypeOf(fieldId.declaringClass)) {
return null
}

return fieldId.safeField
}

fun ClassId.hasField(fieldName: String): Boolean = findFieldOrNull(fieldName) != null
fun ClassId.hasField(fieldId: FieldId): Boolean {
return findFieldByIdOrNull(fieldId) != null
}

fun ClassId.defaultValueModel(): UtModel = when (this) {
intClassId -> UtPrimitiveModel(0)
Expand All @@ -303,11 +310,11 @@ fun ClassId.defaultValueModel(): UtModel = when (this) {
}

// FieldId utils
val FieldId.safeField: Field?
get() = declaringClass.jClass.declaredFields.firstOrNull { it.name == name }

// TODO: maybe cache it somehow in the future
val FieldId.field: Field
get() = declaringClass.jClass.declaredFields.firstOrNull { it.name == name }
?: error("Field $name is not found in class ${declaringClass.jClass.name}")
get() = safeField ?: error("Field $name is not found in class ${declaringClass.jClass.name}")

// https://docstore.mik.ua/orelly/java-ent/jnut/ch03_13.htm
val FieldId.isInnerClassEnclosingClassReference: Boolean
Expand Down
8 changes: 5 additions & 3 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Hierarchy.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ class Hierarchy(private val typeRegistry: TypeRegistry) {
type as? RefType ?: error("$type is not a refType")

val realType = typeRegistry.findRealType(type) as RefType
val realFieldDeclaringType = typeRegistry.findRealType(field.declaringClass.type) as RefType

val ancestorType = ancestors(realType.sootClass.id).lastOrNull { it.declaresField(field.subSignature) }?.type
?: error("No such field ${field.subSignature} found in ${realType.sootClass.name}")
return ChunkId("$ancestorType", field.name)
if (realFieldDeclaringType.sootClass !in ancestors(realType.sootClass.id)) {
error("No such field ${field.subSignature} found in ${realType.sootClass.name}")
}
return ChunkId("$realFieldDeclaringType", field.name)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ sealed class UtAbstractStringBuilderWrapper(className: String) : BaseOverriddenW

val arrayValuesChunkId = typeRegistry.arrayChunkId(charArrayType)

val valuesFieldChunkId = hierarchy.chunkIdForField(utStringClass.type, overriddenClass.valueField)
val valuesFieldChunkId = hierarchy.chunkIdForField(utStringClass.type, utStringClass.valueField)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? Why?

Moreover, if it is true, do we write in this field in all other places in the wrapper? Not only in the resolver

val valuesArrayAddrDescriptor = MemoryChunkDescriptor(valuesFieldChunkId, wrapper.type, charType)
val valuesArrayAddr = findArray(valuesArrayAddrDescriptor, MemoryState.CURRENT).select(wrapper.addr)

Expand Down
10 changes: 5 additions & 5 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import kotlinx.collections.immutable.toPersistentMap
import kotlinx.collections.immutable.toPersistentSet
import org.utbot.common.WorkaroundReason.HACK
import org.utbot.common.WorkaroundReason.REMOVE_ANONYMOUS_CLASSES
import org.utbot.common.findField
import org.utbot.common.unreachableBranch
import org.utbot.common.withAccessibility
import org.utbot.common.workaround
Expand Down Expand Up @@ -90,8 +89,9 @@ import org.utbot.framework.plugin.api.MethodId
import org.utbot.framework.plugin.api.UtMethod
import org.utbot.framework.plugin.api.classId
import org.utbot.framework.plugin.api.id
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.field
import org.utbot.framework.plugin.api.util.jClass
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.signature
import org.utbot.framework.plugin.api.util.utContext
import org.utbot.framework.util.executableId
Expand Down Expand Up @@ -582,7 +582,7 @@ class Traverser(
declaringClass: SootClass,
stmt: Stmt
): SymbolicStateUpdate {
val concreteValue = extractConcreteValue(field, declaringClass)
val concreteValue = extractConcreteValue(field)
val (symbolicResult, symbolicStateUpdate) = toMethodResult(concreteValue, field.type)
val symbolicValue = (symbolicResult as SymbolicSuccess).value

Expand Down Expand Up @@ -634,12 +634,12 @@ class Traverser(
// Some fields are inaccessible with reflection, so we have to instantiate it by ourselves.
// Otherwise, extract it from the class.
// TODO JIRA:1593
private fun extractConcreteValue(field: SootField, declaringClass: SootClass): Any? =
private fun extractConcreteValue(field: SootField): Any? =
when (field.signature) {
SECURITY_FIELD_SIGNATURE -> SecurityManager()
FIELD_FILTER_MAP_FIELD_SIGNATURE -> mapOf(Reflection::class to arrayOf("fieldFilterMap", "methodFilterMap"))
METHOD_FILTER_MAP_FIELD_SIGNATURE -> emptyMap<Class<*>, Array<String>>()
else -> declaringClass.id.jClass.findField(field.name).let { it.withAccessibility { it.get(null) } }
else -> field.fieldId.field.let { it.withAccessibility { it.get(null) } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously, a problem with naming: field.fieldId.field

Moreover, now we have unused declaringClass variable

Copy link
Collaborator Author

@volivan239 volivan239 Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just rewrote using declaringClass, looks better now for me.

UPD. Rolled back, problem still exists

}

private fun isStaticInstanceInMethodResult(id: ClassId, methodResult: MethodResult?) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ class TypeResolver(private val typeRegistry: TypeRegistry, private val hierarchy
hierarchy
.ancestors(type.sootClass.id)
.flatMap { it.fields }
.asReversed() // to take fields of the farthest parent in the distinctBy
.distinctBy { it.name } // TODO we lose hidden fields here JIRA:315
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.utbot.engine

import org.utbot.common.findField
import org.utbot.common.findFieldOrNull
import org.utbot.common.invokeCatching
import org.utbot.common.withAccessibility
import org.utbot.framework.plugin.api.ClassId
Expand Down Expand Up @@ -37,6 +35,7 @@ import org.utbot.framework.plugin.api.UtValueExecutionState
import org.utbot.framework.plugin.api.UtVoidModel
import org.utbot.framework.plugin.api.isMockModel
import org.utbot.framework.plugin.api.util.constructor
import org.utbot.framework.plugin.api.util.field
import org.utbot.framework.plugin.api.util.jClass
import org.utbot.framework.plugin.api.util.method
import org.utbot.framework.plugin.api.util.utContext
Expand Down Expand Up @@ -212,9 +211,8 @@ class ValueConstructor {
val classInstance = javaClass.anyInstance
constructedObjects[model] = classInstance

model.fields.forEach { (field, fieldModel) ->
val declaredField =
javaClass.findFieldOrNull(field.name) ?: error("Can't find field: $field for $javaClass")
model.fields.forEach { (fieldId, fieldModel) ->
val declaredField = fieldId.field
val accessible = declaredField.isAccessible

try {
Expand All @@ -228,7 +226,7 @@ class ValueConstructor {
fieldModel.classId.name,
model.classId.name,
UtConcreteValue(classInstance),
field.name
fieldId.name
)
}
val value = construct(fieldModel, target).value
Expand Down Expand Up @@ -360,7 +358,7 @@ class ValueConstructor {
val instanceClassId = instanceModel.classId
val fieldModel = directSetterModel.fieldModel

val field = instance::class.java.findField(directSetterModel.fieldId.name)
val field = directSetterModel.fieldId.field
val isAccessible = field.isAccessible

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ internal class CgFieldStateManagerImpl(val context: CgContext)
}

// if previous field has type that does not have current field, this field is inaccessible
if (index > 0 && !path[index - 1].type.hasField(fieldPathElement.field.name)) {
if (index > 0 && !path[index - 1].type.hasField(fieldPathElement.field)) {
lastAccessibleIndex = index - 1
break
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import org.utbot.framework.codegen.model.tree.CgEqualTo
import org.utbot.framework.codegen.model.tree.CgErrorTestMethod
import org.utbot.framework.codegen.model.tree.CgExecutableCall
import org.utbot.framework.codegen.model.tree.CgExpression
import org.utbot.framework.codegen.model.tree.CgFieldAccess
import org.utbot.framework.codegen.model.tree.CgGetJavaClass
import org.utbot.framework.codegen.model.tree.CgIfStatement
import org.utbot.framework.codegen.model.tree.CgLiteral
Expand Down Expand Up @@ -928,8 +927,8 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c
private fun FieldId.getAccessExpression(variable: CgVariable): CgExpression =
// Can directly access field only if it is declared in variable class (or in its ancestors)
// and is accessible from current package
if (variable.type.hasField(name) && isAccessibleFrom(testClassPackageName)) {
if (field.isStatic) CgStaticFieldAccess(this) else CgFieldAccess(variable, this)
if (variable.type.hasField(this) && isAccessibleFrom(testClassPackageName)) {
if (field.isStatic) CgStaticFieldAccess(this) else variable[this]
} else {
testClassThisInstance[getFieldValue](variable, stringLiteral(name))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ import org.utbot.framework.codegen.model.tree.CgAllocateInitializedArray
import org.utbot.framework.codegen.model.tree.CgDeclaration
import org.utbot.framework.codegen.model.tree.CgEnumConstantAccess
import org.utbot.framework.codegen.model.tree.CgExpression
import org.utbot.framework.codegen.model.tree.CgFieldAccess
import org.utbot.framework.codegen.model.tree.CgGetJavaClass
import org.utbot.framework.codegen.model.tree.CgLiteral
import org.utbot.framework.codegen.model.tree.CgNotNullAssertion
import org.utbot.framework.codegen.model.tree.CgStaticFieldAccess
import org.utbot.framework.codegen.model.tree.CgValue
import org.utbot.framework.codegen.model.tree.CgVariable
Expand Down Expand Up @@ -47,8 +45,7 @@ import org.utbot.framework.plugin.api.UtPrimitiveModel
import org.utbot.framework.plugin.api.UtReferenceModel
import org.utbot.framework.plugin.api.UtVoidModel
import org.utbot.framework.plugin.api.util.defaultValueModel
import org.utbot.framework.plugin.api.util.field
import org.utbot.framework.plugin.api.util.findFieldOrNull
import org.utbot.framework.plugin.api.util.findFieldByIdOrNull
import org.utbot.framework.plugin.api.util.id
import org.utbot.framework.plugin.api.util.intClassId
import org.utbot.framework.plugin.api.util.isArray
Expand Down Expand Up @@ -128,21 +125,17 @@ internal class CgVariableConstructor(val context: CgContext) :
}

for ((fieldId, fieldModel) in model.fields) {
val field = fieldId.field
val variableForField = getOrCreateVariable(fieldModel)
val fieldFromVariableSpecifiedType = obj.type.findFieldOrNull(field.name)
val field = obj.type.findFieldByIdOrNull(fieldId)

// we cannot set field directly if variable declared type does not have such field
// or we cannot directly create variable for field with the specified type (it is private, for example)
// Example:
// Object heapByteBuffer = createInstance("java.nio.HeapByteBuffer");
// branchRegisterRequest.byteBuffer = heapByteBuffer;
// byteBuffer is field of type ByteBuffer and upper line is incorrect
val canFieldBeDirectlySetByVariableAndFieldTypeRestrictions =
fieldFromVariableSpecifiedType != null && fieldFromVariableSpecifiedType.type.id == variableForField.type
if (canFieldBeDirectlySetByVariableAndFieldTypeRestrictions && fieldId.canBeSetIn(testClassPackageName)) {
// TODO: check if it is correct to use declaringClass of a field here
val fieldAccess = if (field.isStatic) CgStaticFieldAccess(fieldId) else CgFieldAccess(obj, fieldId)
if (field != null && field.type.id == variableForField.type && fieldId.canBeSetIn(testClassPackageName)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Damtev or @EgorkaKulikov, please, look over these changes with code generation

Copy link
Member

@Damtev Damtev Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no functional changes, but I am a bit confused with this TODO removing. Are you sure it is resolved?

val fieldAccess = if (field.isStatic) CgStaticFieldAccess(fieldId) else obj[fieldId]
fieldAccess `=` variableForField
} else {
// composite models must not have info about static fields, hence only non-static fields are set here
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.utbot.framework.codegen.model.util

import org.utbot.common.findDirectAccessedFieldOrNull
import org.utbot.framework.codegen.model.constructor.tree.CgCallableAccessManager
import org.utbot.framework.codegen.model.tree.CgArrayElementAccess
import org.utbot.framework.codegen.model.tree.CgDecrement
Expand All @@ -16,6 +17,7 @@ import org.utbot.framework.codegen.model.tree.CgLessThan
import org.utbot.framework.codegen.model.tree.CgLiteral
import org.utbot.framework.codegen.model.tree.CgStaticFieldAccess
import org.utbot.framework.codegen.model.tree.CgThisInstance
import org.utbot.framework.codegen.model.tree.CgTypeCast
import org.utbot.framework.codegen.model.tree.CgVariable
import org.utbot.framework.plugin.api.ClassId
import org.utbot.framework.plugin.api.CodegenLanguage
Expand All @@ -25,9 +27,11 @@ import org.utbot.framework.plugin.api.util.booleanClassId
import org.utbot.framework.plugin.api.util.byteClassId
import org.utbot.framework.plugin.api.util.charClassId
import org.utbot.framework.plugin.api.util.doubleClassId
import org.utbot.framework.plugin.api.util.field
import org.utbot.framework.plugin.api.util.floatClassId
import org.utbot.framework.plugin.api.util.intClassId
import org.utbot.framework.plugin.api.util.isArray
import org.utbot.framework.plugin.api.util.jClass
import org.utbot.framework.plugin.api.util.longClassId
import org.utbot.framework.plugin.api.util.objectClassId
import org.utbot.framework.plugin.api.util.shortClassId
Expand Down Expand Up @@ -72,7 +76,11 @@ fun stringLiteral(string: String) = CgLiteral(stringClassId, string)

// non-static fields
operator fun CgExpression.get(fieldId: FieldId): CgFieldAccess =
CgFieldAccess(this, fieldId)
if (type.jClass.findDirectAccessedFieldOrNull(fieldId.name) != fieldId.field) {
CgFieldAccess(CgTypeCast(fieldId.declaringClass, this), fieldId)
} else {
CgFieldAccess(this, fieldId)
}
Comment on lines -75 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functionality was just a shortcut for a CgFieldAccess constructor. I don't think we should use more complex logic with additional checks here.

Also, this logic does not always work - there is no check that fieldId.declaringClass is accessible, so there is possible situation with cast to a private class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the best place to move this logic to? Before calling this there are checks that fieldId is accessible, aren't they enough?

Also, I believe that this logic should solve the removed TODO: it checks if we should use obj or declaring class as a caller. But maybe I misunderstood the TODO


// static fields
// TODO: unused receiver
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ internal class CgKotlinRenderer(context: CgContext, printer: CgPrinter = CgPrint
// Property access

override fun visit(element: CgFieldAccess) {
if (element.caller is CgTypeCast) print("(")
element.caller.accept(this)
if (element.caller is CgTypeCast) print(")")
Comment on lines +124 to +126
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If lhs is typecast, we need brackets for correct priority. Without this, the generated code will be like Succ as Super.f and not (Succ as Super).f.

This is a little bit of a hack, but I didn't find more general solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely a hack, which is also not total - it does not cover similar cases for a method invocation and an array access. We need to find another solution.

renderAccess(element.caller)
print(element.fieldId.name)
}
Expand Down
Loading