Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8267989: Exceptions thrown during upcalls should be handled #543

Closed
@@ -43,6 +43,7 @@
import java.util.function.Consumer;

import static jdk.internal.foreign.PlatformLayouts.*;
import static sun.security.action.GetIntegerAction.privilegedGetProperty;

/**
* A C linker implements the C Application Binary Interface (ABI) calling conventions.
@@ -114,6 +115,13 @@
*/
public interface CLinker {

/**
* The value returned by the process when the target of an upcall throws an exception.
*
* @see CLinker#upcallStub(MethodHandle, FunctionDescriptor, ResourceScope)
*/
int ERR_UNCAUGHT_EXCEPTION = privilegedGetProperty("jdk.incubator.foreign.uncaught_exception_code", 1);
JornVernee marked this conversation as resolved.
Show resolved Hide resolved

/**
* Returns the C linker for the current platform.
* <p>
@@ -224,9 +232,14 @@ static SymbolLookup systemLookup() {
* 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>
* Any exceptions that occur during an upcall should be handled during the upcall. The target method handle
Copy link
Collaborator

@mcimadamore mcimadamore May 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best to drop the first sentence, and go straight to the point, which is "The target method handle shoud not...".

Which reminds me, do we want to apply same logic we do for var handle combinators to detect if a method handle contains exceptions in its guts (by looking at signature of direct MH, or by looking for certain combinators).

Copy link
Member Author

@JornVernee JornVernee May 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped the first sentence, I'll see about adding a check for exceptions.

* should not throw any exceptions. If the target method handle does throw an exception, it will be handle by
* calling {@link System#exit System.exit(ERR_UNCAUGHT_EXCEPTION)}. (See {@link #ERR_UNCAUGHT_EXCEPTION})
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
* <p>
* This method is <a href="package-summary.html#restricted"><em>restricted</em></a>.
* 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
@@ -295,6 +295,9 @@ private static Object invokeInterpBindings(Object[] moves, MethodHandle leaf,
} else {
return returnMoves;
}
} catch(Throwable t) {
SharedUtils.handleUncaughtException(t);
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
return null;
}
}

@@ -79,6 +79,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 {
@@ -97,6 +98,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);
}
@@ -359,14 +362,25 @@ private static MethodHandle reachabilityFenceHandle(Class<?> type) {
return MH_REACHBILITY_FENCE.asType(MethodType.methodType(void.class, type));
}

static void handleUncaughtException(Throwable t) {
if (t != null) {
t.printStackTrace();
System.exit(ERR_UNCAUGHT_EXCEPTION);
}
}

static MethodHandle wrapWithAllocator(MethodHandle specializedHandle,
int allocatorPos, long bufferCopySize,
boolean upcall) {
// insert try-finally to close the NativeScope used for Binding.Copy
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
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
} else {
closer = MH_HANDLE_UNCAUGHT_EXCEPTION;
}
insertPos = 1;
} else {
closer = identity(specializedHandle.type().returnType()); // (V) -> V
@@ -28,7 +28,7 @@
* @modules jdk.incubator.foreign/jdk.internal.foreign
* @build NativeTestHelper CallGeneratorHelper TestUpcall
*
* @run testng/othervm
* @run testng/othervm/timeout=720
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
* --enable-native-access=ALL-UNNAMED
* TestUpcall
*/
@@ -0,0 +1,83 @@
/*
* 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.test.lib.Utils;
import org.testng.annotations.Test;

import java.io.IOException;
import java.nio.file.Paths;
import java.util.concurrent.ThreadLocalRandom;

import static org.testng.Assert.assertEquals;

public class TestUpcallException {
JornVernee marked this conversation as resolved.
Show resolved Hide resolved

@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 {
int exitCode = ThreadLocalRandom.current().nextInt();

Process process = new ProcessBuilder()
.inheritIO()
.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,
"-Djdk.incubator.foreign.uncaught_exception_code=" + exitCode,
"-cp", Utils.TEST_CLASS_PATH,
"ThrowingUpcall")
.start();

int result = process.waitFor();
assertEquals(result, exitCode);
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
}

}
@@ -0,0 +1,75 @@
/*
* 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 static jdk.incubator.foreign.CLinker.C_POINTER;

public class ThrowingUpcall {

private static final MethodHandle downcall;
private 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, Throwable.class));
} catch (ReflectiveOperationException e) {
throw new ExceptionInInitializerError(e);
}
}

public static void throwException(Throwable t) throws Throwable {
throw t;
}

public static void main(String[] args) throws Throwable {
test(new Throwable());
}

public static void test(Throwable throwable) throws Throwable {
MethodHandle target = MethodHandles.insertArguments(MH_throwException, 0, throwable);
try (ResourceScope scope = ResourceScope.newConfinedScope()) {
MemoryAddress stub = CLinker.getInstance().upcallStub(target, FunctionDescriptor.ofVoid(), scope);

downcall.invokeExact(stub); // should call System.exit(1);
}
}

}