Skip to content

Commit bbe81a9

Browse files
msimacekmsimacek
committed
Handle passing stale lineno when PFrame wasn't requested but is present
1 parent 5e5b604 commit bbe81a9

File tree

9 files changed

+60
-47
lines changed

9 files changed

+60
-47
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextFrameBuiltins.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,18 @@ static Object get(PFrame frame,
7373
@CApiBuiltin(ret = Int, args = {PyFrameObject}, call = Direct)
7474
abstract static class PyFrame_GetLineNumber extends CApiUnaryBuiltinNode {
7575
@Specialization
76-
static int get(PFrame frame) {
77-
// do not sync location here since we have no VirtualFrame
78-
return frame.getLine();
76+
static Object get(PFrame frame,
77+
@Cached FrameBuiltins.LinenoNode linenoNode) {
78+
return linenoNode.execute(null, frame, PNone.NO_VALUE);
7979
}
8080
}
8181

8282
@CApiBuiltin(ret = Int, args = {PyFrameObject}, call = Direct)
8383
abstract static class PyFrame_GetLasti extends CApiUnaryBuiltinNode {
8484
@Specialization
85-
static int get(PFrame frame) {
86-
return frame.getLasti();
85+
static Object get(PFrame frame,
86+
@Cached FrameBuiltins.GetLastiNode getLastiNode) {
87+
return getLastiNode.execute(null, frame);
8788
}
8889
}
8990

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextSlotBuiltins.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
9999
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess;
100100
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageLen;
101+
import com.oracle.graal.python.builtins.objects.frame.FrameBuiltins;
101102
import com.oracle.graal.python.builtins.objects.frame.PFrame;
102103
import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
103104
import com.oracle.graal.python.builtins.objects.getsetdescriptor.GetSetDescriptor;
@@ -398,8 +399,9 @@ static Object get(GetSetDescriptor object) {
398399
@CApiBuiltin(ret = Int, args = {PyFrameObject}, call = Ignored)
399400
abstract static class GraalPyPrivate_Get_PyFrameObject_f_lineno extends CApiUnaryBuiltinNode {
400401
@Specialization
401-
static int get(PFrame frame) {
402-
return frame.getLine();
402+
static Object get(PFrame frame,
403+
@Cached FrameBuiltins.LinenoNode linenoNode) {
404+
return linenoNode.execute(null, frame, PNone.NO_VALUE);
403405
}
404406
}
405407

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextTracebackBuiltins.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
5858
import com.oracle.graal.python.builtins.objects.frame.PFrame;
5959
import com.oracle.graal.python.builtins.objects.traceback.PTraceback;
60+
import com.oracle.graal.python.nodes.frame.ReadFrameNode;
6061
import com.oracle.graal.python.runtime.PythonContext;
6162
import com.oracle.graal.python.runtime.object.PFactory;
6263
import com.oracle.truffle.api.dsl.Bind;
@@ -86,13 +87,15 @@ abstract static class PyTraceBack_Here extends CApiUnaryBuiltinNode {
8687
static int tbHere(PFrame frame,
8788
@Bind Node inliningTarget,
8889
@Bind PythonContext context,
90+
@Cached ReadFrameNode readFrameNode,
8991
@Cached CExtCommonNodes.ReadAndClearNativeException readAndClearNativeException,
9092
@Cached CExtCommonNodes.TransformExceptionToNativeNode transformExceptionToNativeNode) {
9193
PythonLanguage language = context.getLanguage(inliningTarget);
9294
PythonContext.PythonThreadState threadState = context.getThreadState(language);
9395
Object currentException = readAndClearNativeException.execute(inliningTarget, threadState);
9496
if (currentException instanceof PBaseException) {
9597
Object traceback = ExceptionNodes.GetTracebackNode.executeUncached(currentException);
98+
frame = readFrameNode.ensureFresh(null, frame);
9699
PTraceback newTraceback = PFactory.createTraceback(language, frame, frame.getLine(), traceback instanceof PTraceback ptb ? ptb : null);
97100
ExceptionNodes.SetTracebackNode.executeUncached(currentException, newTraceback);
98101
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/FrameBuiltins.java‎

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import com.oracle.graal.python.builtins.PythonBuiltins;
3939
import com.oracle.graal.python.builtins.objects.PNone;
4040
import com.oracle.graal.python.builtins.objects.code.PCode;
41-
import com.oracle.graal.python.builtins.objects.function.PArguments;
4241
import com.oracle.graal.python.builtins.objects.getsetdescriptor.DescriptorDeleteMarker;
4342
import com.oracle.graal.python.builtins.objects.module.PythonModule;
4443
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins;
@@ -51,7 +50,6 @@
5150
import com.oracle.graal.python.nodes.ErrorMessages;
5251
import com.oracle.graal.python.nodes.PRaiseNode;
5352
import com.oracle.graal.python.nodes.frame.GetFrameLocalsNode;
54-
import com.oracle.graal.python.nodes.frame.MaterializeFrameNode;
5553
import com.oracle.graal.python.nodes.frame.ReadFrameNode;
5654
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5755
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
@@ -72,7 +70,6 @@
7270
import com.oracle.truffle.api.dsl.Specialization;
7371
import com.oracle.truffle.api.frame.VirtualFrame;
7472
import com.oracle.truffle.api.nodes.Node;
75-
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
7673
import com.oracle.truffle.api.strings.TruffleString;
7774

7875
@CoreFunctions(extendClasses = PythonBuiltinClassType.PFrame)
@@ -91,12 +88,10 @@ abstract static class ReprNode extends PythonUnaryBuiltinNode {
9188
@Specialization
9289
TruffleString repr(VirtualFrame frame, PFrame self,
9390
@Cached GetCodeNode getCodeNode,
94-
@Bind Node inliningTarget,
95-
@Cached InlinedConditionProfile profile,
96-
@Cached MaterializeFrameNode materializeFrameNode,
91+
@Cached ReadFrameNode readFrameNode,
9792
@Cached SimpleTruffleStringFormatNode simpleTruffleStringFormatNode) {
93+
self = readFrameNode.ensureFresh(frame, self);
9894
PCode code = getCodeNode.executeObject(frame, self);
99-
LinenoNode.syncLocationIfNeeded(frame, self, inliningTarget, profile, materializeFrameNode);
10095
int lineno = self.getLine();
10196
return simpleTruffleStringFormatNode.format("<frame at 0x%s, file '%s', line %d, code %s>",
10297
objectHashCodeAsHexString(self), code.getFilename(), lineno, code.getName());
@@ -154,43 +149,26 @@ public static GetBuiltinsNode create() {
154149
public abstract static class LinenoNode extends PythonBinaryBuiltinNode {
155150

156151
@Specialization
157-
Object delete(VirtualFrame frame, PFrame self, DescriptorDeleteMarker ignored,
152+
Object delete(PFrame self, DescriptorDeleteMarker ignored,
158153
@Bind Node inliningTarget,
159154
@Cached @Cached.Exclusive PRaiseNode raise) {
160-
raise.raise(inliningTarget, PythonBuiltinClassType.AttributeError, ErrorMessages.CANNOT_DELETE);
161-
return PNone.NONE;
155+
throw raise.raise(inliningTarget, PythonBuiltinClassType.AttributeError, ErrorMessages.CANNOT_DELETE);
162156
}
163157

164158
@Specialization(guards = "isNoValue(newLineno)")
165159
int get(VirtualFrame frame, PFrame self, Object newLineno,
166-
@Bind Node inliningTarget,
167-
@Cached @Cached.Exclusive InlinedConditionProfile profile,
168-
@Cached @Cached.Exclusive MaterializeFrameNode frameNode) {
169-
syncLocationIfNeeded(frame, self, inliningTarget, profile, frameNode);
170-
return self.getLine();
171-
}
172-
173-
static void syncLocationIfNeeded(VirtualFrame frame, PFrame self, Node inliningTarget, InlinedConditionProfile isCurrentFrameProfile, MaterializeFrameNode materializeNode) {
174-
// Special case because this builtin can be called without going through an invoke node:
175-
// we need to sync the location of the frame if and only if 'self' represents the
176-
// current frame. If 'self' represents another frame on the stack, the location is
177-
// already set
178-
if (isCurrentFrameProfile.profile(inliningTarget, frame != null && PArguments.getCurrentFrameInfo(frame) == self.getRef())) {
179-
PFrame pyFrame = materializeNode.executeOnStack(false, false, frame);
180-
assert pyFrame == self;
181-
}
160+
@Cached @Cached.Exclusive ReadFrameNode readFrameNode) {
161+
return readFrameNode.ensureFresh(frame, self).getLine();
182162
}
183163

184-
@SuppressWarnings("truffle-static-method") // this is used for location here
185164
@Specialization(guards = {"!isNoValue(newLineno)", "!isDeleteMarker(newLineno)"})
186-
PNone set(VirtualFrame frame, PFrame self, Object newLineno,
165+
static PNone set(VirtualFrame frame, PFrame self, Object newLineno,
187166
@Bind Node inliningTarget,
188-
@Cached @Cached.Exclusive InlinedConditionProfile isCurrentFrameProfile,
189-
@Cached @Cached.Exclusive MaterializeFrameNode materializeNode,
167+
@Cached @Cached.Exclusive ReadFrameNode readFrameNode,
190168
@Cached @Cached.Exclusive PRaiseNode raise,
191169
@Cached PyLongCheckExactNode isLong,
192170
@Cached PyLongAsLongAndOverflowNode toLong) {
193-
syncLocationIfNeeded(frame, self, inliningTarget, isCurrentFrameProfile, materializeNode);
171+
readFrameNode.ensureFresh(frame, self);
194172
if (self.isTraceArgument()) {
195173
if (isLong.execute(inliningTarget, newLineno)) {
196174
try {
@@ -207,7 +185,7 @@ PNone set(VirtualFrame frame, PFrame self, Object newLineno,
207185
throw raise.raise(inliningTarget, PythonBuiltinClassType.ValueError, ErrorMessages.LINENO_MUST_BE_AN_INTEGER);
208186
}
209187
} else {
210-
PythonContext context = getContext();
188+
PythonContext context = PythonContext.get(inliningTarget);
211189
throw raise.raise(inliningTarget, PythonBuiltinClassType.ValueError, ErrorMessages.CANT_JUMP_FROM_S_EVENT,
212190
context.getThreadState(context.getLanguage(inliningTarget)).getTracingWhat().pythonName);
213191
}
@@ -219,8 +197,9 @@ PNone set(VirtualFrame frame, PFrame self, Object newLineno,
219197
@GenerateNodeFactory
220198
public abstract static class GetLastiNode extends PythonUnaryBuiltinNode {
221199
@Specialization
222-
int get(PFrame self) {
223-
return self.getLasti();
200+
int get(VirtualFrame frame, PFrame self,
201+
@Cached ReadFrameNode readFrameNode) {
202+
return readFrameNode.ensureFresh(frame, self).getLasti();
224203
}
225204
}
226205

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/PFrame.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ public final class PFrame extends PythonBuiltinObject {
9898

9999
private PFrame.Reference backref = null;
100100

101+
private boolean stale;
102+
101103
public Object getLocalTraceFun() {
102104
return localTraceFun;
103105
}
@@ -215,13 +217,22 @@ public PFrame(PythonLanguage lang, @SuppressWarnings("unused") Object threadStat
215217
* In most cases, you should use {@link GetFrameLocalsNode}.
216218
*/
217219
public MaterializedFrame getLocals() {
220+
assert !stale;
218221
return locals;
219222
}
220223

221224
public void setLocals(MaterializedFrame locals) {
222225
this.locals = locals;
223226
}
224227

228+
public boolean isStale() {
229+
return stale;
230+
}
231+
232+
public void setStale(boolean stale) {
233+
this.stale = stale;
234+
}
235+
225236
/**
226237
* Use {@link GetFrameLocalsNode} instead of accessing this directly.
227238
*/
@@ -275,6 +286,7 @@ public boolean didJump() {
275286

276287
@TruffleBoundary
277288
public int getLine() {
289+
assert !stale;
278290
if (line == UNINITIALIZED_LINE) {
279291
if (location == null) {
280292
line = -1;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/GetFrameLocalsNode.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ static Object doLoop(VirtualFrame frame, Node inliningTarget, PFrame pyFrame,
9999
@Cached InlinedBranchProfile create,
100100
@Cached(inline = false) CopyLocalsToDict copyLocalsToDict,
101101
@Cached ReadFrameNode readFrameNode) {
102-
if (pyFrame.getLocals() == null) {
103-
pyFrame = readFrameNode.getFrameForReference(frame, pyFrame.getRef(), ReadFrameNode.AllFramesSelector.INSTANCE, 0, true);
102+
if (pyFrame.isStale() || pyFrame.getLocals() == null) {
103+
pyFrame = readFrameNode.refreshFrame(frame, pyFrame.getRef(), true);
104104
}
105105
MaterializedFrame locals = pyFrame.getLocals();
106106
// It doesn't have custom locals, so it has to be a builtin dict or null

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ static PFrame freshPFrameForGenerator(Node location, @SuppressWarnings("unused")
190190
static PFrame alreadyEscapedFrame(@SuppressWarnings("unused") Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
191191
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
192192
PFrame pyFrame = getPFrame(frameToMaterialize);
193+
pyFrame.setStale(false);
193194
if (forceSync) {
194195
syncValuesNode.execute(pyFrame, frameToMaterialize, location);
195196
} else {
@@ -243,6 +244,7 @@ private static PFrame doEscapeFrame(Frame frameToMaterialize, PFrame escapedFram
243244

244245
// on a freshly created PFrame, we do always sync the arguments
245246
PArguments.synchronizeArgs(frameToMaterialize, escapedFrame);
247+
escapedFrame.setStale(false);
246248
if (forceSync) {
247249
syncValuesNode.execute(escapedFrame, frameToMaterialize, location);
248250
} else {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadFrameNode.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,21 @@ public final PFrame getCurrentPythonFrame(VirtualFrame frame, boolean needsLocal
139139
return getFrameForReference(frame, frame != null ? PArguments.getCurrentFrameInfo(frame) : null, AllPythonFramesSelector.INSTANCE, 0, needsLocals);
140140
}
141141

142+
public final PFrame refreshFrame(VirtualFrame frame, PFrame.Reference reference, boolean needsLocals) {
143+
return getFrameForReference(frame, reference, 0, needsLocals);
144+
}
145+
146+
public final PFrame ensureFresh(VirtualFrame frame, PFrame pFrame) {
147+
return ensureFresh(frame, pFrame, false);
148+
}
149+
150+
public final PFrame ensureFresh(VirtualFrame frame, PFrame pFrame, boolean needsLocals) {
151+
if (pFrame.isStale() || (needsLocals && pFrame.getLocals() == null) || (frame != null && PArguments.getCurrentFrameInfo(frame) == pFrame.getRef())) {
152+
return refreshFrame(frame, pFrame.getRef(), needsLocals);
153+
}
154+
return pFrame;
155+
}
156+
142157
public final PFrame getFrameForReference(Frame frame, PFrame.Reference startFrameInfo, int level, boolean needsLocals) {
143158
return getFrameForReference(frame, startFrameInfo, AllPythonFramesSelector.INSTANCE, level, needsLocals);
144159
}
@@ -174,7 +189,7 @@ PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.F
174189
if (frame != null && PArguments.getCurrentFrameInfo(frame) == curFrameInfo) {
175190
return materializeFrameNode.execute(this, false, needsLocals, frame);
176191
}
177-
if (curFrameInfo.getPyFrame() != null && (!needsLocals || curFrameInfo.getPyFrame().getLocals() != null)) {
192+
if (curFrameInfo.getPyFrame() != null && !curFrameInfo.getPyFrame().isStale() && (!needsLocals || curFrameInfo.getPyFrame().getLocals() != null)) {
178193
return curFrameInfo.getPyFrame();
179194
}
180195
// We don't have the frame for the reference, fall back to the stack walk

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/ExecutionContext.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,7 @@ protected static void passCallerFrame(VirtualFrame frame, Object[] callArguments
231231
assert thisInfo.getPyFrame() == pyFrame;
232232
assert pyFrame.getRef() == thisInfo;
233233
} else if (thisInfo.getPyFrame() != null) {
234-
// Don't pass stale locals
235-
thisInfo.getPyFrame().setLocals(null);
234+
thisInfo.getPyFrame().setStale(true);
236235
}
237236
PArguments.setCallerFrameInfo(callArguments, thisInfo);
238237
}
@@ -710,7 +709,7 @@ private static Object enterSlowPath(VirtualFrame frame, IndirectCallDataBase cal
710709
callData.getMaterializeFrameNode().executeOnStack(false, CallerFlags.needsLocals(callerFlags), frame);
711710
} else if (info.getPyFrame() != null) {
712711
// Avoid passing stale locals
713-
info.getPyFrame().setLocals(null);
712+
info.getPyFrame().setStale(true);
714713
}
715714
}
716715
AbstractTruffleException curExc = pythonThreadState.getCaughtException();

0 commit comments

Comments
 (0)