Skip to content

Commit fd9b144

Browse files
steve-ssteve-s
committed
[GR-23342] Pass few more tests of test_sys.
PullRequest: graalpython/1263
2 parents 7de3a20 + bfb9351 commit fd9b144

File tree

5 files changed

+160
-56
lines changed

5 files changed

+160
-56
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ private String[] getExecutableList() {
344344
String javaOptions = System.getenv("_JAVA_OPTIONS");
345345
String javaToolOptions = System.getenv("JAVA_TOOL_OPTIONS");
346346
for (String arg : ManagementFactory.getRuntimeMXBean().getInputArguments()) {
347-
if (arg.matches("-Xrunjdwp:transport=dt_socket,server=y,address=\\d+,suspend=y")) {
347+
if (arg.matches("(-Xrunjdwp:|-agentlib:jdwp=).*suspend=y.*")) {
348348
arg = arg.replace("suspend=y", "suspend=n");
349349
}
350350
if ((javaOptions != null && javaOptions.contains(arg)) || (javaToolOptions != null && javaToolOptions.contains(arg))) {

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_sys.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
*graalpython.lib-python.3.test.test_sys.DisplayHookTest.test_custom_displayhook
22
*graalpython.lib-python.3.test.test_sys.ExceptHookTest.test_excepthook_bytes_filename
33
*graalpython.lib-python.3.test.test_sys.ExceptHookTest.test_original_excepthook
4+
*graalpython.lib-python.3.test.test_sys.ExceptHookTest.test_excepthook
45
*graalpython.lib-python.3.test.test_sys.SizeofTest.test_asyncgen_hooks
56
*graalpython.lib-python.3.test.test_sys.SizeofTest.test_default
67
*graalpython.lib-python.3.test.test_sys.SizeofTest.test_errors
@@ -22,6 +23,7 @@
2223
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_ioencoding_nonascii
2324
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_no_duplicates_in_meta_path
2425
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_refcount
26+
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_executable
2527
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_setrecursionlimit_recursion_depth
2628
*graalpython.lib-python.3.test.test_sys.SysModuleTest.test_sys_getwindowsversion_no_instantiation
2729
*graalpython.lib-python.3.test.test_sys.UnraisableHookTest.test_custom_unraisablehook

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

Lines changed: 149 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.nio.channels.Channels;
4949
import java.nio.channels.WritableByteChannel;
5050
import java.util.ArrayList;
51+
import java.util.HashMap;
5152
import java.util.List;
5253
import java.util.Map;
5354

@@ -57,15 +58,12 @@
5758
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
5859
import com.oracle.graal.python.builtins.PythonBuiltins;
5960
import com.oracle.graal.python.builtins.objects.PNone;
60-
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes;
6161
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
62-
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes;
6362
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;
6463
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum.ErrorAndMessagePair;
6564
import com.oracle.graal.python.builtins.objects.function.PArguments;
6665
import com.oracle.graal.python.builtins.objects.list.PList;
6766
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
68-
import com.oracle.graal.python.builtins.objects.str.PString;
6967
import com.oracle.graal.python.nodes.ErrorMessages;
7068
import com.oracle.graal.python.nodes.PGuards;
7169
import com.oracle.graal.python.nodes.expression.CastToListExpressionNode.CastToListNode;
@@ -77,13 +75,17 @@
7775
import com.oracle.graal.python.runtime.PosixResources;
7876
import com.oracle.graal.python.runtime.PythonContext;
7977
import com.oracle.graal.python.runtime.PythonOptions;
78+
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
8079
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
80+
import com.oracle.truffle.api.TruffleFile;
81+
import com.oracle.truffle.api.TruffleLanguage.Env;
8182
import com.oracle.truffle.api.TruffleLogger;
8283
import com.oracle.truffle.api.dsl.Cached;
8384
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
8485
import com.oracle.truffle.api.dsl.NodeFactory;
8586
import com.oracle.truffle.api.dsl.Specialization;
8687
import com.oracle.truffle.api.frame.VirtualFrame;
88+
import com.oracle.truffle.api.interop.UnsupportedMessageException;
8789
import com.oracle.truffle.api.library.CachedLibrary;
8890

8991
@CoreFunctions(defineModule = "_posixsubprocess")
@@ -100,62 +102,140 @@ protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFa
100102
abstract static class ForkExecNode extends PythonBuiltinNode {
101103
private static final TruffleLogger LOGGER = PythonLanguage.getLogger(ForkExecNode.class);
102104

103-
@Child private BytesNodes.ToBytesNode toBytes = BytesNodes.ToBytesNode.create();
104-
105105
@Specialization
106106
int forkExec(VirtualFrame frame, PList args, @SuppressWarnings("unused") PList execList, @SuppressWarnings("unused") boolean closeFds,
107107
@SuppressWarnings("unused") PList fdsToKeep, String cwd, PList env,
108108
int p2cread, int p2cwrite, int c2pread, int c2pwrite,
109109
int errread, int errwrite, @SuppressWarnings("unused") int errpipe_read, int errpipe_write,
110-
@SuppressWarnings("unused") boolean restore_signals, @SuppressWarnings("unused") boolean call_setsid, @SuppressWarnings("unused") PNone preexec_fn,
111-
@Cached SequenceStorageNodes.CopyInternalArrayNode copy) {
110+
@SuppressWarnings("unused") boolean restore_signals, @SuppressWarnings("unused") boolean call_setsid, @SuppressWarnings("unused") PNone preexec_fn) {
112111

113112
PythonContext context = getContext();
114113
Object state = IndirectCallContext.enter(frame, context, this);
115114
try {
116115
return forkExec(args, execList, closeFds, fdsToKeep, cwd, env, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, errpipe_read, errpipe_write, restore_signals, call_setsid,
117-
preexec_fn, copy);
116+
preexec_fn);
118117
} finally {
119118
IndirectCallContext.exit(frame, context, state);
120119
}
121120
}
122121

123122
@TruffleBoundary
124-
private synchronized int forkExec(PList args, @SuppressWarnings("unused") PList execList, @SuppressWarnings("unused") boolean closeFds,
123+
private synchronized int forkExec(PList args, PList execList, @SuppressWarnings("unused") boolean closeFds,
125124
@SuppressWarnings("unused") PList fdsToKeep, String cwd, PList env,
126125
int p2cread, int p2cwrite, int c2pread, int c2pwrite,
127126
int errread, int errwrite, @SuppressWarnings("unused") int errpipe_read, int errpipe_write,
128-
@SuppressWarnings("unused") boolean restore_signals, @SuppressWarnings("unused") boolean call_setsid, @SuppressWarnings("unused") PNone preexec_fn,
129-
@Cached SequenceStorageNodes.CopyInternalArrayNode copy) {
127+
@SuppressWarnings("unused") boolean restore_signals, @SuppressWarnings("unused") boolean call_setsid, @SuppressWarnings("unused") PNone preexec_fn) {
130128
PythonContext context = getContext();
131129
PosixResources resources = context.getResources();
132130
if (!context.isExecutableAccessAllowed()) {
133131
return -1;
134132
}
135133

136-
ArrayList<String> argStrings = new ArrayList<>();
137-
Object[] copyOfInternalArray = copy.execute(args.getSequenceStorage());
138-
for (Object o : copyOfInternalArray) {
139-
if (o instanceof String) {
140-
argStrings.add((String) o);
141-
} else if (o instanceof PString) {
142-
argStrings.add(((PString) o).getValue());
143-
} else {
134+
SequenceStorage argsStorage = args.getSequenceStorage();
135+
ArrayList<String> argStrings = new ArrayList<>(argsStorage.length());
136+
CastToJavaStringNode castToStringNode = CastToJavaStringNode.getUncached();
137+
for (int i = 0; i < argsStorage.length(); i++) {
138+
try {
139+
argStrings.add(castToStringNode.execute(argsStorage.getItemNormalized(i)));
140+
} catch (CannotCastException ex) {
144141
throw raise(PythonBuiltinClassType.OSError, ErrorMessages.ILLEGAL_ARG);
145142
}
146143
}
147144

148-
if (!argStrings.isEmpty()) {
149-
if (argStrings.get(0).equals(context.getOption(PythonOptions.Executable))) {
145+
File cwdFile;
146+
Env truffleEnv = context.getEnv();
147+
if (getSafeTruffleFile(truffleEnv, cwd).exists()) {
148+
cwdFile = new File(cwd);
149+
} else {
150+
throw raise(PythonBuiltinClassType.OSError, ErrorMessages.WORK_DIR_NOT_ACCESSIBLE, cwd);
151+
}
152+
153+
SequenceStorage envStorage = env.getSequenceStorage();
154+
HashMap<String, String> envMap = new HashMap<>(envStorage.length());
155+
PythonObjectLibrary pyLib = PythonObjectLibrary.getUncached();
156+
for (int i = 0; i < envStorage.length(); i++) {
157+
Object keyValue = envStorage.getItemNormalized(i);
158+
if (!(keyValue instanceof PBytes)) {
159+
continue;
160+
}
161+
// NOTE: passing 'null' frame means we took care of the global state in the callers
162+
String str = checkNullBytesAndEncode(pyLib, (PBytes) keyValue);
163+
String[] strings = str.split("=", 2);
164+
if (strings.length == 2) {
165+
envMap.put(strings[0], strings[1]);
166+
}
167+
}
168+
169+
// The execList argument contains a list of paths to executables. They should be tried
170+
// one-by-one until we find one that can be executed. Unless passed explicitly as an
171+
// argument by the user, the list is constructed by the Python wrapper code, which
172+
// creates an entry for each directory in $PATH joined with the executable name
173+
174+
// CPython iterates the executable list trying to call execve for each item until it
175+
// finds one whose execution succeeds. We do the same to be as compatible as possible.
176+
177+
// Moreover, execve allows to set program arguments (argv) including argv[0] to anything
178+
// and independently of that choose the executable. There is nothing like that in the
179+
// ProcessBuilder API, so we have to replace the first argument with the right
180+
// executable path taken from execList
181+
182+
if (argStrings.isEmpty()) {
183+
// CPython fails on OS level and does not raise any python level error, just the
184+
// message is print to stderr by the subprocess
185+
throw raise(PythonBuiltinClassType.OSError, "A NULL argv[0] was passed through an exec system call");
186+
}
187+
188+
LOGGER.fine(() -> "_posixsubprocess.fork_exec: " + String.join(" ", argStrings));
189+
IOException firstError = null;
190+
SequenceStorage execListStorage = execList.getSequenceStorage();
191+
for (int i = 0; i < execListStorage.length(); i++) {
192+
Object item = execListStorage.getItemNormalized(i);
193+
if (!(item instanceof PBytes)) {
194+
throw raise(PythonBuiltinClassType.TypeError, ErrorMessages.EXPECTED_BYTES_P_FOUND, item);
195+
}
196+
String path = checkNullBytesAndEncode(pyLib, (PBytes) item);
197+
int executableListLen = 0;
198+
if (path.equals(context.getOption(PythonOptions.Executable))) {
199+
// In case someone passed to us sys.executable that happens to be java command
200+
// invocation with additional options like classpath, we split it to the
201+
// individual arguments
150202
String[] executableList = PythonOptions.getExecutableList(context);
151203
argStrings.remove(0);
152-
for (int i = executableList.length - 1; i >= 0; i--) {
153-
argStrings.add(0, executableList[i]);
204+
executableListLen = executableList.length;
205+
for (int j = executableListLen - 1; j >= 0; j--) {
206+
argStrings.add(0, executableList[j]);
154207
}
208+
} else {
209+
argStrings.set(0, path);
155210
}
211+
TruffleFile executableFile = getSafeTruffleFile(truffleEnv, argStrings.get(0));
212+
if (executableFile.isExecutable()) {
213+
try {
214+
return exec(argStrings, cwdFile, envMap, p2cwrite, p2cread, c2pwrite, c2pread, errwrite, errpipe_write, resources, errread);
215+
} catch (IOException ex) {
216+
if (firstError == null) {
217+
firstError = ex;
218+
}
219+
}
220+
} else {
221+
LOGGER.finest(() -> "_posixsubprocess.fork_exec not executable: " + executableFile);
222+
}
223+
for (int j = 1; j < executableListLen; j++) {
224+
argStrings.remove(1);
225+
}
226+
}
227+
if (errpipe_write != -1) {
228+
handleIOError(errpipe_write, resources, firstError);
156229
}
230+
return -1;
231+
}
157232

158-
LOGGER.fine(() -> "_posixsubprocess.fork_exec: " + String.join(" ", argStrings));
233+
// Tries executing given arguments, throws IOException if the executable cannot be executed,
234+
// any other error is handled here
235+
private int exec(ArrayList<String> argStrings, File cwd, Map<String, String> env,
236+
int p2cwrite, int p2cread, int c2pwrite, int c2pread,
237+
int errwrite, int errpipe_write, PosixResources resources, int errread) throws IOException {
238+
LOGGER.finest(() -> "_posixsubprocess.fork_exec trying to exec: " + String.join(" ", argStrings));
159239
ProcessBuilder pb = new ProcessBuilder(argStrings);
160240
if (p2cread != -1 && p2cwrite != -1) {
161241
pb.redirectInput(Redirect.PIPE);
@@ -179,30 +259,11 @@ private synchronized int forkExec(PList args, @SuppressWarnings("unused") PList
179259
pb.redirectErrorStream(true);
180260
}
181261

182-
try {
183-
if (getContext().getEnv().getPublicTruffleFile(cwd).exists()) {
184-
pb.directory(new File(cwd));
185-
} else {
186-
throw raise(PythonBuiltinClassType.OSError, ErrorMessages.WORK_DIR_NOT_ACCESSIBLE, cwd);
187-
}
188-
} catch (SecurityException e) {
189-
throw raise(PythonBuiltinClassType.OSError, e);
190-
}
191-
192-
Map<String, String> environment = pb.environment();
193-
for (Object keyValue : env.getSequenceStorage().getInternalArray()) {
194-
if (keyValue instanceof PBytes) {
195-
// NOTE: passing 'null' frame means we took care of the global state in the
196-
// callers
197-
String[] string = new String(toBytes.execute(null, keyValue)).split("=", 2);
198-
if (string.length == 2) {
199-
environment.put(string[0], string[1]);
200-
}
201-
}
202-
}
262+
pb.directory(cwd);
263+
pb.environment().putAll(env);
203264

265+
Process process = pb.start();
204266
try {
205-
Process process = pb.start();
206267
if (p2cwrite != -1) {
207268
// user code is expected to close the unused ends of the pipes
208269
resources.getFileChannel(p2cwrite).close();
@@ -216,25 +277,59 @@ private synchronized int forkExec(PList args, @SuppressWarnings("unused") PList
216277
resources.getFileChannel(errread).close();
217278
resources.fdopen(errread, Channels.newChannel(process.getErrorStream()));
218279
}
219-
220-
return resources.registerChild(process);
221-
} catch (IOException e) {
280+
} catch (IOException ex) {
281+
// We only want to rethrow the IOException that may come out of pb.start()
222282
if (errpipe_write != -1) {
223-
// write exec error information here. Data format: "exception name:hex
224-
// errno:description"
225-
handleIOError(errpipe_write, resources, e);
283+
handleIOError(errpipe_write, resources, ex);
226284
}
227285
return -1;
228286
}
287+
288+
return resources.registerChild(process);
289+
}
290+
291+
private TruffleFile getSafeTruffleFile(Env env, String path) {
292+
try {
293+
return env.getPublicTruffleFile(path);
294+
} catch (SecurityException e) {
295+
throw raise(PythonBuiltinClassType.OSError, e);
296+
}
297+
}
298+
299+
private String checkNullBytesAndEncode(PythonObjectLibrary pyLib, PBytes bytesObj) {
300+
byte[] bytes;
301+
try {
302+
bytes = pyLib.getBufferBytes(bytesObj);
303+
} catch (UnsupportedMessageException e) {
304+
throw new AssertionError(); // should not happen
305+
}
306+
for (byte b : bytes) {
307+
if (b == 0) {
308+
throw raise(PythonBuiltinClassType.ValueError, ErrorMessages.EMBEDDED_NULL_BYTE);
309+
}
310+
}
311+
// Note: we use intentionally the default encoding for the bytes. We're most likely
312+
// getting bytes that the Python wrapper encoded from strings passed to it by the user
313+
// and we should support non-ascii characters supported by the current FS. See
314+
// test_warnings.test_nonascii
315+
return new String(bytes);
229316
}
230317

231318
@TruffleBoundary(allowInlining = true)
232319
private void handleIOError(int errpipe_write, PosixResources resources, IOException e) {
320+
// write exec error information here. Data format: "exception name:hex
321+
// errno:description". The exception can be null if we did not find any file in the
322+
// execList that could be executed
233323
Channel err = resources.getFileChannel(errpipe_write);
234324
if (!(err instanceof WritableByteChannel)) {
235325
throw raise(PythonBuiltinClassType.OSError, ErrorMessages.ERROR_WRITING_FORKEXEC);
236326
} else {
237-
ErrorAndMessagePair pair = OSErrorEnum.fromException(e);
327+
ErrorAndMessagePair pair;
328+
if (e == null) {
329+
pair = new ErrorAndMessagePair(OSErrorEnum.ENOENT, OSErrorEnum.ENOENT.getMessage());
330+
} else {
331+
pair = OSErrorEnum.fromException(e);
332+
}
238333
try {
239334
((WritableByteChannel) err).write(ByteBuffer.wrap(("OSError:" + Long.toHexString(pair.oserror.getNumber()) + ":" + pair.message).getBytes()));
240335
} catch (IOException e1) {
@@ -253,7 +348,6 @@ int forkExecDefault(VirtualFrame frame, Object args, Object executable_list, Obj
253348
@Cached CastToListNode castFdsToKeep,
254349
@Cached CastToJavaStringNode castCwd,
255350
@Cached CastToListNode castEnv,
256-
@Cached SequenceStorageNodes.CopyInternalArrayNode copy,
257351
@CachedLibrary(limit = "3") PythonObjectLibrary lib) {
258352
String actualCwd;
259353
if (cwd instanceof PNone) {
@@ -291,7 +385,7 @@ int forkExecDefault(VirtualFrame frame, Object args, Object executable_list, Obj
291385
lib.asSizeWithState(errpipe_read, PArguments.getThreadState(frame)),
292386
lib.asSizeWithState(errpipe_write, PArguments.getThreadState(frame)),
293387
lib.isTrueWithState(restore_signals, PArguments.getThreadState(frame)),
294-
lib.isTrueWithState(call_setsid, PArguments.getThreadState(frame)), PNone.NO_VALUE, copy);
388+
lib.isTrueWithState(call_setsid, PArguments.getThreadState(frame)), PNone.NO_VALUE);
295389
}
296390

297391
}

0 commit comments

Comments
 (0)