Skip to content

Commit 60a0eac

Browse files
committed
[GR-30333] Fix coverage job.
PullRequest: graalpython/1729
2 parents f0c32d5 + ab8df4e commit 60a0eac

File tree

6 files changed

+121
-35
lines changed

6 files changed

+121
-35
lines changed

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/GraalPythonMain.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import java.util.ListIterator;
4444
import java.util.Map;
4545
import java.util.Set;
46+
import java.util.UUID;
4647
import java.util.function.Function;
4748

4849
import org.graalvm.launcher.AbstractLanguageLauncher;
@@ -778,8 +779,9 @@ protected void printHelp(OptionCategory maxCategory) {
778779
"PYTHONPYCACHEPREFIX: if this is set, GraalPython will write .pyc files in a mirror\n" +
779780
" directory tree at this path, instead of in __pycache__ directories within the source tree.\n" +
780781
"GRAAL_PYTHON_ARGS: the value is added as arguments as if passed on the\n" +
781-
" commandline. There is one special case: any `$$' in the value is replaced\n" +
782-
" with the current process id. To pass a literal `$$', you must escape the\n" +
782+
" commandline. There are two special cases: any `$$' in the value is replaced\n" +
783+
" with the current process id, and any $UUID$ is replaced with random unique string\n" +
784+
" that may contain letters, digits, and '-'. To pass a literal `$$', you must escape the\n" +
783785
" second `$' like so: `$\\$'\n" +
784786
(wantsExperimental ? "\nArguments specific to the Graal Python launcher:\n" +
785787
"--show-version : print the Python version number and continue.\n" +
@@ -1070,14 +1072,15 @@ private static List<String> getDefaultEnvironmentArgs() {
10701072
} else {
10711073
pid = ManagementFactory.getRuntimeMXBean().getName().split("@")[0];
10721074
}
1075+
String uuid = UUID.randomUUID().toString();
10731076
String envArgsOpt = System.getenv("GRAAL_PYTHON_ARGS");
10741077
ArrayList<String> envArgs = new ArrayList<>();
1075-
State s = State.NORMAL;
1076-
StringBuilder sb = new StringBuilder();
10771078
if (envArgsOpt != null) {
1079+
State s = State.NORMAL;
1080+
StringBuilder sb = new StringBuilder();
10781081
for (char x : envArgsOpt.toCharArray()) {
10791082
if (s == State.NORMAL && Character.isWhitespace(x)) {
1080-
addArgument(pid, envArgs, sb);
1083+
addArgument(pid, uuid, envArgs, sb);
10811084
} else {
10821085
if (x == '"') {
10831086
if (s == State.NORMAL) {
@@ -1108,14 +1111,14 @@ private static List<String> getDefaultEnvironmentArgs() {
11081111
}
11091112
}
11101113
}
1111-
addArgument(pid, envArgs, sb);
1114+
addArgument(pid, uuid, envArgs, sb);
11121115
}
11131116
return envArgs;
11141117
}
11151118

1116-
private static void addArgument(String pid, ArrayList<String> envArgs, StringBuilder sb) {
1119+
private static void addArgument(String pid, String uuid, ArrayList<String> envArgs, StringBuilder sb) {
11171120
if (sb.length() > 0) {
1118-
String arg = sb.toString().replace("$$", pid).replace("\\$", "$");
1121+
String arg = sb.toString().replace("$UUID$", uuid).replace("$$", pid).replace("\\$", "$");
11191122
envArgs.add(arg);
11201123
sb.setLength(0);
11211124
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Copyright (c) 2021, 2021, 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+
# Various miscellaneous tests not related to one specific Python feature
41+
42+
import collections.abc
43+
44+
45+
def test_thread_state_and_globals():
46+
for x in ('f', 1, 1.0):
47+
format(x)
48+
assert isinstance(globals(), collections.abc.Mapping)
49+
50+
51+
if __name__ == '__main__':
52+
unittest.main()

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,7 @@ Object doObject(Object value,
10431043
*/
10441044
public abstract static class RecursiveBinaryCheckBaseNode extends PythonBinaryBuiltinNode {
10451045
static final int MAX_EXPLODE_LOOP = 16; // is also shifted to the left by recursion depth
1046+
static final byte NON_RECURSIVE = Byte.MAX_VALUE;
10461047

10471048
@Child private SequenceStorageNodes.LenNode lenNode;
10481049
@Child private GetObjectArrayNode getObjectArrayNode;
@@ -1059,7 +1060,7 @@ protected final RecursiveBinaryCheckBaseNode createRecursive() {
10591060
}
10601061

10611062
protected final RecursiveBinaryCheckBaseNode createNonRecursive() {
1062-
return createRecursive((byte) (PythonOptions.getNodeRecursionLimit() + 1));
1063+
return createRecursive(NON_RECURSIVE);
10631064
}
10641065

10651066
protected RecursiveBinaryCheckBaseNode createRecursive(@SuppressWarnings("unused") byte newDepth) {
@@ -1097,22 +1098,31 @@ final boolean doRecursiveWithNode(VirtualFrame frame, Object instance, PTuple cl
10971098
return false;
10981099
}
10991100

1100-
@Specialization(guards = "depth >= getNodeRecursionLimit()")
1101+
@Specialization(guards = {"depth != NON_RECURSIVE", "depth >= getNodeRecursionLimit()"})
11011102
final boolean doRecursiveWithLoop(VirtualFrame frame, Object instance, PTuple clsTuple,
11021103
@CachedLanguage PythonLanguage language,
11031104
@Cached("createNonRecursive()") RecursiveBinaryCheckBaseNode node) {
11041105
Object state = IndirectCallContext.enter(frame, language, getContextRef(), this);
11051106
try {
11061107
// Note: we need actual recursion to trigger the stack overflow error like CPython
11071108
// Note: we need fresh RecursiveBinaryCheckBaseNode and cannot use "this", because
1108-
// children of this executed by other specializations may assume they'll always get
1109-
// non-null frame
1109+
// other children of this executed by other specializations may assume they'll
1110+
// always get a non-null frame
11101111
return callRecursiveWithNodeTruffleBoundary(instance, clsTuple, node);
11111112
} finally {
11121113
IndirectCallContext.exit(frame, language, getContextRef(), state);
11131114
}
11141115
}
11151116

1117+
@Specialization(guards = "depth == NON_RECURSIVE")
1118+
final boolean doRecursiveWithLoopReuseThis(VirtualFrame frame, Object instance, PTuple clsTuple) {
1119+
// This should be only called by doRecursiveWithLoop, now we have to reuse this to stop
1120+
// recursive node creation. It is OK, because now all specializations should always get
1121+
// null frame
1122+
assert frame == null;
1123+
return callRecursiveWithNodeTruffleBoundary(instance, clsTuple, this);
1124+
}
1125+
11161126
@TruffleBoundary
11171127
private boolean callRecursiveWithNodeTruffleBoundary(Object instance, PTuple clsTuple, RecursiveBinaryCheckBaseNode node) {
11181128
return doRecursiveWithNode(null, instance, clsTuple, node);

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PArguments.java

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.oracle.graal.python.builtins.objects.cell.PCell;
2929
import com.oracle.graal.python.builtins.objects.dict.PDict;
3030
import com.oracle.graal.python.builtins.objects.frame.PFrame;
31+
import com.oracle.graal.python.builtins.objects.frame.PFrame.Reference;
3132
import com.oracle.graal.python.builtins.objects.generator.GeneratorControlData;
3233
import com.oracle.graal.python.builtins.objects.object.PythonObject;
3334
import com.oracle.graal.python.nodes.function.ClassBodyRootNode;
@@ -381,9 +382,35 @@ public static PDict getGeneratorFrameLocals(Object[] arguments) {
381382
return (PDict) arguments[INDEX_CALLER_FRAME_INFO];
382383
}
383384

385+
/**
386+
* Synchronizes the arguments array of a Truffle frame with a {@link PFrame}. Copies only those
387+
* arguments that are necessary to be synchronized between the two.
388+
*
389+
* NOTE: such arguments usually need to be preserved in {@link ThreadState} so that when we are
390+
* materializing a frame restored from {@link ThreadState}, the newly created instance of
391+
* {@link PFrame} will contain those arguments.
392+
*/
393+
public static void synchronizeArgs(Frame frameToMaterialize, PFrame escapedFrame) {
394+
Object[] arguments = frameToMaterialize.getArguments();
395+
Object[] copiedArgs = new Object[arguments.length];
396+
397+
// copy only some carefully picked internal arguments
398+
setSpecialArgument(copiedArgs, getSpecialArgument(arguments));
399+
setGeneratorFrame(copiedArgs, getGeneratorFrameSafe(arguments));
400+
setGlobals(copiedArgs, getGlobals(arguments));
401+
setClosure(copiedArgs, getClosure(arguments));
402+
403+
// copy all user arguments
404+
PythonUtils.arraycopy(arguments, USER_ARGUMENTS_OFFSET, copiedArgs, USER_ARGUMENTS_OFFSET, getUserArgumentLength(arguments));
405+
406+
escapedFrame.setArguments(copiedArgs);
407+
}
408+
384409
public static ThreadState getThreadState(VirtualFrame frame) {
385410
assert frame != null : "cannot get thread state without a frame";
386-
return new ThreadState(PArguments.getCurrentFrameInfo(frame), PArguments.getExceptionUnchecked(frame));
411+
return new ThreadState(PArguments.getCurrentFrameInfo(frame),
412+
PArguments.getExceptionUnchecked(frame),
413+
PArguments.getGlobals(frame));
387414
}
388415

389416
public static ThreadState getThreadStateOrNull(VirtualFrame frame, ConditionProfile hasFrameProfile) {
@@ -394,6 +421,7 @@ public static VirtualFrame frameForCall(ThreadState frame) {
394421
Object[] args = PArguments.create();
395422
PArguments.setCurrentFrameInfo(args, frame.info);
396423
PArguments.setExceptionUnchecked(args, frame.exc);
424+
args[INDEX_GLOBALS_ARGUMENT] = frame.globals;
397425
return Truffle.getRuntime().createVirtualFrame(args, EMTPY_FD);
398426
}
399427

@@ -405,10 +433,12 @@ public static final class ThreadState {
405433
private final PFrame.Reference info;
406434
// The type is object because it is Object in the frame and casting it slows things down
407435
private final Object exc;
436+
private final Object globals;
408437

409-
private ThreadState(PFrame.Reference info, Object exc) {
438+
private ThreadState(Reference info, Object exc, Object globals) {
410439
this.info = info;
411440
this.exc = exc;
441+
this.globals = globals;
412442
}
413443
}
414444
}

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

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
import com.oracle.graal.python.nodes.function.ClassBodyRootNode;
5959
import com.oracle.graal.python.nodes.object.GetClassNode;
6060
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
61-
import com.oracle.graal.python.util.PythonUtils;
6261
import com.oracle.truffle.api.CompilerDirectives;
6362
import com.oracle.truffle.api.dsl.Cached;
6463
import com.oracle.truffle.api.dsl.Cached.Shared;
@@ -125,7 +124,7 @@ public final PFrame execute(VirtualFrame frame, Node location, boolean markAsEsc
125124
static PFrame freshPFrameForGenerator(Node location, @SuppressWarnings("unused") boolean markAsEscaped, @SuppressWarnings("unused") boolean forceSync, Frame frameToMaterialize,
126125
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory) {
127126
PFrame escapedFrame = factory.createPFrame(PArguments.getCurrentFrameInfo(frameToMaterialize), location, PArguments.getGeneratorFrameLocals(frameToMaterialize), false);
128-
syncArgs(frameToMaterialize, escapedFrame);
127+
PArguments.synchronizeArgs(frameToMaterialize, escapedFrame);
129128
PFrame.Reference topFrameRef = PArguments.getCurrentFrameInfo(frameToMaterialize);
130129
topFrameRef.setPyFrame(escapedFrame);
131130
return escapedFrame;
@@ -222,7 +221,7 @@ private static PFrame doEscapeFrame(VirtualFrame frame, Frame frameToMaterialize
222221
topFrameRef.setPyFrame(escapedFrame);
223222

224223
// on a freshly created PFrame, we do always sync the arguments
225-
syncArgs(frameToMaterialize, escapedFrame);
224+
PArguments.synchronizeArgs(frameToMaterialize, escapedFrame);
226225
if (forceSync) {
227226
syncValuesNode.execute(frame, escapedFrame, frameToMaterialize);
228227
}
@@ -232,22 +231,6 @@ private static PFrame doEscapeFrame(VirtualFrame frame, Frame frameToMaterialize
232231
return escapedFrame;
233232
}
234233

235-
private static void syncArgs(Frame frameToMaterialize, PFrame escapedFrame) {
236-
Object[] arguments = frameToMaterialize.getArguments();
237-
Object[] copiedArgs = new Object[arguments.length];
238-
239-
// copy only some carefully picked internal arguments
240-
PArguments.setSpecialArgument(copiedArgs, PArguments.getSpecialArgument(arguments));
241-
PArguments.setGeneratorFrame(copiedArgs, PArguments.getGeneratorFrameSafe(arguments));
242-
PArguments.setGlobals(copiedArgs, PArguments.getGlobals(arguments));
243-
PArguments.setClosure(copiedArgs, PArguments.getClosure(arguments));
244-
245-
// copy all user arguments
246-
PythonUtils.arraycopy(arguments, PArguments.USER_ARGUMENTS_OFFSET, copiedArgs, PArguments.USER_ARGUMENTS_OFFSET, PArguments.getUserArgumentLength(arguments));
247-
248-
escapedFrame.setArguments(copiedArgs);
249-
}
250-
251234
protected static boolean inClassBody(Frame frame) {
252235
return PArguments.getSpecialArgument(frame) instanceof ClassBodyRootNode;
253236
}

mx.graalpython/mx_graalpython.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,7 +1654,7 @@ def python_coverage(args):
16541654
for kwds in variants:
16551655
variant_str = re.sub(r"[^a-zA-Z]", "_", str(kwds))
16561656
for pattern in ["py"]:
1657-
outfile = os.path.join(SUITE.dir, "coverage_%s_%s_$$.lcov" % (variant_str, pattern))
1657+
outfile = os.path.join(SUITE.dir, "coverage_%s_%s_$UUID$.lcov" % (variant_str, pattern))
16581658
if os.path.exists(outfile):
16591659
os.unlink(outfile)
16601660
extra_args = [
@@ -1694,16 +1694,24 @@ def python_coverage(args):
16941694
print('Could not compile', fullname, e)
16951695
""".format(os.path.join(prefix, "lib-python")))
16961696
f.flush()
1697+
lcov_file = 'zero.lcov'
1698+
try:
1699+
# the coverage instrument complains if the file already exists
1700+
os.unlink(lcov_file)
1701+
except OSError:
1702+
pass
1703+
# We use "java" POSIX backend, because the supporting so library is missing in the dev build
16971704
with _dev_pythonhome_context():
16981705
mx.run([
16991706
executable,
17001707
"-S",
17011708
"--experimental-options",
1709+
"--python.PosixModuleBackend=java",
17021710
"--coverage",
17031711
"--coverage.TrackInternal",
17041712
"--coverage.FilterFile=%s/*.py" % prefix,
17051713
"--coverage.Output=lcov",
1706-
"--coverage.OutputFile=zero.lcov",
1714+
"--coverage.OutputFile=" + lcov_file,
17071715
f.name
17081716
])
17091717

0 commit comments

Comments
 (0)