Do not return an abstract class as IMT owner.

Abstract classes don't have an IMT and therefore cannot own one.

Test: 141-class-unload
Bug: 322253629
Change-Id: Ic6bb498d83ec66252710af384c24963be2287055

4 files changed, 99 insertions, 1 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 050896041b..5b00a87217 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -6627,8 +6627,15 @@ static ObjPtr<mirror::Class> GetImtOwner(ObjPtr<mirror::Class> klass)
   DCHECK(imt != nullptr);
   while (klass->HasSuperClass()) {
     ObjPtr<mirror::Class> super_class = klass->GetSuperClass();
-    if (super_class->ShouldHaveImt() && imt != super_class->GetImt(kRuntimePointerSize)) {
+    // Abstract classes cannot have IMTs, so we skip them.
+    while (super_class->IsAbstract()) {
+      DCHECK(super_class->HasSuperClass());
+      super_class = super_class->GetSuperClass();
+    }
+    DCHECK(super_class->ShouldHaveImt());
+    if (imt != super_class->GetImt(kRuntimePointerSize)) {
       // IMT not shared with the super class, return the current class.
+      DCHECK_EQ(klass->GetImt(kRuntimePointerSize), imt) << klass->PrettyClass();
       return klass;
     }
     klass = super_class;
@@ -6651,6 +6658,10 @@ ArtMethod* ClassLinker::AddMethodToConflictTable(ObjPtr<mirror::Class> klass,
   DCHECK(imt_owner != nullptr);
 
   LinearAlloc* linear_alloc = GetAllocatorForClassLoader(imt_owner->GetClassLoader());
+  // If the imt owner is in an image, the imt is also there and not in the
+  // linear alloc.
+  DCHECK_IMPLIES(runtime->GetHeap()->FindSpaceFromObject(imt_owner, /*fail_ok=*/true) == nullptr,
+                 linear_alloc->Contains(klass->GetImt(kRuntimePointerSize)));
 
   // Create a new entry if the existing one is the shared conflict method.
   ArtMethod* new_conflict_method = (conflict_method == runtime->GetImtConflictMethod())
diff --git a/test/141-class-unload/src-ex/AbstractClass.java b/test/141-class-unload/src-ex/AbstractClass.java
new file mode 100644
index 0000000000..de4dccf5db
--- /dev/null
+++ b/test/141-class-unload/src-ex/AbstractClass.java
@@ -0,0 +1,18 @@
+/*
+ * 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.
+ */
+
+public abstract class AbstractClass extends ConflictSuper {
+}
diff --git a/test/141-class-unload/src-ex/ConflictImpl2.java b/test/141-class-unload/src-ex/ConflictImpl2.java
new file mode 100644
index 0000000000..40f986797a
--- /dev/null
+++ b/test/141-class-unload/src-ex/ConflictImpl2.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+
+public class ConflictImpl2 extends AbstractClass {
+}
diff --git a/test/141-class-unload/src/Main.java b/test/141-class-unload/src/Main.java
index 7b7ffa2ea7..5e07430e11 100644
--- a/test/141-class-unload/src/Main.java
+++ b/test/141-class-unload/src/Main.java
@@ -20,8 +20,10 @@ import java.io.FileReader;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
+import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.function.Consumer;
+import java.util.Base64;
 
 public class Main {
     static final String DEX_FILE = System.getenv("DEX_LOCATION") + "/141-class-unload-ex.jar";
@@ -61,6 +63,7 @@ public class Main {
             testCopiedAppImageMethodInStackTrace();
             // Test that the runtime uses the right allocator when creating conflict methods.
             testConflictMethod(constructor);
+            testConflictMethod2(constructor);
         } catch (Exception e) {
             e.printStackTrace(System.out);
         }
@@ -330,6 +333,35 @@ public class Main {
         $noinline$callAllMethods(iface);
     }
 
+    private static void $noinline$invokeConflictMethod2(Constructor<?> constructor)
+            throws Exception {
+        // We need three class loaders to expose the issue: the main one with the top super class,
+        // then a second one with the abstract class which we used to wrongly return as an IMT
+        // owner, and the concrete class in a different class loader.
+        Class<?> cls = Class.forName("dalvik.system.InMemoryDexClassLoader");
+        Constructor<?> inMemoryConstructor =
+                cls.getDeclaredConstructor(ByteBuffer.class, ClassLoader.class);
+        ClassLoader inMemoryLoader = (ClassLoader) inMemoryConstructor.newInstance(
+                ByteBuffer.wrap(DEX_BYTES), ClassLoader.getSystemClassLoader());
+        ClassLoader loader = (ClassLoader) constructor.newInstance(
+                DEX_FILE, LIBRARY_SEARCH_PATH, inMemoryLoader);
+        Class<?> impl = loader.loadClass("ConflictImpl2");
+        ConflictIface iface = (ConflictIface) impl.newInstance();
+        $noinline$callAllMethods(iface);
+    }
+
+    private static void testConflictMethod2(Constructor<?> constructor) throws Exception {
+        // Load and unload a few class loaders to force re-use of the native memory where we
+        // used to allocate the conflict table.
+        for (int i = 0; i < 2; i++) {
+            $noinline$invokeConflictMethod2(constructor);
+            doUnloading();
+        }
+        Class<?> impl = Class.forName("ConflictSuper");
+        ConflictIface iface = (ConflictIface) impl.newInstance();
+        $noinline$callAllMethods(iface);
+    }
+
     private static void testCopiedMethodInStackTrace(Constructor<?> constructor) throws Exception {
         Throwable t = $noinline$createStackTraceWithCopiedMethod(constructor);
         doUnloading();
@@ -431,4 +463,23 @@ public class Main {
 
     public static native void stopJit();
     public static native void startJit();
+
+
+    /* Corresponds to:
+     *
+     * public abstract class AbstractClass extends ConflictSuper { }
+     *
+     */
+    private static final byte[] DEX_BYTES = Base64.getDecoder().decode(
+        "ZGV4CjAzNQAOZ0WGvUad/2dEp77oyy9K2tx8txklUZ1wAgAAcAAAAHhWNBIAAAAAAAAAANwBAAAG" +
+        "AAAAcAAAAAMAAACIAAAAAQAAAJQAAAAAAAAAAAAAAAIAAACgAAAAAQAAALAAAACgAQAA0AAAAOwA" +
+        "AAD0AAAACAEAABkBAAAqAQAALQEAAAIAAAADAAAABAAAAAQAAAACAAAAAAAAAAAAAAAAAAAAAQAA" +
+        "AAAAAAAAAAAAAQQAAAEAAAAAAAAAAQAAAAAAAADLAQAAAAAAAAEAAQABAAAA6AAAAAQAAABwEAEA" +
+        "AAAOABEADgAGPGluaXQ+ABJBYnN0cmFjdENsYXNzLmphdmEAD0xBYnN0cmFjdENsYXNzOwAPTENv" +
+        "bmZsaWN0U3VwZXI7AAFWAJsBfn5EOHsiYmFja2VuZCI6ImRleCIsImNvbXBpbGF0aW9uLW1vZGUi" +
+        "OiJkZWJ1ZyIsImhhcy1jaGVja3N1bXMiOmZhbHNlLCJtaW4tYXBpIjoxLCJzaGEtMSI6ImI3MmIx" +
+        "NWJjODQ2N2Y0M2FhNTdlYjk5ZDAyMjU0Nzg5ODYwZjRlOWEiLCJ2ZXJzaW9uIjoiOC41LjEtZGV2" +
+        "In0AAAABAACBgATQAQAAAAAAAAAMAAAAAAAAAAEAAAAAAAAAAQAAAAYAAABwAAAAAgAAAAMAAACI" +
+        "AAAAAwAAAAEAAACUAAAABQAAAAIAAACgAAAABgAAAAEAAACwAAAAASAAAAEAAADQAAAAAyAAAAEA" +
+        "AADoAAAAAiAAAAYAAADsAAAAACAAAAEAAADLAQAAAxAAAAEAAADYAQAAABAAAAEAAADcAQAA");
 }