Fixes for analysis of reflection and Unsafe

dtim · dtim · commit 99fad0616622 · 2022-08-29T17:29:29.000+03:00
* Refactored the list of fields that can't be accessed via reflection
    to use the single rules for all cases (engine, concrete executor,
    codegen)

  * Updated this list for Java 11 classes

  * Added a check to avoid marking reflection-inaccessible static fields
    as meaningful (they can't be reflexively created by the codegen anyway)

  * Added `jdk.internal` to the list of system packages to avoid
    mocking, disabled mocking for classes that can't be accessed via
    reflection

  * Added a wrapper for `SecurityManager` and override for two related
    methods in [Class]

  * Added UtSettings option to disable sandbox

  * Temporarily disabled `StringExamplesTest.testByteToString` as flaky
diff --git a/utbot-core/src/main/kotlin/org/utbot/common/ReflectionUtil.kt b/utbot-core/src/main/kotlin/org/utbot/common/ReflectionUtil.kt
@@ -72,4 +72,4 @@ inline fun <reified R> Field.withAccessibility(block: () -> R): R {
         isAccessible = prevAccessibility
         setModifiers(this, prevModifiers)
     }
-}
+}
diff --git a/utbot-framework-api/src/main/kotlin/org/utbot/framework/UtSettings.kt b/utbot-framework-api/src/main/kotlin/org/utbot/framework/UtSettings.kt
@@ -406,6 +406,11 @@ object UtSettings {
      */
     var ignoreStaticsFromTrustedLibraries by getBooleanProperty(true)
 
+    /**
+     * Disable sandbox in the concrete executor. All unsafe/dangerous calls will be permitted.
+     */
+    var disableSandbox by getBooleanProperty(false)
+
     override fun toString(): String =
         settingsValues
             .mapKeys { it.key.name }
diff --git a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/Api.kt b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/Api.kt
@@ -922,7 +922,7 @@ open class FieldId(val declaringClass: ClassId, val name: String) {
         return result
     }
 
-    override fun toString() = safeJField.toString()
+    override fun toString() = safeJField?.toString() ?: "[unresolved] $declaringClass.$name"
 }
 
 inline fun <T> withReflection(block: () -> T): T {
diff --git a/utbot-framework/src/main/java/org/utbot/engine/overrides/Class.java b/utbot-framework/src/main/java/org/utbot/engine/overrides/Class.java
@@ -7,4 +7,14 @@ public class Class {
     public static boolean desiredAssertionStatus() {
         return true;
     }
+
+    private void checkMemberAccess(SecurityManager sm, int which,
+                                   java.lang.Class<?> caller, boolean checkProxyInterfaces) {
+        // Do nothing to allow everything
+    }
+
+    private void checkPackageAccess(SecurityManager sm, final ClassLoader ccl,
+                                    boolean checkProxyInterfaces) {
+        // Do nothing to allow everything
+    }
 }
diff --git a/utbot-framework/src/main/java/org/utbot/engine/overrides/security/UtSecurityManager.java b/utbot-framework/src/main/java/org/utbot/engine/overrides/security/UtSecurityManager.java
@@ -0,0 +1,16 @@
+package org.utbot.engine.overrides.security;
+
+import java.security.Permission;
+
+/**
+ * Overridden implementation for [java.lang.SecurityManager] class
+ */
+public class UtSecurityManager {
+    public void checkPermission(Permission perm) {
+        // Do nothing to allow everything
+    }
+
+    public void checkPackageAccess(String pkg) {
+        // Do nothing to allow everything
+    }
+}
diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/MockStrategy.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/MockStrategy.kt
@@ -45,6 +45,7 @@ private val systemPackages = setOf(
     "sun.reflect", // we cannot mock Reflection since mockers are using it during the execution
     "java.awt",
     "sun.misc",
+    "jdk.internal",
     "kotlin.jvm.internal",
     "kotlin.internal"
 )
diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/Mocks.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/Mocks.kt
@@ -19,6 +19,7 @@ import kotlin.reflect.KFunction2
 import kotlin.reflect.KFunction5
 import kotlinx.collections.immutable.persistentListOf
 import org.utbot.engine.util.mockListeners.MockListenerController
+import org.utbot.framework.util.isInaccessibleViaReflection
 import soot.BooleanType
 import soot.RefType
 import soot.Scene
@@ -183,6 +184,7 @@ class Mocker(
         if (isUtMockAssume(mockInfo)) return false // never mock UtMock.assume invocation
         if (isUtMockAssumeOrExecuteConcretely(mockInfo)) return false // never mock UtMock.assumeOrExecuteConcretely invocation
         if (isOverriddenClass(type)) return false  // never mock overriden classes
+        if (type.isInaccessibleViaReflection) return false // never mock classes that we can't process with reflection
         if (isMakeSymbolic(mockInfo)) return true // support for makeSymbolic
         if (type.sootClass.isArtificialEntity) return false // never mock artificial types, i.e. Maps$lambda_computeValue_1__7
         if (!isEngineClass(type) && (type.sootClass.isInnerClass || type.sootClass.isLocal || type.sootClass.isAnonymous)) return false // there is no reason (and maybe no possibility) to mock such classes
diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/ObjectWrappers.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/ObjectWrappers.kt
@@ -13,6 +13,7 @@ import org.utbot.engine.overrides.collections.AssociativeArray
 import org.utbot.engine.overrides.collections.RangeModifiableUnlimitedArray
 import org.utbot.engine.overrides.collections.UtHashMap
 import org.utbot.engine.overrides.collections.UtHashSet
+import org.utbot.engine.overrides.security.UtSecurityManager
 import org.utbot.engine.overrides.strings.UtNativeString
 import org.utbot.engine.overrides.strings.UtString
 import org.utbot.engine.overrides.strings.UtStringBuffer
@@ -86,6 +87,8 @@ val classToWrapper: MutableMap<TypeToBeWrapped, WrapperType> =
         putSootClass(java.util.stream.BaseStream::class, UT_STREAM.className)
         putSootClass(java.util.stream.Stream::class, UT_STREAM.className)
         // TODO primitive streams https://github.com/UnitTestBot/UTBotJava/issues/146
+
+        putSootClass(java.lang.SecurityManager::class, UtSecurityManager::class)
     }
 
 /**
@@ -187,6 +190,9 @@ private val wrappers = mapOf(
     wrap(java.util.stream.BaseStream::class) { _, addr -> objectValue(STREAM_TYPE, addr, CommonStreamWrapper()) },
     wrap(java.util.stream.Stream::class) { _, addr -> objectValue(STREAM_TYPE, addr, CommonStreamWrapper()) },
     // TODO primitive streams https://github.com/UnitTestBot/UTBotJava/issues/146
+
+    // Security-related wrappers
+    wrap(SecurityManager::class) { type, addr -> objectValue(type, addr, SecurityManagerWrapper()) },
 ).also {
     // check every `wrapped` class has a corresponding value in [classToWrapper]
     it.keys.all { key ->
diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/SecurityManagerWrapper.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/SecurityManagerWrapper.kt
@@ -0,0 +1,39 @@
+package org.utbot.engine
+
+import org.utbot.engine.overrides.security.UtSecurityManager
+import org.utbot.framework.plugin.api.UtAssembleModel
+import org.utbot.framework.plugin.api.UtModel
+import org.utbot.framework.plugin.api.UtStatementModel
+import org.utbot.framework.plugin.api.classId
+import org.utbot.framework.util.nextModelName
+import soot.Scene
+import soot.SootClass
+import soot.SootMethod
+
+class SecurityManagerWrapper : BaseOverriddenWrapper(utSecurityManagerClass.name) {
+    private val baseModelName: String = "securityManager"
+
+    override fun Traverser.overrideInvoke(
+        wrapper: ObjectValue,
+        method: SootMethod,
+        parameters: List<SymbolicValue>
+    ): List<InvokeResult>? {
+        // We currently do not overload any [SecurityManager] method symbolically
+        return null
+    }
+
+    override fun value(resolver: Resolver, wrapper: ObjectValue): UtModel = resolver.run {
+        val classId = wrapper.type.classId
+        val addr = holder.concreteAddr(wrapper.addr)
+        val modelName = nextModelName(baseModelName)
+
+        val instantiationChain = mutableListOf<UtStatementModel>()
+        val modificationChain = mutableListOf<UtStatementModel>()
+        return UtAssembleModel(addr, classId, modelName, instantiationChain, modificationChain)
+    }
+
+    companion object {
+        val utSecurityManagerClass: SootClass
+            get() = Scene.v().getSootClass(UtSecurityManager::class.qualifiedName)
+    }
+}
diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/Traverser.kt
@@ -98,6 +98,7 @@ import org.utbot.framework.plugin.api.util.signature
 import org.utbot.framework.plugin.api.util.utContext
 import org.utbot.framework.util.executableId
 import org.utbot.framework.util.graph
+import org.utbot.framework.util.isInaccessibleViaReflection
 import java.lang.reflect.ParameterizedType
 import kotlin.collections.plus
 import kotlin.collections.plusAssign
@@ -631,7 +632,7 @@ class Traverser(
 
         // so, we have to make an update for the local $r0
 
-        return if (stmt is JAssignStmt) {
+        return if (stmt is JAssignStmt && stmt.leftOp is JimpleLocal) {
             val local = stmt.leftOp as JimpleLocal
 
             localMemoryUpdate(local.variable to symbolicValue)
@@ -1798,6 +1799,8 @@ class Traverser(
      */
     private fun isStaticFieldMeaningful(field: SootField) =
         !Modifier.isSynthetic(field.modifiers) &&
+            // we don't want to set fields that cannot be set via reflection anyway
+            !field.fieldId.isInaccessibleViaReflection &&
             // we don't want to set fields from library classes
             !workaround(IGNORE_STATICS_FROM_TRUSTED_LIBRARIES) {
                 ignoreStaticsFromTrustedLibraries && field.declaringClass.isFromTrustedLibrary()
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt
@@ -70,7 +70,6 @@ import org.utbot.framework.codegen.model.util.equalTo
 import org.utbot.framework.codegen.model.util.get
 import org.utbot.framework.codegen.model.util.inc
 import org.utbot.framework.codegen.model.util.isAccessibleFrom
-import org.utbot.framework.codegen.model.util.isInaccessible
 import org.utbot.framework.codegen.model.util.length
 import org.utbot.framework.codegen.model.util.lessThan
 import org.utbot.framework.codegen.model.util.nullLiteral
@@ -139,6 +138,7 @@ import org.utbot.framework.plugin.api.util.objectClassId
 import org.utbot.framework.plugin.api.util.stringClassId
 import org.utbot.framework.plugin.api.util.voidClassId
 import org.utbot.framework.plugin.api.util.wrapIfPrimitive
+import org.utbot.framework.util.isInaccessibleViaReflection
 import org.utbot.framework.util.isUnit
 import org.utbot.summary.SummarySentenceConstants.TAB
 import java.lang.reflect.InvocationTargetException
@@ -273,7 +273,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c
         }
     }
 
