Skip to content

Commit a98da58

Browse files
msimacekmsimacek
committed
Implement exception handling in instrumentation calls
1 parent 160b3bf commit a98da58

File tree

3 files changed

+74
-17
lines changed

3 files changed

+74
-17
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/debug/PythonDebugTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,6 @@ public void testConditionalBreakpointGlobal() throws Throwable {
329329

330330
@Test
331331
public void testReenterArgumentsAndValues() throws Throwable {
332-
// GR-40742
333-
PythonTests.skipOnBytecodeInterpreter();
334332
// Test that after a re-enter, arguments are kept and variables are cleared.
335333
final Source source = Source.newBuilder("python", "" +
336334
"def main():\n" +

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,9 +1169,12 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
11691169
final PythonLanguage language = PythonLanguage.get(this);
11701170
final Assumption noTraceOrProfile = language.noTracingOrProfilingAssumption;
11711171
final InstrumentationSupport instrumentation = instrumentationRoot.getInstrumentation();
1172-
if (instrumentation != null && instrumentationRoot instanceof WrapperNode) {
1173-
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
1174-
wrapper.getProbeNode().onEnter(virtualFrame);
1172+
boolean returnCalled = false;
1173+
if (instrumentation != null) {
1174+
Object result = enterRoot(virtualFrame);
1175+
if (result != null) {
1176+
return result;
1177+
}
11751178
}
11761179

11771180
/*
@@ -1214,19 +1217,20 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
12141217
traceLine(virtualFrame, mutableData, localBC, bci);
12151218
}
12161219
profilingEnabled = isProfilingEnabled(noTraceOrProfile, mutableData);
1217-
if (instrumentation != null) {
1218-
int line = bciToLine(bci);
1219-
int pastLine = mutableData.getPastLine();
1220-
instrumentation.notifyStatement(virtualFrame, pastLine, line);
1221-
mutableData.setPastLine(line);
1222-
mutableData.setPastBci(bci);
1223-
}
12241220

12251221
CompilerAsserts.partialEvaluationConstant(bc);
12261222
CompilerAsserts.partialEvaluationConstant(bci);
12271223
CompilerAsserts.partialEvaluationConstant(stackTop);
12281224

12291225
try {
1226+
if (instrumentation != null) {
1227+
int line = bciToLine(bci);
1228+
int pastLine = mutableData.getPastLine();
1229+
instrumentation.notifyStatement(virtualFrame, pastLine, line);
1230+
mutableData.setPastLine(line);
1231+
mutableData.setPastBci(bci);
1232+
}
1233+
12301234
switch (bc) {
12311235
case OpCodesConstants.LOAD_NONE:
12321236
virtualFrame.setObject(++stackTop, PNone.NONE);
@@ -1676,6 +1680,7 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
16761680
traceOrProfileReturn(virtualFrame, mutableData, value, tracingEnabled, profilingEnabled);
16771681

16781682
if (instrumentation != null && instrumentationRoot instanceof WrapperNode) {
1683+
returnCalled = true;
16791684
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
16801685
wrapper.getProbeNode().onReturnValue(virtualFrame, value);
16811686
}
@@ -2095,6 +2100,7 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
20952100
}
20962101
traceOrProfileYield(virtualFrame, mutableData, value, tracingEnabled, profilingEnabled);
20972102
if (instrumentation != null && instrumentationRoot instanceof WrapperNode) {
2103+
returnCalled = true;
20982104
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
20992105
wrapper.getProbeNode().onReturnValue(virtualFrame, value);
21002106
}
@@ -2188,8 +2194,17 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
21882194
if (instrumentation != null) {
21892195
instrumentation.notifyException(virtualFrame, bciToLine(beginBci), e);
21902196
if (instrumentationRoot instanceof WrapperNode) {
2197+
returnCalled = true;
21912198
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
2192-
wrapper.getProbeNode().onReturnExceptionalOrUnwind(virtualFrame, e, false);
2199+
Object result = wrapper.getProbeNode().onReturnExceptionalOrUnwind(virtualFrame, e, false);
2200+
if (result == ProbeNode.UNWIND_ACTION_REENTER) {
2201+
throw CompilerDirectives.shouldNotReachHere("Reenter shouldn't occur at this point");
2202+
} else if (result != null) {
2203+
if (isGeneratorOrCoroutine) {
2204+
throw CompilerDirectives.shouldNotReachHere("Cannot replace return value of generators");
2205+
}
2206+
return result;
2207+
}
21932208
}
21942209
}
21952210

@@ -2215,7 +2230,7 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
22152230
// Need to handle instrumentation frame unwind
22162231
if (instrumentationRoot instanceof WrapperNode) {
22172232
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
2218-
Object ret = wrapper.getProbeNode().onReturnExceptionalOrUnwind(virtualFrame, e, false);
2233+
Object ret = wrapper.getProbeNode().onReturnExceptionalOrUnwind(virtualFrame, e, returnCalled);
22192234
if (ret == ProbeNode.UNWIND_ACTION_REENTER) {
22202235
if (isGeneratorOrCoroutine) {
22212236
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -2226,13 +2241,37 @@ private Object bytecodeLoop(VirtualFrame virtualFrame, Frame localFrame, Bytecod
22262241
stackTop = getInitialStackTop();
22272242
oparg = 0;
22282243
continue;
2244+
} else if (ret != null) {
2245+
return ret;
22292246
}
22302247
}
22312248
throw e;
22322249
}
22332250
}
22342251
}
22352252

2253+
@InliningCutoff
2254+
private Object enterRoot(VirtualFrame virtualFrame) {
2255+
if (instrumentationRoot instanceof WrapperNode) {
2256+
WrapperNode wrapper = (WrapperNode) instrumentationRoot;
2257+
try {
2258+
wrapper.getProbeNode().onEnter(virtualFrame);
2259+
} catch (Throwable t) {
2260+
Object result = wrapper.getProbeNode().onReturnExceptionalOrUnwind(virtualFrame, t, false);
2261+
if (result == ProbeNode.UNWIND_ACTION_REENTER) {
2262+
// We're at the beginning, reenter means just continue
2263+
return null;
2264+
} else if (result != null) {
2265+
if (co.isGeneratorOrCoroutine()) {
2266+
throw CompilerDirectives.shouldNotReachHere("Cannot replace return value of generators");
2267+
}
2268+
return result;
2269+
}
2270+
}
2271+
}
2272+
return null;
2273+
}
2274+
22362275
private MakeFunctionNode insertMakeFunctionNode(Node[] localNodes, int beginBci, CodeUnit codeUnit) {
22372276
return insertChildNode(localNodes, beginBci, MakeFunctionNodeGen.class, () -> MakeFunctionNode.create(getLanguage(PythonLanguage.class), codeUnit, source));
22382277
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/instrumentation/InstrumentationSupport.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@
4444
import com.oracle.graal.python.compiler.OpCodes;
4545
import com.oracle.graal.python.nodes.BuiltinNames;
4646
import com.oracle.graal.python.nodes.bytecode.PBytecodeRootNode;
47+
import com.oracle.truffle.api.CompilerDirectives;
4748
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
4849
import com.oracle.truffle.api.frame.VirtualFrame;
4950
import com.oracle.truffle.api.instrumentation.InstrumentableNode;
51+
import com.oracle.truffle.api.instrumentation.ProbeNode;
5052
import com.oracle.truffle.api.nodes.Node;
5153

5254
public final class InstrumentationSupport extends Node {
@@ -95,21 +97,39 @@ private InstrumentableNode.WrapperNode getWrapperAtLine(int line) {
9597
}
9698

9799
public void notifyStatement(VirtualFrame frame, int prevLine, int line) {
98-
// TODO exception handling?
99100
if (line != prevLine) {
100101
if (prevLine >= 0) {
101102
InstrumentableNode.WrapperNode wrapper = getWrapperAtLine(prevLine);
102103
if (wrapper != null) {
103-
wrapper.getProbeNode().onReturnValue(frame, null);
104+
try {
105+
wrapper.getProbeNode().onReturnValue(frame, null);
106+
} catch (Throwable t) {
107+
handleException(frame, wrapper, t, true);
108+
throw t;
109+
}
104110
}
105111
}
106112
InstrumentableNode.WrapperNode wrapper = getWrapperAtLine(line);
107113
if (wrapper != null) {
108-
wrapper.getProbeNode().onEnter(frame);
114+
try {
115+
wrapper.getProbeNode().onEnter(frame);
116+
} catch (Throwable t) {
117+
handleException(frame, wrapper, t, false);
118+
throw t;
119+
}
109120
}
110121
}
111122
}
112123

124+
private static void handleException(VirtualFrame frame, InstrumentableNode.WrapperNode wrapper, Throwable t, boolean isReturnCalled) {
125+
Object result = wrapper.getProbeNode().onReturnExceptionalOrUnwind(frame, t, isReturnCalled);
126+
if (result == ProbeNode.UNWIND_ACTION_REENTER) {
127+
throw CompilerDirectives.shouldNotReachHere("Unwind not supported on statement level");
128+
} else if (result != null) {
129+
throw CompilerDirectives.shouldNotReachHere("Cannot replace a return of a statement");
130+
}
131+
}
132+
113133
public void notifyException(VirtualFrame frame, int line, Throwable exception) {
114134
InstrumentableNode.WrapperNode wrapper = getWrapperAtLine(line);
115135
if (wrapper != null) {

0 commit comments

Comments
 (0)