Remove a failing DCHECK for IAE in an edge case.

Also address some other late comments on
    https://android-review.googlesource.com/2146626 .

Test: m test-art-host-gtest
Test: testrunnerpy.py --host --optimizing
Change-Id: Ib43ff9b959431e74a6ada6213780b400bbf265a2

8 files changed, 86 insertions, 13 deletions

diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 40a01920de..854d2737a4 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -187,12 +187,14 @@ void ArtMethod::ThrowInvocationTimeError(ObjPtr<mirror::Object> receiver) {
     // IllegalAccessError.
     DCHECK(IsAbstract());
     ObjPtr<mirror::Class> current = receiver->GetClass();
+    std::string_view name = GetNameView();
+    Signature signature = GetSignature();
     while (current != nullptr) {
       for (ArtMethod& method : current->GetDeclaredMethodsSlice(kRuntimePointerSize)) {
         ArtMethod* np_method = method.GetInterfaceMethodIfProxy(kRuntimePointerSize);
         if (!np_method->IsStatic() &&
-            np_method->GetNameView() == GetNameView() &&
-            np_method->GetSignature() == GetSignature()) {
+            np_method->GetNameView() == name &&
+            np_method->GetSignature() == signature) {
           if (!np_method->IsPublic()) {
             ThrowIllegalAccessErrorForImplementingMethod(receiver->GetClass(), np_method, this);
             return;
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index ca1ab1d8d6..829b3e3a4f 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -8740,12 +8740,9 @@ bool ClassLinker::LinkMethodsHelper<kPointerSize>::FindCopiedMethodsForInterface
       size_t hash = ComputeMethodHash(interface_method);
       auto it1 = declared_virtual_signatures.FindWithHash(interface_method, hash);
       if (it1 != declared_virtual_signatures.end()) {
-        ArtMethod* virtual_method = klass->GetVirtualMethodDuringLinking(*it1, kPointerSize);
-        if (!virtual_method->IsAbstract() && !virtual_method->IsPublic()) {
-          sants.reset();
-          ThrowIllegalAccessErrorForImplementingMethod(klass, virtual_method, interface_method);
-          return false;
-        }
+        // Virtual methods in interfaces are always public.
+        // This is checked by the `DexFileVerifier`.
+        DCHECK(klass->GetVirtualMethodDuringLinking(*it1, kPointerSize)->IsPublic());
         continue;  // This default method is masked by a method declared in this interface.
       }
 
diff --git a/runtime/common_throws.cc b/runtime/common_throws.cc
index feddd5c2e3..ec43f69b19 100644
--- a/runtime/common_throws.cc
+++ b/runtime/common_throws.cc
@@ -241,7 +241,9 @@ void ThrowIllegalAccessErrorForImplementingMethod(ObjPtr<mirror::Class> klass,
                                                   ArtMethod* implementation_method,
                                                   ArtMethod* interface_method)
     REQUIRES_SHARED(Locks::mutator_lock_) {
-  DCHECK(!implementation_method->IsAbstract());
+  // Note: For a non-public abstract implementing method, both `AbstractMethodError` and
+  // `IllegalAccessError` are reasonable. We now follow the RI behaviour and throw the latter,
+  // so we do not assert here that the implementation method is concrete as we did in the past.
   DCHECK(!implementation_method->IsPublic());
   ThrowIllegalAccessError(
       klass,
diff --git a/test/182-method-linking/expected-stdout.txt b/test/182-method-linking/expected-stdout.txt
index 56ad59d046..f97da754f4 100644
--- a/test/182-method-linking/expected-stdout.txt
+++ b/test/182-method-linking/expected-stdout.txt
@@ -48,3 +48,5 @@ Calling pkg1.I1.foo on pkg2.DXI1
 Caught IllegalAccessError
 Calling pkg2.I2.foo on pkg2.DXI2
 Caught IllegalAccessError
+Calling pkg1.I1.foo on InheritingProtectedAbstract
+Caught IllegalAccessError
diff --git a/test/182-method-linking/src/InheritingProtectedAbstract.java b/test/182-method-linking/src/InheritingProtectedAbstract.java
new file mode 100644
index 0000000000..33740af460
--- /dev/null
+++ b/test/182-method-linking/src/InheritingProtectedAbstract.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import pkg1.I1;
+
+class InheritingProtectedAbstract extends ProtectedAbstract implements I1 {
+}
diff --git a/test/182-method-linking/src/Main.java b/test/182-method-linking/src/Main.java
index 638ff654a3..178b0b90ed 100644
--- a/test/182-method-linking/src/Main.java
+++ b/test/182-method-linking/src/Main.java
@@ -110,32 +110,39 @@ public class Main {
         D2I2 d2i2 = new D2I2();
         I2.callI2Foo(d2i2);  // pkg1.D2.foo
 
+        CXI1 cxi1 = new CXI1();
         try {
-            CXI1 cxi1 = new CXI1();
             I1.callI1Foo(cxi1);
         } catch (IllegalAccessError expected) {
             System.out.println("Caught IllegalAccessError");
         }
 
+        CXI2 cxi2 = new CXI2();
         try {
-            CXI2 cxi2 = new CXI2();
             I2.callI2Foo(cxi2);
         } catch (IllegalAccessError expected) {
             System.out.println("Caught IllegalAccessError");
         }
 
+        DXI1 dxi1 = new DXI1();
         try {
-            DXI1 dxi1 = new DXI1();
             I1.callI1Foo(dxi1);
         } catch (IllegalAccessError expected) {
             System.out.println("Caught IllegalAccessError");
         }
 
+        DXI2 dxi2 = new DXI2();
         try {
-            DXI2 dxi2 = new DXI2();
             I2.callI2Foo(dxi2);
         } catch (IllegalAccessError expected) {
             System.out.println("Caught IllegalAccessError");
         }
+
+        InheritingProtectedAbstract ipa = new InheritingProtectedAbstract();
+        try {
+            I1.callI1Foo(ipa);
+        } catch (IllegalAccessError expected) {
+            System.out.println("Caught IllegalAccessError");
+        }
     }
 }
diff --git a/test/182-method-linking/src/ProtectedAbstract.java b/test/182-method-linking/src/ProtectedAbstract.java
new file mode 100644
index 0000000000..c276eebc9b
--- /dev/null
+++ b/test/182-method-linking/src/ProtectedAbstract.java
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This definition is used for compiling but the class used at runtime is in src2/.
+public abstract class ProtectedAbstract {
+    protected /*abstract*/ void foo() {
+        // No body in src2/.
+        throw new Error("Unreachable");
+    }
+}
diff --git a/test/182-method-linking/src2/ProtectedAbstract.java b/test/182-method-linking/src2/ProtectedAbstract.java
new file mode 100644
index 0000000000..0c6468c117
--- /dev/null
+++ b/test/182-method-linking/src2/ProtectedAbstract.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// This is the class ProtectedAbstract used at runtime.
+public abstract class ProtectedAbstract {
+    protected abstract void foo();
+}