fix passing wtable to destroy

Author: ktoso

Makefile

@@ -97,8 +97,7 @@ JEXTRACT_BUILD_DIR="$(BUILD_DIR)/jextract"
 define make_swiftinterface
     $(eval $@_MODULE = $(1))
     $(eval $@_FILENAME = $(2))
-	# eval ${SWIFTC} \
-	eval /Library/Developer/Toolchains/swift-PR-76905-1539.xctoolchain/usr/bin/swiftc \
+	eval ${SWIFTC} \
 		-emit-module-interface-path ${JEXTRACT_BUILD_DIR}/${$@_MODULE}/${$@_FILENAME}.swiftinterface \
 		-emit-module-path ${JEXTRACT_BUILD_DIR}/${$@_MODULE}/${$@_FILENAME}.swiftmodule \
 		-enable-library-evolution \

Samples/SwiftKitSampleApp/src/test/java/org/swift/swiftkit/SwiftArenaTest.java

@@ -35,7 +35,7 @@ static void beforeAll() {
     }
 
     @Test
-    void arena_releaseClassOnClose_class_ok() {
+    public void arena_releaseClassOnClose_class_ok() {
         MySwiftClass unsafelyEscaped = null;
 
         try (var arena = SwiftArena.ofConfined()) {
@@ -53,7 +53,7 @@ void arena_releaseClassOnClose_class_ok() {
     }
 
     @Test
-    void arena_releaseClassOnClose_class_leaked() {
+    public void arena_releaseClassOnClose_class_leaked() {
         String memorySegmentDescription = "<none>";
 
         try {

SwiftKit/build.gradle

@@ -36,4 +36,7 @@ dependencies {
 
 tasks.test {
     useJUnitPlatform()
+    testLogging {
+        events("passed", "skipped", "failed")
+    }
 }

SwiftKit/src/main/java/org/swift/swiftkit/MemorySegmentUtils.java

@@ -0,0 +1,23 @@
+package org.swift.swiftkit;
+
+import java.lang.foreign.MemorySegment;
+import java.lang.foreign.ValueLayout;
+
+public class MemorySegmentUtils {
+    /**
+     * Set the value of `target` to the {@link MemorySegment#address()} of the `memorySegment`,
+     * adjusting for the fact that Swift pointer may be 32 or 64 bit, depending on runtime.
+     *
+     * @param target        the target to set a value on
+     * @param memorySegment the origin of the address value to write into `target`
+     */
+    static void setSwiftPointerAddress(MemorySegment target, MemorySegment memorySegment) {
+        // Write the address of as the value of the newly created pointer.
+        // We need to type-safely set the pointer value which may be 64 or 32-bit.
+        if (SwiftValueLayout.SWIFT_INT == ValueLayout.JAVA_LONG) {
+            target.set(ValueLayout.JAVA_LONG, /*offset=*/0, memorySegment.address());
+        } else {
+            target.set(ValueLayout.JAVA_INT, /*offset=*/0, (int) memorySegment.address());
+        }
+    }
+}

SwiftKit/src/main/java/org/swift/swiftkit/SwiftAnyType.java

@@ -18,7 +18,7 @@
 import java.lang.foreign.MemoryLayout;
 import java.lang.foreign.MemorySegment;
 
-public final class SwiftAnyType implements SwiftMemoryResource {
+public final class SwiftAnyType {
 
     private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
             SwiftValueLayout.SWIFT_POINTER
@@ -48,22 +48,14 @@ public SwiftAnyType(SwiftHeapObject object) {
     }
 
 
-    @Override
     public MemorySegment $memorySegment() {
         return memorySegment;
     }
 
-    @Override
     public GroupLayout $layout() {
         return $LAYOUT;
     }
 
-    @Override
-    public boolean immortal() {
-        // Don't ever try to destroy a type memory segment.
-        return true;
-    }
-
     @Override
     public String toString() {
         return "AnySwiftType{" +

SwiftKit/src/main/java/org/swift/swiftkit/SwiftHeapObject.java

@@ -14,11 +14,9 @@
 
 package org.swift.swiftkit;
 
-import java.lang.foreign.MemorySegment;
-
 /**
  * Represents a wrapper around a Swift heap object, e.g. a {@code class} or an {@code actor}.
  */
-public interface SwiftHeapObject extends SwiftMemoryResource {
+public interface SwiftHeapObject extends SwiftInstance {
     SwiftAnyType $swiftType();
 }

SwiftKit/src/main/java/org/swift/swiftkit/SwiftMemoryResource.java renamed to SwiftKit/src/main/java/org/swift/swiftkit/SwiftInstance.java

@@ -17,17 +17,26 @@
 import java.lang.foreign.GroupLayout;
 import java.lang.foreign.MemorySegment;
 
-public interface SwiftMemoryResource {
+public interface SwiftInstance {
 
     /**
      * The pointer to the instance in memory. I.e. the {@code self} of the Swift object or value.
      */
     MemorySegment $memorySegment();
 
-    /** The in memory layout of an instance of this Swift type. */
+    /**
+     * The in memory layout of an instance of this Swift type.
+     */
     GroupLayout $layout();
 
-    default boolean immortal() {
-        return false;
+    SwiftAnyType $swiftType();
+
+    /**
+     * Returns `true` if this swift instance is a reference type, i.e. a `class` or (`distributed`) `actor`.
+     *
+     * @return `true` if this instance is a reference type, `false` otherwise.
+     */
+    default boolean isReferenceType() {
+        return this instanceof SwiftHeapObject;
     }
 }

SwiftKit/src/main/java/org/swift/swiftkit/SwiftMemoryResourceCleanup.java

@@ -15,26 +15,32 @@
 package org.swift.swiftkit;
 
 import java.lang.foreign.MemorySegment;
+import java.lang.foreign.ValueLayout;
+
+import static org.swift.swiftkit.MemorySegmentUtils.setSwiftPointerAddress;
 
 /**
- * A Swift memory resource cleanup, e.g. count-down a reference count and destroy a class, or destroy struct/enum etc.
+ * A Swift memory instance cleanup, e.g. count-down a reference count and destroy a class, or destroy struct/enum etc.
  */
 sealed interface SwiftMemoryResourceCleanup extends Runnable {
 }
 
-record SwiftHeapObjectCleanup(SwiftHeapObject resource) implements SwiftMemoryResourceCleanup {
+record SwiftHeapObjectCleanup(SwiftHeapObject instance) implements SwiftMemoryResourceCleanup {
 
     @Override
     public void run() throws UnexpectedRetainCountException {
-        long retainedCount = SwiftKit.retainCount(this.resource);
+        // Verify we're only destroying an object that's indeed not retained by anyone else:
+        long retainedCount = SwiftKit.retainCount(this.instance);
         if (retainedCount > 1) {
-            throw new UnexpectedRetainCountException(this.resource, retainedCount, 1);
+            throw new UnexpectedRetainCountException(this.instance, retainedCount, 1);
         }
 
-        System.out.println("Cleanup heap object: " + this.resource);
-        var ty = this.resource.$swiftType();
+        // Destroy (and deinit) the object:
+        var ty = this.instance.$swiftType();
+        SwiftValueWitnessTable.destroy(ty, this.instance.$memorySegment());
 
-        SwiftValueWitnessTable.destroy(ty, this.resource.$memorySegment());
+        // Invalidate the Java wrapper class, in order to prevent effectively use-after-free issues.
+        // FIXME: some trouble with setting the pointer to null, need to figure out an appropriate way to do this
     }
 }
 

SwiftKit/src/main/java/org/swift/swiftkit/SwiftValue.java

@@ -14,5 +14,6 @@
 
 package org.swift.swiftkit;
 
-public interface SwiftValue extends SwiftMemoryResource {
+public interface SwiftValue extends SwiftInstance {
+    SwiftAnyType $swiftType();
 }

SwiftKit/src/main/java/org/swift/swiftkit/SwiftValueWitnessTable.java

@@ -17,10 +17,8 @@
 import java.lang.foreign.*;
 import java.lang.invoke.MethodHandle;
 
-import static java.lang.foreign.ValueLayout.ADDRESS;
 import static java.lang.foreign.ValueLayout.JAVA_BYTE;
 import static org.swift.swiftkit.SwiftKit.getSwiftInt;
-import static org.swift.swiftkit.util.StringUtils.hexString;
 
 public abstract class SwiftValueWitnessTable {
 
@@ -182,10 +180,8 @@ private static class destroy {
                 $LAYOUT.byteOffset(MemoryLayout.PathElement.groupElement("destroy"));
 
         static final FunctionDescriptor DESC = FunctionDescriptor.ofVoid(
-                ValueLayout.ADDRESS
-                // SwiftValueLayout.SWIFT_POINTER // should be a "pointer to the self pointer"
-//                , // the object
-//                SwiftValueLayout.SWIFT_POINTER // the type
+                ValueLayout.ADDRESS, // witness table functions expect a pointer to self pointer
+                ValueLayout.ADDRESS // pointer to the witness table
         );
 
         /**
@@ -212,33 +208,17 @@ static MethodHandle handle(SwiftAnyType ty) {
      * This includes deallocating the Swift managed memory for the object.
      */
     public static void destroy(SwiftAnyType type, MemorySegment object) {
-        System.out.println("Destroy object: " + object);
-        System.out.println("Destroy object type: " + type);
+        var fullTypeMetadata = fullTypeMetadata(type.$memorySegment());
+        var wtable = valueWitnessTable(fullTypeMetadata);
 
-        var handle = destroy.handle(type);
-        System.out.println("Destroy object handle: " + handle);
+        var mh = destroy.handle(type);
 
         try (var arena = Arena.ofConfined()) {
             // we need to make a pointer to the self pointer when calling witness table functions:
-            MemorySegment indirect = arena.allocate(SwiftValueLayout.SWIFT_POINTER);
-
-            // Write the address of as the value of the newly created pointer.
-            // We need to type-safely set the pointer value which may be 64 or 32-bit.
-            if (SwiftValueLayout.SWIFT_INT == ValueLayout.JAVA_LONG) {
-                indirect.set(ValueLayout.JAVA_LONG, /*offset=*/0, object.address());
-            } else {
-                indirect.set(ValueLayout.JAVA_INT, /*offset=*/0, (int) object.address());
-            }
-
-            System.out.println("indirect = " + indirect);
-            if (SwiftValueLayout.SWIFT_INT == ValueLayout.JAVA_LONG) {
-                System.out.println("indirect.get(ValueLayout.JAVA_LONG, 0) = " + hexString(indirect.get(ValueLayout.JAVA_LONG, 0)));
-            } else {
-                System.out.println("indirect.get(ValueLayout.JAVA_INT, 0) = " + hexString(indirect.get(ValueLayout.JAVA_INT, 0)));
-            }
-
-//             handle.invokeExact(object); // NOTE: This does "nothing"
-            handle.invokeExact(indirect);
+            MemorySegment indirect = arena.allocate(SwiftValueLayout.SWIFT_POINTER); // TODO: remove this and just have classes have this always anyway
+            MemorySegmentUtils.setSwiftPointerAddress(indirect, object);
+
+            mh.invokeExact(indirect, wtable);
         } catch (Throwable th) {
             throw new AssertionError("Failed to destroy '" + type + "' at " + object, th);
         }