8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled

Reviewed-by: psandoz, mcimadamore

src/java.base/share/classes/java/lang/System.java

@@ -2398,6 +2398,11 @@ public final class System {
             public long findNative(ClassLoader loader, String entry) {
                 return ClassLoader.findNative(loader, entry);
             }
+
+            @Override
+            public void exit(int statusCode) {
+                Shutdown.exit(statusCode);
+            }
         });
     }
 }

src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java

@@ -395,4 +395,10 @@ public interface JavaLangAccess {
     Object classData(Class<?> c);
 
     long findNative(ClassLoader loader, String entry);
+
+    /**
+     * Direct access to Shutdown.exit to avoid security manager checks
+     * @param statusCode the status code
+     */
+    void exit(int statusCode);
 }

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

@@ -214,8 +214,15 @@ public sealed interface CLinker permits AbstractCLinker {
      * Allocates a native stub with given scope which can be passed to other foreign functions (as a function pointer);
      * calling such a function pointer from native code will result in the execution of the provided method handle.
      *
-     * <p>The returned memory address is associated with the provided scope. When such scope is closed,
+     * <p>
+     * The returned memory address is associated with the provided scope. When such scope is closed,
      * the corresponding native stub will be deallocated.
+     * <p>
+     * The target method handle should not throw any exceptions. If the target method handle does throw an exception,
+     * the VM will exit with a non-zero exit code. To avoid the VM aborting due to an uncaught exception, clients
+     * could wrap all code in the target method handle in a try/catch block that catches any {@link Throwable}, for
+     * instance by using the {@link java.lang.invoke.MethodHandles#catchException(MethodHandle, Class, MethodHandle)}
+     * method handle combinator, and handle exceptions as desired in the corresponding catch block.
      *
      * @param target   the target method handle.
      * @param function the function descriptor.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/CABI.java

@@ -25,9 +25,10 @@
  */
 package jdk.internal.foreign;
 
-import jdk.internal.foreign.abi.SharedUtils;
+import sun.security.action.GetPropertyAction;
 
 import static jdk.incubator.foreign.MemoryLayouts.ADDRESS;
+import static sun.security.action.GetPropertyAction.privilegedGetProperty;
 
 public enum CABI {
     SysV,
@@ -38,8 +39,8 @@ public enum CABI {
     private static final CABI current;
 
     static {
-        String arch = System.getProperty("os.arch");
+        String arch = privilegedGetProperty("os.arch");
-        String os = System.getProperty("os.name");
+        String os = privilegedGetProperty("os.name");
         long addressSize = ADDRESS.bitSize();
         // might be running in a 32-bit VM on a 64-bit platform.
         // addressSize will be correctly 32

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

@@ -295,6 +295,9 @@ public class ProgrammableUpcallHandler {
             } else {
                 return returnMoves;
             }
+        } catch(Throwable t) {
+            SharedUtils.handleUncaughtException(t);
+            return null;
         }
     }
 

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

@@ -37,6 +37,8 @@ import jdk.incubator.foreign.SegmentAllocator;
 import jdk.incubator.foreign.SequenceLayout;
 import jdk.incubator.foreign.CLinker;
 import jdk.incubator.foreign.ValueLayout;
+import jdk.internal.access.JavaLangAccess;
+import jdk.internal.access.SharedSecrets;
 import jdk.internal.foreign.CABI;
 import jdk.internal.foreign.MemoryAddressImpl;
 import jdk.internal.foreign.Utils;
@@ -73,6 +75,8 @@ import static jdk.incubator.foreign.CLinker.*;
 
 public class SharedUtils {
 
+    private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
+
     private static final MethodHandle MH_ALLOC_BUFFER;
     private static final MethodHandle MH_BASEADDRESS;
     private static final MethodHandle MH_BUFFER_COPY;
@@ -80,6 +84,7 @@ public class SharedUtils {
     private static final MethodHandle MH_MAKE_CONTEXT_BOUNDED_ALLOCATOR;
     private static final MethodHandle MH_CLOSE_CONTEXT;
     private static final MethodHandle MH_REACHBILITY_FENCE;
+    private static final MethodHandle MH_HANDLE_UNCAUGHT_EXCEPTION;
 
     static {
         try {
@@ -98,6 +103,8 @@ public class SharedUtils {
                     methodType(void.class));
             MH_REACHBILITY_FENCE = lookup.findStatic(Reference.class, "reachabilityFence",
                     methodType(void.class, Object.class));
+            MH_HANDLE_UNCAUGHT_EXCEPTION = lookup.findStatic(SharedUtils.class, "handleUncaughtException",
+                    methodType(void.class, Throwable.class));
         } catch (ReflectiveOperationException e) {
             throw new BootstrapMethodError(e);
         }
@@ -361,6 +368,13 @@ public class SharedUtils {
         return MH_REACHBILITY_FENCE.asType(MethodType.methodType(void.class, type));
     }
 
+    static void handleUncaughtException(Throwable t) {
+        if (t != null) {
+            t.printStackTrace();
+            JLA.exit(1);
+        }
+    }
+
     static MethodHandle wrapWithAllocator(MethodHandle specializedHandle,
                                           int allocatorPos, long bufferCopySize,
                                           boolean upcall) {
@@ -368,7 +382,11 @@ public class SharedUtils {
         MethodHandle closer;
         int insertPos;
         if (specializedHandle.type().returnType() == void.class) {
-            closer = empty(methodType(void.class, Throwable.class)); // (Throwable) -> void
+            if (!upcall) {
+                closer = empty(methodType(void.class, Throwable.class)); // (Throwable) -> void
+            } else {
+                closer = MH_HANDLE_UNCAUGHT_EXCEPTION;
+            }
             insertPos = 1;
         } else {
             closer = identity(specializedHandle.type().returnType()); // (V) -> V

test/jdk/java/foreign/TestUpcallException.java

@@ -0,0 +1,105 @@
+/*
+ * 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.
+ */
+
+/*
+ * @test
+ * @requires ((os.arch == "amd64" | os.arch == "x86_64") & sun.arch.data.model == "64") | os.arch == "aarch64"
+ * @library /test/lib
+ * @modules jdk.incubator.foreign/jdk.internal.foreign
+ * @build ThrowingUpcall TestUpcallException
+ *
+ * @run testng/othervm/native
+ *   --enable-native-access=ALL-UNNAMED
+ *   TestUpcallException
+ */
+
+import jdk.incubator.foreign.CLinker;
+import jdk.incubator.foreign.FunctionDescriptor;
+import jdk.incubator.foreign.ResourceScope;
+import jdk.test.lib.Utils;
+import org.testng.annotations.Test;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.file.Paths;
+import java.util.List;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertTrue;
+
+public class TestUpcallException {
+
+    @Test
+    public void testExceptionInterpreted() throws InterruptedException, IOException {
+        boolean useSpec = false;
+        run(useSpec);
+    }
+
+    @Test
+    public void testExceptionSpecialized() throws IOException, InterruptedException {
+        boolean useSpec = true;
+        run(useSpec);
+    }
+
+    private void run(boolean useSpec) throws IOException, InterruptedException {
+        Process process = new ProcessBuilder()
+            .command(
+                Paths.get(Utils.TEST_JDK)
+                     .resolve("bin")
+                     .resolve("java")
+                     .toAbsolutePath()
+                     .toString(),
+                "--add-modules", "jdk.incubator.foreign",
+                "--enable-native-access=ALL-UNNAMED",
+                "-Djava.library.path=" + System.getProperty("java.library.path"),
+                "-Djdk.internal.foreign.ProgrammableUpcallHandler.USE_SPEC=" + useSpec,
+                "-cp", Utils.TEST_CLASS_PATH,
+                "ThrowingUpcall")
+            .start();
+
+        int result = process.waitFor();
+        assertNotEquals(result, 0);
+
+        List<String> outLines = linesFromStream(process.getInputStream());
+        outLines.forEach(System.out::println);
+        List<String> errLines = linesFromStream(process.getErrorStream());
+        errLines.forEach(System.err::println);
+
+        // Exception message would be found in stack trace
+        String shouldInclude = "Testing upcall exceptions";
+        assertTrue(linesContain(errLines, shouldInclude), "Did not find '" + shouldInclude + "' in stderr");
+    }
+
+    private boolean linesContain(List<String> errLines, String shouldInclude) {
+        return errLines.stream().anyMatch(line -> line.contains(shouldInclude));
+    }
+
+    private static List<String> linesFromStream(InputStream stream) throws IOException {
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(stream))) {
+            return reader.lines().toList();
+        }
+    }
+}

test/jdk/java/foreign/ThrowingUpcall.java

@@ -0,0 +1,79 @@
+/*
+ * 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.FunctionDescriptor;
+import jdk.incubator.foreign.MemoryAddress;
+import jdk.incubator.foreign.ResourceScope;
+import jdk.incubator.foreign.SymbolLookup;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.security.Permission;
+
+import static jdk.incubator.foreign.CLinker.C_POINTER;
+
+public class ThrowingUpcall {
+
+    private static final MethodHandle downcall;
+    public static final MethodHandle MH_throwException;
+
+    static {
+        System.loadLibrary("TestUpcall");
+        SymbolLookup lookup = SymbolLookup.loaderLookup();
+        downcall = CLinker.getInstance().downcallHandle(
+            lookup.lookup("f0_V__").orElseThrow(),
+            MethodType.methodType(void.class, MemoryAddress.class),
+            FunctionDescriptor.ofVoid(C_POINTER)
+        );
+
+        try {
+            MH_throwException = MethodHandles.lookup().findStatic(ThrowingUpcall.class, "throwException",
+                    MethodType.methodType(void.class));
+        } catch (ReflectiveOperationException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
+
+    public static void throwException() throws Throwable {
+        throw new Throwable("Testing upcall exceptions");
+    }
+
+    public static void main(String[] args) throws Throwable {
+        test();
+    }
+
+    public static void test() throws Throwable {
+        MethodHandle handle = MH_throwException;
+        MethodHandle invoker = MethodHandles.exactInvoker(MethodType.methodType(void.class));
+        handle = MethodHandles.insertArguments(invoker, 0, handle);
+
+        try (ResourceScope scope = ResourceScope.newConfinedScope()) {
+            MemoryAddress stub = CLinker.getInstance().upcallStub(handle, FunctionDescriptor.ofVoid(), scope);
+
+            downcall.invokeExact(stub); // should call Shutdown.exit(1);
+        }
+    }
+
+}