Revert "Fix inadequate detection of package private accesses"
This reverts commit 65be8bcf65bac299cf8d84d600c65119ff0a2258.
Reason for revert: Failures on internal bot
Change-Id: I08e1f0e7fa5dead0a4a273600002f3a69a9b2110
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index 68700e4..8760fb8 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -24,7 +24,6 @@
import com.android.tools.r8.graph.DefaultInstanceInitializerCode;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
-import com.android.tools.r8.graph.DexClassAndField;
import com.android.tools.r8.graph.DexClassAndMethod;
import com.android.tools.r8.graph.DexEncodedField;
import com.android.tools.r8.graph.DexEncodedMember;
@@ -2107,8 +2106,8 @@
}
}
- // Searches for a reference to a non-private, non-public class, field or method declared in the
- // same package as [source].
+ // Searches for a reference to a non-public class, field or method declared in the same package
+ // as [source].
public static class IllegalAccessDetector extends UseRegistryWithResult<Boolean, ProgramMethod> {
private final AppView<? extends AppInfoWithClassHierarchy> appView;
@@ -2119,143 +2118,106 @@
this.appView = appView;
}
- private boolean checkFoundPackagePrivateAccess() {
- assert getResult();
- return true;
- }
+ private boolean checkFieldReference(DexField field) {
+ DexType baseType =
+ appView.graphLens().lookupType(field.holder.toBaseType(appView.dexItemFactory()));
+ if (baseType.isClassType() && baseType.isSamePackage(getContext().getHolderType())) {
+ if (checkTypeReference(field.holder) || checkTypeReference(field.type)) {
+ return true;
+ }
- private boolean setFoundPackagePrivateAccess() {
- setResult(true);
- return true;
- }
-
- private static boolean continueSearchForPackagePrivateAccess() {
+ DexEncodedField definition = appView.appInfo().resolveField(field).getResolvedField();
+ if (definition == null || !definition.accessFlags.isPublic()) {
+ setResult(true);
+ return true;
+ }
+ }
return false;
}
- private boolean checkFieldReference(DexField field) {
- return internalCheckFieldReference(field, appView.graphLens());
- }
-
- private boolean checkRewrittenFieldReference(DexField field) {
- return internalCheckFieldReference(field, GraphLens.getIdentityLens());
- }
-
- private boolean internalCheckFieldReference(DexField field, GraphLens graphLens) {
- DexField rewrittenField = graphLens.lookupField(field);
- assert rewrittenField.getHolderType().isClassType();
- DexType rewrittenFieldHolder = rewrittenField.getHolderType();
- if (rewrittenFieldHolder.isSamePackage(getContext().getHolderType())) {
- if (checkRewrittenTypeReference(rewrittenFieldHolder)) {
- return checkFoundPackagePrivateAccess();
- }
- DexClassAndField resolvedField =
- appView.appInfo().resolveField(rewrittenField).getResolutionPair();
- if (resolvedField == null) {
- return setFoundPackagePrivateAccess();
- }
- if (resolvedField.getHolder() != getContext().getHolder()
- && !resolvedField.getAccessFlags().isPublic()) {
- return setFoundPackagePrivateAccess();
- }
- }
- return continueSearchForPackagePrivateAccess();
- }
-
- private boolean checkRewrittenMethodReference(
- DexMethod rewrittenMethod, OptionalBool isInterface) {
- DexType baseType = rewrittenMethod.getHolderType().toBaseType(appView.dexItemFactory());
+ private boolean checkMethodReference(DexMethod method, OptionalBool isInterface) {
+ DexType baseType =
+ appView.graphLens().lookupType(method.holder.toBaseType(appView.dexItemFactory()));
if (baseType.isClassType() && baseType.isSamePackage(getContext().getHolderType())) {
- if (checkTypeReference(rewrittenMethod.getHolderType())) {
- return checkFoundPackagePrivateAccess();
+ if (checkTypeReference(method.holder)
+ || checkTypeReference(method.proto.returnType)
+ || Iterables.any(method.getParameters(), this::checkTypeReference)) {
+ return true;
}
+
MethodResolutionResult resolutionResult =
isInterface.isUnknown()
- ? appView.appInfo().unsafeResolveMethodDueToDexFormat(rewrittenMethod)
- : appView.appInfo().resolveMethod(rewrittenMethod, isInterface.isTrue());
- if (!resolutionResult.isSingleResolution()) {
- return setFoundPackagePrivateAccess();
- }
- DexClassAndMethod resolvedMethod =
- resolutionResult.asSingleResolution().getResolutionPair();
- if (resolvedMethod.getHolder() != getContext().getHolder()
- && !resolvedMethod.getAccessFlags().isPublic()) {
- return setFoundPackagePrivateAccess();
+ ? appView.appInfo().unsafeResolveMethodDueToDexFormatLegacy(method)
+ : appView.appInfo().resolveMethodLegacy(method, isInterface.isTrue());
+ if (!resolutionResult.isSingleResolution()
+ || !resolutionResult.asSingleResolution().getResolvedMethod().isPublic()) {
+ setResult(true);
+ return true;
}
}
- return continueSearchForPackagePrivateAccess();
+ return false;
}
private boolean checkTypeReference(DexType type) {
- return internalCheckTypeReference(type, appView.graphLens());
- }
-
- private boolean checkRewrittenTypeReference(DexType type) {
- return internalCheckTypeReference(type, GraphLens.getIdentityLens());
- }
-
- private boolean internalCheckTypeReference(DexType type, GraphLens graphLens) {
- DexType baseType = graphLens.lookupType(type.toBaseType(appView.dexItemFactory()));
+ DexType baseType = appView.graphLens().lookupType(type.toBaseType(appView.dexItemFactory()));
if (baseType.isClassType() && baseType.isSamePackage(getContext().getHolderType())) {
DexClass clazz = appView.definitionFor(baseType);
- if (clazz == null || !clazz.isPublic()) {
- return setFoundPackagePrivateAccess();
+ if (clazz == null || !clazz.accessFlags.isPublic()) {
+ setResult(true);
+ return true;
}
}
- return continueSearchForPackagePrivateAccess();
+ return false;
}
@Override
public void registerInitClass(DexType clazz) {
- if (appView.initClassLens().isFinal()) {
- // The InitClass lens is always rewritten up until the most recent graph lens, so first map
- // the class type to the most recent graph lens.
- DexType rewrittenType = appView.graphLens().lookupType(clazz);
- DexField initClassField = appView.initClassLens().getInitClassField(rewrittenType);
- checkRewrittenFieldReference(initClassField);
- } else {
- checkTypeReference(clazz);
- }
+ checkTypeReference(clazz);
}
@Override
public void registerInvokeVirtual(DexMethod method) {
- MethodLookupResult lookup = appView.graphLens().lookupInvokeVirtual(method, getContext());
- checkRewrittenMethodReference(lookup.getReference(), OptionalBool.FALSE);
+ MethodLookupResult lookup =
+ appView.graphLens().lookupMethod(method, getContext().getReference(), VIRTUAL);
+ checkMethodReference(lookup.getReference(), OptionalBool.FALSE);
}
@Override
public void registerInvokeDirect(DexMethod method) {
- MethodLookupResult lookup = appView.graphLens().lookupInvokeDirect(method, getContext());
- checkRewrittenMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
+ MethodLookupResult lookup =
+ appView.graphLens().lookupMethod(method, getContext().getReference(), DIRECT);
+ checkMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
}
@Override
public void registerInvokeStatic(DexMethod method) {
- MethodLookupResult lookup = appView.graphLens().lookupInvokeStatic(method, getContext());
- checkRewrittenMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
+ MethodLookupResult lookup =
+ appView.graphLens().lookupMethod(method, getContext().getReference(), Type.STATIC);
+ checkMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
}
@Override
public void registerInvokeInterface(DexMethod method) {
- MethodLookupResult lookup = appView.graphLens().lookupInvokeInterface(method, getContext());
- checkRewrittenMethodReference(lookup.getReference(), OptionalBool.TRUE);
+ MethodLookupResult lookup =
+ appView.graphLens().lookupMethod(method, getContext().getReference(), Type.INTERFACE);
+ checkMethodReference(lookup.getReference(), OptionalBool.TRUE);
}
@Override
public void registerInvokeSuper(DexMethod method) {
- MethodLookupResult lookup = appView.graphLens().lookupInvokeSuper(method, getContext());
- checkRewrittenMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
+ MethodLookupResult lookup =
+ appView.graphLens().lookupMethod(method, getContext().getReference(), Type.SUPER);
+ checkMethodReference(lookup.getReference(), OptionalBool.UNKNOWN);
}
@Override
public void registerInstanceFieldWrite(DexField field) {
- checkFieldReference(field);
+ checkFieldReference(appView.graphLens().lookupField(field));
}
@Override
public void registerInstanceFieldRead(DexField field) {
- checkFieldReference(field);
+ checkFieldReference(appView.graphLens().lookupField(field));
}
@Override
@@ -2265,12 +2227,12 @@
@Override
public void registerStaticFieldRead(DexField field) {
- checkFieldReference(field);
+ checkFieldReference(appView.graphLens().lookupField(field));
}
@Override
public void registerStaticFieldWrite(DexField field) {
- checkFieldReference(field);
+ checkFieldReference(appView.graphLens().lookupField(field));
}
@Override
diff --git a/src/test/java/com/android/tools/r8/classmerging/horizontal/InitClassToPackagePrivateFieldWithCrossPackageMergingTest.java b/src/test/java/com/android/tools/r8/classmerging/horizontal/InitClassToPackagePrivateFieldWithCrossPackageMergingTest.java
index af9f880..ed2130d 100644
--- a/src/test/java/com/android/tools/r8/classmerging/horizontal/InitClassToPackagePrivateFieldWithCrossPackageMergingTest.java
+++ b/src/test/java/com/android/tools/r8/classmerging/horizontal/InitClassToPackagePrivateFieldWithCrossPackageMergingTest.java
@@ -9,7 +9,7 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
-import com.android.tools.r8.utils.codeinspector.HorizontallyMergedClassesInspector;
+import com.android.tools.r8.references.Reference;
import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Test;
@@ -47,13 +47,16 @@
.addProgramClassFileData(getProgramClassFileData())
.addKeepMainRule(Main.class)
.addHorizontallyMergedClassesInspector(
- HorizontallyMergedClassesInspector::assertNoClassesMerged)
+ inspector ->
+ inspector.assertMergedInto(
+ Reference.classFromClass(A.class),
+ Reference.classFromDescriptor(NEW_B_DESCRIPTOR)))
.enableInliningAnnotations()
.enableNoHorizontalClassMergingAnnotations()
.setMinApi(parameters.getApiLevel())
.compile()
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("Hello, world!");
+ .assertFailureWithErrorThatThrows(IllegalAccessError.class);
}
private List<byte[]> getProgramClassFileData() throws Exception {
diff --git a/src/test/java/com/android/tools/r8/internal/YouTubeV1719Test.java b/src/test/java/com/android/tools/r8/internal/YouTubeV1719Test.java
index c78212b..dbeb66b 100644
--- a/src/test/java/com/android/tools/r8/internal/YouTubeV1719Test.java
+++ b/src/test/java/com/android/tools/r8/internal/YouTubeV1719Test.java
@@ -40,7 +40,6 @@
import java.nio.file.Paths;
import java.util.concurrent.ExecutionException;
import org.junit.After;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -192,7 +191,6 @@
* Validates that when all protos are kept and the proto optmization is enabled, the generated
* proto schemas are identical to the proto schemas in the input.
*/
- @Ignore
@Test
public void testProtoRewriting() throws Exception {
assumeTrue(shouldRunSlowTests());
diff --git a/src/test/java/com/android/tools/r8/kotlin/R8KotlinAccessorTest.java b/src/test/java/com/android/tools/r8/kotlin/R8KotlinAccessorTest.java
index 483f18b..907ca27 100644
--- a/src/test/java/com/android/tools/r8/kotlin/R8KotlinAccessorTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/R8KotlinAccessorTest.java
@@ -19,7 +19,6 @@
import com.android.tools.r8.kotlin.TestKotlinClass.AccessorKind;
import com.android.tools.r8.kotlin.TestKotlinClass.Visibility;
import com.android.tools.r8.naming.MemberNaming;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
@@ -220,9 +219,7 @@
runTest(PROPERTIES_PACKAGE_NAME, mainClass)
.inspect(
inspector -> {
- if (allowAccessModification
- || (testParameters.isDexRuntime()
- && testParameters.getApiLevel().isGreaterThan(AndroidApiLevel.B))) {
+ if (allowAccessModification) {
checkClassIsRemoved(inspector, testedClass.getOuterClassName());
return;
}
diff --git a/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java b/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
index 527e568..59ba99b 100644
--- a/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
+++ b/src/test/java/com/android/tools/r8/kotlin/R8KotlinPropertiesTest.java
@@ -11,7 +11,6 @@
import com.android.tools.r8.kotlin.TestKotlinClass.Visibility;
import com.android.tools.r8.naming.MemberNaming;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
-import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
@@ -510,9 +509,7 @@
inspector -> {
checkClassIsRemoved(inspector, testedClass.getClassName());
- if (allowAccessModification
- || (testParameters.isDexRuntime()
- && testParameters.getApiLevel().isGreaterThan(AndroidApiLevel.B))) {
+ if (allowAccessModification) {
checkClassIsRemoved(inspector, testedClass.getOuterClassName());
return;
}