-    private fun <E> Map<FieldId, E>.accessibleFields(): Map<FieldId, E> = filterKeys { !it.isInaccessible }
+    private fun <E> Map<FieldId, E>.accessibleFields(): Map<FieldId, E> = filterKeys { !it.isInaccessibleViaReflection }
 
     /**
      * @return expression for [java.lang.Class] of the given [classId]
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/util/FieldIdUtil.kt
@@ -22,11 +22,3 @@ fun FieldId.isAccessibleFrom(packageName: String): Boolean {
  * Whether or not a field can be set without reflection
  */
 fun FieldId.canBeSetIn(packageName: String): Boolean = isAccessibleFrom(packageName) && !isFinal
-
-private val systemClassId = System::class.id
-
-/**
- * Security field is inaccessible in Runtime even via reflection.
- */
-val FieldId.isInaccessible: Boolean
-    get() = name == "security" && declaringClass == systemClassId
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt
@@ -30,6 +30,7 @@ import org.utbot.framework.plugin.api.util.singleExecutableId
 import org.utbot.framework.plugin.api.util.utContext
 import org.utbot.framework.plugin.api.util.withUtContext
 import org.utbot.framework.plugin.api.withReflection
+import org.utbot.framework.util.isInaccessibleViaReflection
 import org.utbot.instrumentation.instrumentation.ArgumentList
 import org.utbot.instrumentation.instrumentation.Instrumentation
 import org.utbot.instrumentation.instrumentation.InvokeInstrumentation
