Version 2.0.34
Cherry pick: Fix incorrect accessibility check in ResolutionResult
CL: https://r8-review.googlesource.com/c/r8/+/49023
Cherry pick: Reproduce NPE due to incorrect accessibility judgement
CL: https://r8-review.googlesource.com/c/r8/+/49022
Bug: 149831282
Change-Id: Iad7c96f13d61e40263cdabffe7401918966dec9d
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 7944c37..579277e 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "2.0.33";
+ public static final String LABEL = "2.0.34";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
index 229d8eb..ad22d6d 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -142,21 +142,11 @@
public DexEncodedMethod lookupInvokeSpecialTarget(
DexProgramClass context, AppInfoWithSubtyping appInfo) {
// If the resolution is non-accessible then no target exists.
- if (!isAccessibleFrom(context, appInfo)) {
- return null;
+ if (isAccessibleFrom(context, appInfo)) {
+ return internalInvokeSpecialOrSuper(
+ context, appInfo, (sup, sub) -> isSuperclass(sup, sub, appInfo));
}
- DexEncodedMethod target =
- internalInvokeSpecialOrSuper(
- context, appInfo, (sup, sub) -> isSuperclass(sup, sub, appInfo));
- if (target == null) {
- return null;
- }
- // Should we check access control again?
- DexClass holder = appInfo.definitionFor(target.method.holder);
- if (!AccessControl.isMethodAccessible(target, holder, context, appInfo)) {
- return null;
- }
- return target;
+ return null;
}
/**
@@ -177,19 +167,10 @@
@Override
public DexEncodedMethod lookupInvokeSuperTarget(
DexProgramClass context, AppInfoWithSubtyping appInfo) {
- if (!isAccessibleFrom(context, appInfo)) {
- return null;
+ if (isAccessibleFrom(context, appInfo)) {
+ return lookupInvokeSuperTarget(context.asDexClass(), appInfo);
}
- DexEncodedMethod target = lookupInvokeSuperTarget(context.asDexClass(), appInfo);
- if (target == null) {
- return null;
- }
- // Should we check access control again?
- DexClass holder = appInfo.definitionFor(target.method.holder);
- if (!AccessControl.isMethodAccessible(target, holder, context, appInfo)) {
- return null;
- }
- return target;
+ return null;
}
@Override
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 1290489..ea7d4b3 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -1408,4 +1408,16 @@
throws IOException, CompilationFailedException {
return buildOnDexRuntime(parameters, Arrays.asList(paths));
}
+
+ public static String binaryName(Class<?> clazz) {
+ return DescriptorUtils.getBinaryNameFromJavaType(typeName(clazz));
+ }
+
+ public static String descriptor(Class<?> clazz) {
+ return DescriptorUtils.javaTypeToDescriptor(typeName(clazz));
+ }
+
+ public static String typeName(Class<?> clazz) {
+ return clazz.getTypeName();
+ }
}
diff --git a/src/test/java/com/android/tools/r8/shaking/B149831282.java b/src/test/java/com/android/tools/r8/shaking/B149831282.java
new file mode 100644
index 0000000..910d44b
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/B149831282.java
@@ -0,0 +1,107 @@
+// Copyright (c) 2020, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking;
+
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NeverMerge;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestBuilder;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class B149831282 extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public B149831282(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testRuntime() throws Exception {
+ testForRuntime(parameters)
+ .apply(this::addProgramInputs)
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("In A.m()");
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .apply(this::addProgramInputs)
+ .addKeepMainRule(TestClass.class)
+ .enableInliningAnnotations()
+ .enableMergeAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutputLines("In A.m()");
+ }
+
+ private void addProgramInputs(TestBuilder<?, ?> builder) throws Exception {
+ builder
+ .addProgramClasses(A.class, B.class)
+ .addProgramClassFileData(
+ transformer(TestClass.class)
+ .transformTypeInsnInMethod(
+ "main",
+ (opcode, type, continuation) -> {
+ assertEquals(binaryName(C.class), type);
+ continuation.apply(opcode, "b149831282/C");
+ })
+ .transformMethodInsnInMethod(
+ "main",
+ (opcode, owner, name, descriptor, isInterface, continuation) -> {
+ assertEquals(binaryName(C.class), owner);
+ continuation.apply(opcode, "b149831282/C", name, descriptor, isInterface);
+ })
+ .transform())
+ .addProgramClassFileData(
+ transformer(C.class).setClassDescriptor("Lb149831282/C;").transform());
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ new C().m();
+ }
+ }
+
+ @NeverMerge
+ static class A {
+
+ @NeverInline
+ protected void m() {
+ System.out.println("In A.m()");
+ }
+ }
+
+ @NeverMerge
+ public static class B extends A {}
+
+ @NeverClassInline
+ public static class /*b149831282.*/ C extends B {
+
+ @NeverInline
+ @Override
+ public void m() {
+ super.m();
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
index 598f6f1..52777d9 100644
--- a/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
+++ b/src/test/java/com/android/tools/r8/transformers/ClassFileTransformer.java
@@ -343,6 +343,18 @@
void apply(int opcode, String owner, String name, String descriptor, boolean isInterface);
}
+ /** Abstraction of the MethodVisitor.visitTypeInsn method with its continuation. */
+ @FunctionalInterface
+ public interface TypeInsnTransform {
+ void visitTypeInsn(int opcode, String type, TypeInsnTransformContinuation continuation);
+ }
+
+ /** Continuation for transforming a method. Will continue with the super visitor if called. */
+ @FunctionalInterface
+ public interface TypeInsnTransformContinuation {
+ void apply(int opcode, String type);
+ }
+
public ClassFileTransformer transformMethodInsnInMethod(
String methodName, MethodInsnTransform transform) {
return addMethodTransformer(
@@ -360,6 +372,21 @@
});
}
+ public ClassFileTransformer transformTypeInsnInMethod(
+ String methodName, TypeInsnTransform transform) {
+ return addMethodTransformer(
+ new MethodTransformer() {
+ @Override
+ public void visitTypeInsn(int opcode, String type) {
+ if (getContext().method.getMethodName().equals(methodName)) {
+ transform.visitTypeInsn(opcode, type, super::visitTypeInsn);
+ } else {
+ super.visitTypeInsn(opcode, type);
+ }
+ }
+ });
+ }
+
/** Abstraction of the MethodVisitor.visitLdcInsn method with its continuation. */
@FunctionalInterface
public interface LdcInsnTransform {