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

Commit c0d10972 authored by Makoto Onuki's avatar Makoto Onuki
Browse files

Make text filter policy resolution more intuitive 

- Previously, text policies would always win over annotation based
policies.

For example, even if a class has `@Remove`, a package-wide "keep" policy
would still override it. But that was kind of unintuitive and
made package policies less useful.

Now, text policies will not override it if the fallback policy is
more narrowly scoped.

Meaning:
    - Package text policies won't override class annotations
    - Class text policies won't override member annotations

- We couldn't previously do it because InMemoryOutputFilter couldn't tell
apart if a fallback policy is explicitly set (e.g. with annotations)
or if it's just the default policy.

In order to solve this problem, now we have a field "isDefault"
in FilterPolicyWithReason.

- This CL inherits the "'experimental' policies won't override
'supported' policies" behavior from the previous CL.
However, the previous implementation didn't allow making
a @KeepPartialClass as "experimental". (to make all unsupported
experimental while keeping annotated methods as-is.)

This is because `@KeepPartialClass` is considered to be "supported"
at the class level, and "experimental" class policy wouldn't override
it. So this means, for example, even if we added
"class ActivityThread experimental" in the policy file, it was
ignored. But we want to do it to make all unsupported methods in it
as experimental.

To solve it, now we have a new filter policy, ExperimentalClass.

ExperimentalClass is basically the same as KeepClass -- specifically,
the class itself is considered to have "Keep" -- but it distributes
"experimental" to the members.

ExperimentalClass (at the class level) _is_ considered to be supported,
because it means "Keep" for the class itself.
But its members will get "Experimental", which is considered to be
unsupported.

- The above changes will allow, for example, marking ActivityThread
(which currently has @RavenwoodKeepPartialClass) as "experimental" and
enable all its methods. Note some of its members have `@Ignore`
annotations, and the class-wide "experimental" policy won't override
it, because method annotations are more narrowly scoped. If we want to
change it, we'd use a method level text policy.

- See the javadoc on InMemoryOutputFilter and FilterPolicy for more
details.

Bug: 292141694
Flag: TEST_ONLY
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh -s
Change-Id: I85c62dcb14aca4e1fbc6c06330bfc8c57abcdce8
parent cae12a92

ravenwood/tools/hoststubgen/annotations-src/android/hosttest/annotation/HostSideTestStaticInitializerKeep.java

+1 −4
Original line number Diff line number Diff line
@@ -15,9 +15,6 @@ 
 */

package android.hosttest.annotation;



import static java.lang.annotation.ElementType.CONSTRUCTOR;

import static java.lang.annotation.ElementType.FIELD;

import static java.lang.annotation.ElementType.METHOD;

import static java.lang.annotation.ElementType.TYPE;



import java.lang.annotation.Retention;
@@ -27,7 +24,7 @@ import java.lang.annotation.Target; 
/**

 * Only used for HostStubGen tests. It's not used by Ravenwood.

 */

@Target({TYPE, FIELD, METHOD, CONSTRUCTOR})

@Target({TYPE})

@Retention(RetentionPolicy.CLASS)

public @interface HostSideTestStaticInitializerKeep {

}

ravenwood/tools/hoststubgen/helper-runtime-src/com/android/hoststubgen/hosthelper/HostTestUtils.java

