8268888: Upstream 8268230: Foreign Linker API & Windows user32/kernel…

…32: String conversion seems broken

Reviewed-by: mcimadamore

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java

@@ -41,6 +41,7 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 import java.util.function.Consumer;
 
@@ -285,7 +286,7 @@ static <T extends MemoryLayout> T asVarArg(T layout) {
     }
 
     /**
-     * Converts a Java string into a null-terminated C string, using the platform's default charset,
+     * Converts a Java string into a UTF-8 encoded, null-terminated C string,
      * storing the result into a native memory segment allocated using the provided allocator.
      * <p>
      * This method always replaces malformed-input and unmappable-character
@@ -300,11 +301,11 @@ static <T extends MemoryLayout> T asVarArg(T layout) {
     static MemorySegment toCString(String str, SegmentAllocator allocator) {
         Objects.requireNonNull(str);
         Objects.requireNonNull(allocator);
-        return toCString(str.getBytes(), allocator);
+        return toCString(str.getBytes(StandardCharsets.UTF_8), allocator);
     }
 
     /**
-     * Converts a Java string into a null-terminated C string, using the platform's default charset,
+     * Converts a Java string into a UTF-8 encoded, null-terminated C string,
      * storing the result into a native memory segment associated with the provided resource scope.
      * <p>
      * This method always replaces malformed-input and unmappable-character
@@ -323,48 +324,7 @@ static MemorySegment toCString(String str, ResourceScope scope) {
     }
 
     /**
-     * Converts a Java string into a null-terminated C string, using the given {@linkplain java.nio.charset.Charset charset},
-     * storing the result into a new native memory segment native memory segment allocated using the provided allocator.
-     * <p>
-     * This method always replaces malformed-input and unmappable-character
-     * sequences with this charset's default replacement byte array.  The
-     * {@link java.nio.charset.CharsetEncoder} class should be used when more
-     * control over the encoding process is required.
-     *
-     * @param str the Java string to be converted into a C string.
-     * @param charset The {@link java.nio.charset.Charset} to be used to compute the contents of the C string.
-     * @param allocator the allocator to be used for the native segment allocation.
-     * @return a new native memory segment containing the converted C string.
-     */
-    static MemorySegment toCString(String str, Charset charset, SegmentAllocator allocator) {
-        Objects.requireNonNull(str);
-        Objects.requireNonNull(charset);
-        Objects.requireNonNull(allocator);
-        return toCString(str.getBytes(charset), allocator);
-    }
-
-    /**
-     * Converts a Java string into a null-terminated C string, using the given {@linkplain java.nio.charset.Charset charset},
-     * storing the result into a native memory segment associated with the provided resource scope.
-     * <p>
-     * This method always replaces malformed-input and unmappable-character
-     * sequences with this charset's default replacement byte array.  The
-     * {@link java.nio.charset.CharsetEncoder} class should be used when more
-     * control over the encoding process is required.
-     *
-     * @param str the Java string to be converted into a C string.
-     * @param charset The {@link java.nio.charset.Charset} to be used to compute the contents of the C string.
-     * @param scope the resource scope to be associated with the returned segment.
-     * @return a new native memory segment containing the converted C string.
-     * @throws IllegalStateException if {@code scope} has been already closed, or if access occurs from a thread other
-     * than the thread owning {@code scope}.
-     */
-    static MemorySegment toCString(String str, Charset charset, ResourceScope scope) {
-        return toCString(str, charset, SegmentAllocator.ofScope(scope));
-    }
-
-    /**
-     * Converts a null-terminated C string stored at given address into a Java string, using the platform's default charset.
+     * Converts a UTF-8 encoded, null-terminated C string stored at given address into a Java string.
      * <p>
      * This method always replaces malformed-input and unmappable-character
      * sequences with this charset's default replacement string.  The {@link
@@ -378,49 +338,21 @@ static MemorySegment toCString(String str, Charset charset, ResourceScope scope)
      *
      * @param addr the address at which the string is stored.
      * @return a Java string with the contents of the null-terminated C string at given address.
-     * @throws IllegalArgumentException if the size of the native string is greater than the largest string supported by the platform.
+     * @throws IllegalArgumentException if the size of the native string is greater than the largest string supported by the platform,
+     * or if {@code addr == MemoryAddress.NULL}.
      * @throws IllegalCallerException if access to this method occurs from a module {@code M} and the command line option
      * {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.
      */
     @CallerSensitive
     static String toJavaString(MemoryAddress addr) {
         Reflection.ensureNativeAccess(Reflection.getCallerClass());
-        Objects.requireNonNull(addr);
-        return SharedUtils.toJavaStringInternal(NativeMemorySegmentImpl.EVERYTHING, addr.toRawLongValue(), Charset.defaultCharset());
+        SharedUtils.checkAddress(addr);
+        return SharedUtils.toJavaStringInternal(NativeMemorySegmentImpl.EVERYTHING, addr.toRawLongValue());
     }
 
     /**
-     * Converts a null-terminated C string stored at given address into a Java string, using the given {@linkplain java.nio.charset.Charset charset}.
-     * <p>
-     * This method always replaces malformed-input and unmappable-character
-     * sequences with this charset's default replacement string.  The {@link
-     * java.nio.charset.CharsetDecoder} class should be used when more control
-     * over the decoding process is required.
-     * <p>
-     * This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
-     * Restricted methods 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 the address at which the string is stored.
-     * @param charset The {@link java.nio.charset.Charset} to be used to compute the contents of the Java string.
-     * @return a Java string with the contents of the null-terminated C string at given address.
-     * @throws IllegalArgumentException if the size of the native string is greater than the largest string supported by the platform.
-     * @throws IllegalCallerException if access to this method occurs from a module {@code M} and the command line option
-     * {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
-     * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.
-     */
-    @CallerSensitive
-    static String toJavaString(MemoryAddress addr, Charset charset) {
-        Reflection.ensureNativeAccess(Reflection.getCallerClass());
-        Objects.requireNonNull(addr);
-        Objects.requireNonNull(charset);
-        return SharedUtils.toJavaStringInternal(NativeMemorySegmentImpl.EVERYTHING, addr.toRawLongValue(), charset);
-    }
-
-    /**
-     * Converts a null-terminated C string stored at given address into a Java string, using the platform's default charset.
+     * Converts a UTF-8 encoded, null-terminated C string stored at given address into a Java string.
      * <p>
      * This method always replaces malformed-input and unmappable-character
      * sequences with this charset's default replacement string.  The {@link
@@ -434,27 +366,7 @@ static String toJavaString(MemoryAddress addr, Charset charset) {
      */
     static String toJavaString(MemorySegment addr) {
         Objects.requireNonNull(addr);
-        return SharedUtils.toJavaStringInternal(addr, 0L, Charset.defaultCharset());
-    }
-
-    /**
-     * Converts a null-terminated C string stored at given address into a Java string, using the given {@linkplain java.nio.charset.Charset charset}.
-     * <p>
-     * This method always replaces malformed-input and unmappable-character
-     * sequences with this charset's default replacement string.  The {@link
-     * java.nio.charset.CharsetDecoder} class should be used when more control
-     * over the decoding process is required.
-     * @param addr the address at which the string is stored.
-     * @param charset The {@link java.nio.charset.Charset} to be used to compute the contents of the Java string.
-     * @return a Java string with the contents of the null-terminated C string at given address.
-     * @throws IllegalArgumentException if the size of the native string is greater than the largest string supported by the platform.
-     * @throws IllegalStateException if the size of the native string is greater than the size of the segment
-     * associated with {@code addr}, or if {@code addr} is associated with a segment that is <em>not alive</em>.
-     */
-    static String toJavaString(MemorySegment addr, Charset charset) {
-        Objects.requireNonNull(addr);
-        Objects.requireNonNull(charset);
-        return SharedUtils.toJavaStringInternal(addr, 0L, charset);
+        return SharedUtils.toJavaStringInternal(addr, 0L);
     }
 
     private static void copy(MemorySegment addr, byte[] bytes) {
@@ -505,13 +417,14 @@ static MemoryAddress allocateMemory(long size) {
      *
      * @param addr memory address of the native memory to be freed
      * @throws IllegalCallerException if access to this method occurs from a module {@code M} and the command line option
+     * @throws IllegalArgumentException if {@code addr == MemoryAddress.NULL}.
      * {@code --enable-native-access} is either absent, or does not mention the module name {@code M}, or
      * {@code ALL-UNNAMED} in case {@code M} is an unnamed module.
      */
     @CallerSensitive
     static void freeMemory(MemoryAddress addr) {
         Reflection.ensureNativeAccess(Reflection.getCallerClass());
-        Objects.requireNonNull(addr);
+        SharedUtils.checkAddress(addr);
         SharedUtils.freeMemoryInternal(addr);
     }
 

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/SharedUtils.java

@@ -53,6 +53,8 @@
 import java.lang.invoke.VarHandle;
 import java.lang.ref.Reference;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -282,12 +284,12 @@ public static CLinker getSystemLinker() {
         };
     }
 
-    public static String toJavaStringInternal(MemorySegment segment, long start, Charset charset) {
+    public static String toJavaStringInternal(MemorySegment segment, long start) {
         int len = strlen(segment, start);
         byte[] bytes = new byte[len];
         MemorySegment.ofArray(bytes)
                 .copyFrom(segment.asSlice(start, len));
-        return new String(bytes, charset);
+        return new String(bytes, StandardCharsets.UTF_8);
     }
 
     private static int strlen(MemorySegment segment, long start) {
@@ -447,6 +449,14 @@ private static class AllocHolder {
     }
 
     public static MemoryAddress checkSymbol(Addressable symbol) {
+        return checkAddressable(symbol, "Symbol is NULL");
+    }
+
+    public static MemoryAddress checkAddress(MemoryAddress address) {
+        return checkAddressable(address, "Address is NULL");
+    }
+
+    private static MemoryAddress checkAddressable(Addressable symbol, String msg) {
         Objects.requireNonNull(symbol);
         MemoryAddress symbolAddr = symbol.address();
         if (symbolAddr.equals(MemoryAddress.NULL))

test/jdk/java/foreign/TestNULLTarget.java → test/jdk/java/foreign/TestNULLAddress.java

@@ -27,7 +27,7 @@
  * @modules jdk.incubator.foreign
  * @run testng/othervm
  *     --enable-native-access=ALL-UNNAMED
- *     TestNULLTarget
+ *     TestNULLAddress
  */
 
 import jdk.incubator.foreign.Addressable;
@@ -38,8 +38,9 @@
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodType;
+import java.nio.charset.Charset;
 
-public class TestNULLTarget {
+public class TestNULLAddress {
 
     static final CLinker LINKER = CLinker.getInstance();
 
@@ -58,4 +59,14 @@ public void testNULLVirtual() throws Throwable {
                 FunctionDescriptor.ofVoid());
         mh.invokeExact((Addressable) MemoryAddress.NULL);
     }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void testNULLtoJavaString() {
+        CLinker.toJavaString(MemoryAddress.NULL);
+    }
+
+    @Test(expectedExceptions = IllegalArgumentException.class)
+    public void testNULLfreeMemory() {
+        CLinker.freeMemory(MemoryAddress.NULL);
+    }
 }

test/jdk/java/foreign/TestStringEncoding.java

@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ *  DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ *  This code is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 only, as
+ *  published by the Free Software Foundation.
+ *
+ *  This code is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ *  version 2 for more details (a copy is included in the LICENSE file that
+ *  accompanied this code).
+ *
+ *  You should have received a copy of the GNU General Public License version
+ *  2 along with this work; if not, write to the Free Software Foundation,
+ *  Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ *   Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ *  or visit www.oracle.com if you need additional information or have any
+ *  questions.
+ *
+ */
+
+import jdk.incubator.foreign.CLinker;
+import jdk.incubator.foreign.MemorySegment;
+
+import jdk.incubator.foreign.ResourceScope;
+import org.testng.annotations.*;
+import static org.testng.Assert.*;
+
+/*
+ * @test
+ * @requires ((os.arch == "amd64" | os.arch == "x86_64") & sun.arch.data.model == "64") | os.arch == "aarch64"
+ * @run testng TestStringEncoding
+ */
+
+public class TestStringEncoding {
+
+    @Test(dataProvider = "strings")
+    public void testStrings(String testString, int expectedByteLength) {
+        try (ResourceScope scope = ResourceScope.newConfinedScope()) {
+            MemorySegment text = CLinker.toCString(testString, scope);
+
+            assertEquals(text.byteSize(), expectedByteLength);
+
+            String roundTrip = CLinker.toJavaString(text);
+            assertEquals(roundTrip, testString);
+        }
+    }
+
+    @DataProvider
+    public static Object[][] strings() {
+        return new Object[][] {
+            { "testing",               8 },
+            { "",                      1 },
+            { "X",                     2 },
+            { "12345",                 6 },
+            { "yen \u00A5",            7 }, // in UTF-8 2 bytes: 0xC2 0xA5
+            { "snowman \u26C4",       12 }, // in UTF-8 three bytes: 0xE2 0x9B 0x84
+            { "rainbow \uD83C\uDF08", 13 }  // in UTF-8 four bytes: 0xF0 0x9F 0x8C 0x88
+        };
+    }
+}

test/jdk/java/foreign/handles/lookup_module/handle/lookup/MethodHandleLookup.java

@@ -58,10 +58,7 @@ static Object[][] restrictedMethods() {
                             MethodType.methodType(CLinker.class)), "CLinker::getInstance" },
                     { MethodHandles.lookup().findStatic(CLinker.class, "toJavaString",
                             MethodType.methodType(String.class, MemoryAddress.class)),
-                            "CLinker::toJavaString/1" },
-                    { MethodHandles.lookup().findStatic(CLinker.class, "toJavaString",
-                            MethodType.methodType(String.class, MemoryAddress.class, Charset.class)),
-                            "CLinker::toJavaString/2" },
+                            "CLinker::toJavaString" },
                     { MethodHandles.lookup().findStatic(CLinker.class, "allocateMemory",
                             MethodType.methodType(MemoryAddress.class, long.class)),
                             "CLinker::allocateMemory" },