8252872: NativeScope should take Addressable instead of MemoryAddress

Reviewed-by: sundar, jvernee
openjdk · Sep 7, 2020 · 5f63553 · 5f63553
1 parent a064fb9
commit 5f63553
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 95 deletions.
diff --git a/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CSupport.java b/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CSupport.java
@@ -225,11 +225,16 @@ public interface VaList extends Addressable, AutoCloseable {
 
         /**
          * Constructs a new {@code VaList} instance out of a memory address pointing to an existing C {@code va_list}.
+         * <p>
+         * This method is <em>restricted</em>. Restricted method are unsafe, and, if used incorrectly, their use might crash
+         * the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
+         * restricted methods, and use safe and supported functionalities, where possible.
          *
          * @param address a memory address pointing to an existing C {@code va_list}.
          * @return a new {@code VaList} instance backed by the C {@code va_list} at {@code address}.
          */
-        static VaList ofAddress(MemoryAddress address) {
+        static VaList ofAddressRestricted(MemoryAddress address) {
+            Utils.checkRestrictedAccess("VaList.ofAddressRestricted");
             return SharedUtils.newVaListOfAddress(address);
         }
 
@@ -299,7 +304,7 @@ interface Builder {
              * @return this builder.
              * @throws IllegalArgumentException if the given memory layout is not compatible with {@code int}
              */
-            Builder vargFromInt(MemoryLayout layout, int value);
+            Builder vargFromInt(ValueLayout layout, int value);
 
             /**
              * Adds a native value represented as a {@code long} to the C {@code va_list} being constructed.
@@ -309,7 +314,7 @@ interface Builder {
              * @return this builder.
              * @throws IllegalArgumentException if the given memory layout is not compatible with {@code long}
              */
-            Builder vargFromLong(MemoryLayout layout, long value);
+            Builder vargFromLong(ValueLayout layout, long value);
 
             /**
              * Adds a native value represented as a {@code double} to the C {@code va_list} being constructed.
@@ -319,17 +324,17 @@ interface Builder {
              * @return this builder.
              * @throws IllegalArgumentException if the given memory layout is not compatible with {@code double}
              */
-            Builder vargFromDouble(MemoryLayout layout, double value);
+            Builder vargFromDouble(ValueLayout layout, double value);
 
             /**
              * Adds a native value represented as a {@code MemoryAddress} to the C {@code va_list} being constructed.
              *
              * @param layout the native layout of the value.
-             * @param value the value, represented as a {@code MemoryAddress}.
+             * @param value the value, represented as a {@code Addressable}.
              * @return this builder.
              * @throws IllegalArgumentException if the given memory layout is not compatible with {@code MemoryAddress}
              */
-            Builder vargFromAddress(MemoryLayout layout, MemoryAddress value);
+            Builder vargFromAddress(ValueLayout layout, Addressable value);
 
             /**
              * Adds a native value represented as a {@code MemorySegment} to the C {@code va_list} being constructed.
@@ -339,7 +344,7 @@ interface Builder {
              * @return this builder.
              * @throws IllegalArgumentException if the given memory layout is not compatible with {@code MemorySegment}
              */
-            Builder vargFromSegment(MemoryLayout layout, MemorySegment value);
+            Builder vargFromSegment(GroupLayout layout, MemorySegment value);
         }
     }
 
@@ -891,6 +896,10 @@ private static MemorySegment toCString(byte[] bytes, NativeScope scope) {
 
     /**
      * Allocate memory of given size using malloc.
+     * <p>
+     * This method is <em>restricted</em>. Restricted method are unsafe, and, if used incorrectly, their use might crash
+     * the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
+     * restricted methods, and use safe and supported functionalities, where possible.
      *
      * @param size memory size to be allocated
      * @return addr memory address of the allocated memory
@@ -902,6 +911,10 @@ public static MemoryAddress allocateMemoryRestricted(long size) {
 
     /**
      * Free the memory pointed by the given memory address.
+     * <p>
+     * This method is <em>restricted</em>. Restricted method are unsafe, and, if used incorrectly, their use might crash
+     * the JVM or, worse, silently result in memory corruption. Thus, clients should refrain from depending on
+     * restricted methods, and use safe and supported functionalities, where possible.
      *
      * @param addr memory address of the native memory to be freed
      */

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/NativeScope.java b/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/NativeScope.java
@@ -85,7 +85,7 @@ public interface NativeScope extends AutoCloseable {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a byte value.
      */
-    default MemorySegment allocate(MemoryLayout layout, byte value) {
+    default MemorySegment allocate(ValueLayout layout, byte value) {
         VarHandle handle = layout.varHandle(byte.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
@@ -103,7 +103,7 @@ default MemorySegment allocate(MemoryLayout layout, byte value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a short value.
      */
-    default MemorySegment allocate(MemoryLayout layout, short value) {
+    default MemorySegment allocate(ValueLayout layout, short value) {
         VarHandle handle = layout.varHandle(short.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
@@ -121,7 +121,7 @@ default MemorySegment allocate(MemoryLayout layout, short value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a int value.
      */
-    default MemorySegment allocate(MemoryLayout layout, int value) {
+    default MemorySegment allocate(ValueLayout layout, int value) {
         VarHandle handle = layout.varHandle(int.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
@@ -139,7 +139,7 @@ default MemorySegment allocate(MemoryLayout layout, int value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a float value.
      */
-    default MemorySegment allocate(MemoryLayout layout, float value) {
+    default MemorySegment allocate(ValueLayout layout, float value) {
         VarHandle handle = layout.varHandle(float.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
@@ -157,7 +157,7 @@ default MemorySegment allocate(MemoryLayout layout, float value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a long value.
      */
-    default MemorySegment allocate(MemoryLayout layout, long value) {
+    default MemorySegment allocate(ValueLayout layout, long value) {
         VarHandle handle = layout.varHandle(long.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
@@ -175,15 +175,16 @@ default MemorySegment allocate(MemoryLayout layout, long value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize()} does not conform to the size of a double value.
      */
-    default MemorySegment allocate(MemoryLayout layout, double value) {
+    default MemorySegment allocate(ValueLayout layout, double value) {
         VarHandle handle = layout.varHandle(double.class);
         MemorySegment addr = allocate(layout);
         handle.set(addr, value);
         return addr;
     }
 
     /**
-     * Allocate a block of memory in this native scope with given layout and initialize it with given address value.
+     * Allocate a block of memory in this native scope with given layout and initialize it with given address value
+     * (expressed as an {@link Addressable} instance).
      * The address value might be narrowed according to the platform address size (see {@link MemoryLayouts#ADDRESS}).
      * The segment returned by this method cannot be closed. Moreover, the returned
      * segment must conform to the layout alignment constraints.
@@ -194,13 +195,13 @@ default MemorySegment allocate(MemoryLayout layout, double value) {
      * {@code limit() - size() < layout.byteSize()}.
      * @throws IllegalArgumentException if {@code layout.byteSize() != MemoryLayouts.ADDRESS.byteSize()}.
      */
-    default MemorySegment allocate(MemoryLayout layout, MemoryAddress value) {
+    default MemorySegment allocate(ValueLayout layout, Addressable value) {
         if (MemoryLayouts.ADDRESS.byteSize() != layout.byteSize()) {
             throw new IllegalArgumentException("Layout size mismatch - " + layout.byteSize() + " != " + MemoryLayouts.ADDRESS.byteSize());
         }
         switch ((int)layout.byteSize()) {
-            case 4: return allocate(layout, (int)value.toRawLongValue());
-            case 8: return allocate(layout, value.toRawLongValue());
+            case 4: return allocate(layout, (int)value.address().toRawLongValue());
+            case 8: return allocate(layout, value.address().toRawLongValue());
             default: throw new UnsupportedOperationException("Unsupported pointer size"); // should not get here
         }
     }
@@ -322,16 +323,16 @@ default MemorySegment allocateArray(ValueLayout elementLayout, double[] array) {
      * {@code limit() - size() < (elementLayout.byteSize() * array.length)}.
      * @throws IllegalArgumentException if {@code layout.byteSize() != MemoryLayouts.ADDRESS.byteSize()}.
      */
-    default MemorySegment allocateArray(ValueLayout elementLayout, MemoryAddress[] array) {
+    default MemorySegment allocateArray(ValueLayout elementLayout, Addressable[] array) {
         if (MemoryLayouts.ADDRESS.byteSize() != elementLayout.byteSize()) {
             throw new IllegalArgumentException("Layout size mismatch - " + elementLayout.byteSize() + " != " + MemoryLayouts.ADDRESS.byteSize());
         }
         switch ((int)elementLayout.byteSize()) {
             case 4: return copyArrayWithSwapIfNeeded(Stream.of(array)
-                            .mapToInt(a -> (int)a.toRawLongValue()).toArray(),
+                            .mapToInt(a -> (int)a.address().toRawLongValue()).toArray(),
                             elementLayout, MemorySegment::ofArray);
             case 8: return copyArrayWithSwapIfNeeded(Stream.of(array)
-                            .mapToLong(MemoryAddress::toRawLongValue).toArray(),
+                            .mapToLong(a -> a.address().toRawLongValue()).toArray(),
                             elementLayout, MemorySegment::ofArray);
             default: throw new UnsupportedOperationException("Unsupported pointer size"); // should not get here
         }

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64VaList.java b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/AArch64VaList.java
@@ -25,13 +25,7 @@
  */
 package jdk.internal.foreign.abi.aarch64;
 
-import jdk.incubator.foreign.CSupport;
-import jdk.incubator.foreign.GroupLayout;
-import jdk.incubator.foreign.MemoryAddress;
-import jdk.incubator.foreign.MemoryHandles;
-import jdk.incubator.foreign.MemoryLayout;
-import jdk.incubator.foreign.MemorySegment;
-import jdk.incubator.foreign.NativeScope;
+import jdk.incubator.foreign.*;
 import jdk.internal.foreign.NativeMemorySegmentImpl;
 import jdk.internal.foreign.Utils;
 import jdk.internal.foreign.abi.SharedUtils;
@@ -444,27 +438,27 @@ static class Builder implements CSupport.VaList.Builder {
         }
 
         @Override
-        public Builder vargFromInt(MemoryLayout layout, int value) {
+        public Builder vargFromInt(ValueLayout layout, int value) {
             return arg(int.class, layout, value);
         }
 
         @Override
-        public Builder vargFromLong(MemoryLayout layout, long value) {
+        public Builder vargFromLong(ValueLayout layout, long value) {
             return arg(long.class, layout, value);
         }
 
         @Override
-        public Builder vargFromDouble(MemoryLayout layout, double value) {
+        public Builder vargFromDouble(ValueLayout layout, double value) {
             return arg(double.class, layout, value);
         }
 
         @Override
-        public Builder vargFromAddress(MemoryLayout layout, MemoryAddress value) {
-            return arg(MemoryAddress.class, layout, value);
+        public Builder vargFromAddress(ValueLayout layout, Addressable value) {
+            return arg(MemoryAddress.class, layout, value.address());
         }
 
         @Override
-        public Builder vargFromSegment(MemoryLayout layout, MemorySegment value) {
+        public Builder vargFromSegment(GroupLayout layout, MemorySegment value) {
             return arg(MemorySegment.class, layout, value);
         }
 

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/sysv/SysVVaList.java b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/sysv/SysVVaList.java
@@ -25,13 +25,7 @@
  */
 package jdk.internal.foreign.abi.x64.sysv;
 
-import jdk.incubator.foreign.CSupport;
-import jdk.incubator.foreign.GroupLayout;
-import jdk.incubator.foreign.MemoryAddress;
-import jdk.incubator.foreign.MemoryHandles;
-import jdk.incubator.foreign.MemoryLayout;
-import jdk.incubator.foreign.MemorySegment;
-import jdk.incubator.foreign.NativeScope;
+import jdk.incubator.foreign.*;
 import jdk.internal.foreign.NativeMemorySegmentImpl;
 import jdk.internal.foreign.Utils;
 import jdk.internal.foreign.abi.SharedUtils;
@@ -375,27 +369,27 @@ public Builder(SharedUtils.Allocator allocator) {
         }
 
         @Override
-        public Builder vargFromInt(MemoryLayout layout, int value) {
+        public Builder vargFromInt(ValueLayout layout, int value) {
             return arg(int.class, layout, value);
         }
 
         @Override
-        public Builder vargFromLong(MemoryLayout layout, long value) {
+        public Builder vargFromLong(ValueLayout layout, long value) {
             return arg(long.class, layout, value);
         }
 
         @Override
-        public Builder vargFromDouble(MemoryLayout layout, double value) {
+        public Builder vargFromDouble(ValueLayout layout, double value) {
             return arg(double.class, layout, value);
         }
 
         @Override
-        public Builder vargFromAddress(MemoryLayout layout, MemoryAddress value) {
-            return arg(MemoryAddress.class, layout, value);
+        public Builder vargFromAddress(ValueLayout layout, Addressable value) {
+            return arg(MemoryAddress.class, layout, value.address());
         }
 
         @Override
-        public Builder vargFromSegment(MemoryLayout layout, MemorySegment value) {
+        public Builder vargFromSegment(GroupLayout layout, MemorySegment value) {
             return arg(MemorySegment.class, layout, value);
         }
 

diff --git a/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/windows/WinVaList.java b/src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/x64/windows/WinVaList.java
@@ -25,13 +25,8 @@
  */
 package jdk.internal.foreign.abi.x64.windows;
 
-import jdk.incubator.foreign.CSupport;
+import jdk.incubator.foreign.*;
 import jdk.incubator.foreign.CSupport.VaList;
-import jdk.incubator.foreign.MemoryAddress;
-import jdk.incubator.foreign.MemoryHandles;
-import jdk.incubator.foreign.MemoryLayout;
-import jdk.incubator.foreign.MemorySegment;
-import jdk.incubator.foreign.NativeScope;
 import jdk.internal.foreign.abi.SharedUtils;
 import jdk.internal.foreign.abi.SharedUtils.SimpleVaArg;
 
@@ -206,27 +201,27 @@ private Builder arg(Class<?> carrier, MemoryLayout layout, Object value) {
         }
 
         @Override
-        public Builder vargFromInt(MemoryLayout layout, int value) {
+        public Builder vargFromInt(ValueLayout layout, int value) {
             return arg(int.class, layout, value);
         }
 
         @Override
-        public Builder vargFromLong(MemoryLayout layout, long value) {
+        public Builder vargFromLong(ValueLayout layout, long value) {
             return arg(long.class, layout, value);
         }
 
         @Override
-        public Builder vargFromDouble(MemoryLayout layout, double value) {
+        public Builder vargFromDouble(ValueLayout layout, double value) {
             return arg(double.class, layout, value);
         }
 
         @Override
-        public Builder vargFromAddress(MemoryLayout layout, MemoryAddress value) {
-            return arg(MemoryAddress.class, layout, value);
+        public Builder vargFromAddress(ValueLayout layout, Addressable value) {
+            return arg(MemoryAddress.class, layout, value.address());
         }
 
         @Override
-        public Builder vargFromSegment(MemoryLayout layout, MemorySegment value) {
+        public Builder vargFromSegment(GroupLayout layout, MemorySegment value) {
             return arg(MemorySegment.class, layout, value);
         }
 

diff --git a/test/jdk/java/foreign/StdLibTest.java b/test/jdk/java/foreign/StdLibTest.java
@@ -44,15 +44,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import jdk.incubator.foreign.CSupport;
-import jdk.incubator.foreign.ForeignLinker;
-import jdk.incubator.foreign.FunctionDescriptor;
-import jdk.incubator.foreign.LibraryLookup;
-import jdk.incubator.foreign.MemoryAddress;
-import jdk.incubator.foreign.MemoryLayout;
-import jdk.incubator.foreign.MemorySegment;
-import jdk.incubator.foreign.NativeScope;
-import jdk.incubator.foreign.SequenceLayout;
+import jdk.incubator.foreign.*;
 
 import static jdk.incubator.foreign.MemoryAccess.*;
 
@@ -426,14 +418,14 @@ enum PrintfArg implements Consumer<VaList.Builder> {
         DOUBLE(double.class, asVarArg(C_DOUBLE), "%.4f", 1.2345d, 1.2345d, VaList.Builder::vargFromDouble);
 
         final Class<?> carrier;
-        final MemoryLayout layout;
+        final ValueLayout layout;
         final String format;
         final Object nativeValue;
         final Object javaValue;
         @SuppressWarnings("rawtypes")
         final VaListBuilderCall builderCall;
 
-        <Z> PrintfArg(Class<?> carrier, MemoryLayout layout, String format, Z nativeValue, Object javaValue, VaListBuilderCall<Z> builderCall) {
+        <Z> PrintfArg(Class<?> carrier, ValueLayout layout, String format, Z nativeValue, Object javaValue, VaListBuilderCall<Z> builderCall) {
             this.carrier = carrier;
             this.layout = layout;
             this.format = format;
@@ -449,7 +441,7 @@ public void accept(VaList.Builder builder) {
         }
 
         interface VaListBuilderCall<V> {
-            void build(VaList.Builder builder, MemoryLayout layout, V value);
+            void build(VaList.Builder builder, ValueLayout layout, V value);
         }
     }