Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9e771969 authored by Makoto Onuki's avatar Makoto Onuki
Browse files

Allow "experimental" class to be always loadable.

Previously, when a class with <clinit> is "experimental", even
the "<clinit>" would throw, which would render the class to be
unloadable, which would cause lot of problems.

Now, "experimental API hook" returns a boolean. If it returns "false"
(instead of throwing), we skip the rest of the method body, which
essentially simulates `@Ignore`.

For now, only <clinit> can return false.

It'd be nice if we can support other methods too, but:
- <init> must call super.<init>, so we can't inject an early return.
- For other methods, generating a stack map frame was kind of painful
unless it's void.

Supporting non-<clinit> methods isn't really critical at all, so let's
just ignore it for now.

Bug: 292141694
Flag: TEST_ONLY
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh -r
Change-Id: I68e5356465a2e6c4ae053d406ef0740b77968685
parent e6020b6c
Loading
Loading
Loading
Loading
+8 −1
Original line number Diff line number Diff line
@@ -44,8 +44,15 @@ public class RavenwoodExperimentalApiChecker {
     * Check if experimental APIs are enabled, and if not, throws
     * {@link RavenwoodUnsupportedApiException}.
     */
    public static void onExperimentalApiCalled(Class<?> clazz, String method, String desc) {
    public static boolean onExperimentalApiCalled(Class<?> clazz, String method, String desc) {
        // Even when experimental APIs are disabled, we don't want to throw from <clinit>.
        // because that'd make the class unloadable. Instead, we return false to skip the rest of
        // the code.
        if ("<clinit>".equals(method)) {
            return isExperimentalApiEnabled();
        }
        onExperimentalApiCalled(2);
        return true;
    }

    /**
+24 −0
Original line number Diff line number Diff line
@@ -21,6 +21,9 @@ import java.lang.reflect.AnnotatedElement;
 * Utilities used in the host side test environment.
 */
public class HostTestUtils {
    public static final String CLASS_INTERNAL_NAME = getInternalName(HostTestUtils.class);
    public static final  String CLASS_DESCRIPTOR = "L" + CLASS_INTERNAL_NAME + ";";

    private HostTestUtils() {
    }

@@ -58,4 +61,25 @@ public class HostTestUtils {
        }
        return null;
    }

    public static final String ASSERT_THAT_HOOK_RETURNED_TRUE = "assertThatHookReturnedTrue";
    public static final String ASSERT_THAT_HOOK_RETURNED_FALSE = "assertThatHookReturnedFalse";

    /**
     * Used to assert a boolean method returned true.
     */
    public static void assertThatHookReturnedTrue(boolean b) {
        if (!b) {
            throw new IllegalStateException("The hook must return true for this method");
        }
    }

    /**
     * Used to assert a boolean method returned false.
     */
    public static void assertThatHookReturnedFalse(boolean b) {
        if (b) {
            throw new IllegalStateException("The hook must return false for this method");
        }
    }
}
+42 −0
Original line number Diff line number Diff line
@@ -268,6 +268,37 @@ fun writeByteCodeToReturn(methodDescriptor: String, writer: MethodVisitor) {
    }
}

/**
 * Write bytecode to "RETURN" that matches the method's return type with the type's empty value,
 * according to [methodDescriptor].
 */
fun writeByteCodeToReturnDefault(methodDescriptor: String, writer: MethodVisitor) {
    when (Type.getReturnType(methodDescriptor)) {
        Type.VOID_TYPE -> writer.visitInsn(Opcodes.RETURN)
        Type.BOOLEAN_TYPE, Type.BYTE_TYPE, Type.CHAR_TYPE, Type.SHORT_TYPE,
        Type.INT_TYPE -> {
            writer.visitInsn(Opcodes.ICONST_0)
            writer.visitInsn(Opcodes.IRETURN)
        }
        Type.LONG_TYPE -> {
            writer.visitInsn(Opcodes.LCONST_0)
            writer.visitInsn(Opcodes.LRETURN)
        }
        Type.FLOAT_TYPE -> {
            writer.visitInsn(Opcodes.FCONST_0)
            writer.visitInsn(Opcodes.FRETURN)
        }
        Type.DOUBLE_TYPE -> {
            writer.visitInsn(Opcodes.DCONST_0)
            writer.visitInsn(Opcodes.DRETURN)
        }
        else -> {
            writer.visitInsn(Opcodes.ACONST_NULL)
            writer.visitInsn(Opcodes.ARETURN)
        }
    }
}