@@ -210,15 +211,6 @@ object UtExecutionInstrumentation : Instrumentation<UtConcreteExecutionResult> {
         }
     }
 
-    private val inaccessibleViaReflectionFields = setOf(
-        "security" to "java.lang.System",
-        "fieldFilterMap" to "sun.reflect.Reflection",
-        "methodFilterMap" to "sun.reflect.Reflection"
-    )
-
-    private val FieldId.isInaccessibleViaReflection: Boolean
-        get() = (name to declaringClass.name) in inaccessibleViaReflectionFields
-
     private fun sortOutException(exception: Throwable): UtExecutionFailure {
         if (exception is TimeoutException) {
             return UtTimeoutException(exception)
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtModelConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtModelConstructor.kt
@@ -28,6 +28,7 @@ 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
+import org.utbot.framework.util.isInaccessibleViaReflection
 import org.utbot.framework.util.valueToClassId
 import java.lang.reflect.Modifier
 import java.util.IdentityHashMap
@@ -281,7 +282,9 @@ internal class UtModelConstructor(
         generateSequence(javaClazz) { it.superclass }.forEach { clazz ->
             val allFields = clazz.declaredFields
             allFields
+                .asSequence()
                 .filter { !(Modifier.isFinal(it.modifiers) && Modifier.isStatic(it.modifiers)) } // TODO: what about static final fields?
+                .filterNot { it.fieldId.isInaccessibleViaReflection }
                 .forEach { it.withAccessibility { fields[it.fieldId] = construct(it.get(value), it.type.id) } }
         }
         return utModel
diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/util/ReflectionUtils.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/util/ReflectionUtils.kt
@@ -0,0 +1,30 @@
+package org.utbot.framework.util
+
+import org.utbot.framework.plugin.api.FieldId
+import soot.RefType
+
+/**
+ * Several fields are inaccessible in runtime even via reflection
+ */
+val FieldId.isInaccessibleViaReflection: Boolean
+    get() {
+        val declaringClassName = declaringClass.name
+        return declaringClassName in inaccessibleViaReflectionClasses ||
+                (name to declaringClassName) in inaccessibleViaReflectionFields
+    }
+
+val RefType.isInaccessibleViaReflection: Boolean
+    get() {
+        return className in inaccessibleViaReflectionClasses
+    }
+
+private val inaccessibleViaReflectionClasses = setOf(
+    "jdk.internal.reflect.ReflectionFactory",
+    "jdk.internal.reflect.Reflection",
+    "jdk.internal.loader.ClassLoaderValue",
+    "sun.reflect.Reflection",
+)
+
+private val inaccessibleViaReflectionFields = setOf(
+    "security" to "java.lang.System",
+)
diff --git a/utbot-framework/src/test/kotlin/org/utbot/examples/UtValueTestCaseChecker.kt b/utbot-framework/src/test/kotlin/org/utbot/examples/UtValueTestCaseChecker.kt
@@ -2890,3 +2890,16 @@ inline fun <reified T> withUsingReflectionForMaximizingCoverage(maximizeCoverage
         UtSettings.maximizeCoverageUsingReflection = prev
     }
 }
+
+/**
+ * Run [block] with disabled sandbox in the concrete executor
+ */
+inline fun <reified T> withoutSandbox(block: () -> T): T {
+    val prev = UtSettings.disableSandbox
+    UtSettings.disableSandbox = true
+    try {
+        return block()
+    } finally {
+        UtSettings.disableSandbox = prev
+    }
+}
diff --git a/utbot-framework/src/test/kotlin/org/utbot/examples/exceptions/JvmCrashExamplesTest.kt b/utbot-framework/src/test/kotlin/org/utbot/examples/exceptions/JvmCrashExamplesTest.kt
@@ -5,6 +5,9 @@ import org.utbot.examples.DoNotCalculate
 import org.utbot.examples.eq
 import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
+import org.utbot.examples.withoutSandbox
+import org.utbot.framework.codegen.CodeGeneration
+import org.utbot.framework.plugin.api.CodegenLanguage
 
 internal class JvmCrashExamplesTest : UtValueTestCaseChecker(testClass = JvmCrashExamples::class) {
     @Test
@@ -17,11 +20,23 @@ internal class JvmCrashExamplesTest : UtValueTestCaseChecker(testClass = JvmCras
     }
 
     @Test
-    @Disabled("Java 11 transition")
     fun testCrash() {
+        withoutSandbox {
+            check(
+                JvmCrashExamples::crash,
+                eq(1), // we expect only one execution after minimization
+                // It seems that we can't calculate coverage when the child JVM has crashed
+                coverage = DoNotCalculate
+            )
+        }
+    }
+
+    @Test
+    fun testCrashPrivileged() {
         check(
-            JvmCrashExamples::crash,
+            JvmCrashExamples::crashPrivileged,
             eq(1), // we expect only one execution after minimization
+            // It seems that we can't calculate coverage when the child JVM has crashed
             coverage = DoNotCalculate
         )
     }
diff --git a/utbot-framework/src/test/kotlin/org/utbot/examples/strings/StringExamplesTest.kt b/utbot-framework/src/test/kotlin/org/utbot/examples/strings/StringExamplesTest.kt
@@ -26,6 +26,7 @@ internal class StringExamplesTest : UtValueTestCaseChecker(
     )
 ) {
     @Test
+    @Disabled("Flaky test: https://github.com/UnitTestBot/UTBotJava/issues/131 (will be enabled in new strings PR)")
     fun testByteToString() {
         // TODO related to the https://github.com/UnitTestBot/UTBotJava/issues/131
         withSolverTimeoutInMillis(5000) {
diff --git a/utbot-framework/src/test/kotlin/org/utbot/examples/unsafe/UnsafeOperationsTest.kt b/utbot-framework/src/test/kotlin/org/utbot/examples/unsafe/UnsafeOperationsTest.kt
@@ -0,0 +1,41 @@
+package org.utbot.examples.unsafe
+
+import org.junit.jupiter.api.Test
+import org.utbot.examples.DoNotCalculate
+import org.utbot.examples.UtValueTestCaseChecker
+import org.utbot.examples.eq
+import org.utbot.examples.withoutSandbox
+import org.utbot.framework.codegen.CodeGeneration
+import org.utbot.framework.plugin.api.CodegenLanguage
+import org.utbot.framework.plugin.api.MockStrategyApi
+
+internal class UnsafeOperationsTest : UtValueTestCaseChecker(testClass = UnsafeOperations::class) {
+    @Test
+    fun checkGetAddressSizeOrZero() {
+        withoutSandbox {
+            check(
+                UnsafeOperations::getAddressSizeOrZero,
+                eq(1),
+                { r -> r!! > 0 },
+                // Coverage matcher fails (branches: 0/0, instructions: 15/21, lines: 0/0)
+                // TODO: check coverage calculation: https://github.com/UnitTestBot/UTBotJava/issues/807
+                coverage = DoNotCalculate
+            )
+        }
+    }
+
+    @Test
+    fun checkGetAddressSizeOrZeroWithMocks() {
+        withoutSandbox {
+            check(
+                UnsafeOperations::getAddressSizeOrZero,
+                eq(1),
+                { r -> r!! > 0 },
+                // Coverage matcher fails (branches: 0/0, instructions: 15/21, lines: 0/0)
+                // TODO: check coverage calculation: https://github.com/UnitTestBot/UTBotJava/issues/807
+                coverage = DoNotCalculate,
+                mockStrategy = MockStrategyApi.OTHER_CLASSES
+            )
+        }
+    }
+}
diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt
diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcessRunner.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcessRunner.kt
diff --git a/utbot-sample/src/main/java/org/utbot/examples/exceptions/JvmCrashExamples.java b/utbot-sample/src/main/java/org/utbot/examples/exceptions/JvmCrashExamples.java
diff --git a/utbot-sample/src/main/java/org/utbot/examples/unsafe/UnsafeOperations.java b/utbot-sample/src/main/java/org/utbot/examples/unsafe/UnsafeOperations.java

Original file line number	Diff line number	Diff line change
`@@ -72,4 +72,4 @@ inline fun <reified R> Field.withAccessibility(block: () -> R): R {`
`72`	`72`	`isAccessible = prevAccessibility`
`73`	`73`	`setModifiers(this, prevModifiers)`
`74`	`74`	`}`
`75`		`-}`
	`75`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -922,7 +922,7 @@ open class FieldId(val declaringClass: ClassId, val name: String) {`
`922`	`922`	`return result`
`923`	`923`	`}`
`924`	`924`
`925`		`- override fun toString() = safeJField.toString()`
	`925`	`+ override fun toString() = safeJField?.toString() ?: "[unresolved] $declaringClass.$name"`
`926`	`926`	`}`
`927`	`927`
`928`	`928`	`inline fun <T> withReflection(block: () -> T): T {`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ private val systemPackages = setOf(`
`45`	`45`	`"sun.reflect", // we cannot mock Reflection since mockers are using it during the execution`
`46`	`46`	`"java.awt",`
`47`	`47`	`"sun.misc",`
	`48`	`+ "jdk.internal",`
`48`	`49`	`"kotlin.jvm.internal",`
`49`	`50`	`"kotlin.internal"`
`50`	`51`	`)`