diff options
| author | 2024-09-03 18:00:17 +0000 | |
|---|---|---|
| committer | 2024-09-03 18:00:17 +0000 | |
| commit | 1566e17ed5b70d9ce8527761558e94770a8122f4 (patch) | |
| tree | a2ad36e1564f3c79ea6633c045c1b3d2e48993d7 /tools | |
| parent | 10bf2e16ff89e4c41ad7237767a0c9d44a168af3 (diff) | |
[HostStubGen] Multiple improvements
1. Consolidate SubstituteAnd[Stub/Keep] into a single Substitute policy.
There is little reason to support SubstituteKeep, so make Substitute
implicitly Stub. In production, methods are never annotated with both
Stub/Keep + Substitute anyways.
2. Update ClassFilter to support nested classes.
3. Rewrite AnnotationFilter with the following improvements:
- Previously, when a method is annotated as Substitute, the method
itself will get the Remove policy, and the replacement method will
get the Stub/Keep policy. In the new implementation, the annotated
method will preserve its Substitute policy, which will not only be
useful for future improvements, but also be consistent with how
text policy filter is implemented.
- Process the entire class all at once and cache its results; the
previous implementation re-run the same operations multiple times.
- Previously, the annotation allowlist was not properly enforced when
Substitute annotation is used.
Bug: 292141694
Flag: EXEMPT host test change only
Test: atest hoststubgen-test-tiny-test
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh
Change-Id: Iee223c5d0814351eccdcfc884da64bc507af83b7
Diffstat (limited to 'tools')
11 files changed, 322 insertions, 369 deletions
diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/AnnotationBasedFilter.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/AnnotationBasedFilter.kt index 248121c63d78..e25e882a8571 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/AnnotationBasedFilter.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/AnnotationBasedFilter.kt @@ -17,7 +17,6 @@ package com.android.hoststubgen.filters import com.android.hoststubgen.ClassParseException import com.android.hoststubgen.HostStubGenErrors -import com.android.hoststubgen.HostStubGenInternalException import com.android.hoststubgen.InvalidAnnotationException import com.android.hoststubgen.addNonNullElement import com.android.hoststubgen.asm.CLASS_INITIALIZER_DESC @@ -35,53 +34,58 @@ import org.objectweb.asm.tree.ClassNode // TODO: Detect invalid cases, such as... // - Class's visibility is lower than the members'. -// - HostSideTestSubstituteWith is set, but it doesn't have @Stub or @Keep /** * [OutputFilter] using Java annotations. */ class AnnotationBasedFilter( - private val errors: HostStubGenErrors, - private val classes: ClassNodes, - stubAnnotations_: Set<String>, - keepAnnotations_: Set<String>, - stubClassAnnotations_: Set<String>, - keepClassAnnotations_: Set<String>, - throwAnnotations_: Set<String>, - removeAnnotations_: Set<String>, - substituteAnnotations_: Set<String>, - nativeSubstituteAnnotations_: Set<String>, - classLoadHookAnnotations_: Set<String>, - keepStaticInitializerAnnotations_: Set<String>, - private val annotationAllowedClassesFilter: ClassFilter, - fallback: OutputFilter, + private val errors: HostStubGenErrors, + private val classes: ClassNodes, + stubAnnotations_: Set<String>, + keepAnnotations_: Set<String>, + stubClassAnnotations_: Set<String>, + keepClassAnnotations_: Set<String>, + throwAnnotations_: Set<String>, + removeAnnotations_: Set<String>, + substituteAnnotations_: Set<String>, + nativeSubstituteAnnotations_: Set<String>, + classLoadHookAnnotations_: Set<String>, + keepStaticInitializerAnnotations_: Set<String>, + private val annotationAllowedClassesFilter: ClassFilter, + fallback: OutputFilter, ) : DelegatingFilter(fallback) { - private var stubAnnotations = convertToInternalNames(stubAnnotations_) - private var keepAnnotations = convertToInternalNames(keepAnnotations_) - private var stubClassAnnotations = convertToInternalNames(stubClassAnnotations_) - private var keepClassAnnotations = convertToInternalNames(keepClassAnnotations_) - private var throwAnnotations = convertToInternalNames(throwAnnotations_) - private var removeAnnotations = convertToInternalNames(removeAnnotations_) - private var substituteAnnotations = convertToInternalNames(substituteAnnotations_) - private var nativeSubstituteAnnotations = convertToInternalNames(nativeSubstituteAnnotations_) - private var classLoadHookAnnotations = convertToInternalNames(classLoadHookAnnotations_) - private var keepStaticInitializerAnnotations = - convertToInternalNames(keepStaticInitializerAnnotations_) + private val stubAnnotations = convertToInternalNames(stubAnnotations_) + private val keepAnnotations = convertToInternalNames(keepAnnotations_) + private val stubClassAnnotations = convertToInternalNames(stubClassAnnotations_) + private val keepClassAnnotations = convertToInternalNames(keepClassAnnotations_) + private val throwAnnotations = convertToInternalNames(throwAnnotations_) + private val removeAnnotations = convertToInternalNames(removeAnnotations_) + private val substituteAnnotations = convertToInternalNames(substituteAnnotations_) + private val nativeSubstituteAnnotations = convertToInternalNames(nativeSubstituteAnnotations_) + private val classLoadHookAnnotations = convertToInternalNames(classLoadHookAnnotations_) + private val keepStaticInitializerAnnotations = + convertToInternalNames(keepStaticInitializerAnnotations_) /** Annotations that control API visibility. */ - private var visibilityAnnotations: Set<String> = convertToInternalNames( - stubAnnotations_ + - keepAnnotations_ + - stubClassAnnotations_ + - keepClassAnnotations_ + - throwAnnotations_ + - removeAnnotations_) + private val visibilityAnnotations = stubAnnotations + + keepAnnotations + + stubClassAnnotations + + keepClassAnnotations + + throwAnnotations + + removeAnnotations + + substituteAnnotations + + /** All the annotations we use. */ + private val allAnnotations = visibilityAnnotations + + nativeSubstituteAnnotations + + classLoadHookAnnotations + + keepStaticInitializerAnnotations /** * All the annotations we use. Note, this one is in a [convertToJvmNames] format unlike * other ones, because of how it's used. */ - private var allAnnotations: Set<String> = convertToJvmNames( + private val allAnnotationClasses: Set<String> = convertToJvmNames( stubAnnotations_ + keepAnnotations_ + stubClassAnnotations_ + @@ -90,203 +94,75 @@ class AnnotationBasedFilter( removeAnnotations_ + substituteAnnotations_ + nativeSubstituteAnnotations_ + - classLoadHookAnnotations_) - - private val substitutionHelper = SubstitutionHelper() - - private val reasonAnnotation = "annotation" - private val reasonClassAnnotation = "class-annotation" - - /** - * Throw if an item has more than one visibility annotations. - * - * name1 - 4 are only used in exception messages. We take them as separate strings - * to avoid unnecessary string concatenations. - */ - private fun detectInvalidAnnotations( - visibles: List<AnnotationNode>?, - invisibles: List<AnnotationNode>?, - type: String, - name1: String, - name2: String, - name3: String, - ) { - var count = 0 - for (an in visibles ?: emptyList()) { - if (visibilityAnnotations.contains(an.desc)) { - count++ - } - } - for (an in invisibles ?: emptyList()) { - if (visibilityAnnotations.contains(an.desc)) { - count++ - } - } - if (count > 1) { - val description = if (name2 == "" && name3 == "") { - "$type $name1" - } else { - "$type $name1.$name2$name3" - } - throw InvalidAnnotationException( - "Found more than one visibility annotations on $description") + classLoadHookAnnotations_ + + keepStaticInitializerAnnotations_ + ) + + private val policyCache = mutableMapOf<String, ClassAnnotations>() + + private val AnnotationNode.policy: FilterPolicyWithReason? get() { + return when (desc) { + in stubAnnotations -> FilterPolicy.Stub.withReason(REASON_ANNOTATION) + in stubClassAnnotations -> FilterPolicy.StubClass.withReason(REASON_CLASS_ANNOTATION) + in keepAnnotations -> FilterPolicy.Keep.withReason(REASON_ANNOTATION) + in keepClassAnnotations -> FilterPolicy.KeepClass.withReason(REASON_CLASS_ANNOTATION) + in substituteAnnotations -> FilterPolicy.Substitute.withReason(REASON_ANNOTATION) + in throwAnnotations -> FilterPolicy.Throw.withReason(REASON_ANNOTATION) + in removeAnnotations -> FilterPolicy.Remove.withReason(REASON_ANNOTATION) + else -> null } } - fun findAnyAnnotation( - className: String, - anyAnnotations: Set<String>, - visibleAnnotations: List<AnnotationNode>?, - invisibleAnnotations: List<AnnotationNode>?, - ): AnnotationNode? { - val ret = findAnyAnnotation(anyAnnotations, visibleAnnotations, invisibleAnnotations) - - if (ret != null) { - if (!annotationAllowedClassesFilter.matches(className)) { - throw InvalidAnnotationException( - "Class ${className.toHumanReadableClassName()} is not allowed to have " + - "Ravenwood annotations. Contact g/ravenwood for more details.") - } - } - - return ret - } - - /** - * Find a visibility annotation. - * - * name1 - 4 are only used in exception messages. - */ - private fun findAnnotation( - className: String, - visibles: List<AnnotationNode>?, - invisibles: List<AnnotationNode>?, - type: String, - name1: String, - name2: String = "", - name3: String = "", - ): FilterPolicyWithReason? { - detectInvalidAnnotations(visibles, invisibles, type, name1, name2, name3) - - findAnyAnnotation(className, stubAnnotations, visibles, invisibles)?.let { - return FilterPolicy.Stub.withReason(reasonAnnotation) - } - findAnyAnnotation(className, stubClassAnnotations, visibles, invisibles)?.let { - return FilterPolicy.StubClass.withReason(reasonClassAnnotation) - } - findAnyAnnotation(className, keepAnnotations, visibles, invisibles)?.let { - return FilterPolicy.Keep.withReason(reasonAnnotation) - } - findAnyAnnotation(className, keepClassAnnotations, visibles, invisibles)?.let { - return FilterPolicy.KeepClass.withReason(reasonClassAnnotation) - } - findAnyAnnotation(className, throwAnnotations, visibles, invisibles)?.let { - return FilterPolicy.Throw.withReason(reasonAnnotation) - } - findAnyAnnotation(className, removeAnnotations, visibles, invisibles)?.let { - return FilterPolicy.Remove.withReason(reasonAnnotation) - } - - return null + private fun getAnnotationPolicy(cn: ClassNode): ClassAnnotations { + return policyCache.getOrPut(cn.name) { ClassAnnotations(cn) } } override fun getPolicyForClass(className: String): FilterPolicyWithReason { - val cn = classes.getClass(className) - - findAnnotation( - cn.name, - cn.visibleAnnotations, - cn.invisibleAnnotations, - "class", - className)?.let { - return it - } - // If it's any of the annotations, then always keep it. - if (allAnnotations.contains(className)) { + if (allAnnotationClasses.contains(className)) { return FilterPolicy.KeepClass.withReason("HostStubGen Annotation") } - return super.getPolicyForClass(className) + val cn = classes.getClass(className) + return getAnnotationPolicy(cn).classPolicy ?: super.getPolicyForClass(className) } - override fun getPolicyForField( - className: String, - fieldName: String - ): FilterPolicyWithReason { + override fun getPolicyForField(className: String, fieldName: String): FilterPolicyWithReason { val cn = classes.getClass(className) - - cn.fields?.firstOrNull { it.name == fieldName }?.let {fn -> - findAnnotation( - cn.name, - fn.visibleAnnotations, - fn.invisibleAnnotations, - "field", - className, - fieldName - )?.let { policy -> - // If the item has an annotation, then use it. - return policy - } - } - return super.getPolicyForField(className, fieldName) + return getAnnotationPolicy(cn).fieldPolicies[fieldName] + ?: super.getPolicyForField(className, fieldName) } override fun getPolicyForMethod( - className: String, - methodName: String, - descriptor: String + className: String, + methodName: String, + descriptor: String ): FilterPolicyWithReason { val cn = classes.getClass(className) if (methodName == CLASS_INITIALIZER_NAME && descriptor == CLASS_INITIALIZER_DESC) { - findAnyAnnotation(cn.name, keepStaticInitializerAnnotations, - cn.visibleAnnotations, cn.invisibleAnnotations)?.let { - return FilterPolicy.Keep.withReason(reasonAnnotation) + if (cn.findAnyAnnotation(keepStaticInitializerAnnotations) != null) { + return FilterPolicy.Keep.withReason(REASON_ANNOTATION) } } - cn.methods?.firstOrNull { it.name == methodName && it.desc == descriptor }?.let { mn -> - // @SubstituteWith is going to complicate the policy here, so we ask helper - // what to do. - substitutionHelper.getPolicyFromSubstitution(cn, mn.name, mn.desc)?.let { - return it - } - - // If there's no substitution, then we check the annotation. - findAnnotation( - cn.name, - mn.visibleAnnotations, - mn.invisibleAnnotations, - "method", - className, - methodName, - descriptor - )?.let { policy -> - return policy - } - } - return super.getPolicyForMethod(className, methodName, descriptor) + return getAnnotationPolicy(cn).methodPolicies[MethodKey(methodName, descriptor)] + ?: super.getPolicyForMethod(className, methodName, descriptor) } override fun getRenameTo( - className: String, - methodName: String, - descriptor: String + className: String, + methodName: String, + descriptor: String ): String? { val cn = classes.getClass(className) - - // If the method has a "substitute with" annotation, then return its "value" parameter. - cn.methods?.firstOrNull { it.name == methodName && it.desc == descriptor }?.let { mn -> - return substitutionHelper.getRenameTo(cn, mn.name, mn.desc) - } - return null + return getAnnotationPolicy(cn).renamedMethods[MethodKey(methodName, descriptor)] + ?: super.getRenameTo(className, methodName, descriptor) } override fun getNativeSubstitutionClass(className: String): String? { classes.getClass(className).let { cn -> - findAnyAnnotation(nativeSubstituteAnnotations, - cn.visibleAnnotations, cn.invisibleAnnotations)?.let { an -> + cn.findAnyAnnotation(nativeSubstituteAnnotations)?.let { an -> return getAnnotationField(an, "value")?.toJvmClassName() } } @@ -295,8 +171,7 @@ class AnnotationBasedFilter( override fun getClassLoadHooks(className: String): List<String> { val e = classes.getClass(className).let { cn -> - findAnyAnnotation(classLoadHookAnnotations, - cn.visibleAnnotations, cn.invisibleAnnotations)?.let { an -> + cn.findAnyAnnotation(classLoadHookAnnotations)?.let { an -> getAnnotationField(an, "value")?.toHumanReadableMethodName() } } @@ -306,102 +181,130 @@ class AnnotationBasedFilter( private data class MethodKey(val name: String, val desc: String) /** - * In order to handle substitution, we need to build a reverse mapping of substitution - * methods. + * Every time we see a class, we scan all its methods for substitution attributes, + * and compute (implicit) policies caused by them. + * + * For example, for the following methods: * - * This class automatically builds such a map internally that the above methods can - * take advantage of. + * @Substitute(suffix = "_host") + * private void foo() { + * // This isn't supported on the host side. + * } + * private void foo_host() { + * // Host side implementation + * } + * + * We internally handle them as: + * + * foo() -> Substitute + * foo_host() -> Stub, and then rename it to foo(). */ - private inner class SubstitutionHelper { - private var currentClass: ClassNode? = null - - private var policiesFromSubstitution = mutableMapOf<MethodKey, FilterPolicyWithReason>() - private var substituteToMethods = mutableMapOf<MethodKey, String>() - - fun getPolicyFromSubstitution(cn: ClassNode, methodName: String, descriptor: String): - FilterPolicyWithReason? { - setClass(cn) - return policiesFromSubstitution[MethodKey(methodName, descriptor)] - } + private inner class ClassAnnotations(cn: ClassNode) { + + val classPolicy: FilterPolicyWithReason? + val fieldPolicies = mutableMapOf<String, FilterPolicyWithReason>() + val methodPolicies = mutableMapOf<MethodKey, FilterPolicyWithReason>() + val renamedMethods = mutableMapOf<MethodKey, String>() + + init { + val allowAnnotation = annotationAllowedClassesFilter.matches(cn.name) + detectInvalidAnnotations( + cn.name, allowAnnotation, + cn.visibleAnnotations, cn.invisibleAnnotations, + "class", cn.name + ) + classPolicy = cn.findAnyAnnotation(visibilityAnnotations)?.policy + + for (fn in cn.fields ?: emptyList()) { + detectInvalidAnnotations( + cn.name, allowAnnotation, + fn.visibleAnnotations, fn.invisibleAnnotations, + "field", cn.name, fn.name + ) + fn.findAnyAnnotation(visibilityAnnotations)?.policy?.let { + fieldPolicies[fn.name] = it + } + } - fun getRenameTo(cn: ClassNode, methodName: String, descriptor: String): String? { - setClass(cn) - return substituteToMethods[MethodKey(methodName, descriptor)] + for (mn in cn.methods ?: emptyList()) { + detectInvalidAnnotations( + cn.name, allowAnnotation, + mn.visibleAnnotations, mn.invisibleAnnotations, + "method", cn.name, mn.name, mn.desc + ) + + val an = mn.findAnyAnnotation(visibilityAnnotations) ?: continue + val policy = an.policy ?: continue + methodPolicies[MethodKey(mn.name, mn.desc)] = policy + + if (policy.policy != FilterPolicy.Substitute) continue + + // Handle substitution + val suffix = getAnnotationField(an, "suffix", false) ?: "\$ravenwood" + val replacement = mn.name + suffix + + if (replacement == mn.name) { + errors.onErrorFound("@SubstituteWith require a different name") + } else { + // The replacement method has to be renamed + methodPolicies[MethodKey(replacement, mn.desc)] = + FilterPolicy.Stub.withReason(REASON_ANNOTATION) + renamedMethods[MethodKey(replacement, mn.desc)] = mn.name + + log.v("Substitution found: %s%s -> %s", replacement, mn.desc, mn.name) + } + } } /** - * Every time we see a different class, we scan all its methods for substitution attributes, - * and compute (implicit) policies caused by them. - * - * For example, for the following methods: + * Throw if an item has more than one visibility annotations, or the class is not allowed * - * @Stub - * @Substitute(suffix = "_host") - * private void foo() { - * // This isn't supported on the host side. - * } - * private void foo_host() { - * // Host side implementation - * } - * - * We internally handle them as: - * - * foo() -> Remove - * foo_host() -> Stub, and then rename it to foo(). + * name1 - 4 are only used in exception messages. We take them as separate strings + * to avoid unnecessary string concatenations. */ - private fun setClass(cn: ClassNode) { - if (currentClass == cn) { - return + private fun detectInvalidAnnotations( + className: String, + allowAnnotation: Boolean, + visibles: List<AnnotationNode>?, + invisibles: List<AnnotationNode>?, + type: String, + name1: String, + name2: String = "", + name3: String = "", + ) { + var count = 0 + var visibleCount = 0 + for (an in visibles ?: emptyList()) { + if (visibilityAnnotations.contains(an.desc)) { + visibleCount++ + } + if (allAnnotations.contains(an.desc)) { + count++ + } } - // If the class is changing, we'll rebuild the internal structure. - currentClass = cn - - policiesFromSubstitution.clear() - substituteToMethods.clear() - - for (mn in cn.methods ?: emptyList()) { - findAnyAnnotation(substituteAnnotations, - mn.visibleAnnotations, - mn.invisibleAnnotations)?.let { an -> - - // Find the policy for this method. - val policy = outermostFilter.getPolicyForMethod(cn.name, mn.name, mn.desc) - .policy.resolveClassWidePolicy() - // Make sure it's either Stub or Keep. - if (!(policy.needsInStub || policy.needsInImpl)) { - // TODO: Use the real annotation names in the message - errors.onErrorFound("@SubstituteWith must have either @Stub or @Keep") - return@let - } - if (!policy.isUsableWithMethods) { - throw HostStubGenInternalException("Policy $policy shouldn't show up here") - } - - val suffix = getAnnotationField(an, "suffix", false) ?: "\$ravenwood" - val renameFrom = mn.name + suffix - val renameTo = mn.name - - if (renameFrom == renameTo) { - errors.onErrorFound("@SubstituteWith have a different name") - return@let - } - - // This mn has "SubstituteWith". This means, - // 1. Re move the "rename-to" method, so add it to substitutedMethods. - policiesFromSubstitution[MethodKey(renameTo, mn.desc)] = - FilterPolicy.Remove.withReason("substitute-to") - - // If the policy is "stub", use "stub". - // Otherwise, it must be "keep" or "throw", but there's no point in using - // "throw", so let's use "keep". - val newPolicy = if (policy.needsInStub) policy else FilterPolicy.Keep - // 2. We also keep the from-to in the map. - policiesFromSubstitution[MethodKey(renameFrom, mn.desc)] = - newPolicy.withReason("substitute-from") - substituteToMethods[MethodKey(renameFrom, mn.desc)] = renameTo - - log.v("Substitution found: %s%s -> %s", renameFrom, mn.desc, renameTo) + for (an in invisibles ?: emptyList()) { + if (visibilityAnnotations.contains(an.desc)) { + visibleCount++ } + if (allAnnotations.contains(an.desc)) { + count++ + } + } + if (count > 0 && !allowAnnotation) { + throw InvalidAnnotationException( + "Class ${className.toHumanReadableClassName()} is not allowed to have " + + "Ravenwood annotations. Contact g/ravenwood for more details." + ) + } + if (visibleCount > 1) { + val description = if (name2 == "" && name3 == "") { + "$type $name1" + } else { + "$type $name1.$name2$name3" + } + throw InvalidAnnotationException( + "Found more than one visibility annotations on $description" + ) } } } @@ -409,8 +312,11 @@ class AnnotationBasedFilter( /** * Return the (String) value of 'value' parameter from an annotation. */ - private fun getAnnotationField(an: AnnotationNode, name: String, - required: Boolean = true): String? { + private fun getAnnotationField( + an: AnnotationNode, + name: String, + required: Boolean = true + ): String? { try { val suffix = findAnnotationValueAsString(an, name) if (suffix == null && required) { @@ -424,6 +330,9 @@ class AnnotationBasedFilter( } companion object { + private const val REASON_ANNOTATION = "annotation" + private const val REASON_CLASS_ANNOTATION = "class-annotation" + /** * Convert from human-readable type names (e.g. "com.android.TypeName") to the internal type * names (e.g. "Lcom/android/TypeName). diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/ConstantFilter.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/ConstantFilter.kt index 678e6eae0be6..27d98b81d52e 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/ConstantFilter.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/ConstantFilter.kt @@ -25,24 +25,15 @@ import com.android.hoststubgen.HostStubGenInternalException * * @param policy the policy. Cannot be a "substitute" policy. */ -class ConstantFilter( - policy: FilterPolicy, - val reason: String -) : OutputFilter() { - val classPolicy: FilterPolicy - val fieldPolicy: FilterPolicy - val methodPolicy: FilterPolicy +class ConstantFilter(policy: FilterPolicy, private val reason: String) : OutputFilter() { + + private val classPolicy: FilterPolicy + private val fieldPolicy: FilterPolicy + private val methodPolicy: FilterPolicy init { - if (policy.isSubstitute) { - throw HostStubGenInternalException( - "ConstantFilter doesn't allow substitution policies.") - } - if (policy.isClassWidePolicy) { - // We prevent it, because there's no point in using class-wide policies because - // all members get othe same policy too anyway. - throw HostStubGenInternalException( - "ConstantFilter doesn't allow class-wide policies.") + if (!policy.isUsableWithDefault) { + throw HostStubGenInternalException("ConstantFilter doesn't support $policy.") } methodPolicy = policy @@ -63,10 +54,10 @@ class ConstantFilter( } override fun getPolicyForMethod( - className: String, - methodName: String, - descriptor: String, - ): FilterPolicyWithReason { + className: String, + methodName: String, + descriptor: String, + ): FilterPolicyWithReason { return methodPolicy.withReason(reason) } } diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/FilterPolicy.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/FilterPolicy.kt index f839444abb46..0db4935c3386 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/FilterPolicy.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/FilterPolicy.kt @@ -40,14 +40,9 @@ enum class FilterPolicy { KeepClass, /** - * Same as [Stub], but replace it with a "substitution" method. Only usable with methods. + * Only usable with methods. Replace a method with a "substitution" method. */ - SubstituteAndStub, - - /** - * Same as [Keep], but replace it with a "substitution" method. Only usable with methods. - */ - SubstituteAndKeep, + Substitute, /** * Only usable with methods. The item will be kept in the impl jar file, but when called, @@ -57,7 +52,7 @@ enum class FilterPolicy { /** * Only usable with methods. The item will be kept in the impl jar file, but when called, - * it'll no-op. Currently only supported for methods returning `void`. + * it'll no-op. */ Ignore, @@ -66,11 +61,8 @@ enum class FilterPolicy { */ Remove; - val isSubstitute: Boolean - get() = this == SubstituteAndStub || this == SubstituteAndKeep - val needsInStub: Boolean - get() = this == Stub || this == StubClass || this == SubstituteAndStub || this == Ignore + get() = this == Stub || this == StubClass || this == Substitute || this == Ignore val needsInImpl: Boolean get() = this != Remove @@ -102,6 +94,15 @@ enum class FilterPolicy { } } + /** Returns whether a policy can be used as default policy. */ + val isUsableWithDefault: Boolean + get() { + return when (this) { + Stub, Keep, Throw, Remove -> true + else -> false + } + } + /** Returns whether a policy is a class-wide one. */ val isClassWidePolicy: Boolean get() { @@ -116,19 +117,11 @@ enum class FilterPolicy { get() { return when (this) { // TODO: handle native method with no substitution as being unsupported - Stub, StubClass, Keep, KeepClass, SubstituteAndStub, SubstituteAndKeep -> true + Stub, StubClass, Keep, KeepClass, Substitute -> true else -> false } } - fun getSubstitutionBasePolicy(): FilterPolicy { - return when (this) { - SubstituteAndKeep -> Keep - SubstituteAndStub -> Stub - else -> this - } - } - /** * Convert {Stub,Keep}Class to the corresponding Stub or Keep. */ diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt index 53bcf103cad4..7c301d3dcfbd 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt @@ -240,7 +240,7 @@ fun createFilterFromTextPolicyFile( imf.setPolicyForMethod(className, name, signature, policy.withReason(FILTER_REASON)) - if (policy.isSubstitute) { + if (policy == FilterPolicy.Substitute) { val fromName = fields[3].substring(1) if (fromName == name) { @@ -248,10 +248,9 @@ fun createFilterFromTextPolicyFile( "Substitution must have a different name") } - // Set the policy for the "from" method. + // Set the policy for the "from" method. imf.setPolicyForMethod(className, fromName, signature, - policy.getSubstitutionBasePolicy() - .withReason(FILTER_REASON)) + FilterPolicy.Stub.withReason(FILTER_REASON)) val classAndMethod = splitWithLastPeriod(fromName) if (classAndMethod != null) { @@ -355,9 +354,7 @@ private fun parsePolicy(s: String): FilterPolicy { "i", "ignore" -> FilterPolicy.Ignore else -> { if (s.startsWith("@")) { - FilterPolicy.SubstituteAndStub - } else if (s.startsWith("%")) { - FilterPolicy.SubstituteAndKeep + FilterPolicy.Substitute } else { throw ParseException("Invalid policy \"$s\"") } diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/utils/ClassFilter.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/utils/ClassFilter.kt index 01a7ab3eacfa..7440b9410a9b 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/utils/ClassFilter.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/utils/ClassFilter.kt @@ -24,19 +24,19 @@ import java.io.File /** * General purpose filter for class names. */ -class ClassFilter private constructor ( - val defaultResult: Boolean, +class ClassFilter private constructor( + private val defaultResult: Boolean, ) { - private data class FilterElement( - val allowed: Boolean, - val internalName: String, - val isPrefix: Boolean, + private class FilterElement( + val allowed: Boolean, + val internalName: String, + val isPrefix: Boolean, ) { fun matches(classInternalName: String): Boolean { - if (isPrefix) { - return classInternalName.startsWith(internalName) + return if (isPrefix) { + classInternalName.startsWith(internalName) } else { - return classInternalName == internalName + classInternalName == internalName } } } @@ -54,15 +54,16 @@ class ClassFilter private constructor ( return it } - var result = defaultResult - run outer@{ - elements.forEach { e -> - if (e.matches(classInternalName)) { - result = e.allowed - return@outer // break equivalent. - } + val testClasses = sequence { + // Yield itself and its outer class(es) one by one + var idx = classInternalName.length + while (idx > 0) { + yield(classInternalName.substring(0, idx)) + idx = classInternalName.lastIndexOf('$', idx - 1) } } + + val result = elements.find { testClasses.any(it::matches) }?.allowed ?: defaultResult cache[classInternalName] = result return result @@ -87,9 +88,9 @@ class ClassFilter private constructor ( /** Build a filter from a string (for unit tests). */ fun buildFromString( - filterString: String, - defaultResult: Boolean, - filenameForErrorMessage: String + filterString: String, + defaultResult: Boolean, + filenameForErrorMessage: String ): ClassFilter { val ret = ClassFilter(defaultResult) @@ -119,17 +120,20 @@ class ClassFilter private constructor ( // Handle wildcard -- e.g. "package.name.*" if (line.endsWith(".*")) { - ret.elements.add(FilterElement( - allow, line.substring(0, line.length - 2).toJvmClassName(), true)) + ret.elements.add( + FilterElement( + allow, line.substring(0, line.length - 2).toJvmClassName(), true + ) + ) return@forEach } // Any other uses of "*" would be an error. if (line.contains('*')) { throw ParseException( - "Wildcard (*) can only show up as the last element", - filenameForErrorMessage, - lineNo + "Wildcard (*) can only show up as the last element", + filenameForErrorMessage, + lineNo ) } ret.elements.add(FilterElement(allow, line.toJvmClassName(), false)) diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/BaseAdapter.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/BaseAdapter.kt index bad0449f1efd..6fd96f24c93e 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/BaseAdapter.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/visitors/BaseAdapter.kt @@ -187,7 +187,7 @@ abstract class BaseAdapter ( // Instead of this method, we rename the substitute-to method with the original // name, in the "Maybe rename the method" part below. val policy = filter.getPolicyForMethod(currentClassName, name, descriptor) - if (policy.policy.isSubstitute) { + if (policy.policy == FilterPolicy.Substitute) { log.d("Skipping %s%s %s", name, descriptor, policy) return null } diff --git a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/01-hoststubgen-test-tiny-framework-orig-dump.txt b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/01-hoststubgen-test-tiny-framework-orig-dump.txt index 845e1d08ce92..538ae2044a04 100644 --- a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/01-hoststubgen-test-tiny-framework-orig-dump.txt +++ b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/01-hoststubgen-test-tiny-framework-orig-dump.txt @@ -513,8 +513,6 @@ public class com.android.hoststubgen.test.tinyframework.TinyFrameworkAnnotations 0 10 0 this Lcom/android/hoststubgen/test/tinyframework/TinyFrameworkAnnotations; 0 10 1 value I RuntimeInvisibleAnnotations: - x: #x() - android.hosttest.annotation.HostSideTestStub x: #x(#x=s#x) android.hosttest.annotation.HostSideTestSubstitute( suffix="_host" @@ -539,8 +537,6 @@ public class com.android.hoststubgen.test.tinyframework.TinyFrameworkAnnotations descriptor: (I)I flags: (0x0109) ACC_PUBLIC, ACC_STATIC, ACC_NATIVE RuntimeInvisibleAnnotations: - x: #x() - android.hosttest.annotation.HostSideTestStub x: #x(#x=s#x) android.hosttest.annotation.HostSideTestSubstitute( suffix="_host" diff --git a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/03-hoststubgen-test-tiny-framework-host-impl-dump.txt b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/03-hoststubgen-test-tiny-framework-host-impl-dump.txt index c6b9c7a9e4f1..f8c60e231832 100644 --- a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/03-hoststubgen-test-tiny-framework-host-impl-dump.txt +++ b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/03-hoststubgen-test-tiny-framework-host-impl-dump.txt @@ -98,6 +98,28 @@ RuntimeVisibleAnnotations: java.lang.annotation.Retention( value=Ljava/lang/annotation/RetentionPolicy;.CLASS ) +## Class: android/hosttest/annotation/HostSideTestStaticInitializerKeep.class + Compiled from "HostSideTestStaticInitializerKeep.java" +public interface android.hosttest.annotation.HostSideTestStaticInitializerKeep extends java.lang.annotation.Annotation + minor version: 0 + major version: 61 + flags: (0x2601) ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT, ACC_ANNOTATION + this_class: #x // android/hosttest/annotation/HostSideTestStaticInitializerKeep + super_class: #x // java/lang/Object + interfaces: 1, fields: 0, methods: 0, attributes: 2 +} +SourceFile: "HostSideTestStaticInitializerKeep.java" +RuntimeVisibleAnnotations: + x: #x() + com.android.hoststubgen.hosthelper.HostStubGenKeptInImpl + x: #x(#x=[e#x.#x,e#x.#x,e#x.#x,e#x.#x]) + java.lang.annotation.Target( + value=[Ljava/lang/annotation/ElementType;.TYPE,Ljava/lang/annotation/ElementType;.FIELD,Ljava/lang/annotation/ElementType;.METHOD,Ljava/lang/annotation/ElementType;.CONSTRUCTOR] + ) + x: #x(#x=e#x.#x) + java.lang.annotation.Retention( + value=Ljava/lang/annotation/RetentionPolicy;.CLASS + ) ## Class: android/hosttest/annotation/HostSideTestStub.class Compiled from "HostSideTestStub.java" public interface android.hosttest.annotation.HostSideTestStub extends java.lang.annotation.Annotation diff --git a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/13-hoststubgen-test-tiny-framework-host-ext-impl-dump.txt b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/13-hoststubgen-test-tiny-framework-host-ext-impl-dump.txt index da434a615c81..11610b7176a6 100644 --- a/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/13-hoststubgen-test-tiny-framework-host-ext-impl-dump.txt +++ b/tools/hoststubgen/hoststubgen/test-tiny-framework/golden-output/13-hoststubgen-test-tiny-framework-host-ext-impl-dump.txt @@ -136,6 +136,37 @@ RuntimeVisibleAnnotations: java.lang.annotation.Retention( value=Ljava/lang/annotation/RetentionPolicy;.CLASS ) +## Class: android/hosttest/annotation/HostSideTestStaticInitializerKeep.class + Compiled from "HostSideTestStaticInitializerKeep.java" +public interface android.hosttest.annotation.HostSideTestStaticInitializerKeep extends java.lang.annotation.Annotation + minor version: 0 + major version: 61 + flags: (0x2601) ACC_PUBLIC, ACC_INTERFACE, ACC_ABSTRACT, ACC_ANNOTATION + this_class: #x // android/hosttest/annotation/HostSideTestStaticInitializerKeep + super_class: #x // java/lang/Object + interfaces: 1, fields: 0, methods: 1, attributes: 2 + private static {}; + descriptor: ()V + flags: (0x000a) ACC_PRIVATE, ACC_STATIC + Code: + stack=2, locals=0, args_size=0 + x: ldc #x // class android/hosttest/annotation/HostSideTestStaticInitializerKeep + x: ldc #x // String com.android.hoststubgen.hosthelper.HostTestUtils.logClassLoaded + x: invokestatic #x // Method com/android/hoststubgen/hosthelper/HostTestUtils.onClassLoaded:(Ljava/lang/Class;Ljava/lang/String;)V + x: return +} +SourceFile: "HostSideTestStaticInitializerKeep.java" +RuntimeVisibleAnnotations: + x: #x() + com.android.hoststubgen.hosthelper.HostStubGenKeptInImpl + x: #x(#x=[e#x.#x,e#x.#x,e#x.#x,e#x.#x]) + java.lang.annotation.Target( + value=[Ljava/lang/annotation/ElementType;.TYPE,Ljava/lang/annotation/ElementType;.FIELD,Ljava/lang/annotation/ElementType;.METHOD,Ljava/lang/annotation/ElementType;.CONSTRUCTOR] + ) + x: #x(#x=e#x.#x) + java.lang.annotation.Retention( + value=Ljava/lang/annotation/RetentionPolicy;.CLASS + ) ## Class: android/hosttest/annotation/HostSideTestStub.class Compiled from "HostSideTestStub.java" public interface android.hosttest.annotation.HostSideTestStub extends java.lang.annotation.Annotation diff --git a/tools/hoststubgen/hoststubgen/test-tiny-framework/tiny-framework/src/com/android/hoststubgen/test/tinyframework/TinyFrameworkAnnotations.java b/tools/hoststubgen/hoststubgen/test-tiny-framework/tiny-framework/src/com/android/hoststubgen/test/tinyframework/TinyFrameworkAnnotations.java index 30dfc80fc60b..f7a24e8fbfcd 100644 --- a/tools/hoststubgen/hoststubgen/test-tiny-framework/tiny-framework/src/com/android/hoststubgen/test/tinyframework/TinyFrameworkAnnotations.java +++ b/tools/hoststubgen/hoststubgen/test-tiny-framework/tiny-framework/src/com/android/hoststubgen/test/tinyframework/TinyFrameworkAnnotations.java @@ -59,7 +59,6 @@ public class TinyFrameworkAnnotations { throw new RuntimeException(); } - @HostSideTestStub @HostSideTestSubstitute(suffix = "_host") public int addTwo(int value) { throw new RuntimeException("not supported on host side"); @@ -69,7 +68,6 @@ public class TinyFrameworkAnnotations { return value + 2; } - @HostSideTestStub @HostSideTestSubstitute(suffix = "_host") public static native int nativeAddThree(int value); diff --git a/tools/hoststubgen/hoststubgen/test/com/android/hoststubgen/utils/ClassFilterTest.kt b/tools/hoststubgen/hoststubgen/test/com/android/hoststubgen/utils/ClassFilterTest.kt index f6515142ccdb..85b6e80f84c4 100644 --- a/tools/hoststubgen/hoststubgen/test/com/android/hoststubgen/utils/ClassFilterTest.kt +++ b/tools/hoststubgen/hoststubgen/test/com/android/hoststubgen/utils/ClassFilterTest.kt @@ -72,6 +72,18 @@ class ClassFilterTest { } @Test + fun testNestedClass() { + val f = ClassFilter.buildFromString("a.b.c\nm.n.o\$p\n", false, "X") + assertThat(f.matches("a/b/c")).isEqualTo(true) + assertThat(f.matches("a/b/c\$d")).isEqualTo(true) + assertThat(f.matches("a/b/c\$d\$e")).isEqualTo(true) + assertThat(f.matches("m/n/o")).isEqualTo(false) + assertThat(f.matches("m/n/o\$p")).isEqualTo(true) + assertThat(f.matches("m/n/o\$p\$r")).isEqualTo(true) + assertThat(f.matches("m/n/o\$p\$r\$")).isEqualTo(true) + } + + @Test fun testBadFilter1() { try { ClassFilter.buildFromString(""" |