/**
 * Write bytecode to pop the 2 uninitialized instances out of the stack
 * after performing constructor redirection.
@@ -553,3 +584,14 @@ abstract class UnifiedVisitor {
        }
    }
}

/**
 * Data class to store visitFrame() arguments.
 */
data class FrameInfo(
    var type: Int,
    var numLocal: Int,
    var local: Array<out Any?>?,
    var numStack: Int,
    var stack: Array<out Any?>?
)
 No newline at end of file
+78 −26
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@
package com.android.hoststubgen.visitors

import com.android.hoststubgen.HostStubGenErrors
import com.android.hoststubgen.HostStubGenInternalException
import com.android.hoststubgen.asm.CLASS_INITIALIZER_DESC
import com.android.hoststubgen.asm.CLASS_INITIALIZER_NAME
import com.android.hoststubgen.asm.CTOR_NAME
@@ -31,6 +32,7 @@ import com.android.hoststubgen.asm.reasonParam
import com.android.hoststubgen.asm.toJvmClassName
import com.android.hoststubgen.asm.writeByteCodeToPushArguments
import com.android.hoststubgen.asm.writeByteCodeToReturn
import com.android.hoststubgen.asm.writeByteCodeToReturnDefault
import com.android.hoststubgen.filters.FilterPolicy
import com.android.hoststubgen.filters.FilterPolicyWithReason
import com.android.hoststubgen.filters.OutputFilter
@@ -40,12 +42,13 @@ import com.android.hoststubgen.hosthelper.HostStubGenProcessedAsKeep
import com.android.hoststubgen.hosthelper.HostStubGenProcessedAsSubstitute
import com.android.hoststubgen.hosthelper.HostStubGenProcessedAsThrow
import com.android.hoststubgen.hosthelper.HostStubGenProcessedAsThrowButSupported
import com.android.hoststubgen.hosthelper.HostTestUtils
import com.android.hoststubgen.log
import com.android.hoststubgen.utils.ClassDescriptorSet
import java.lang.annotation.RetentionPolicy
import org.objectweb.asm.AnnotationVisitor
import org.objectweb.asm.ClassVisitor
import org.objectweb.asm.FieldVisitor
import org.objectweb.asm.Label
import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Opcodes
import org.objectweb.asm.Opcodes.INVOKEINTERFACE
@@ -53,6 +56,7 @@ import org.objectweb.asm.Opcodes.INVOKESPECIAL
import org.objectweb.asm.Opcodes.INVOKESTATIC
import org.objectweb.asm.Opcodes.INVOKEVIRTUAL
import org.objectweb.asm.Type
import java.lang.annotation.RetentionPolicy

const val OPCODE_VERSION = Opcodes.ASM9

