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 {