Skip to content

Commit a80312d

Browse files
fangererfangerer
committed
[GR-18797] [GR-18938] Do not implicitly assume top frame if no caller is given.
PullRequest: graalpython/698
2 parents 8037b49 + e0e4bce commit a80312d

File tree

8 files changed

+186
-41
lines changed

8 files changed

+186
-41
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_backref_chain.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
2+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
3+
#
4+
# The Universal Permissive License (UPL), Version 1.0
5+
#
6+
# Subject to the condition set forth below, permission is hereby granted to any
7+
# person obtaining a copy of this software, associated documentation and/or
8+
# data (collectively the "Software"), free of charge and under any and all
9+
# copyright rights in the Software, and any and all patent rights owned or
10+
# freely licensable by each licensor hereunder covering either (i) the
11+
# unmodified Software as contributed to or provided by such licensor, or (ii)
12+
# the Larger Works (as defined below), to deal in both
13+
#
14+
# (a) the Software, and
15+
#
16+
# (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
17+
# one is included with the Software each a "Larger Work" to which the Software
18+
# is contributed by such licensors),
19+
#
20+
# without restriction, including without limitation the rights to copy, create
21+
# derivative works of, display, perform, and distribute the Software and make,
22+
# use, sell, offer for sale, import, export, have made, and have sold the
23+
# Software and the Larger Work(s), and to sublicense the foregoing rights on
24+
# either these or other terms.
25+
#
26+
# This license is subject to the following condition:
27+
#
28+
# The above copyright notice and either this complete permission notice or at a
29+
# minimum a reference to the UPL must be included in all copies or substantial
30+
# portions of the Software.
31+
#
32+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
33+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
34+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
35+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
36+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
37+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
38+
# SOFTWARE.
39+
40+
import sys
41+
42+
def get_frame():
43+
return sys._getframe(0)
44+
45+
def foo():
46+
f = get_frame()
47+
stack = []
48+
while f:
49+
stack.append(f)
50+
f = f.f_back
51+
return stack
52+
53+
def bar():
54+
return foo()
55+
56+
# NOTE: we need to call it in the module because the test framework will already provide the materialized caller frame
57+
# and then the test will succeed in any case.
58+
stack = bar()
59+
60+
def test_backref_chain():
61+
actual_fnames = [n.f_code.co_name for n in stack]
62+
expected_prefix = ['get_frame', 'foo', 'bar']
63+
assert len(stack) >= len(expected_prefix)
64+
assert expected_prefix == actual_fnames[:len(expected_prefix)]
65+

‎graalpython/com.oracle.graal.python.test/src/tests/test_frame_tests.py‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
1+
# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
22
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
33
#
44
# The Universal Permissive License (UPL), Version 1.0
@@ -64,6 +64,48 @@ def foo():
6464
return sys._getframe(0).f_back
6565
assert foo().f_locals['a'] == 'test_backref'
6666

67+
def get_frame():
68+
return sys._getframe(0)
69+
70+
def get_frame_caller():
71+
return get_frame()
72+
73+
def do_stackwalk(f):
74+
stack = []
75+
while f:
76+
stack.append(f)
77+
f = f.f_back
78+
return stack
79+
80+
stack = do_stackwalk(get_frame_caller())
81+
actual_fnames = [n.f_code.co_name for n in stack]
82+
expected_prefix = ['get_frame', 'get_frame_caller', 'test_backref']
83+
assert len(stack) >= len(expected_prefix)
84+
assert expected_prefix == actual_fnames[:len(expected_prefix)]
85+
86+
87+
def test_backref_recursive():
88+
def get_frame():
89+
return sys._getframe(0)
90+
91+
def foo(i):
92+
if i == 1:
93+
f = get_frame()
94+
stack = []
95+
while f:
96+
stack.append(f)
97+
f = f.f_back
98+
return stack
99+
else:
100+
# This recursive call will cause
101+
return foo(i+1)
102+
103+
def bar():
104+
return foo(0)
105+
106+
s = bar()
107+
print([n.f_code for n in s])
108+
67109

68110
def test_code():
69111
code = sys._getframe().f_code

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import java.util.function.Supplier;
4141
import java.util.logging.Level;
4242