@@ -444,7 +448,7 @@ class ImplGeneratingAdapter(
                    }
                    log.v("Making method experimental...")
                    innerVisitor = innerVisitor?.let {
                        MethodCallHookInjectingAdapter(
                        ReturnDecidingMethodCallHookInjectingAdapter(
                            name,
                            descriptor,
                            listOf(options.experimentalMethodCallHook!!),
@@ -497,30 +501,7 @@ class ImplGeneratingAdapter(
        next: MethodVisitor?
    ) : BodyReplacingMethodVisitor(createBody, next) {
        override fun emitNewCode() {
            when (Type.getReturnType(descriptor)) {
                Type.VOID_TYPE -> visitInsn(Opcodes.RETURN)
                Type.BOOLEAN_TYPE, Type.BYTE_TYPE, Type.CHAR_TYPE, Type.SHORT_TYPE,
                Type.INT_TYPE -> {
                    visitInsn(Opcodes.ICONST_0)
                    visitInsn(Opcodes.IRETURN)
                }
                Type.LONG_TYPE -> {
                    visitInsn(Opcodes.LCONST_0)
                    visitInsn(Opcodes.LRETURN)
                }
                Type.FLOAT_TYPE -> {
                    visitInsn(Opcodes.FCONST_0)
                    visitInsn(Opcodes.FRETURN)
                }
                Type.DOUBLE_TYPE -> {
                    visitInsn(Opcodes.DCONST_0)
                    visitInsn(Opcodes.DRETURN)
                }
                else -> {
                    visitInsn(Opcodes.ACONST_NULL)
                    visitInsn(Opcodes.ARETURN)
                }
            }
            writeByteCodeToReturnDefault(descriptor, this)
            visitMaxs(0, 0) // We let ASM figure them out.
        }
    }
@@ -604,6 +585,77 @@ class ImplGeneratingAdapter(
        }
    }


    /**
     * Similar to [MethodCallHookInjectingAdapter], but the hook must return boolean.
     *
     * The hook must always return true, except in <clinit>, it can return false. If the hook
     * returns false, we return from the method, without running any of the remaining code.
     *
     * Can we use false for other methods?
     * - No, for <init>, beucase we always must call super's <init>. We can never return
     *   early from <init.
     * - For other methods, we could.
     */
    private inner class ReturnDecidingMethodCallHookInjectingAdapter(
        val name: String,
        val descriptor: String,
        val hooks: List<String>,
        next: MethodVisitor?,
    ) : MethodVisitor(OPCODE_VERSION, next) {
        /**
         * Whether or not the hook can return false to skip the rest of the method body.
         */
        val allowReturn = name == "<clinit>"

        override fun visitCode() {
            super.visitCode() // Note it's possible nested visitor added code too

            hooks.forEach { hook ->
                mv.visitLdcInsn(Type.getType("L$currentClassName;"))
                visitLdcInsn(name)
                visitLdcInsn(descriptor)

                val (clazz, method) = hook.parseClassMethod()
                visitMethodInsn(INVOKESTATIC, clazz, method,
                    "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;)Z",
                    false
                )
                if (!allowReturn) {
                    // We don't allow "return". The hook must always return true.
                    visitMethodInsn(INVOKESTATIC, HostTestUtils.CLASS_INTERNAL_NAME,
                        HostTestUtils.ASSERT_THAT_HOOK_RETURNED_TRUE,
                        "(Z)V",
                        false
                    )
                } else {
                    var label = Label()

                    visitJumpInsn(Opcodes.IFNE, label)
                    if (Type.getReturnType(descriptor) != Type.VOID_TYPE) {
                        // If we ever want to use it for non-void method,
                        // The below visitFrame() call needs to accommodate that.
                        // (Need some investigation to implement it properly...)
                        throw HostStubGenInternalException(
                            "This feature for non-void method isn't implemented yet")
                    }
                    writeByteCodeToReturnDefault(descriptor, this)

                    visitLabel(label)

                    // We need to inject a frame.
                    //
                    // We assume our method cal is the first thing we do in this method
                    // and the frame can be empty.
                    // In reality, it's possible the "next" visitors created more with frames,
                    // in super.visitCode(), in that case this would probably break...
                    //
                    super.visitFrame(Opcodes.F_NEW, 0, null, 0, null)
                }
            }
        }
    }

    /**
     * Inject a class load hook call.
     */
+141 −28

File changed.

Preview size limit exceeded, changes collapsed.

Loading