From e1e37c4ec61e4ed23b5bf8027f1c97832858302f Mon Sep 17 00:00:00 2001 From: Tomas Stupka <tomas.stupka@oracle.com> Date: Thu, 25 Jan 2018 17:17:59 +0100 Subject: [PATCH] ensure that isNull in RNullMR is set back to its previous state after a NFI call --- .../truffle/r/engine/interop/RNullMR.java | 18 ++++---- .../r/ffi/impl/nfi/TruffleNFI_Call.java | 16 +++---- .../truffle/r/runtime/context/RContext.java | 6 ++- .../runtime/interop/RNullMRContextState.java | 45 +++++++++++++++++++ .../r/test/engine/interop/AbstractMRTest.java | 3 +- .../test/engine/interop/RFunctionMRTest.java | 37 ++++++++------- 6 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/interop/RNullMRContextState.java diff --git a/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RNullMR.java b/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RNullMR.java index dde35783c5..472757526d 100644 --- a/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RNullMR.java +++ b/com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/interop/RNullMR.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2018, 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 @@ -28,20 +28,18 @@ import com.oracle.truffle.api.interop.Resolve; import com.oracle.truffle.api.interop.TruffleObject; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.r.ffi.impl.interop.NativePointer; +import com.oracle.truffle.r.runtime.context.RContext; import com.oracle.truffle.r.runtime.data.NativeDataAccess; import com.oracle.truffle.r.runtime.data.RNull; +import com.oracle.truffle.r.runtime.interop.RNullMRContextState; @MessageResolution(receiverType = RNull.class) public class RNullMR { - /** - * Workaround to avoid NFI converting {@link RNull} to {@code null}. - */ - private static boolean isNull = true; @Resolve(message = "IS_NULL") public abstract static class RNullIsNullNode extends Node { protected Object access(@SuppressWarnings("unused") RNull receiver) { - return isNull; + return RContext.getInstance().stateRNullMR.isNull(); } } @@ -81,9 +79,13 @@ public class RNullMR { } } + /** + * Workaround to avoid NFI converting {@link RNull} to {@code null}. + */ static boolean setIsNull(boolean value) { - boolean prev = isNull; - isNull = value; + RNullMRContextState state = RContext.getInstance().stateRNullMR; + boolean prev = state.isNull(); + state.setIsNull(value); return prev; } } diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_Call.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_Call.java index 2e89e25493..98bea5a094 100644 --- a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_Call.java +++ b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_Call.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2018, 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 @@ -90,11 +90,11 @@ public class TruffleNFI_Call implements CallRFFI { @Cached("nativeCallInfo.address.asTruffleObject()") TruffleObject cachedAddress, @Cached("getFunction(cachedArgsLength)") TruffleObject cachedFunction) { Object result = null; - boolean isNullSetting = prepareCall(nativeCallInfo.name, args, ffiWrapNodes); Object[] realArgs = new Object[cachedArgsLength + 1]; - System.arraycopy(args, 0, realArgs, 1, cachedArgsLength); - realArgs[0] = cachedAddress; + boolean isNullSetting = prepareCall(nativeCallInfo.name, args, ffiWrapNodes); try { + System.arraycopy(args, 0, realArgs, 1, cachedArgsLength); + realArgs[0] = cachedAddress; result = ForeignAccess.sendExecute(executeNode, cachedFunction, realArgs); return unwrap.execute(result); } catch (InteropException ex) { @@ -112,11 +112,11 @@ public class TruffleNFI_Call implements CallRFFI { @Cached("create()") FFIUnwrapNode unwrap, @Cached("createExecute(cachedArgsLength)") Node executeNode) { Object result = null; - boolean isNullSetting = prepareCall(nativeCallInfo.name, args, ffiWrapNodes); Object[] realArgs = new Object[cachedArgsLength + 1]; - System.arraycopy(args, 0, realArgs, 1, cachedArgsLength); - realArgs[0] = nativeCallInfo.address.asTruffleObject(); + boolean isNullSetting = prepareCall(nativeCallInfo.name, args, ffiWrapNodes); try { + System.arraycopy(args, 0, realArgs, 1, cachedArgsLength); + realArgs[0] = nativeCallInfo.address.asTruffleObject(); result = ForeignAccess.sendExecute(executeNode, getFunction(cachedArgsLength), realArgs); return unwrap.execute(result); } catch (InteropException ex) { @@ -181,6 +181,7 @@ public class TruffleNFI_Call implements CallRFFI { } private static void prepareReturn(String name, Object result, boolean isNullSetting) { + RContext.getRForeignAccessFactory().setIsNull(isNullSetting); if (traceEnabled()) { traceDownCallReturn(name, result); } @@ -191,7 +192,6 @@ public class TruffleNFI_Call implements CallRFFI { nfiCtx.setLastUpCallException(null); throw lastUpCallEx; } - RContext.getRForeignAccessFactory().setIsNull(isNullSetting); } @Override diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/RContext.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/RContext.java index 1fbb949422..45ef9e3c0c 100644 --- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/RContext.java +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/context/RContext.java @@ -103,6 +103,7 @@ import com.oracle.truffle.r.runtime.ffi.RFFIContext; import com.oracle.truffle.r.runtime.ffi.RFFIFactory; import com.oracle.truffle.r.runtime.instrument.InstrumentationState; import com.oracle.truffle.r.runtime.interop.FastrInteropTryContextState; +import com.oracle.truffle.r.runtime.interop.RNullMRContextState; import com.oracle.truffle.r.runtime.nodes.RCodeBuilder; import com.oracle.truffle.r.runtime.nodes.RSyntaxNode; import com.oracle.truffle.r.runtime.rng.RRNG; @@ -350,6 +351,7 @@ public final class RContext { public final InstrumentationState stateInstrumentation; public final ContextStateImpl stateInternalCode; public final DLL.ContextStateImpl stateDLL; + public final RNullMRContextState stateRNullMR; @CompilationFinal private RFFIContext stateRFFI; @@ -370,7 +372,7 @@ public final class RContext { private ContextState[] contextStates() { return new ContextState[]{stateREnvVars, stateRLocale, stateRProfile, stateTempPath, stateROptions, stateREnvironment, stateRErrorHandling, stateRConnection, stateStdConnections, stateRNG, stateRFFI, - stateRSerialize, stateLazyDBCache, stateInstrumentation, stateDLL}; + stateRSerialize, stateLazyDBCache, stateInstrumentation, stateDLL, stateRNullMR}; } public static void setEmbedded() { @@ -454,6 +456,7 @@ public final class RContext { this.stateInstrumentation = InstrumentationState.newContextState(instrumenter); this.stateInternalCode = ContextStateImpl.newContextState(); this.stateDLL = DLL.ContextStateImpl.newContextState(); + this.stateRNullMR = RNullMRContextState.newContextState(); this.engine = RContext.getRRuntimeASTAccess().createEngine(this); state.add(State.CONSTRUCTED); @@ -527,6 +530,7 @@ public final class RContext { stateLazyDBCache.initialize(this); stateInstrumentation.initialize(this); stateInternalCode.initialize(this); + stateRNullMR.initialize(this); state.add(State.INITIALIZED); if (!embedded) { diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/interop/RNullMRContextState.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/interop/RNullMRContextState.java new file mode 100644 index 0000000000..cdb89afb1e --- /dev/null +++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/interop/RNullMRContextState.java @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2018, 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. + */ +package com.oracle.truffle.r.runtime.interop; + +import com.oracle.truffle.r.runtime.context.RContext; +import com.oracle.truffle.r.runtime.data.RNull; + +public class RNullMRContextState implements RContext.ContextState { + /** + * Workaround to avoid NFI converting {@link RNull} to {@code null}. + */ + private boolean isNull = true; + + public static RNullMRContextState newContextState() { + return new RNullMRContextState(); + } + + public boolean isNull() { + return isNull; + } + + public void setIsNull(boolean isNull) { + this.isNull = isNull; + } +} diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/AbstractMRTest.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/AbstractMRTest.java index f626efc810..23edd664d4 100644 --- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/AbstractMRTest.java +++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/AbstractMRTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2018, 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 @@ -46,7 +46,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import org.graalvm.polyglot.Context; import org.junit.Test; -import static com.oracle.truffle.r.test.generate.FastRSession.execInContext; import java.util.concurrent.Callable; public abstract class AbstractMRTest { diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/RFunctionMRTest.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/RFunctionMRTest.java index a4068db583..deb0bbd202 100644 --- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/RFunctionMRTest.java +++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/engine/interop/RFunctionMRTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2018, 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 @@ -42,29 +42,32 @@ public class RFunctionMRTest extends AbstractMRTest { @Test public void testExecute() throws UnsupportedTypeException, ArityException, UnsupportedMessageException { - RFunction f = create("function() {}"); - assertTrue(ForeignAccess.sendIsExecutable(Message.IS_EXECUTABLE.createNode(), f)); + execInContext(() -> { + RFunction f = create("function() {}"); + assertTrue(ForeignAccess.sendIsExecutable(Message.IS_EXECUTABLE.createNode(), f)); - TruffleObject result = (TruffleObject) ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f); - assertTrue(ForeignAccess.sendIsNull(Message.IS_NULL.createNode(), result)); + TruffleObject result = (TruffleObject) ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f); + assertTrue(ForeignAccess.sendIsNull(Message.IS_NULL.createNode(), result)); - f = create("function() {1L}"); - assertEquals(1, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); + f = create("function() {1L}"); + assertEquals(1, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); - f = create("function() {1}"); - assertEquals(1.0, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); + f = create("function() {1}"); + assertEquals(1.0, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); - f = create("function() {TRUE}"); - assertEquals(true, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); + f = create("function() {TRUE}"); + assertEquals(true, ForeignAccess.sendExecute(Message.createExecute(0).createNode(), f)); - f = create("function(a) {a}"); - assertEquals("abc", ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, "abc")); + f = create("function(a) {a}"); + assertEquals("abc", ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, "abc")); - f = create("function(a) { is.logical(a) }"); - assertEquals(true, ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, true)); + f = create("function(a) { is.logical(a) }"); + assertEquals(true, ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, true)); - f = create("function(a) { as.external.short(a) }"); - assertTrue(ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, 123) instanceof Short); + f = create("function(a) { as.external.short(a) }"); + assertTrue(ForeignAccess.sendExecute(Message.createExecute(1).createNode(), f, 123) instanceof Short); + return null; + }); } -- GitLab