+2 −1
Original line number Diff line number Diff line
@@ -52,7 +52,8 @@ public class HostTestUtils { 
                case HostStubGenProcessedAsIgnore a -> a.reason();

                case HostStubGenProcessedAsThrow a -> a.reason();

                case HostStubGenProcessedAsThrowButSupported a -> a.reason();

                // case HostStubGenProcessedAsSubstitute a -> a.reason();

                case HostStubGenProcessedAsExperimental a -> a.reason();

                case HostStubGenProcessedAsSubstitute a -> a.reason();

                default -> null;

            };

            if (reason != null && !reason.isEmpty()) {

ravenwood/tools/hoststubgen/lib/com/android/hoststubgen/HostStubGenClassProcessor.kt

+11 −6
Original line number Diff line number Diff line
@@ -32,13 +32,13 @@ import com.android.hoststubgen.utils.ClassPredicate 
import com.android.hoststubgen.visitors.ImplGeneratingAdapter

import com.android.hoststubgen.visitors.JdkPatchVisitor

import com.android.hoststubgen.visitors.PackageRedirectRemapper

import java.io.PrintWriter

import org.objectweb.asm.ClassReader

import org.objectweb.asm.ClassVisitor

import org.objectweb.asm.ClassWriter

import org.objectweb.asm.commons.ClassRemapper

import org.objectweb.asm.util.CheckClassAdapter

import org.objectweb.asm.util.TraceClassVisitor

import java.io.PrintWriter



/**

 * This class implements bytecode transformation of HostStubGen.
@@ -58,14 +58,15 @@ class HostStubGenClassProcessor( 
        // Connect to the base visitor

        var outVisitor: ClassVisitor = base



        if (!options.disableJdkPatch.get) {

            outVisitor = JdkPatchVisitor(outVisitor)

        }



        // This should be the innermost visitor. This one checks the final bytecode.

        if (options.enableClassChecker.get) {

            outVisitor = CheckClassAdapter(outVisitor)

        }



        if (!options.disableJdkPatch.get) {

            outVisitor = JdkPatchVisitor(outVisitor)

        }



        // Remapping should happen at the end.

        outVisitor = ClassRemapper(outVisitor, remapper)
@@ -155,7 +156,11 @@ class HostStubGenClassProcessor( 
            var filter: OutputFilter



            // The first filter is for the default policy from the command line options.

            filter = ConstantFilter(options.defaultPolicy.get, "default-by-options")

            filter = ConstantFilter(

                options.defaultPolicy.get.resolveDefaultForClass(),

                options.defaultPolicy.get.resolveDefaultForFields(),

                options.defaultPolicy.get.resolveDefaultForMethods(),

            )



            // Next, we build a filter that preserves all native methods by default

            filter = KeepNativeFilter(allClasses, filter)

ravenwood/tools/hoststubgen/lib/com/android/hoststubgen/filters/AnnotationBasedFilter.kt

+1 −1
Original line number Diff line number Diff line
@@ -65,7 +65,7 @@ class AnnotationBasedFilter( 
     * policy.

     */

    var annotationAllowedMembers: OutputFilter =

        ConstantFilter(FilterPolicy.Remove, "default disallowed")

        ConstantFilter(FilterPolicy.Remove.withReason("default disallowed"))



    private val keepAnnotations = convertToInternalNames(keepAnnotations_)

    private val keepClassAnnotations = convertToInternalNames(keepClassAnnotations_)

ravenwood/tools/hoststubgen/lib/com/android/hoststubgen/filters/ConstantFilter.kt

+13 −21
Original line number Diff line number Diff line
@@ -22,38 +22,30 @@ import com.android.hoststubgen.HostStubGenInternalException 
 * [OutputFilter] with a given policy. Used to represent the default policy.

 *

 * This is used as the last fallback filter.

 *

 * @param policy the policy. Cannot be a "substitute" policy.

 */

class ConstantFilter(

    policy: FilterPolicy,

    private val reason: String

    private val classPolicy: FilterPolicyWithReason,

    private val fieldPolicy: FilterPolicyWithReason = classPolicy,

    private val methodPolicy: FilterPolicyWithReason = classPolicy,

) : OutputFilter() {



    private val classPolicy: FilterPolicy

    private val fieldPolicy: FilterPolicy

    private val methodPolicy: FilterPolicy



    init {

        if (!policy.isUsableWithDefault) {

            throw HostStubGenInternalException("ConstantFilter doesn't support $policy.")

        if (!classPolicy.policy.isUsableWithClasses) {

            throw HostStubGenInternalException("${classPolicy.policy} can't be used for classes")

        }

        methodPolicy = policy



        // If the default policy is "throw", we convert it to "keep" for classes and fields.

        classPolicy = when (policy) {

            FilterPolicy.Throw -> FilterPolicy.Keep

            else -> policy

        if (!fieldPolicy.policy.isUsableWithFields) {

            throw HostStubGenInternalException("${fieldPolicy.policy} can't be used for fields")

        }

        if (!methodPolicy.policy.isUsableWithMethods) {

            throw HostStubGenInternalException("${methodPolicy.policy} can't be used for methods")

        }

        fieldPolicy = classPolicy

    }



    override fun getPolicyForClass(className: String): FilterPolicyWithReason {

        return classPolicy.withReason(reason)

        return classPolicy

    }



    override fun getPolicyForField(className: String, fieldName: String): FilterPolicyWithReason {

        return fieldPolicy.withReason(reason)

        return fieldPolicy

    }



    override fun getPolicyForMethod(
@@ -61,6 +53,6 @@ class ConstantFilter( 
        methodName: String,

        descriptor: String,

    ): FilterPolicyWithReason {

        return methodPolicy.withReason(reason)

        return methodPolicy

    }

}