Skip to content

Prohibit to set static fields from library classes #697 #699

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

Merged
merged 7 commits into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions utbot-core/src/main/kotlin/org/utbot/common/HackUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,46 @@ inline fun <T> heuristic(reason: WorkaroundReason, block: () -> T): T = block()

/**
* Explains reason for applied workaround.
*
* Workarounds are:
* - HACK - hacks behaviour for contest. Shall be removed sometimes
* - MAKE_SYMBOLIC - Returns a new symbolic value with proper type instead of function result (i.e. for wrappers)
* - IGNORE_SORT_INEQUALITY -- Ignores pairs of particular sorts in stores and selects
* - RUN_CONCRETE -- Runs something concretely instead of symbolic run
* - REMOVE_ANONYMOUS_CLASSES -- Remove anonymous classes from the results passed to the code generation till it doesn't support their generation
* - IGNORE_MODEL_TYPES_INEQUALITY -- Ignore the fact that model before and model after have different types
* - LONG_CODE_FRAGMENTS -- Comment too long blocks of code due to JVM restrictions [65536 bytes](https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.3)
* - ARRAY_ELEMENT_TYPES_ALWAYS_NULLABLE -- Can't infer nullability for array elements from allocation statement, so make them nullable
* Note:
* - MAKE_SYMBOLIC can lose additional path constraints or branches from function call
*/
enum class WorkaroundReason {
/**
* Hacks behaviour for contest. Shall be removed sometimes
*/
HACK,
/**
* Returns a new symbolic value with proper type instead of function result (i.e. for wrappers)
*
* [MAKE_SYMBOLIC] can lose additional path constraints or branches from function call
*/
MAKE_SYMBOLIC,
/**
* Ignores pairs of particular sorts in stores and selects
*/
IGNORE_SORT_INEQUALITY,
/**
* Runs something concretely instead of symbolic run
*/
RUN_CONCRETE,
/**
* Remove anonymous classes from the results passed to the code generation till it doesn't support their generation
*/
REMOVE_ANONYMOUS_CLASSES,
/**
* Ignore the fact that model before and model after have different types
*/
IGNORE_MODEL_TYPES_INEQUALITY,
/**
* Comment too long blocks of code due to JVM restrictions [65536 bytes](https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.3)
*/
LONG_CODE_FRAGMENTS,
/**
* Can't infer nullability for array elements from allocation statement, so make them nullable
*/
ARRAY_ELEMENT_TYPES_ALWAYS_NULLABLE,
/**
* We won't branch on static field from trusted libraries and won't add them to staticsBefore. For now, it saves us
* from setting [SecurityManager] and other suspicious stuff, but it can lead to coverage regression and thus it
* requires thorough [investigation](https://github.com/UnitTestBot/UTBotJava/issues/716).
*/
IGNORE_STATICS_FROM_TRUSTED_LIBRARIES,
}
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,13 @@ object UtSettings {
*/
var skipTestGenerationForSyntheticMethods by getBooleanProperty(true)

/**
* Flag that indicates whether should we branch on and set static fields from trusted libraries or not.
*
* @see [org.utbot.common.WorkaroundReason.IGNORE_STATICS_FROM_TRUSTED_LIBRARIES]
*/
var ignoreStaticsFromTrustedLibraries by getBooleanProperty(true)

override fun toString(): String =
settingsValues
.mapKeys { it.key.name }
Expand Down
8 changes: 4 additions & 4 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Memory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ data class Memory( // TODO: split purely symbolic memory and information about s
private val concrete: PersistentMap<UtAddrExpression, Concrete> = persistentHashMapOf(),
private val mockInfos: PersistentList<MockInfoEnriched> = persistentListOf(),
private val staticInstanceStorage: PersistentMap<ClassId, ObjectValue> = persistentHashMapOf(),
private val initializedStaticFields: PersistentMap<FieldId, Any?> = persistentHashMapOf(),
private val initializedStaticFields: PersistentSet<FieldId> = persistentHashSetOf(),
private val staticFieldsStates: PersistentMap<FieldId, FieldStates> = persistentHashMapOf(),
private val meaningfulStaticFields: PersistentSet<FieldId> = persistentHashSetOf(),
private val addrToArrayType: PersistentMap<UtAddrExpression, ArrayType> = persistentHashMapOf(),
Expand Down Expand Up @@ -290,7 +290,7 @@ data class Memory( // TODO: split purely symbolic memory and information about s
concrete = concrete.putAll(update.concrete),
mockInfos = mockInfos.mergeWithUpdate(update.mockInfos),
staticInstanceStorage = staticInstanceStorage.putAll(update.staticInstanceStorage),
initializedStaticFields = initializedStaticFields.putAll(update.initializedStaticFields),
initializedStaticFields = initializedStaticFields.addAll(update.initializedStaticFields),
staticFieldsStates = previousMemoryStates.toPersistentMap().putAll(updatedStaticFields),
meaningfulStaticFields = meaningfulStaticFields.addAll(update.meaningfulStaticFields),
addrToArrayType = addrToArrayType.putAll(update.addrToArrayType),
Expand Down Expand Up @@ -963,7 +963,7 @@ data class MemoryUpdate(
val concrete: PersistentMap<UtAddrExpression, Concrete> = persistentHashMapOf(),
val mockInfos: PersistentList<MockInfoEnriched> = persistentListOf(),
val staticInstanceStorage: PersistentMap<ClassId, ObjectValue> = persistentHashMapOf(),
val initializedStaticFields: PersistentMap<FieldId, Any?> = persistentHashMapOf(),
val initializedStaticFields: PersistentSet<FieldId> = persistentHashSetOf(),
val staticFieldsUpdates: PersistentList<StaticFieldMemoryUpdateInfo> = persistentListOf(),
val meaningfulStaticFields: PersistentSet<FieldId> = persistentHashSetOf(),
val addrToArrayType: PersistentMap<UtAddrExpression, ArrayType> = persistentHashMapOf(),
Expand All @@ -982,7 +982,7 @@ data class MemoryUpdate(
concrete = concrete.putAll(other.concrete),
mockInfos = mockInfos.mergeWithUpdate(other.mockInfos),
staticInstanceStorage = staticInstanceStorage.putAll(other.staticInstanceStorage),
initializedStaticFields = initializedStaticFields.putAll(other.initializedStaticFields),
initializedStaticFields = initializedStaticFields.addAll(other.initializedStaticFields),
staticFieldsUpdates = staticFieldsUpdates.addAll(other.staticFieldsUpdates),
meaningfulStaticFields = meaningfulStaticFields.addAll(other.meaningfulStaticFields),
addrToArrayType = addrToArrayType.putAll(other.addrToArrayType),
Expand Down
32 changes: 25 additions & 7 deletions utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package org.utbot.engine

import kotlinx.collections.immutable.persistentHashMapOf
import kotlinx.collections.immutable.persistentHashSetOf
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.persistentSetOf
import kotlinx.collections.immutable.toPersistentList
import kotlinx.collections.immutable.toPersistentMap
import kotlinx.collections.immutable.toPersistentSet
import org.utbot.common.WorkaroundReason.HACK
import org.utbot.framework.UtSettings.ignoreStaticsFromTrustedLibraries
import org.utbot.common.WorkaroundReason.IGNORE_STATICS_FROM_TRUSTED_LIBRARIES
import org.utbot.common.WorkaroundReason.REMOVE_ANONYMOUS_CLASSES
import org.utbot.common.unreachableBranch
import org.utbot.common.withAccessibility
Expand Down Expand Up @@ -112,6 +114,7 @@ import soot.DoubleType
import soot.FloatType
import soot.IntType
import soot.LongType
import soot.Modifier
import soot.PrimType
import soot.RefLikeType
import soot.RefType
Expand Down Expand Up @@ -560,7 +563,7 @@ class Traverser(
}

val initializedStaticFieldsMemoryUpdate = MemoryUpdate(
initializedStaticFields = staticFields.associate { it.first.fieldId to it.second.single() }.toPersistentMap(),
initializedStaticFields = staticFields.map { it.first.fieldId }.toPersistentSet(),
meaningfulStaticFields = meaningfulStaticFields.map { it.first.fieldId }.toPersistentSet(),
symbolicEnumValues = enumConstantSymbolicValues.toPersistentList()
)
Expand Down Expand Up @@ -596,7 +599,7 @@ class Traverser(

// Collects memory updates
val initializedFieldUpdate =
MemoryUpdate(initializedStaticFields = persistentHashMapOf(fieldId to concreteValue))
MemoryUpdate(initializedStaticFields = persistentHashSetOf(fieldId))

val objectUpdate = objectUpdate(
instance = findOrCreateStaticObject(declaringClass.type),
Expand Down Expand Up @@ -823,7 +826,7 @@ class Traverser(
val staticFieldMemoryUpdate = StaticFieldMemoryUpdateInfo(fieldId, value)
val touchedStaticFields = persistentListOf(staticFieldMemoryUpdate)
queuedSymbolicStateUpdates += MemoryUpdate(staticFieldsUpdates = touchedStaticFields)
if (!environment.method.isStaticInitializer && !fieldId.isSynthetic) {
if (!environment.method.isStaticInitializer && isStaticFieldMeaningful(left.field)) {
queuedSymbolicStateUpdates += MemoryUpdate(meaningfulStaticFields = persistentSetOf(fieldId))
}
}
Expand Down Expand Up @@ -1780,13 +1783,27 @@ class Traverser(
queuedSymbolicStateUpdates += MemoryUpdate(staticFieldsUpdates = touchedStaticFields)

// TODO filter enum constant static fields JIRA:1681
if (!environment.method.isStaticInitializer && !fieldId.isSynthetic) {
if (!environment.method.isStaticInitializer && isStaticFieldMeaningful(field)) {
queuedSymbolicStateUpdates += MemoryUpdate(meaningfulStaticFields = persistentSetOf(fieldId))
}

return createdField
}

/**
* For now the field is `meaningful` if it is safe to set, that is, it is not an internal system field nor a
* synthetic field. This filter is needed to prohibit changing internal fields, which can break up our own
* code and which are useless for the user.
*
* @return `true` if the field is meaningful, `false` otherwise.
*/
private fun isStaticFieldMeaningful(field: SootField) =
!Modifier.isSynthetic(field.modifiers) &&
// we don't want to set fields from library classes
!workaround(IGNORE_STATICS_FROM_TRUSTED_LIBRARIES) {
ignoreStaticsFromTrustedLibraries && field.declaringClass.isFromTrustedLibrary()
}

/**
* Locates object represents static fields of particular class.
*
Expand Down Expand Up @@ -3321,10 +3338,11 @@ class Traverser(
val updatedFields = staticFieldsUpdates.mapTo(mutableSetOf()) { it.fieldId }
val objectUpdates = mutableListOf<UtNamedStore>()

// we assign unbounded symbolic variables for every non-final field of the class
// we assign unbounded symbolic variables for every non-final meaningful field of the class
// fields from predefined library classes are excluded, because there are not meaningful
typeResolver
.findFields(declaringClass.type)
.filter { !it.isFinal && it.fieldId in updatedFields }
.filter { !it.isFinal && it.fieldId in updatedFields && isStaticFieldMeaningful(it) }
.forEach {
// remove updates from clinit, because we'll replace those values
// with new unbounded symbolic variable
Expand Down
Loading