Skip to content

Commit ee620d0

Browse files
steve-sDSouzaM
authored andcommitted
Make PException#location follow Truffle contract
1 parent 37089af commit ee620d0

File tree

3 files changed

+39
-13
lines changed

3 files changed

+39
-13
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/PBaseException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public LazyTraceback internalReifyException(PFrame.Reference curFrameInfo) {
322322
**/
323323
public PException getExceptionForReraise(LazyTraceback reraiseTraceback) {
324324
setTraceback(reraiseTraceback);
325-
return PException.fromObject(this, exception.getLocation(), false);
325+
return PException.fromObject(this, null, exception.getLocation(), false);
326326
}
327327

328328
@ExportMessage

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/traceback/MaterializeLazyTracebackNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -71,7 +71,7 @@
7171
* frames that the exception has passed through during the unwinding plus the frame where it was
7272
* caught. It doesn't include the frames above it (to the top). Secondly, the traceback is never
7373
* frozen. The traceback accumulates more frames when the exception gets reraised. To correct the
74-
* mismatch between Truffle and Python eception handling, we need to wrap {@link PException}s in
74+
* mismatch between Truffle and Python exception handling, we need to wrap {@link PException}s in
7575
* {@link LazyTraceback} objects when caught and adhere to particular rules of exception handling
7676
* mentioned below.
7777
* </p>

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/exception/PException.java

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
* object must be created for each throw.
8282
*/
8383
@ExportLibrary(value = InteropLibrary.class, delegateTo = "pythonException")
84+
@SuppressWarnings({"serial"})
8485
public final class PException extends AbstractTruffleException {
8586
private static final long serialVersionUID = -6437116280384996361L;
8687

@@ -101,39 +102,64 @@ public final class PException extends AbstractTruffleException {
101102
private boolean skipFirstTracebackFrame;
102103
private int tracebackFrameCount;
103104

105+
/**
106+
* We create new {@link PException} instances wrapping the same {@link PBaseException} as the
107+
* stack is unwound when the exception is reraised. See {@link MaterializeLazyTracebackNode} for
108+
* more details.
109+
* <p>
110+
* Since the new {@link PException} for reraise will be thrown from different frame than the
111+
* initial {@link PException}, Truffle will think that it belongs to that frame and not to the
112+
* original frame. Truffle expects the {@link AbstractTruffleException#getLocation()} to belong
113+
* the (AST of the root node of) frame where the exception was thrown. If we just copy the
114+
* location from the initial {@link PException} to the {@link PException} constructed for
115+
* reraise, the location would not satisfy that property, but at the same time we like to keep
116+
* the original location to use it in {@link InteropLibrary#getSourceLocation} message.
117+
*/
118+
private final Node originalLocation;
119+
104120
private PException(Object pythonException, Node node) {
105121
super(node);
106122
this.pythonException = pythonException;
123+
this.originalLocation = node;
107124
}
108125

109-
private PException(Object pythonException, Node node, Throwable wrapped) {
110-
super(null, wrapped, UNLIMITED_STACK_TRACE, node);
126+
private PException(Object pythonException, Node location, Node originalLocation, Throwable wrapped) {
127+
super(null, wrapped, UNLIMITED_STACK_TRACE, location);
111128
this.pythonException = pythonException;
129+
this.originalLocation = originalLocation;
112130
assert PyExceptionInstanceCheckNode.executeUncached(pythonException);
113131
}
114132

115-
public static PException fromObject(Object pythonException, Node node, boolean withJavaStacktrace) {
133+
public static PException fromObject(Object pythonException, Node location, boolean withJavaStacktrace) {
134+
return fromObject(pythonException, location, location, withJavaStacktrace);
135+
}
136+
137+
public static PException fromObject(Object pythonException, Node location, Node originalLocation, boolean withJavaStacktrace) {
116138
Throwable wrapped = null;
117139
if (withJavaStacktrace) {
118140
// Create a carrier for the java stacktrace as PException cannot have one
119141
wrapped = createStacktraceCarrier();
120142
}
121-
return fromObject(pythonException, node, wrapped);
143+
return fromObject(pythonException, location, originalLocation, wrapped);
122144
}
123145

124146
@TruffleBoundary
125147
private static RuntimeException createStacktraceCarrier() {
126148
return new RuntimeException();
127149
}
128150

151+
public static PException fromObject(Object pythonException, Node location, Throwable wrapped) {
152+
return fromObject(pythonException, location, location, wrapped);
153+
}
154+
129155
/*
130156
* Note: we use this method to convert a Java StackOverflowError into a Python RecursionError.
131157
* At the time when this is done, some Java stack frames were already unwinded but there is no
132158
* guarantee on how many. Therefore, it is important that this method is simple. In particular,
133159
* do not add calls if that can be avoided.
134160
*/
135-
public static PException fromObject(Object pythonException, Node node, Throwable wrapped) {
136-
PException pException = new PException(pythonException, node, wrapped);
161+
public static PException fromObject(Object pythonException, Node node, Node originalLocation, Throwable wrapped) {
162+
PException pException = new PException(pythonException, node, originalLocation, wrapped);
137163
if (pythonException instanceof PBaseException managedException) {
138164
managedException.setException(pException);
139165
}
@@ -146,7 +172,7 @@ public static PException fromExceptionInfo(Object pythonException, boolean withJ
146172
// Create a carrier for the java stacktrace as PException cannot have one
147173
wrapped = createStacktraceCarrier();
148174
}
149-
PException pException = new PException(pythonException, null, wrapped);
175+
PException pException = new PException(pythonException, null, null, wrapped);
150176
pException.reified = true;
151177
if (pythonException instanceof PBaseException managedException) {
152178
managedException.setException(pException);
@@ -394,7 +420,7 @@ public void notifyAddedTracebackFrame(boolean visible) {
394420
*/
395421
public PException getExceptionForReraise(boolean rootNodeVisible) {
396422
ensureReified();
397-
PException pe = PException.fromObject(pythonException, getLocation(), false);
423+
PException pe = PException.fromObject(pythonException, null, getLocation(), false);
398424
if (rootNodeVisible) {
399425
pe.skipFirstTracebackFrame();
400426
}
@@ -444,15 +470,15 @@ RuntimeException throwException(@Exclusive @Cached GilNode gil) {
444470
@ExportMessage
445471
@SuppressWarnings("static-method")
446472
boolean hasSourceLocation() {
447-
return getLocation() != null && getLocation().getEncapsulatingSourceSection() != null;
473+
return originalLocation != null && originalLocation.getEncapsulatingSourceSection() != null;
448474
}
449475

450476
@ExportMessage(name = "getSourceLocation")
451477
SourceSection getExceptionSourceLocation(
452478
@Bind("$node") Node inliningTarget,
453479
@Cached InlinedBranchProfile unsupportedProfile) throws UnsupportedMessageException {
454480
if (hasSourceLocation()) {
455-
return getLocation().getEncapsulatingSourceSection();
481+
return originalLocation.getEncapsulatingSourceSection();
456482
}
457483
unsupportedProfile.enter(inliningTarget);
458484
throw UnsupportedMessageException.create();

0 commit comments

Comments
 (0)