Skip to content

Commit 707219f

Browse files
steve-ssteve-s
committed
[GR-20427] [GR-20743] [GR-15731] Backport fixes: native memory leaks, wrong LibHandle type, RootTag tagged node throws ReturnException.
PullRequest: fastr/2318
2 parents 55573d7 + 9d4304e commit 707219f

File tree

22 files changed

+919
-356
lines changed

22 files changed

+919
-356
lines changed

‎com.oracle.truffle.r.engine/src/com/oracle/truffle/r/engine/RRuntimeASTAccessImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2014, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -299,7 +299,7 @@ public boolean isTaggedWith(Node node, Class<?> tag) {
299299
// The only child of the RootBodyNode that is instrumentable should be the body.
300300
// Any other child of RootBodyNode should not be instrumentable, e.g. SaveArgumentsNode,
301301
// etc.
302-
return node.getParent() instanceof RootBodyNode;
302+
return RASTUtils.unwrapParent(node) instanceof RootBodyNode;
303303
}
304304
if (node instanceof RootBodyNode) {
305305
return (tag == RootTag.class);

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/llvm/TruffleLLVM_Call.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
package com.oracle.truffle.r.ffi.impl.llvm;
2424

2525
import com.oracle.truffle.api.CompilerDirectives;
26+
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
2627
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
2728
import com.oracle.truffle.api.dsl.Cached;
2829
import com.oracle.truffle.api.dsl.ImportStatic;
@@ -75,10 +76,10 @@ static final class ContextStateImpl implements RContext.ContextState {
7576
private TruffleObject setCallbacksAddress;
7677
private TruffleObject callbacks;
7778

78-
public Object doubleArrayType;
79-
public Object intArrayType;
80-
public Object longArrayType;
81-
public Object byteArrayType;
79+
@CompilationFinal public Object doubleArrayType;
80+
@CompilationFinal public Object intArrayType;
81+
@CompilationFinal public Object longArrayType;
82+
@CompilationFinal public Object byteArrayType;
8283

8384
@Override
8485
public ContextState initialize(RContext contextA) {

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/mixed/TruffleMixed_Context.java‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -52,11 +52,18 @@ public final class TruffleMixed_Context extends RFFIContext {
5252
private final TruffleNFI_Context nfiContext;
5353

5454
TruffleMixed_Context(RFFIContextState rffiContextState) {
55-
super(rffiContextState, new TruffleMixed_C(), new BaseRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE, TruffleNFI_DownCallNodeFactory.INSTANCE), new TruffleMixed_Call(), new TruffleMixed_DLL(),
55+
super(rffiContextState, new TruffleMixed_C(),
56+
createBaseDowncallNode(),
57+
new TruffleMixed_Call(),
58+
new TruffleMixed_DLL(),
5659
new TruffleLLVM_UserRng(),
57-
new ZipRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE), new PCRERFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
58-
new LapackRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE), new StatsRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
59-
new ToolsRFFI(), new REmbedRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE), new MiscRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE));
60+
new ZipRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
61+
new PCRERFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
62+
new LapackRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
63+
createStatsDowncallNode(),
64+
new ToolsRFFI(),
65+
new REmbedRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE),
66+
new MiscRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE));
6067
llvmContext = new TruffleLLVM_Context(rffiContextState) {
6168
@Override
6269
protected void addLibRToDLLContextState(RContext context, DLLInfo libR) {
@@ -75,6 +82,15 @@ protected void addLibRToDLLContextState(RContext context, DLLInfo libR) {
7582

7683
}
7784

85+
private static StatsRFFI createStatsDowncallNode() {
86+
return new StatsRFFI(RContext.getInstance().isLLVMPackage("stats") ? TruffleLLVM_DownCallNodeFactory.INSTANCE : TruffleNFI_DownCallNodeFactory.INSTANCE);
87+
}
88+
89+
private static BaseRFFI createBaseDowncallNode() {
90+
return RContext.getInstance().isLLVMPackage("base") ? new BaseRFFI(TruffleLLVM_DownCallNodeFactory.INSTANCE, TruffleNFI_DownCallNodeFactory.INSTANCE)
91+
: new BaseRFFI(TruffleNFI_DownCallNodeFactory.INSTANCE, TruffleNFI_DownCallNodeFactory.INSTANCE);
92+
}
93+
7894
@Override
7995
public Object getSulongArrayType(Object arrayElement) {
8096
return llvmContext.getSulongArrayType(arrayElement);

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/mixed/TruffleMixed_DLL.java‎

Lines changed: 4 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -24,8 +24,6 @@
2424

2525
import com.oracle.truffle.api.TruffleFile;
2626
import com.oracle.truffle.api.TruffleLanguage;
27-
import java.util.HashSet;
28-
import java.util.Set;
2927

3028
import com.oracle.truffle.api.dsl.Cached;
3129
import com.oracle.truffle.api.dsl.CachedContext;
@@ -38,7 +36,6 @@
3836
import com.oracle.truffle.r.ffi.impl.mixed.TruffleMixed_DLLFactory.TruffleMixed_DLSymNodeGen;
3937
import com.oracle.truffle.r.ffi.impl.nfi.TruffleNFI_DLL;
4038
import com.oracle.truffle.r.ffi.impl.nfi.TruffleNFI_DLL.NFIHandle;
41-
import com.oracle.truffle.r.runtime.context.FastROptions;
4239
import com.oracle.truffle.r.runtime.context.RContext;
4340
import com.oracle.truffle.r.runtime.context.TruffleRLanguage;
4441
import com.oracle.truffle.r.runtime.data.RDataFactory;
@@ -51,47 +48,7 @@ public class TruffleMixed_DLL implements DLLRFFI {
5148
private final TruffleLLVM_DLL llvmDllRFFI = new TruffleLLVM_DLL();
5249
private final TruffleNFI_DLL nfiDllRFFI = new TruffleNFI_DLL();
5350

54-
private final Set<String> explicitPackages;
55-
private final boolean isLLVMDefault;
56-
5751
TruffleMixed_DLL() {
58-
if ("llvm".equals(System.getenv().get("FASTR_RFFI"))) {
59-
isLLVMDefault = true;
60-
explicitPackages = java.util.Collections.emptySet();
61-
} else {
62-
String backendOpt = RContext.getInstance().getOption(FastROptions.BackEnd);
63-
String explicitPkgsOpt;
64-
if ("native".equals(backendOpt)) {
65-
isLLVMDefault = false;
66-
explicitPkgsOpt = RContext.getInstance().getOption(FastROptions.BackEndLLVM);
67-
} else {
68-
// llvm
69-
isLLVMDefault = true;
70-
explicitPkgsOpt = RContext.getInstance().getOption(FastROptions.BackEndNative);
71-
}
72-
73-
String[] explicitPkgsOptSplit = explicitPkgsOpt == null ? null : explicitPkgsOpt.split(",");
74-
if (explicitPkgsOptSplit == null || explicitPkgsOptSplit.length == 0) {
75-
explicitPackages = java.util.Collections.emptySet();
76-
} else {
77-
explicitPackages = new HashSet<>();
78-
for (String pkg : explicitPkgsOptSplit) {
79-
explicitPackages.add(pkg);
80-
}
81-
}
82-
}
83-
}
84-
85-
boolean isLLVMPackage(TruffleFile libPath) {
86-
if (explicitPackages.isEmpty()) {
87-
return isLLVMDefault;
88-
}
89-
90-
assert libPath != null;
91-
String libName = libPath.getName();
92-
libName = libName.substring(0, libName.lastIndexOf('.'));
93-
boolean isExplicitPkg = explicitPackages.contains(libName);
94-
return isExplicitPkg ^ isLLVMDefault;
9552
}
9653

9754
@Override
@@ -113,23 +70,22 @@ abstract static class TruffleMixed_DLOpenNode extends Node implements DLOpenNode
11370

11471
private final DLOpenNode llvmDllOpenNode;
11572
private final DLOpenNode nfiDllOpenNode;
116-
private final TruffleMixed_DLL dllRffi;
11773

11874
TruffleMixed_DLOpenNode(TruffleMixed_DLL dllRffi) {
119-
this.dllRffi = dllRffi;
12075
this.llvmDllOpenNode = dllRffi.llvmDllRFFI.createDLOpenNode();
12176
this.nfiDllOpenNode = dllRffi.nfiDllRFFI.createDLOpenNode();
12277
}
12378

12479
@Specialization
12580
public LibHandle exec(String path, boolean local, boolean now,
12681
@CachedContext(TruffleRLanguage.class) TruffleLanguage.ContextReference<RContext> ctxRef) throws UnsatisfiedLinkError {
127-
TruffleFile libPath = ctxRef.get().getSafeTruffleFile(path);
82+
RContext context = ctxRef.get();
83+
TruffleFile libPath = context.getSafeTruffleFile(path);
12884
if (!libPath.exists()) {
12985
throw new UnsatisfiedLinkError(String.format("Shared library %s not found", path));
13086
}
13187

132-
boolean useLLVM = dllRffi.isLLVMPackage(libPath);
88+
boolean useLLVM = context.isLLVMPackage(libPath);
13389

13490
LibHandle nfiLibHandle = nfiDllOpenNode.execute(path, local, now);
13591
if (useLLVM) {

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_Context.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import com.oracle.truffle.r.runtime.ffi.ToolsRFFI;
7171
import com.oracle.truffle.r.runtime.ffi.ZipRFFI;
7272

73+
import com.oracle.truffle.r.runtime.ffi.util.NativeMemory;
7374
import sun.misc.Unsafe;
7475

7576
class UnsafeAdapter {
@@ -239,7 +240,7 @@ private long initCallbacksAddress() {
239240
private void initCallbacks(RContext context) {
240241
if (context.getKind() == ContextKind.SHARE_NOTHING) {
241242
// create and fill a new callbacks table
242-
callbacks = UnsafeAdapter.UNSAFE.allocateMemory(Callbacks.values().length * Unsafe.ARRAY_LONG_INDEX_SCALE);
243+
callbacks = NativeMemory.allocate(Callbacks.values().length * Unsafe.ARRAY_LONG_INDEX_SCALE, "callbacks");
243244
InteropLibrary interop = InteropLibrary.getFactory().getUncached();
244245
Object addCallback;
245246
try {
@@ -383,7 +384,7 @@ private static synchronized void initializeLock() {
383384
public void beforeDispose(RContext context) {
384385
switch (context.getKind()) {
385386
case SHARE_NOTHING:
386-
UnsafeAdapter.UNSAFE.freeMemory(callbacks);
387+
NativeMemory.free(callbacks, "callbacks");
387388
break;
388389
case SHARE_ALL:
389390
case SHARE_PARENT_RO:
@@ -416,7 +417,7 @@ public void afterDowncall(Object beforeValue, RFFIFactory.Type rffiType) {
416417
super.afterDowncall(tokens[0], rffiType);
417418
popCallbacks((long) tokens[1]);
418419
for (Long ptr : transientAllocations.pop()) {
419-
UnsafeAdapter.UNSAFE.freeMemory(ptr);
420+
NativeMemory.free(ptr, "Rf_alloc");
420421
}
421422
RuntimeException lastUpCallEx = getLastUpCallException();
422423
setLastUpCallException(null);

‎com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nfi/TruffleNFI_UpCallsRFFIImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
import com.oracle.truffle.r.runtime.ffi.FFIWrap.FFIDownCallWrap;
4545
import com.oracle.truffle.r.runtime.ffi.NativeFunction;
4646
import com.oracle.truffle.r.runtime.ffi.RFFIFactory;
47-
import com.oracle.truffle.r.runtime.ffi.UnsafeAdapter;
47+
import com.oracle.truffle.r.runtime.ffi.util.NativeMemory;
4848

4949
public class TruffleNFI_UpCallsRFFIImpl extends JavaUpCallsRFFIImpl {
5050

@@ -75,7 +75,7 @@ public Object Rf_mkCharLenCE(Object bytes, int len, int encoding) {
7575

7676
@Override
7777
public Object R_alloc(int n, int size) {
78-
long result = UnsafeAdapter.UNSAFE.allocateMemory(n * size);
78+
long result = NativeMemory.allocate(n * size, "R_alloc");
7979
getContext().transientAllocations.peek().add(result);
8080
return result;
8181
}

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionBodyNode.java‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,18 +23,22 @@
2323
package com.oracle.truffle.r.nodes.function;
2424

2525
import com.oracle.truffle.api.CompilerDirectives;
26+
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
2627
import com.oracle.truffle.api.frame.FrameDescriptor;
2728
import com.oracle.truffle.api.frame.FrameSlot;
2829
import com.oracle.truffle.api.frame.FrameSlotKind;
2930
import com.oracle.truffle.api.frame.VirtualFrame;
3031
import com.oracle.truffle.api.nodes.Node;
3132
import com.oracle.truffle.api.profiles.BranchProfile;
33+
import com.oracle.truffle.api.profiles.ConditionProfile;
3234
import com.oracle.truffle.api.source.SourceSection;
3335
import com.oracle.truffle.r.runtime.RArguments;
3436
import com.oracle.truffle.r.runtime.RArguments.DispatchArgs;
3537
import com.oracle.truffle.r.runtime.RArguments.S3Args;
3638
import com.oracle.truffle.r.runtime.RArguments.S4Args;
39+
import com.oracle.truffle.r.runtime.RInternalError;
3740
import com.oracle.truffle.r.runtime.RRuntime;
41+
import com.oracle.truffle.r.runtime.ReturnException;
3842
import com.oracle.truffle.r.runtime.RootBodyNode;
3943
import com.oracle.truffle.r.runtime.env.frame.FrameSlotChangeMonitor;
4044
import com.oracle.truffle.r.runtime.nodes.RNode;
@@ -46,16 +50,34 @@ public final class FunctionBodyNode extends Node implements RootBodyNode {
4650
@Child private SetupS3ArgsNode setupS3Args;
4751
@Child private SetupS4ArgsNode setupS4Args;
4852

53+
// Profiling for catching ReturnException
54+
private final BranchProfile returnExceptionProfile = BranchProfile.create();
55+
@CompilationFinal private ConditionProfile returnTopLevelProfile;
56+
4957
public FunctionBodyNode(SaveArgumentsNode saveArguments, RNode body) {
5058
this.body = body;
5159
this.saveArguments = saveArguments;
5260
}
5361

5462
@Override
5563
public Object visibleExecute(VirtualFrame frame) {
56-
setupDispatchSlots(frame);
57-
saveArguments.execute(frame);
58-
return body.visibleExecute(frame);
64+
try {
65+
setupDispatchSlots(frame);
66+
saveArguments.execute(frame);
67+
return body.visibleExecute(frame);
68+
} catch (ReturnException ex) {
69+
returnExceptionProfile.enter();
70+
if (profileReturnToTopLevel(ex.getTarget() == RArguments.getCall(frame))) {
71+
Object result = ex.getResult();
72+
if (CompilerDirectives.inInterpreter() && result == null) {
73+
throw RInternalError.shouldNotReachHere("invalid null from ReturnException.getResult() of " + this);
74+
}
75+
return result;
76+
} else {
77+
// re-thrown until it reaches its target
78+
throw ex;
79+
}
80+
}
5981
}
6082

6183
@Override
@@ -68,6 +90,14 @@ public SourceSection getSourceSection() {
6890
return body.getSourceSection();
6991
}
7092

93+
public boolean profileReturnToTopLevel(boolean condition) {
94+
if (returnTopLevelProfile == null) {
95+
CompilerDirectives.transferToInterpreterAndInvalidate();
96+
returnTopLevelProfile = ConditionProfile.createBinaryProfile();
97+
}
98+
return returnTopLevelProfile.profile(condition);
99+
}
100+
71101
private void setupDispatchSlots(VirtualFrame frame) {
72102
DispatchArgs dispatchArgs = RArguments.getDispatchArgs(frame);
73103
if (dispatchArgs == null) {

‎com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/FunctionDefinitionNode.java‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -132,10 +132,8 @@ public final class FunctionDefinitionNode extends RRootNode implements RSyntaxNo
132132
@CompilationFinal private FrameSlot handlerStackSlot;
133133
@CompilationFinal private FrameSlot restartStackSlot;
134134

135-
/**
136-
* Profiling for catching {@link ReturnException}s.
137-
*/
138-
private final ConditionProfile returnTopLevelProfile = ConditionProfile.createBinaryProfile();
135+
// Profiling for catching ReturnException thrown from an exit handler
136+
@CompilationFinal private ConditionProfile returnTopLevelProfile;
139137

140138
public static FunctionDefinitionNode create(TruffleRLanguage language, SourceSection src, FrameDescriptor frameDesc, SourceSection[] argSourceSections, SaveArgumentsNode saveArguments,
141139
RSyntaxNode body,
@@ -288,15 +286,9 @@ public Object execute(VirtualFrame frame) {
288286
}
289287
return result;
290288
} catch (ReturnException ex) {
291-
if (returnTopLevelProfile.profile(ex.getTarget() == RArguments.getCall(frame))) {
292-
Object result = ex.getResult();
293-
if (CompilerDirectives.inInterpreter() && result == null) {
294-
throw RInternalError.shouldNotReachHere("invalid null from ReturnException.getResult() of " + this);
295-
}
296-
return result;
297-
} else {
298-
throw ex;
299-
}
289+
// here we just re-throw, the check whether this function is the target of the return is
290+
// done in the function body node
291+
throw ex;
300292
} catch (BreakException e) {
301293
breakProfile.enter();
302294
throw e;
@@ -391,7 +383,7 @@ public Object execute(VirtualFrame frame) {
391383
}
392384
}
393385
} catch (ReturnException ex) {
394-
if (returnTopLevelProfile.profile(ex.getTarget() == RArguments.getCall(frame))) {
386+
if (profileReturnToTopLevel(ex.getTarget() == RArguments.getCall(frame))) {
395387
return ex.getResult();
396388
} else {
397389
throw ex;
@@ -405,6 +397,14 @@ public Object execute(VirtualFrame frame) {
405397
}
406398
}
407399

400+
public boolean profileReturnToTopLevel(boolean condition) {
401+
if (returnTopLevelProfile == null) {
402+
CompilerDirectives.transferToInterpreterAndInvalidate();
403+
returnTopLevelProfile = ConditionProfile.createBinaryProfile();
404+
}
405+
return returnTopLevelProfile.profile(condition);
406+
}
407+
408408
private static RPairList getCurrentOnExitList(VirtualFrame frame, FrameSlot slot) {
409409
try {
410410
return (RPairList) FrameSlotChangeMonitor.getObject(slot, frame);

0 commit comments

Comments
 (0)