43+
import org.graalvm.nativeimage.ImageInfo;
44+
4345
import com.oracle.graal.python.PythonLanguage;
4446
import com.oracle.graal.python.builtins.modules.ArrayModuleBuiltins;
4547
import com.oracle.graal.python.builtins.modules.AstModuleBuiltins;
@@ -164,6 +166,7 @@
164166
import com.oracle.graal.python.builtins.objects.type.TypeNodes.GetNameNode;
165167
import com.oracle.graal.python.builtins.objects.zipimporter.ZipImporterBuiltins;
166168
import com.oracle.graal.python.nodes.BuiltinNames;
169+
import com.oracle.graal.python.nodes.call.GenericInvokeNode;
167170
import com.oracle.graal.python.runtime.PythonContext;
168171
import com.oracle.graal.python.runtime.PythonCore;
169172
import com.oracle.graal.python.runtime.PythonParser;
@@ -184,8 +187,6 @@
184187
import com.oracle.truffle.api.source.Source;
185188
import com.oracle.truffle.api.source.SourceSection;
186189

187-
import org.graalvm.nativeimage.ImageInfo;
188-
189190
/**
190191
* The core is intended to the immutable part of the interpreter, including most modules and most
191192
* types.
@@ -635,7 +636,7 @@ private void loadFile(String s, String prefix) {
635636
// use an anonymous module for the side-effects
636637
mod = factory().createPythonModule("__anonymous__");
637638
}
638-
callTarget.call(PArguments.withGlobals(mod));
639+
GenericInvokeNode.getUncached().execute(null, callTarget, PArguments.withGlobals(mod));
639640
}
640641

641642
public PythonObjectFactory factory() {

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
import com.oracle.truffle.api.dsl.TypeSystemReference;
9595
import com.oracle.truffle.api.frame.VirtualFrame;
9696
import com.oracle.truffle.api.nodes.LanguageInfo;
97+
import com.oracle.truffle.api.profiles.ConditionProfile;
9798
import com.oracle.truffle.llvm.api.Toolchain;
9899

99100
@CoreFunctions(defineModule = "sys")
@@ -350,42 +351,54 @@ public abstract static class GetFrameNode extends PythonBuiltinNode {
350351
@Specialization
351352
PFrame first(VirtualFrame frame, @SuppressWarnings("unused") PNone arg,
352353
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
353-
return escapeFrame(frame, 0, readCallerNode);
354+
PFrame requested = escapeFrame(frame, 0, readCallerNode);
355+
// there must always be *the current frame*
356+
assert requested != null : "frame must not be null";
357+
return requested;
354358
}
355359

356360
@Specialization
357361
PFrame counted(VirtualFrame frame, int num,
358-
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
359-
return escapeFrame(frame, num, readCallerNode);
362+
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode,
363+
@Shared("callStackDepthProfile") @Cached("createBinaryProfile()") ConditionProfile callStackDepthProfile) {
364+
PFrame requested = escapeFrame(frame, num, readCallerNode);
365+
if (callStackDepthProfile.profile(requested == null)) {
366+
throw raiseCallStackDepth();
367+
}
368+
return requested;
360369
}
361370

362371
@Specialization(rewriteOn = ArithmeticException.class)
363372
PFrame countedLong(VirtualFrame frame, long num,
364-
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
365-
return counted(frame, PInt.intValueExact(num), readCallerNode);
373+
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode,
374+
@Shared("callStackDepthProfile") @Cached("createBinaryProfile()") ConditionProfile callStackDepthProfile) {
375+
return counted(frame, PInt.intValueExact(num), readCallerNode, callStackDepthProfile);
366376
}
367377

368378
@Specialization
369379
PFrame countedLongOvf(VirtualFrame frame, long num,
370-
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
380+
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode,
381+
@Shared("callStackDepthProfile") @Cached("createBinaryProfile()") ConditionProfile callStackDepthProfile) {
371382
try {
372-
return counted(frame, PInt.intValueExact(num), readCallerNode);
383+
return counted(frame, PInt.intValueExact(num), readCallerNode, callStackDepthProfile);
373384
} catch (ArithmeticException e) {
374385
throw raiseCallStackDepth();
375386
}
376387
}
377388

378389
@Specialization(rewriteOn = ArithmeticException.class)
379390
PFrame countedPInt(VirtualFrame frame, PInt num,
380-
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
381-
return counted(frame, num.intValueExact(), readCallerNode);
391+
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode,
392+
@Shared("callStackDepthProfile") @Cached("createBinaryProfile()") ConditionProfile callStackDepthProfile) {
393+
return counted(frame, num.intValueExact(), readCallerNode, callStackDepthProfile);
382394
}
383395

384396
@Specialization
385397
PFrame countedPIntOvf(VirtualFrame frame, PInt num,
386-
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode) {
398+
@Shared("caller") @Cached ReadCallerFrameNode readCallerNode,
399+
@Shared("callStackDepthProfile") @Cached("createBinaryProfile()") ConditionProfile callStackDepthProfile) {
387400
try {
388-
return counted(frame, num.intValueExact(), readCallerNode);
401+
return counted(frame, num.intValueExact(), readCallerNode, callStackDepthProfile);
389402
} catch (ArithmeticException e) {
390403
throw raiseCallStackDepth();
391404
}

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.oracle.graal.python.builtins.PythonBuiltins;
3434
import com.oracle.graal.python.builtins.objects.PNone;
3535
import com.oracle.graal.python.builtins.objects.code.CodeNodes;
36+
import com.oracle.graal.python.builtins.objects.frame.PFrame.Reference;
3637
import com.oracle.graal.python.builtins.objects.function.PArguments;
3738
import com.oracle.graal.python.builtins.objects.module.PythonModule;
3839
import com.oracle.graal.python.builtins.objects.object.ObjectBuiltins.DictNode;
@@ -172,6 +173,7 @@ public abstract static class GetBackrefNode extends PythonBuiltinNode {
172173
Object getBackref(VirtualFrame frame, PFrame self,
173174
@Cached BranchProfile noBackref,
174175
@Cached BranchProfile topRef,
176+
@Cached("createBinaryProfile()") ConditionProfile notMaterialized,
175177
@Cached ReadCallerFrameNode readCallerFrame) {
176178
PFrame.Reference backref;
177179
for (PFrame cur = self;; cur = backref.getPyFrame()) {
@@ -199,15 +201,37 @@ Object getBackref(VirtualFrame frame, PFrame self,
199201
}
200202
}
201203

202-
if (backref.getPyFrame() == null) {
204+
if (backref == Reference.EMPTY) {
203205
return PNone.NONE;
204206
} else if (!PRootNode.isPythonInternal(backref.getCallNode().getRootNode())) {
205-
return backref.getPyFrame();
207+
PFrame fback = materialize(frame, readCallerFrame, backref, notMaterialized);
208+
assert fback.getRef() == backref;
209+
return fback;
206210
}
207211

208212
assert backref.getPyFrame() != null;
209213
}
210214
}
215+
216+
private static PFrame materialize(VirtualFrame frame, ReadCallerFrameNode readCallerFrameNode, PFrame.Reference backref, ConditionProfile notMaterialized) {
217+
if (notMaterialized.profile(backref.getPyFrame() == null)) {
218+
// Special case: the backref's PFrame object is not yet available; this is because
219+
// the frame is still on the stack. So we need to find and materialize it.
220+
for (int i = 0;; i++) {
221+
PFrame caller = readCallerFrameNode.executeWith(frame, i);
222+
if (caller == null) {
223+
break;
224+
} else if (caller.getRef() == backref) {
225+
// now, the PFrame object is available since the readCallerFrameNode
226+
// materialized it
227+
assert backref.getPyFrame() != null;
228+
return caller;
229+
}
230+
}
231+
assert false : "could not find frame of backref on the stack";
232+
}
233+
return backref.getPyFrame();
234+
}
211235
}
212236

213237
@Builtin(name = "clear", minNumOfPositionalArgs = 1)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/control/TopLevelExceptionHandler.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public class TopLevelExceptionHandler extends RootNode {
9494

9595
@Child private CreateArgumentsNode createArgs = CreateArgumentsNode.create();
9696
@Child private LookupAndCallUnaryNode callStrNode = LookupAndCallUnaryNode.create(__STR__);
97-
@Child private CallNode callNode = CallNode.create();
97+
@Child private CallNode exceptionHookCallNode = CallNode.create();
9898
@Child private MaterializeFrameNode materializeFrameNode = MaterializeFrameNodeGen.create();
9999
@Child private PythonObjectFactory factory;
100100
@Child private GetTracebackNode getTracebackNode;
@@ -187,7 +187,10 @@ private void printExc(VirtualFrame frame, PException e) {
187187
if (PythonOptions.getOption(theContext, PythonOptions.AlwaysRunExcepthook)) {
188188
if (hook != PNone.NO_VALUE) {
189189
try {
190-
callNode.execute(frame, hook, new Object[]{type, value, tb}, PKeyword.EMPTY_KEYWORDS);
190+
// Note: it is important to pass frame 'null' because that will cause the
191+
// CallNode to tread the invoke like a foreign call and access the top frame ref
192+
// in the context.
193+
exceptionHookCallNode.execute(null, hook, new Object[]{type, value, tb}, PKeyword.EMPTY_KEYWORDS);
191194
} catch (PException internalError) {
192195
// More complex handling of errors in exception printing is done in our
193196
// Python code, if we get here, we just fall back to the launcher

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

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,6 @@ public static final class CalleeContext extends Node {
195195
@Child private MaterializeFrameNode materializeNode;
196196

197197
@CompilationFinal private boolean everEscaped = false;
198-
@CompilationFinal private boolean firstRequest = true;
199198

200199
/**
201200
* Wrap the execution of a Python callee called from a Python frame.
@@ -230,28 +229,22 @@ public void exit(VirtualFrame frame, PRootNode node) {
230229
// This assumption acts as our branch profile here
231230
PFrame.Reference callerInfo = PArguments.getCallerFrameInfo(frame);
232231
if (callerInfo == null) {
233-
if (firstRequest) {
234-
// we didn't request the caller frame reference. now we need it.
235-
CompilerDirectives.transferToInterpreterAndInvalidate();
236-
firstRequest = false;
237-
238-
// n.b. We need to use 'ReadCallerFrameNode.getCallerFrame' instead of
239-
// 'Truffle.getRuntime().getCallerFrame()' because we still need to skip
240-
// non-Python frames, even if we do not skip frames of builtin functions.
241-
Frame callerFrame = ReadCallerFrameNode.getCallerFrame(info, FrameInstance.FrameAccess.READ_ONLY, false, 0);
242-
if (PArguments.isPythonFrame(callerFrame)) {
243-
callerInfo = PArguments.getCurrentFrameInfo(callerFrame);
244-
} else {
245-
// TODO: frames: an assertion should be that this is one of our
246-
// entry point call nodes
247-
callerInfo = PFrame.Reference.EMPTY;
248-
}
232+
// we didn't request the caller frame reference. now we need it.
233+
CompilerDirectives.transferToInterpreterAndInvalidate();
234+
235+
// n.b. We need to use 'ReadCallerFrameNode.getCallerFrame' instead of
236+
// 'Truffle.getRuntime().getCallerFrame()' because we still need to skip
237+
// non-Python frames, even if we do not skip frames of builtin functions.
238+
Frame callerFrame = ReadCallerFrameNode.getCallerFrame(info, FrameInstance.FrameAccess.READ_ONLY, false, 0);
239+
if (PArguments.isPythonFrame(callerFrame)) {
240+
callerInfo = PArguments.getCurrentFrameInfo(callerFrame);
249241
} else {
250-
// caller info was requested, it must be here if there is
251-
// any. If it isn't, we're in a top-frame.
252-
assert node.needsCallerFrame();
242+
// TODO: frames: an assertion should be that this is one of our
243+
// entry point call nodes
253244
callerInfo = PFrame.Reference.EMPTY;
254245
}
246+
// ReadCallerFrameNode.getCallerFrame must have the assumption invalidated
247+
assert node.needsCallerFrame() : "stack walk did not invalidate caller frame assumption";
255248
}
256249

257250
// force the frame so that it can be accessed later

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
5353
import com.oracle.graal.python.builtins.objects.dict.PDict;
5454
import com.oracle.graal.python.builtins.objects.frame.PFrame;
55+
import com.oracle.graal.python.builtins.objects.frame.PFrame.Reference;
5556
import com.oracle.graal.python.builtins.objects.list.PList;
5657
import com.oracle.graal.python.builtins.objects.module.PythonModule;
5758
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
@@ -106,8 +107,11 @@ public final class PythonContext {
106107

107108
// TODO: frame: make these three ThreadLocal
108109

109-
/* the reference to the last top frame on the Python stack during interop calls */
110-
private PFrame.Reference topframeref = null;
110+
/*
111+
* The reference to the last top frame on the Python stack during interop calls. Initially, this
112+
* is EMPTY representing the top frame.
113+
*/
114+
private PFrame.Reference topframeref = Reference.EMPTY;
111115

112116
/* corresponds to 'PyThreadState.curexc_*' */
113117
private PException currentException;

0 commit comments

Comments
 (0)