8268230: Foreign Linker API & Windows user32/kernel32: String convers…

…ion 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;
 
@@ -286,7 +287,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
@@ -301,11 +302,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
@@ -324,48 +325,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
@@ -389,41 +349,11 @@ static MemorySegment toCString(String str, Charset charset, ResourceScope scope)
     static String toJavaString(MemoryAddress addr) {
         Reflection.ensureNativeAccess(Reflection.getCallerClass());
         SharedUtils.checkAddress(addr);
-        return SharedUtils.toJavaStringInternal(NativeMemorySegmentImpl.EVERYTHING, addr.toRawLongValue(), Charset.defaultCharset());
+        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,
-     * 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, Charset charset) {
-        Reflection.ensureNativeAccess(Reflection.getCallerClass());
-        SharedUtils.checkAddress(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
@@ -437,27 +367,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) {

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

@@ -54,6 +54,7 @@
 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;
@@ -285,12 +286,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) {

test/jdk/java/foreign/TestNULLAddress.java

@@ -65,11 +65,6 @@ public void testNULLtoJavaString() {
         CLinker.toJavaString(MemoryAddress.NULL);
     }
 
-    @Test(expectedExceptions = IllegalArgumentException.class)
-    public void testNULLtoJavaStringCharset() {
-        CLinker.toJavaString(MemoryAddress.NULL, Charset.defaultCharset());
-    }
-
     @Test(expectedExceptions = IllegalArgumentException.class)
     public void testNULLfreeMemory() {
         CLinker.freeMemory(MemoryAddress.NULL);

test/jdk/java/foreign/TestStringEncoding.java

@@ -0,0 +1,63 @@
+/*
+ * 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
+ * @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" },